From 2091305b796d5552fd991c527a0359a0b4d8fde0 Mon Sep 17 00:00:00 2001

From: David Teigland <teigland@redhat.com>

Date: Mon, 20 Dec 2021 13:38:23 -0600

Subject: [PATCH 02/54] 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;

-- 

2.34.3