diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 991299561..c306a3aa9 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -981,11 +981,25 @@ int lvmcache_dev_is_unchosen_duplicate(struct device *dev) * The actual filters are evaluated too early, before a complete * picture of all PVs is available, to eliminate these duplicates. * - * By removing the filtered duplicates from unused_duplicate_devs, we remove + * By removing some duplicates from unused_duplicate_devs here, we remove * the restrictions that are placed on using duplicate devs or VGs with * duplicate devs. * - * There may other kinds of duplicates that we want to ignore. + * In cases where we know that two duplicates refer to the same underlying + * storage, and we know which dev path to use, it's best for us to just + * use that one preferred device path and ignore the others. It is the cases + * where we are unsure whether dups refer to the same underlying storage where + * we need to keep the unused duplicate referenced in the + * unused_duplicate_devs list, and restrict what we allow done with it. + * + * In the case of md components, we usually filter these out in filter-md, + * but in the special case of md superblocks <= 1.0 where the superblock + * is at the end of the device, filter-md doesn't always eliminate them + * first, so we eliminate them here. + * + * There may other kinds of duplicates that we want to eliminate at + * this point (using the knowledge from the scan) that we couldn't + * eliminate in the filters prior to the scan. */ static void _filter_duplicate_devs(struct cmd_context *cmd) @@ -1004,6 +1018,34 @@ static void _filter_duplicate_devs(struct cmd_context *cmd) dm_free(devl); } } + + if (dm_list_empty(&_unused_duplicate_devs)) + _found_duplicate_pvs = 0; +} + +static void _warn_duplicate_devs(struct cmd_context *cmd) +{ + char uuid[64] __attribute__((aligned(8))); + struct lvmcache_info *info; + struct device_list *devl, *devl2; + + dm_list_iterate_items_safe(devl, devl2, &_unused_duplicate_devs) { + if (!id_write_format((const struct id *)devl->dev->pvid, uuid, sizeof(uuid))) + stack; + + log_warn("WARNING: Not using device %s for PV %s.", dev_name(devl->dev), uuid); + } + + dm_list_iterate_items_safe(devl, devl2, &_unused_duplicate_devs) { + /* info for the preferred device that we're actually using */ + info = lvmcache_info_from_pvid(devl->dev->pvid, NULL, 0); + + if (!id_write_format((const struct id *)info->dev->pvid, uuid, sizeof(uuid))) + stack; + + log_warn("WARNING: PV %s prefers device %s because %s.", + uuid, dev_name(info->dev), info->dev->duplicate_prefer_reason); + } } /* @@ -1028,7 +1070,6 @@ static void _choose_preferred_devs(struct cmd_context *cmd, struct dm_list *del_cache_devs, struct dm_list *add_cache_devs) { - char uuid[64] __attribute__((aligned(8))); const char *reason; struct dm_list altdevs; struct dm_list new_unused; @@ -1229,9 +1270,7 @@ next: alt = devl; } - if (!id_write_format((const struct id *)dev1->pvid, uuid, sizeof(uuid))) - stack; - log_warn("WARNING: PV %s prefers device %s because %s.", uuid, dev_name(dev1), reason); + dev1->duplicate_prefer_reason = reason; } if (dev1 != info->dev) { @@ -1480,11 +1519,21 @@ int lvmcache_label_scan(struct cmd_context *cmd) dm_list_splice(&_unused_duplicate_devs, &del_cache_devs); /* - * We might want to move the duplicate device warnings until - * after this filtering so that we can skip warning about - * duplicates that we are filtering out. + * This may remove some entries from the unused_duplicates list for + * devs that we know are the same underlying dev. */ _filter_duplicate_devs(cmd); + + /* + * Warn about remaining duplicates that may actually be separate copies of + * the same device. + */ + _warn_duplicate_devs(cmd); + + if (!_found_duplicate_pvs && lvmetad_used()) { + log_warn("WARNING: Disabling lvmetad cache which does not support duplicate PVs."); + lvmetad_set_disabled(cmd, LVMETAD_DISABLE_REASON_DUPLICATES); + } } /* Perform any format-specific scanning e.g. text files */ @@ -1509,6 +1558,53 @@ int lvmcache_label_scan(struct cmd_context *cmd) return r; } +/* + * When not using lvmetad, lvmcache_label_scan() detects duplicates in + * the basic label_scan(), then filters out some dups, and chooses + * preferred duplicates to use. + * + * When using lvmetad, pvscan --cache does not use lvmcache_label_scan(), + * only label_scan() which detects the duplicates. This function is used + * after pvscan's label_scan() to filter out some dups, print any warnings, + * and disable lvmetad if any dups are left. + */ + +void lvmcache_pvscan_duplicate_check(struct cmd_context *cmd) +{ + struct device_list *devl; + + /* Check if label_scan() detected any dups. */ + if (!_found_duplicate_pvs) + return; + + /* + * Once all the dups are identified, they are moved from the + * "found" list to the "unused" list to sort out. + */ + dm_list_splice(&_unused_duplicate_devs, &_found_duplicate_devs); + + /* + * Remove items from the dups list that we know are the same + * underlying dev, e.g. md components, that we want to just ignore. + */ + _filter_duplicate_devs(cmd); + + /* + * If no more dups after ignoring some, then we can use lvmetad. + */ + if (!_found_duplicate_pvs) + return; + + /* Duplicates are found where we would have to pick one, so disable lvmetad. */ + + dm_list_iterate_items(devl, &_unused_duplicate_devs) + log_warn("WARNING: found device with duplicate %s", dev_name(devl->dev)); + + log_warn("WARNING: Disabling lvmetad cache which does not support duplicate PVs."); + lvmetad_set_disabled(cmd, LVMETAD_DISABLE_REASON_DUPLICATES); + lvmetad_make_unused(cmd); +} + int lvmcache_get_vgnameids(struct cmd_context *cmd, int include_internal, struct dm_list *vgnameids) { @@ -2303,14 +2399,8 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, */ if (!created) { if (info->dev != dev) { - log_warn("WARNING: PV %s on %s was already found on %s.", - uuid, dev_name(dev), dev_name(info->dev)); - - if (!_found_duplicate_pvs && lvmetad_used()) { - log_warn("WARNING: Disabling lvmetad cache which does not support duplicate PVs."); - lvmetad_set_disabled(labeller->fmt->cmd, LVMETAD_DISABLE_REASON_DUPLICATES); - } - _found_duplicate_pvs = 1; + log_debug_cache("PV %s on %s was already found on %s.", + uuid, dev_name(dev), dev_name(info->dev)); strncpy(dev->pvid, pvid_s, sizeof(dev->pvid)); @@ -2328,6 +2418,7 @@ struct lvmcache_info *lvmcache_add(struct labeller *labeller, devl->dev = dev; dm_list_add(&_found_duplicate_devs, &devl->list); + _found_duplicate_pvs = 1; return NULL; } diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index a2a7f07e6..b988be66c 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -188,6 +188,8 @@ uint64_t lvmcache_smallest_mda_size(struct lvmcache_info *info); int lvmcache_found_duplicate_pvs(void); +void lvmcache_pvscan_duplicate_check(struct cmd_context *cmd); + int lvmcache_get_unused_duplicate_devs(struct cmd_context *cmd, struct dm_list *head); int vg_has_duplicate_pvs(struct volume_group *vg); diff --git a/lib/cache/lvmetad.c b/lib/cache/lvmetad.c index 235f72b6f..a1ab41aab 100644 --- a/lib/cache/lvmetad.c +++ b/lib/cache/lvmetad.c @@ -2350,6 +2350,8 @@ int lvmetad_pvscan_all_devs(struct cmd_context *cmd, int do_wait) label_scan(cmd); + lvmcache_pvscan_duplicate_check(cmd); + if (lvmcache_found_duplicate_pvs()) { log_warn("WARNING: Scan found duplicate PVs."); return 0; diff --git a/lib/device/device.h b/lib/device/device.h index 42659cd0f..c8ccd73d9 100644 --- a/lib/device/device.h +++ b/lib/device/device.h @@ -75,6 +75,7 @@ struct device { uint64_t size; uint64_t end; struct dev_ext ext; + const char *duplicate_prefer_reason; const char *vgid; /* if device is an LV */ const char *lvid; /* if device is an LV */ diff --git a/lib/filters/filter-md.c b/lib/filters/filter-md.c index bb8a7cf42..ab97b5946 100644 --- a/lib/filters/filter-md.c +++ b/lib/filters/filter-md.c @@ -20,13 +20,77 @@ #define MSG_SKIPPING "%s: Skipping md component device" -static int _ignore_md(struct device *dev, int full) +/* + * The purpose of these functions is to ignore md component devices, + * e.g. if /dev/md0 is a raid1 composed of /dev/loop0 and /dev/loop1, + * lvm wants to deal with md0 and ignore loop0 and loop1. md0 should + * pass the filter, and loop0,loop1 should not pass the filter so lvm + * will ignore them. + * + * (This is assuming lvm.conf md_component_detection=1.) + * + * If lvm does *not* ignore the components, then lvm will read lvm + * labels from the md dev and from the component devs, and will see + * them all as duplicates of each other. LVM duplicate resolution + * will then kick in and keep the md dev around to use and ignore + * the components. + * + * It is better to exclude the components as early as possible during + * lvm processing, ideally before lvm even looks for labels on the + * components, so that duplicate resolution can be avoided. There are + * a number of ways that md components can be excluded earlier than + * the duplicate resolution phase: + * + * - When external_device_info_source="udev", lvm discovers a device is + * an md component by asking udev during the initial filtering phase. + * However, lvm's default is to not use udev for this. The + * alternative is "native" detection in which lvm tries to detect + * md components itself. + * + * - When using native detection, lvm's md filter looks for the md + * superblock at the start of devices. It will see the md superblock + * on the components, exclude them in the md filter, and avoid + * handling them later in duplicate resolution. + * + * - When using native detection, lvm's md filter will not detect + * components when the md device has an older superblock version that + * places the superblock at the end of the device. This case will + * fall back to duplicate resolution to exclude components. + * + * A variation of the description above occurs for lvm commands that + * intend to create new PVs on devices (pvcreate, vgcreate, vgextend). + * For these commands, the native md filter also reads the end of all + * devices to check for the odd md superblocks. + * + * (The reason that external_device_info_source is not set to udev by + * default is that there have be issues with udev not being promptly + * or reliably updated about md state changes, causing the udev info + * that lvm uses to be occasionally wrong.) + */ + +/* + * Returns 0 if: + * the device is an md component and it should be ignored. + * + * Returns 1 if: + * the device is not md component and should not be ignored. + * + * The actual md device will pass this filter and should be used, + * it is the md component devices that we are trying to exclude + * that will not pass. + */ + +static int _passes_md_filter(struct device *dev, int full) { int ret; - + + /* + * When md_component_dectection=0, don't even try to skip md + * components. + */ if (!md_filtering()) return 1; - + ret = dev_is_md(dev, NULL, full); if (ret == -EAGAIN) { @@ -36,6 +100,9 @@ static int _ignore_md(struct device *dev, int full) return 1; } + if (ret == 0) + return 1; + if (ret == 1) { if (dev->ext.src == DEV_EXT_NONE) log_debug_devs(MSG_SKIPPING, dev_name(dev)); @@ -54,16 +121,16 @@ static int _ignore_md(struct device *dev, int full) return 1; } -static int _ignore_md_lite(struct dev_filter *f __attribute__((unused)), - struct device *dev) +static int _passes_md_filter_lite(struct dev_filter *f __attribute__((unused)), + struct device *dev) { - return _ignore_md(dev, 0); + return _passes_md_filter(dev, 0); } -static int _ignore_md_full(struct dev_filter *f __attribute__((unused)), - struct device *dev) +static int _passes_md_filter_full(struct dev_filter *f __attribute__((unused)), + struct device *dev) { - return _ignore_md(dev, 1); + return _passes_md_filter(dev, 1); } static void _destroy(struct dev_filter *f) @@ -91,9 +158,9 @@ struct dev_filter *md_filter_create(struct cmd_context *cmd, struct dev_types *d */ if (cmd->use_full_md_check) - f->passes_filter = _ignore_md_full; + f->passes_filter = _passes_md_filter_full; else - f->passes_filter = _ignore_md_lite; + f->passes_filter = _passes_md_filter_lite; f->destroy = _destroy; f->use_count = 0; diff --git a/test/shell/process-each-duplicate-pvs.sh b/test/shell/process-each-duplicate-pvs.sh index 98dd285da..9ce4e142b 100644 --- a/test/shell/process-each-duplicate-pvs.sh +++ b/test/shell/process-each-duplicate-pvs.sh @@ -72,7 +72,7 @@ not grep duplicate main not grep $vg2 main not grep $UUID2 main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn # Find which is the preferred dev and which is the duplicate. @@ -119,7 +119,7 @@ grep "$dev2" main | grep $vg1 grep "$dev1" main | grep $UUID1 grep "$dev2" main | grep $UUID1 -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn # @@ -136,7 +136,7 @@ grep "$dev1" main not grep "$dev2" main grep "$UUID1" main grep "$vg1" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn pvs -o+uuid "$dev2" 2>&1 | tee out @@ -149,7 +149,7 @@ grep "$dev2" main not grep "$dev1" main grep "$UUID1" main grep "$vg1" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn pvs -o+uuid,duplicate "$dev1" "$dev2" 2>&1 | tee out @@ -225,7 +225,7 @@ grep -v WARNING out > main || true not grep "$dev1" main grep "$dev2" main -not grep "was already found on" warn +not grep "Not using device" warn not grep "prefers device" warn @@ -238,7 +238,7 @@ grep -v WARNING out > main || true grep "$dev1" main not grep "$dev2" main -not grep "was already found on" warn +not grep "Not using device" warn not grep "prefers device" warn # PV size and minor is still reported correctly for each. @@ -306,7 +306,7 @@ grep -v WARNING out > main || true test "$(grep -c "$UUID3" main)" -eq 1 not grep "$UUID4" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn # Both appear with 'pvs -a' @@ -325,7 +325,7 @@ grep "$dev4" main grep $UUID3 main not grep $UUID4 main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn # Show each dev individually and both together @@ -339,7 +339,7 @@ grep -v WARNING out > main || true grep "$dev3" main not grep "$dev4" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn pvs -o+uuid "$dev4" 2>&1 | tee out @@ -351,7 +351,7 @@ grep -v WARNING out > main || true not grep "$dev3" main grep "$dev4" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn pvs -o+uuid "$dev3" "$dev4" 2>&1 | tee out @@ -363,7 +363,7 @@ grep -v WARNING out > main || true grep "$dev3" main grep "$dev4" main -grep "was already found on" warn +grep "Not using device" warn grep "prefers device" warn # Same sizes shown. diff --git a/test/shell/pv-duplicate-uuid.sh b/test/shell/pv-duplicate-uuid.sh index 43eb830b7..2c121d747 100644 --- a/test/shell/pv-duplicate-uuid.sh +++ b/test/shell/pv-duplicate-uuid.sh @@ -26,7 +26,6 @@ pvcreate --config "devices{filter=[\"a|$dev3|\",\"r|.*|\"]} global/use_lvmetad=0 pvscan --cache 2>&1 | tee out if test -e LOCAL_LVMETAD; then - grep "was already found" out grep "WARNING: Disabling lvmetad cache which does not support duplicate PVs." out fi @@ -37,7 +36,7 @@ grep -v WARNING out > main || true test "$(grep -c $UUID1 main)" -eq 1 -COUNT=$(grep --count "was already found" warn) +COUNT=$(grep --count "Not using device" warn) [ "$COUNT" -eq 2 ] pvs -o+uuid --config "devices{filter=[\"a|$dev2|\",\"r|.*|\"]}" 2>&1 | tee out @@ -50,5 +49,5 @@ not grep "$dev1" main grep "$dev2" main not grep "$dev3" main -not grep "was already found" warn +not grep "Not using device" warn