From 55bea5a26812ed573bc6f5c534113e140fcbbd4e Mon Sep 17 00:00:00 2001 From: Marian Csontos Date: Fri, 3 Dec 2021 17:11:27 +0100 Subject: [PATCH] Revert "pvscan: only add device args to dev cache" This reverts commit 33e47182f773c1a902b533580b63a803906de55d. --- lib/device/dev-cache.c | 204 +++----------------------------- lib/device/dev-cache.h | 6 +- lib/device/device_id.c | 27 ++--- lib/device/device_id.h | 1 - test/shell/devicesfile-basic.sh | 2 +- tools/pvscan.c | 58 ++++----- 6 files changed, 52 insertions(+), 246 deletions(-) diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c index 33b75a9a9..c6e5f68cf 100644 --- a/lib/device/dev-cache.c +++ b/lib/device/dev-cache.c @@ -1852,7 +1852,7 @@ int setup_devices_file(struct cmd_context *cmd) * Add all system devices to dev-cache, and attempt to * match all devices_file entries to dev-cache entries. */ -int setup_devices(struct cmd_context *cmd) +static int _setup_devices(struct cmd_context *cmd, int no_file_match) { int file_exists; int lock_mode = 0; @@ -1979,6 +1979,13 @@ int setup_devices(struct cmd_context *cmd) */ dev_cache_scan(cmd); + /* + * The caller uses "no_file_match" if it wants to match specific devs + * itself, instead of matching everything in device_ids_match. + */ + if (no_file_match && cmd->enable_devices_file) + return 1; + /* * Match entries from cmd->use_devices with device structs in dev-cache. */ @@ -1987,6 +1994,16 @@ int setup_devices(struct cmd_context *cmd) return 1; } +int setup_devices(struct cmd_context *cmd) +{ + return _setup_devices(cmd, 0); +} + +int setup_devices_no_file_match(struct cmd_context *cmd) +{ + return _setup_devices(cmd, 1); +} + /* * The alternative to setup_devices() when the command is interested * in using only one PV. @@ -2055,188 +2072,3 @@ int setup_device(struct cmd_context *cmd, const char *devname) return 1; } -/* - * pvscan --cache is specialized/optimized to look only at command args, - * so this just sets up the devices file, then individual devices are - * added to dev-cache and matched with device_ids later in pvscan. - */ - -int setup_devices_for_pvscan_cache(struct cmd_context *cmd) -{ - if (cmd->enable_devices_list) { - if (!_setup_devices_list(cmd)) - return_0; - return 1; - } - - if (!setup_devices_file(cmd)) - return_0; - - if (!cmd->enable_devices_file) - return 1; - - if (!devices_file_exists(cmd)) { - log_debug("Devices file not found, ignoring."); - cmd->enable_devices_file = 0; - return 1; - } - - if (!lock_devices_file(cmd, LOCK_SH)) { - log_error("Failed to lock the devices file to read."); - return 0; - } - - if (!device_ids_read(cmd)) { - log_error("Failed to read the devices file."); - unlock_devices_file(cmd); - return 0; - } - - unlock_devices_file(cmd); - return 1; -} - - -/* Get a device name from a devno. */ - -static char *_get_devname_from_devno(struct cmd_context *cmd, dev_t devno) -{ - char path[PATH_MAX]; - char devname[PATH_MAX]; - char namebuf[NAME_LEN]; - char line[1024]; - int major = MAJOR(devno); - int minor = MINOR(devno); - int line_major; - int line_minor; - uint64_t line_blocks; - DIR *dir; - struct dirent *dirent; - FILE *fp; - - /* - * $ ls /sys/dev/block/8:0/device/block/ - * sda - */ - if (major_is_scsi_device(cmd->dev_types, major)) { - if (dm_snprintf(path, sizeof(path), "%sdev/block/%d:%d/device/block", - dm_sysfs_dir(), major, minor) < 0) { - return NULL; - } - - if (!(dir = opendir(path))) - return NULL; - - while ((dirent = readdir(dir))) { - if (dirent->d_name[0] == '.') - continue; - if (dm_snprintf(devname, sizeof(devname), "/dev/%s", dirent->d_name) < 0) { - devname[0] = '\0'; - stack; - } - break; - } - closedir(dir); - - if (devname[0]) { - log_debug("Found %s for %d:%d from sys", devname, major, minor); - return _strdup(devname); - } - return NULL; - } - - /* - * $ cat /sys/dev/block/253:3/dm/name - * mpatha - */ - if (major == cmd->dev_types->device_mapper_major) { - if (dm_snprintf(path, sizeof(path), "%sdev/block/%d:%d/dm/name", - dm_sysfs_dir(), major, minor) < 0) { - return NULL; - } - - if (!get_sysfs_value(path, namebuf, sizeof(namebuf), 0)) - return NULL; - - if (dm_snprintf(devname, sizeof(devname), "/dev/mapper/%s", namebuf) < 0) { - devname[0] = '\0'; - stack; - } - - if (devname[0]) { - log_debug("Found %s for %d:%d from sys", devname, major, minor); - return _strdup(devname); - } - return NULL; - } - - /* - * /proc/partitions lists - * major minor #blocks name - */ - - if (!(fp = fopen("/proc/partitions", "r"))) - return NULL; - - while (fgets(line, sizeof(line), fp)) { - if (sscanf(line, "%u %u %llu %s", &line_major, &line_minor, (unsigned long long *)&line_blocks, namebuf) != 4) - continue; - if (line_major != major) - continue; - if (line_minor != minor) - continue; - - if (dm_snprintf(devname, sizeof(devname), "/dev/%s", namebuf) < 0) { - devname[0] = '\0'; - stack; - } - break; - } - fclose(fp); - - if (devname[0]) { - log_debug("Found %s for %d:%d from proc", devname, major, minor); - return _strdup(devname); - } - - /* - * If necessary, this could continue searching by stat'ing /dev entries. - */ - - return NULL; -} - -int setup_devname_in_dev_cache(struct cmd_context *cmd, const char *devname) -{ - struct stat buf; - struct device *dev; - - if (stat(devname, &buf) < 0) { - log_error("Cannot access device %s.", devname); - return 0; - } - - if (!S_ISBLK(buf.st_mode)) { - log_error("Invaild device type %s.", devname); - return 0; - } - - if (!_insert_dev(devname, buf.st_rdev)) - return_0; - - if (!(dev = (struct device *) dm_hash_lookup(_cache.names, devname))) - return_0; - - return 1; -} - -int setup_devno_in_dev_cache(struct cmd_context *cmd, dev_t devno) -{ - const char *devname; - - if (!(devname = _get_devname_from_devno(cmd, devno))) - return_0; - - return setup_devname_in_dev_cache(cmd, devname); -} - diff --git a/lib/device/dev-cache.h b/lib/device/dev-cache.h index 143848d6d..635dc4fc9 100644 --- a/lib/device/dev-cache.h +++ b/lib/device/dev-cache.h @@ -77,11 +77,7 @@ int get_dm_uuid_from_sysfs(char *buf, size_t buf_size, int major, int minor); int setup_devices_file(struct cmd_context *cmd); int setup_devices(struct cmd_context *cmd); +int setup_devices_no_file_match(struct cmd_context *cmd); int setup_device(struct cmd_context *cmd, const char *devname); -/* Normal device setup functions are split up for pvscan optimization. */ -int setup_devices_for_pvscan_cache(struct cmd_context *cmd); -int setup_devname_in_dev_cache(struct cmd_context *cmd, const char *devname); -int setup_devno_in_dev_cache(struct cmd_context *cmd, dev_t devno); - #endif diff --git a/lib/device/device_id.c b/lib/device/device_id.c index 167bf661b..eb06109ff 100644 --- a/lib/device/device_id.c +++ b/lib/device/device_id.c @@ -1534,22 +1534,6 @@ int device_ids_match_dev(struct cmd_context *cmd, struct device *dev) * passes the filter. */ -void device_ids_match_device_list(struct cmd_context *cmd) -{ - struct dev_use *du; - - dm_list_iterate_items(du, &cmd->use_devices) { - if (du->dev) - continue; - if (!(du->dev = dev_cache_get(cmd, du->devname, NULL))) { - log_warn("Device not found for %s.", du->devname); - } else { - /* Should we set dev->id? Which idtype? Use --deviceidtype? */ - du->dev->flags |= DEV_MATCHED_USE_ID; - } - } -} - void device_ids_match(struct cmd_context *cmd) { struct dev_iter *iter; @@ -1557,7 +1541,16 @@ void device_ids_match(struct cmd_context *cmd) struct device *dev; if (cmd->enable_devices_list) { - device_ids_match_device_list(cmd); + dm_list_iterate_items(du, &cmd->use_devices) { + if (du->dev) + continue; + if (!(du->dev = dev_cache_get(cmd, du->devname, NULL))) { + log_warn("Device not found for %s.", du->devname); + } else { + /* Should we set dev->id? Which idtype? Use --deviceidtype? */ + du->dev->flags |= DEV_MATCHED_USE_ID; + } + } return; } diff --git a/lib/device/device_id.h b/lib/device/device_id.h index 0ada35c94..939b3a0f4 100644 --- a/lib/device/device_id.h +++ b/lib/device/device_id.h @@ -32,7 +32,6 @@ int device_id_add(struct cmd_context *cmd, struct device *dev, const char *pvid, void device_id_pvremove(struct cmd_context *cmd, struct device *dev); void device_ids_match(struct cmd_context *cmd); int device_ids_match_dev(struct cmd_context *cmd, struct device *dev); -void device_ids_match_device_list(struct cmd_context *cmd); void device_ids_validate(struct cmd_context *cmd, struct dm_list *scanned_devs, int *device_ids_invalid, int noupdate); int device_ids_version_unchanged(struct cmd_context *cmd); void device_ids_find_renamed_devs(struct cmd_context *cmd, struct dm_list *dev_list, int *search_count, int noupdate); diff --git a/test/shell/devicesfile-basic.sh b/test/shell/devicesfile-basic.sh index 9c3455c76..7ba9e2c7f 100644 --- a/test/shell/devicesfile-basic.sh +++ b/test/shell/devicesfile-basic.sh @@ -283,7 +283,7 @@ not ls "$RUNDIR/lvm/pvs_online/$PVID3" # arg in devices list _clear_online_files pvscan --devices "$dev3" --cache -aay "$dev3" -pvscan --devices "$dev4","$dev3" --cache -aay "$dev4" +pvscan --devices "$dev4" --cache -aay "$dev4" check lv_field $vg2/$lv2 lv_active "active" vgchange -an $vg2 diff --git a/tools/pvscan.c b/tools/pvscan.c index 95d593d57..8e2611361 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -857,21 +857,11 @@ static int _get_devs_from_saved_vg(struct cmd_context *cmd, const char *vgname, devno = MKDEV(file_major, file_minor); - if (!setup_devno_in_dev_cache(cmd, devno)) { - log_error_pvscan(cmd, "No device set up for %d:%d PVID %s", file_major, file_minor, pvid); - goto bad; - } - if (!(dev = dev_cache_get_by_devt(cmd, devno, NULL, NULL))) { log_error_pvscan(cmd, "No device found for %d:%d PVID %s", file_major, file_minor, pvid); goto bad; } - /* - * Do not need to match device_id here, see comment after - * get_devs_from_saved_vg about relying on pvid online file. - */ - name1 = dev_name(dev); name2 = pvl->pv->device_hint; @@ -1109,11 +1099,17 @@ static int _pvscan_aa(struct cmd_context *cmd, struct pvscan_aa_params *pp, * PROCESS_SKIP_SCAN: we have already done lvmcache_label_scan * so tell process_each to skip it. */ + if (do_all) + read_flags |= PROCESS_SKIP_SCAN; + /* + * When the command is processing specific devs (not all), it + * has done setup_devices_no_file_match() to avoid matching ids + * fo all devs unnecessarily, but now that we're falling back + * to process_each_vg() we need to complete the id matching. + */ if (!do_all) - lvmcache_label_scan(cmd); - - read_flags |= PROCESS_SKIP_SCAN; + device_ids_match(cmd); ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, read_flags, 0, handle, _pvscan_aa_single); } @@ -1196,15 +1192,11 @@ static int _get_args_devs(struct cmd_context *cmd, struct dm_list *pvscan_args, /* in common usage, no dev will be found for a devno */ dm_list_iterate_items(arg, pvscan_args) { - if (arg->devname) { - if (!setup_devname_in_dev_cache(cmd, arg->devname)) - log_error_pvscan(cmd, "No device set up for name arg %s", arg->devname); + if (arg->devname) arg->dev = dev_cache_get(cmd, arg->devname, NULL); - } else if (arg->devno) { - if (!setup_devno_in_dev_cache(cmd, arg->devno)) - log_error_pvscan(cmd, "No device set up for devno arg %d", (int)arg->devno); + else if (arg->devno) arg->dev = dev_cache_get_by_devt(cmd, arg->devno, NULL, NULL); - } else + else return_0; } @@ -1680,13 +1672,11 @@ static int _pvscan_cache_args(struct cmd_context *cmd, int argc, char **argv, cmd->pvscan_cache_single = 1; /* - * Special pvscan-specific setup steps to avoid looking - * at any devices except for device args. - * Read devices file and determine if devices file will be used. - * Does not do dev_cache_scan (adds nothing to dev-cache), and - * does not do any device id matching. + * "no_file_match" means that when the devices file is used, + * setup_devices will skip matching devs to devices file entries. + * Specific devs must be matched later with device_ids_match_dev(). */ - if (!setup_devices_for_pvscan_cache(cmd)) { + if (!setup_devices_no_file_match(cmd)) { log_error_pvscan(cmd, "Failed to set up devices."); return 0; } @@ -1745,21 +1735,17 @@ static int _pvscan_cache_args(struct cmd_context *cmd, int argc, char **argv, log_debug("pvscan_cache_args: filter devs nodata"); /* - * Match dev args with the devices file because special/optimized - * device setup was used above which does not check the devices file. - * If a match fails here do not exclude it, that will be done below by - * passes_filter() which runs filter-deviceid. The - * relax_deviceid_filter case needs to be able to work around + * Match dev args with the devices file because + * setup_devices_no_file_match() was used above which skipped checking + * the devices file. If a match fails here do not exclude it, that + * will be done below by passes_filter() which runs filter-deviceid. + * The relax_deviceid_filter case needs to be able to work around * unmatching devs. */ - if (cmd->enable_devices_file) { - dm_list_iterate_items(devl, &pvscan_devs) + dm_list_iterate_items_safe(devl, devl2, &pvscan_devs) device_ids_match_dev(cmd, devl->dev); - } - if (cmd->enable_devices_list) - device_ids_match_device_list(cmd); if (cmd->enable_devices_file && device_ids_use_devname(cmd)) { relax_deviceid_filter = 1;