From e38d974b5036ee4beca9cf97fd40c1ad2d0d6d54 Mon Sep 17 00:00:00 2001 From: David Teigland Date: Fri, 3 Nov 2023 15:15:59 -0500 Subject: [PATCH] lvmdevices: handle empty fields in new check and update Expand the recent commit 37773c1055913 "lvmdevices: new output and options for check and update" to specifically cover entries with empty fields. --- test/shell/devicesfile-misc.sh | 186 +++++++++++++++++++++++++++++++++ tools/lvmdevices.c | 59 +++++++++-- 2 files changed, 234 insertions(+), 11 deletions(-) diff --git a/test/shell/devicesfile-misc.sh b/test/shell/devicesfile-misc.sh index 109209f13..75d79f819 100644 --- a/test/shell/devicesfile-misc.sh +++ b/test/shell/devicesfile-misc.sh @@ -444,6 +444,192 @@ grep IDTYPE=sys_wwid out1 grep IDNAME=$WWID1 out1 lvmdevices --check +# +# lvmdevices --check|--update with empty fields +# + +# PV with wwid, set DEVNAME=. PVID=. +sed -e "s|DEVNAME=$dev1|DEVNAME=.|" orig > tmp1 +sed -e "s|PVID=$PVID1|PVID=.|" tmp1 > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID1 out +grep DEVNAME=$dev1 out +grep "old none" out +lvmdevices --update +grep PVID=$PVID1 "$DF" |tee out +grep DEVNAME=$dev1 out +lvmdevices --check + +# PV with wwid, set DEVNAME=. +sed -e "s|DEVNAME=$dev1|DEVNAME=.|" orig > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID1 out +grep DEVNAME=$dev1 out +grep "old none" out +lvmdevices --update +grep PVID=$PVID1 "$DF" |tee out +grep DEVNAME=$dev1 out +lvmdevices --check + +# PV with wwid, set PVID=. +sed -e "s|PVID=$PVID1|PVID=.|" orig > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID1 out +grep DEVNAME=$dev1 out +grep "old none" out +lvmdevices --update +grep PVID=$PVID1 "$DF" |tee out +grep DEVNAME=$dev1 out +lvmdevices --check + +# PV with wwid, set IDNAME=. DEVNAME=. +sed -e "s|IDNAME=$WWID1|IDNAME=.|" orig > tmp1 +sed -e "s|DEVNAME=$dev1|DEVNAME=.|" tmp1 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep PVID=$PVID1 out |tee out1 +grep 'device not found' out1 +not lvmdevices --check --refresh |tee out +grep PVID=$PVID1 out |tee out1 +grep IDNAME=$WWID1 out1 +grep DEVNAME=$dev1 out1 +grep "old none" out1 +lvmdevices --update --refresh +grep PVID=$PVID1 "$DF" |tee out +grep IDNAME=$WWID1 out +grep DEVNAME=$dev1 out +lvmdevices --check + +# PV with wwid, set IDNAME=. +sed -e "s|IDNAME=$WWID1|IDNAME=.|" orig > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep PVID=$PVID1 out |tee out1 +grep 'device not found' out1 +not lvmdevices --check --refresh |tee out +grep PVID=$PVID1 out |tee out1 +grep IDNAME=$WWID1 out1 +grep DEVNAME=$dev1 out1 +grep "old none" out1 +lvmdevices --update --refresh +grep PVID=$PVID1 "$DF" |tee out +grep IDNAME=$WWID1 out +grep DEVNAME=$dev1 out +lvmdevices --check + +# PV without wwid, set IDNAME=. DEVNAME=. +sed -e "s|IDNAME=$dev3|IDNAME=.|" orig > tmp1 +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" tmp1 > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID3 out |tee out1 +grep IDNAME=$dev3 out1 +grep DEVNAME=$dev3 out1 +grep "old none" out1 +lvmdevices --update +grep PVID=$PVID3 "$DF" |tee out +grep IDNAME=$dev3 out +grep DEVNAME=$dev3 out +lvmdevices --check + +# PV without wwid, set IDNAME=. +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" orig > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID3 out |tee out1 +grep IDNAME=$dev3 out1 +grep DEVNAME=$dev3 out1 +grep "old none" out1 +lvmdevices --update +grep PVID=$PVID3 "$DF" |tee out +grep IDNAME=$dev3 out +grep DEVNAME=$dev3 out +lvmdevices --check + +# PV without wwid, set DEVNAME=. +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" orig > "$DF" +cat "$DF" +not lvmdevices --check |tee out +grep PVID=$PVID3 out |tee out1 +grep IDNAME=$dev3 out1 +grep DEVNAME=$dev3 out1 +grep "old none" out1 +lvmdevices --update +grep PVID=$PVID3 "$DF" |tee out +grep IDNAME=$dev3 out +grep DEVNAME=$dev3 out +lvmdevices --check + +# Without a wwid or a pvid, an entry is indeterminate; there's not enough +# info to definitively know what the entry refers to. These entries will +# never be useful and should be removed. It could be argued that a +# devname entry with a valid device name set in IDNAME and/or DEVNAME +# should be updated with whatever PVID happens to be found on that device. +# By that logic, any PV that's found on that device would be used by lvm, +# and that violates a central rule of the devices file: that lvm will not +# use a PV unless it's definitively identified in the devices file. For +# example, consider a PV and a clone of that PV on a different device. +# The devices file should guarantee that the correct PV and not the clone +# will be used. The user needs to intervene to select one if lvm cannot +# tell the difference. + +# PV without wwid, set PVID=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# PV without wwid, set PVID=. DEVNAME=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" tmp1 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# PV without wwid, set PVID=. IDNAME=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|IDNAME=$dev3|IDNAME=.|" tmp1 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# PV without wwid, set PVID=. IDNAME=. DEVNAME=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|IDNAME=$dev3|IDNAME=.|" tmp1 > tmp2 +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" tmp2 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# indeterminate cases with additional change + +# PV without wwid, set PVID=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|DEVNAME=$dev3|DEVNAME=$DEVNAME_NONE|" tmp1 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# PV without wwid, set PVID=. DEVNAME=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|DEVNAME=$dev3|DEVNAME=.|" tmp1 > tmp2 +sed -e "s|IDNAME=$dev3|IDNAME=$DEVNAME_NONE|" tmp2 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +# PV without wwid, set PVID=. IDNAME=. +sed -e "s|PVID=$PVID3|PVID=.|" orig > tmp1 +sed -e "s|IDNAME=$dev3|IDNAME=.|" tmp1 > tmp2 +sed -e "s|DEVNAME=$dev3|DEVNAME=$DEVNAME_NONE|" tmp2 > "$DF" +cat "$DF" +lvmdevices --check |tee out +grep indeterminate out + +cp orig "$DF" vgchange -an $vg1 lvremove -y $vg1 diff --git a/tools/lvmdevices.c b/tools/lvmdevices.c index 7a1ccac3d..5bc14fce8 100644 --- a/tools/lvmdevices.c +++ b/tools/lvmdevices.c @@ -143,6 +143,7 @@ static void _print_check(struct cmd_context *cmd) int idname_same; int idtype_same; int devname_same; + int part_same; char *part; dm_list_init(&use_old); @@ -198,8 +199,8 @@ restart1: goto next1; } - pvid_same = (du_old->pvid && du_new->pvid && !strcmp(du_old->pvid, du_new->pvid)); - devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)); + pvid_same = (du_old->pvid && du_new->pvid && !strcmp(du_old->pvid, du_new->pvid)) || (!du_old->pvid && !du_new->pvid); + devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)) || (!du_old->devname && !du_new->devname); if (pvid_same && devname_same) { log_verbose("IDTYPE=%s IDNAME=%s DEVNAME=%s PVID=%s%s: no change", @@ -283,8 +284,8 @@ restart2: goto next2; } - idname_same = (du_old->idname && du_new->idname && !strcmp(du_old->idname, du_new->idname)); - devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)); + idname_same = (du_old->idname && du_new->idname && !strcmp(du_old->idname, du_new->idname)) || (!du_old->idname && !du_new->idname); + devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)) || (!du_old->devname && !du_new->devname); if (idname_same && devname_same) { log_verbose("IDTYPE=%s IDNAME=%s DEVNAME=%s PVID=%s%s: no change", @@ -369,8 +370,8 @@ restart3: } idtype_same = (du_old->idtype == du_new->idtype); - idname_same = (du_old->idname && du_new->idname && !strcmp(du_old->idname, du_new->idname)); - devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)); + idname_same = (du_old->idname && du_new->idname && !strcmp(du_old->idname, du_new->idname)) || (!du_old->idname && !du_new->idname); + devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)) || (!du_old->devname && !du_new->devname); if (idtype_same && idname_same && devname_same) { /* this case will probably be caught earlier */ @@ -440,11 +441,47 @@ restart3: } } + /* + * Handle old and new entries that remain and are identical. + * This covers entries that do not have enough valid fields + * set to definitively identify a PV. + */ +restart4: + dm_list_iterate_items(du_old, &use_old) { + dm_list_iterate_items(du_new, &use_new) { + idtype_same = (du_old->idtype == du_new->idtype); + idname_same = (du_old->idname && du_new->idname && !strcmp(du_old->idname, du_new->idname)) || (!du_old->idname && !du_new->idname); + devname_same = (du_old->devname && du_new->devname && !strcmp(du_old->devname, du_new->devname)) || (!du_old->devname && !du_new->devname); + pvid_same = (du_old->pvid && du_new->pvid && !strcmp(du_old->pvid, du_new->pvid)) || (!du_old->pvid && !du_new->pvid); + part_same = (du_old->part == du_new->part); + + if (idtype_same && idname_same && devname_same && pvid_same && part_same) { + part = _part_str(du_old); + + log_print_unless_silent("IDTYPE=%s IDNAME=%s DEVNAME=%s PVID=%s%s: indeterminate", + idtype_to_str(du_new->idtype), + du_new->idname ?: "none", + du_new->devname ?: "none", + du_new->pvid ?: "none", + part); + + dm_list_del(&du_old->list); + dm_list_del(&du_new->list); + dm_list_add(&done_old, &du_old->list); + dm_list_add(&done_new, &du_new->list); + goto restart4; + } + } + } + /* * Entries remaining on old/new lists can't be directly - * correlated by loops above, to print field-specific - * changes. So, just print remaining old entries as - * being removed and remaing new entries as being added. + * correlated by loops above. + * Just print remaining old entries as being removed and + * remaing new entries as being added. + * If we find specific cases that reach here, we may + * want to add loops above to detect and print them + * more specifically. */ dm_list_iterate_items(du_old, &use_old) { @@ -454,7 +491,7 @@ restart3: idtype_to_str(du_old->idtype), du_old->idname ?: "none", du_old->devname ?: "none", - du_old->pvid, + du_old->pvid ?: "none", part); } @@ -465,7 +502,7 @@ restart3: idtype_to_str(du_new->idtype), du_new->idname ?: "none", du_new->devname ?: "none", - du_new->pvid, + du_new->pvid ?: "none", part); }