From 3bb02b18eb0750c98dfdd2d0e5d3c4ba3781b3f0 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 | 86 +++++++++++++++++++++++++++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 12 deletions(-) diff --git a/tools/toollib.c b/tools/toollib.c index 1b3883e7f..175581afe 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); } } @@ -2129,11 +2130,11 @@ out: return r; } -static int _device_list_remove(struct dm_list *all_devices, struct device *dev) +static int _device_list_remove(struct dm_list *devices, struct device *dev) { struct device_list *devl; - dm_list_iterate_items(devl, all_devices) { + dm_list_iterate_items(devl, devices) { if (devl->dev == dev) { dm_list_del(&devl->list); return 1; @@ -2200,6 +2201,7 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, process_single_pv_fn_t process_single_pv) { struct physical_volume *pv; + struct device_list *devl, *devl2; struct pv_list *pvl; const char *pv_name; int process_pv; @@ -2250,12 +2252,50 @@ 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. Other + * entries on all_devices may have the same pv id. + * These are duplicates of the current pv and should be + * processed here also. + * + * This is a fairly narrow, special case. Is this loop + * too expensive to check for each pv? Should this + * check for duplicates only be enabled with an option? + */ + + dm_list_iterate_items_safe(devl, devl2, all_devices) { + char id[ID_LEN + 1] __attribute__((aligned(8))); + strncpy(&id[0], devl->dev->pvid, ID_LEN); + id[ID_LEN] = '\0'; + + log_verbose("Check for duplicate of pv %s on dev %s %s", + pv_name, dev_name(devl->dev), id); + + if (!id_equal((struct id *) &devl->dev->pvid, &pv->id)) + continue; + + log_warn("Processing duplicate of PV %s on device %s.", + pv_name, dev_name(devl->dev)); + + /* 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 +2390,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 dm_str_list *sl; struct device_list *devl; int process_all_pvs; int process_all_devices; @@ -2366,25 +2407,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 +2442,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 +2458,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 device for 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 +2476,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