From 853b7c91a095165920ad8f7216337721e2d8c016 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Wed, 7 Jan 2015 16:16:53 -0600 Subject: toollib: handle duplicate pvs in process_each_pv WIP --- tools/toollib.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file 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; -- cgit v1.2.1