From c1f246fedfc349c25749da501e68a7f70bd122b0 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Fri, 9 Jan 2015 14:55:16 -0600 Subject: [PATCH] toollib: handle duplicate pvs in process_in_pv Processes a PV once for each time a device with its PV ID exists on the command line. This fixes a regression in the case where: . devices /dev/sdA and /dev/sdB where clones (same PV ID) . the cached VG references /dev/sdA . before the regression, the command: pvs /dev/sdB would display the cached device clone /dev/sdA . after the regression, pvs /dev/sdB would display nothing, causing vgimportclone /dev/sdB to fail. . with this fix, pvs /dev/sdB displays /dev/sdA Also, pvs /dev/sdA /dev/sdB will report two lines, one for each device on the command line, but /dev/sdA is displayed for each. This only works without lvmetad. --- lib/cache/lvmcache.c | 2 +- tools/toollib.c | 139 +++++++++++++++++++++++++++++++------------ 2 files changed, 101 insertions(+), 40 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 416907e99..932b1ca33 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -1539,7 +1539,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid, //else if (dm_is_dm_major(MAJOR(existing->dev->dev)) && //dm_is_dm_major(MAJOR(dev->dev))) // - else if (!strcmp(pvid_s, existing->dev->pvid)) + else if (!strcmp(pvid_s, existing->dev->pvid)) log_error("Found duplicate PV %s: using %s not " "%s", pvid, dev_name(dev), dev_name(existing->dev)); diff --git a/tools/toollib.c b/tools/toollib.c index 1b3883e7f..c534e89d2 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -18,6 +18,12 @@ #include #include +struct device_id_list { + struct dm_list list; + struct device *dev; + char pvid[ID_LEN + 1]; +}; + const char *command_name(struct cmd_context *cmd) { return cmd->command->name; @@ -2079,20 +2085,21 @@ static int _get_arg_devices(struct cmd_context *cmd, struct dm_list *arg_devices) { struct dm_str_list *sl; - struct device_list *devl; + struct device_id_list *dil; int ret_max = ECMD_PROCESSED; dm_list_iterate_items(sl, arg_pvnames) { - if (!(devl = dm_pool_alloc(cmd->mem, sizeof(*devl)))) { - log_error("device_list alloc failed."); + if (!(dil = dm_pool_alloc(cmd->mem, sizeof(*dil)))) { + log_error("device_id_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); + if (!(dil->dev = dev_cache_get(sl->str, cmd->filter))) { + log_error("Failed to find device for physical volume \"%s\".", sl->str); ret_max = ECMD_FAILED; } else { - dm_list_add(arg_devices, &devl->list); + strncpy(dil->pvid, dil->dev->pvid, ID_LEN); + dm_list_add(arg_devices, &dil->list); } } @@ -2103,7 +2110,7 @@ static int _get_all_devices(struct cmd_context *cmd, struct dm_list *all_devices { struct dev_iter *iter; struct device *dev; - struct device_list *devl; + struct device_id_list *dil; int r = ECMD_FAILED; lvmcache_seed_infos_from_lvmetad(cmd); @@ -2114,13 +2121,13 @@ static int _get_all_devices(struct cmd_context *cmd, struct dm_list *all_devices } while ((dev = dev_iter_get(iter))) { - if (!(devl = dm_pool_alloc(cmd->mem, sizeof(*devl)))) { - log_error("device_list alloc failed."); + if (!(dil = dm_pool_alloc(cmd->mem, sizeof(*dil)))) { + log_error("device_id_list alloc failed."); goto out; } - devl->dev = dev; - dm_list_add(all_devices, &devl->list); + dil->dev = dev; + dm_list_add(all_devices, &dil->list); } r = ECMD_PROCESSED; @@ -2129,13 +2136,13 @@ 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; + struct device_id_list *dil; - dm_list_iterate_items(devl, all_devices) { - if (devl->dev == dev) { - dm_list_del(&devl->list); + dm_list_iterate_items(dil, devices) { + if (dil->dev == dev) { + dm_list_del(&dil->list); return 1; } } @@ -2143,16 +2150,28 @@ static int _device_list_remove(struct dm_list *all_devices, struct device *dev) return 0; } -static int _device_list_match(struct dm_list *devices, struct device *dev) +static struct device_id_list *_device_list_find_dev(struct dm_list *devices, struct device *dev) { - struct device_list *devl; + struct device_id_list *dil; - dm_list_iterate_items(devl, devices) { - if (devl->dev == dev) - return 1; + dm_list_iterate_items(dil, devices) { + if (dil->dev == dev) + return dil; } - return 0; + return NULL; +} + +static struct device_id_list *_device_list_find_pvid(struct dm_list *devices, struct physical_volume *pv) +{ + struct device_id_list *dil; + + dm_list_iterate_items(dil, devices) { + if (id_equal((struct id *) dil->pvid, &pv->id)) + return dil; + } + + return NULL; } static int _process_device_list(struct cmd_context *cmd, struct dm_list *all_devices, @@ -2160,7 +2179,7 @@ static int _process_device_list(struct cmd_context *cmd, struct dm_list *all_dev { struct physical_volume pv_dummy; struct physical_volume *pv; - struct device_list *devl; + struct device_id_list *dil; int ret_max = ECMD_PROCESSED; int ret = 0; @@ -2168,17 +2187,17 @@ static int _process_device_list(struct cmd_context *cmd, struct dm_list *all_dev * Pretend that each device is a PV with dummy values. * FIXME Formalise this extension or find an alternative. */ - dm_list_iterate_items(devl, all_devices) { + dm_list_iterate_items(dil, all_devices) { if (sigint_caught()) return_ECMD_FAILED; memset(&pv_dummy, 0, sizeof(pv_dummy)); dm_list_init(&pv_dummy.tags); dm_list_init(&pv_dummy.segments); - pv_dummy.dev = devl->dev; + pv_dummy.dev = dil->dev; pv = &pv_dummy; - log_very_verbose("Processing device %s.", dev_name(devl->dev)); + log_very_verbose("Processing device %s.", dev_name(dil->dev)); ret = process_single_pv(cmd, NULL, pv, handle); @@ -2201,6 +2220,7 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, { struct physical_volume *pv; struct pv_list *pvl; + struct device_id_list *dil; const char *pv_name; int process_pv; int dev_found; @@ -2219,9 +2239,17 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, /* Remove each arg_devices entry as it is processed. */ if (!process_pv && !dm_list_empty(arg_devices) && - _device_list_match(arg_devices, pv->dev)) { + (dil = _device_list_find_dev(arg_devices, pv->dev))) { process_pv = 1; - _device_list_remove(arg_devices, pv->dev); + _device_list_remove(arg_devices, dil->dev); + } + + /* Select the PV if the device arg has the same pvid. */ + + if (!process_pv && !dm_list_empty(arg_devices) && + (dil = _device_list_find_pvid(arg_devices, pv))) { + process_pv = 1; + _device_list_remove(arg_devices, dil->dev); } if (!process_pv && !dm_list_empty(arg_tags) && @@ -2257,6 +2285,35 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, if (ret > ret_max) ret_max = ret; } + + /* + * This is a very rare and obscure case where multiple + * duplicate devices are specified on the command line + * referring to this PV. In this case we want to + * process this PV once for each specified device. + */ + + if (!skip && !dm_list_empty(arg_devices)) { + while ((dil = _device_list_find_pvid(arg_devices, pv))) { + _device_list_remove(arg_devices, dil->dev); + + /* + * This will simply display the same PV + * multiple times. Further changes would + * be needed to make this display the details + * of dil->dev instead of pv->dev. + */ + + log_very_verbose("Processing PV %s device %s in VG %s.", + pv_name, dev_name(dil->dev), vg->name); + + ret = process_single_pv(cmd, vg, pv, handle); + if (ret != ECMD_PROCESSED) + stack; + if (ret > ret_max) + ret_max = ret; + } + } } /* @@ -2278,7 +2335,7 @@ static int _process_pvs_in_vg(struct cmd_context *cmd, * Each PV is removed from arg_devices and all_devices when it is * processed. Any names remaining in arg_devices were not found, and * should produce an error. Any devices remaining in all_devices were - * not found and should be processed by process_all_devices(). + * not found and should be processed by process_device_list(). */ static int _process_pvs_in_vgs(struct cmd_context *cmd, uint32_t flags, struct dm_list *all_vgnameids, @@ -2347,10 +2404,10 @@ int process_each_pv(struct cmd_context *cmd, { struct dm_list arg_tags; /* str_list */ struct dm_list arg_pvnames; /* str_list */ - struct dm_list arg_devices; /* device_list */ + struct dm_list arg_devices; /* device_id_list */ struct dm_list all_vgnameids; /* vgnameid_list */ - struct dm_list all_devices; /* device_list */ - struct device_list *devl; + struct dm_list all_devices; /* device_id_list */ + struct device_id_list *dil; int process_all_pvs; int process_all_devices; int ret_max = ECMD_PROCESSED; @@ -2362,6 +2419,15 @@ int process_each_pv(struct cmd_context *cmd, dm_list_init(&all_vgnameids); dm_list_init(&all_devices); + /* + * Read all the vgs first because this has the effect of initializing + * other device/lvmcache info that is needed when creating device lists. + */ + if ((ret = _get_vgnameids_on_system(cmd, &all_vgnameids, only_this_vgname, 1) != ECMD_PROCESSED)) { + stack; + return ret; + } + /* * Create two lists from argv: * arg_pvnames: pvs explicitly named in argv @@ -2396,11 +2462,6 @@ int process_each_pv(struct cmd_context *cmd, return ret; } - if ((ret = _get_vgnameids_on_system(cmd, &all_vgnameids, only_this_vgname, 1) != ECMD_PROCESSED)) { - stack; - return ret; - } - ret = _process_pvs_in_vgs(cmd, flags, &all_vgnameids, &all_devices, &arg_devices, &arg_tags, process_all_pvs, handle, process_single_pv); @@ -2409,8 +2470,8 @@ int process_each_pv(struct cmd_context *cmd, if (ret > ret_max) ret_max = ret; - dm_list_iterate_items(devl, &arg_devices) { - log_error("Failed to find physical volume \"%s\".", dev_name(devl->dev)); + dm_list_iterate_items(dil, &arg_devices) { + log_error("Failed to find physical volume \"%s\".", dev_name(dil->dev)); ret_max = ECMD_FAILED; } -- 2.43.5