toollib: handle duplicate pvs in process_each_pvdev-dct-fix-process-pv

WIP

1 files changed, 62 insertions, 10 deletions

diff --git a/tools/toollib.c b/tools/toollib.c
index 1b3883e7f..9d375d4aa 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -2079,19 +2079,20 @@ static int _get_arg_devices(struct cmd_context *cmd,
 			    struct dm_list *arg_devices)
 {
 	struct dm_str_list *sl;
+	struct dm_str_list *sl2;
 	struct device_list *devl;
 	int ret_max = ECMD_PROCESSED;
 
-	dm_list_iterate_items(sl, arg_pvnames) {
+	dm_list_iterate_items_safe(sl, sl2, arg_pvnames) {
 		if (!(devl = dm_pool_alloc(cmd->mem, sizeof(*devl)))) {
 			log_error("device_list alloc failed.");
 			return ECMD_FAILED;
 		}
 
 		if (!(devl->dev = dev_cache_get(sl->str, cmd->filter))) {
-			log_error("Failed to find physical volume \"%s\".", sl->str);
 			ret_max = ECMD_FAILED;
 		} else {
+			dm_list_del(&sl->list);
 			dm_list_add(arg_devices, &devl->list);
 		}
 	}
@@ -2199,7 +2200,9 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 			      void *handle,
 			      process_single_pv_fn_t process_single_pv)
 {
+	struct lvmcache_info *info;
 	struct physical_volume *pv;
+	struct device_list *devl, *devl2;
 	struct pv_list *pvl;
 	const char *pv_name;
 	int process_pv;
@@ -2250,12 +2253,39 @@ static int _process_pvs_in_vg(struct cmd_context *cmd,
 				continue;
 			}
 
-			if (!skip) {
+			if (skip)
+				continue;
+
+			ret = process_single_pv(cmd, vg, pv, handle);
+			if (ret != ECMD_PROCESSED)
+				stack;
+			if (ret > ret_max)
+				ret_max = ret;
+
+			/*
+			 * Search for and process duplicates of this PV.
+			 * This is a fairly narrow, special case.  Other entries on
+			 * all_devices may have the same pv id.  These are duplicates
+			 * of the current pv and should be processed here also.
+			 * FIXME: Is this loop too expensive to check for each pv?
+			 */
+			dm_list_iterate_items_safe(devl, devl2, all_devices) {
+				if (!id_equal((struct id *) &devl->dev->pvid, &pv->id))
+					continue;
+
+				log_verbose("Processing duplicate PV %s in VG %s.",
+					    pv_name, vg->name);
+
+				/* Change pv->dev to devl->dev? */
+
 				ret = process_single_pv(cmd, vg, pv, handle);
 				if (ret != ECMD_PROCESSED)
 					stack;
 				if (ret > ret_max)
 					ret_max = ret;
+
+				_device_list_remove(all_devices, devl->dev);
+				_device_list_remove(arg_devices, devl->dev);
 			}
 		}
 
@@ -2350,6 +2380,7 @@ int process_each_pv(struct cmd_context *cmd,
 	struct dm_list arg_devices;	/* device_list */
 	struct dm_list all_vgnameids;	/* vgnameid_list */
 	struct dm_list all_devices;	/* device_list */
+	struct str_list *sl;
 	struct device_list *devl;
 	int process_all_pvs;
 	int process_all_devices;
@@ -2366,25 +2397,30 @@ int process_each_pv(struct cmd_context *cmd,
 	 * Create two lists from argv:
 	 * arg_pvnames: pvs explicitly named in argv
 	 * arg_tags: tags explicitly named in argv
-	 *
-	 * Then convert arg_pvnames, which are free-form, user-specified,
-	 * names/paths into arg_devices which can be used to match below.
 	 */
 	if ((ret = _get_arg_pvnames(cmd, argc, argv, &arg_pvnames, &arg_tags)) != ECMD_PROCESSED) {
 		stack;
 		return ret;
 	}
 
+	/*
+	 * It's important to check for empty arg_pvnames before we resolve the
+	 * pv names into devices and remove arg_pvnames entries.
+	 */
 	process_all_pvs = dm_list_empty(&arg_pvnames) && dm_list_empty(&arg_tags);
 
 	process_all_devices = process_all_pvs &&
 			   (cmd->command->flags & ENABLE_ALL_DEVS) &&
 			   arg_count(cmd, all_ARG);
 
-	if ((ret = _get_arg_devices(cmd, &arg_pvnames, &arg_devices) != ECMD_PROCESSED)) {
-		/* get_arg_devices reports the error for any PV names not found. */
-		ret_max = ECMD_FAILED;
-	}
+	/*
+	 * Resolve arg_pvnames, which are free-form, user-specified,
+	 * names/paths into arg_devices which can be used to match below.
+	 * arg_pvnames entries are removed as corresponding devices are found.
+	 * Any pv names remaining in arg_pvnames were not found and will be
+	 * reported at the end.
+	 */
+	_get_arg_devices(cmd, &arg_pvnames, &arg_devices);
 
 	/*
 	 * If the caller wants to process all devices (not just PVs), then all PVs
@@ -2396,6 +2432,9 @@ int process_each_pv(struct cmd_context *cmd,
 		return ret;
 	}
 
+	/*
+	 * Get all VGs on the system.  PVs are first processed per VG.
+	 */
 	if ((ret = _get_vgnameids_on_system(cmd, &all_vgnameids, only_this_vgname, 1) != ECMD_PROCESSED)) {
 		stack;
 		return ret;
@@ -2409,6 +2448,16 @@ int process_each_pv(struct cmd_context *cmd,
 	if (ret > ret_max)
 		ret_max = ret;
 
+	/*
+	 * PVs and devices that were not found in a VG remain on arg_pvnames
+	 * and arg_devices lists.
+	 */
+
+	dm_list_iterate_items(sl, &arg_pvnames) {
+		log_error("Failed to find physical volume \"%s\".", sl->str);
+		ret_max = ECMD_FAILED;
+	}
+
 	dm_list_iterate_items(devl, &arg_devices) {
 		log_error("Failed to find physical volume \"%s\".", dev_name(devl->dev));
 		ret_max = ECMD_FAILED;
@@ -2417,6 +2466,9 @@ int process_each_pv(struct cmd_context *cmd,
 	if (!process_all_devices)
 		goto out;
 
+	/*
+	 * The command wants to process all devices.
+	 */
 	ret = _process_device_list(cmd, &all_devices, handle, process_single_pv);
 	if (ret != ECMD_PROCESSED)
 		stack;