From bc52b7cb0cb4aacd63d599bd635c36141c6c9d51 Mon Sep 17 00:00:00 2001 From: Marian Csontos Date: Thu, 13 Jul 2023 15:43:37 +0200 Subject: [PATCH] Fix previous commit Resolves: #2212295 #2208039 #2204467 --- 0009-lvresize-fix-multiple-mounts.patch | 102 +++ ...-trailing-underscores-in-t10-wwid-fr.patch | 247 +++++++ ...ndling-of-non-PV-with-duplicate-seri.patch | 82 +++ ...-leading-and-trailing-spaces-for-sys.patch | 693 ++++++++++++++++++ ...-RAID1-allocator-uses-one-disk-for-b.patch | 79 ++ ...grity-caching-ensure-raid-redundancy.patch | 117 +++ 6 files changed, 1320 insertions(+) create mode 100644 0009-lvresize-fix-multiple-mounts.patch create mode 100644 0010-device_id-ignore-trailing-underscores-in-t10-wwid-fr.patch create mode 100644 0011-device_id-fix-handling-of-non-PV-with-duplicate-seri.patch create mode 100644 0012-device_id-ignore-leading-and-trailing-spaces-for-sys.patch create mode 100644 0013-Fix-multisegment-RAID1-allocator-uses-one-disk-for-b.patch create mode 100644 0014-tests-integrity-caching-ensure-raid-redundancy.patch diff --git a/0009-lvresize-fix-multiple-mounts.patch b/0009-lvresize-fix-multiple-mounts.patch new file mode 100644 index 0000000..f0e672c --- /dev/null +++ b/0009-lvresize-fix-multiple-mounts.patch @@ -0,0 +1,102 @@ +From e96cdaca1d2fec1d225ff09ef81f66edd7df7513 Mon Sep 17 00:00:00 2001 +From: David Teigland +Date: Fri, 16 Jun 2023 12:06:40 -0500 +Subject: [PATCH 09/14] lvresize: fix multiple mounts + +which was mistaken as a mounted LV that had been renamed. + +(cherry picked from commit 7c3eca833ff7878d6d32198ed76380c91fdc15fc) +--- + lib/device/filesystem.c | 47 +++++++++++++++++++++-------------------- + 1 file changed, 24 insertions(+), 23 deletions(-) + +diff --git a/lib/device/filesystem.c b/lib/device/filesystem.c +index 2163276ed..bca29747a 100644 +--- a/lib/device/filesystem.c ++++ b/lib/device/filesystem.c +@@ -243,8 +243,6 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l + FILE *fme = NULL; + struct mntent *me; + int renamed = 0; +- int found_dir = 0; +- int found_dev = 0; + int dev_match, dir_match; + + if (stat(lv_path, &st_lv) < 0) { +@@ -281,6 +279,9 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l + } + endmntent(fme); + ++ if (mtab_mntpath[0]) ++ log_debug("%s mtab mntpath %s", display_lvname(lv), mtab_mntpath); ++ + /* + * In mtab dir path, replace each ascii space character with the + * four characters \040 which is how /proc/mounts represents spaces. +@@ -319,15 +320,31 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l + if (strcmp(fstype, proc_fstype)) + continue; + ++ /* ++ * When an LV is mounted on two dirs, it appears in /proc/mounts twice as ++ * /dev/mapper/vg-lvol0 on /foo type xfs ... ++ * /dev/mapper/vg-lvol0 on /bar type xfs ... ++ * All entries match dm_devpath, one entry matches mntpath, ++ * and other entries don't match mntpath. ++ * ++ * When an LV is mounted on one dir, and is renamed from lvol0 to lvol1, ++ * it appears in /proc/mounts once as ++ * /dev/mapper/vg-lvol0 on /foo type xfs ... ++ */ ++ + dir_match = !strcmp(mtab_mntpath, proc_mntpath); + dev_match = !strcmp(dm_devpath, proc_devpath); + +- if (dir_match) +- found_dir++; +- if (dev_match) +- found_dev++; ++ if (!dir_match && !dev_match) ++ continue; ++ ++ if (dev_match && !dir_match) { ++ log_debug("LV %s mounted at %s also mounted at %s.", ++ dm_devpath, mtab_mntpath, proc_mntpath); ++ continue; ++ } + +- if (dir_match != dev_match) { ++ if (!dev_match && dir_match) { + log_error("LV %s mounted at %s may have been renamed (from %s).", + dm_devpath, proc_mntpath, proc_devpath); + renamed = 1; +@@ -337,26 +354,10 @@ int fs_mount_state_is_misnamed(struct cmd_context *cmd, struct logical_volume *l + if (fclose(fp)) + stack; + +- /* +- * Don't try resizing if: +- * - different device names apppear for the mount point +- * (LVs probably renamed while mounted), or +- * - the mount point for the LV appears multiple times, or +- * - the LV device is listed for multiple mounts. +- */ + if (renamed) { + log_error("File system resizing not supported: fs utilities do not support renamed devices."); + return 1; + } +- /* These two are likely detected as renamed, but include checks in case. */ +- if (found_dir > 1) { +- log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", mtab_mntpath); +- return 1; +- } +- if (found_dev > 1) { +- log_error("File system resizing not supported: %s appears more than once in /proc/mounts.", dm_devpath); +- return 1; +- } + return 0; + } + +-- +2.41.0 + diff --git a/0010-device_id-ignore-trailing-underscores-in-t10-wwid-fr.patch b/0010-device_id-ignore-trailing-underscores-in-t10-wwid-fr.patch new file mode 100644 index 0000000..d7ba1cb --- /dev/null +++ b/0010-device_id-ignore-trailing-underscores-in-t10-wwid-fr.patch @@ -0,0 +1,247 @@ +From 9e35daea0212e12ea556c04830a91db08a7f3505 Mon Sep 17 00:00:00 2001 +From: David Teigland +Date: Fri, 19 May 2023 12:52:48 -0500 +Subject: [PATCH 10/14] device_id: ignore trailing underscores in t10 wwid from + devices file + +In previous lvm versions, trailing spaces at the end of a t10 wwid would +be replaced with underscores, so the IDNAME string in system.devices +would look something like "t10.123_". Current versions of lvm ignore +trailing spaces in a t10 wwid, so the IDNAME string used would be +"t10.123". The different values would cause lvm to not recognize a +device in system.devices with the trailing _. Fix this by ignoring +trailing underscores in the IDNAME string from system.devices. + +(cherry picked from commit 4cdb178968b44125c41dee6dd28997283c0afefa) +--- + lib/device/device_id.c | 46 ++++++++++-- + test/shell/devicesfile-vpd-ids.sh | 113 ++++++++++++++++++++++++++++++ + 2 files changed, 152 insertions(+), 7 deletions(-) + +diff --git a/lib/device/device_id.c b/lib/device/device_id.c +index 79da12884..7db6c9b86 100644 +--- a/lib/device/device_id.c ++++ b/lib/device/device_id.c +@@ -1728,7 +1728,8 @@ static int _match_dm_devnames(struct cmd_context *cmd, struct device *dev, + return 0; + } + +-static void _reduce_underscores(char *in, int in_len, char *out, int out_size) ++/* More than one _ in a row is replaced with one _ */ ++static void _reduce_repeating_underscores(char *in, int in_len, char *out, int out_size) + { + int us = 0, i, j = 0; + +@@ -1750,6 +1751,17 @@ static void _reduce_underscores(char *in, int in_len, char *out, int out_size) + } + } + ++/* Remove any _ at the end of the string. */ ++static void _remove_trailing_underscores(char *buf) ++{ ++ char *end; ++ ++ end = buf + strlen(buf) - 1; ++ while ((end > buf) && (*end == '_')) ++ end--; ++ end[1] = '\0'; ++} ++ + /* + * du is a devices file entry. dev is any device on the system. + * check if du is for dev by comparing the device's ids to du->idname. +@@ -1764,6 +1776,7 @@ static void _reduce_underscores(char *in, int in_len, char *out, int out_size) + static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct device *dev) + { + char du_t10[DEV_WWID_SIZE] = { 0 }; ++ char id_t10[DEV_WWID_SIZE]; + struct dev_id *id; + const char *idname; + int part; +@@ -1818,10 +1831,17 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + * for IDNAME were saved in the past with each space replaced + * by one _. Now we convert multiple spaces to a single _. + * So, convert a df entry with the old style to the new shorter +- * style to compare. ++ * style to compare. Also, in past versions, trailing spaces ++ * in the wwid would be replaced by _, but now trailing spaces ++ * are ignored. This means devices file entries created by ++ * past versions may have _ at the end of the IDNAME string. ++ * So, exclude trailing underscores when comparing a t10 wwid ++ * from a device with a t10 wwid in the devices file. + */ +- if (du->idtype == DEV_ID_TYPE_SYS_WWID && !strncmp(du->idname, "t10", 3) && strstr(du->idname, "__")) +- _reduce_underscores(du->idname, strlen(du->idname), du_t10, sizeof(du_t10) - 1); ++ if (du->idtype == DEV_ID_TYPE_SYS_WWID && !strncmp(du->idname, "t10", 3) && strchr(du->idname, '_')) { ++ _reduce_repeating_underscores(du->idname, strlen(du->idname), du_t10, sizeof(du_t10) - 1); ++ _remove_trailing_underscores(du_t10); ++ } + + /* + * Try to match du with ids that have already been read for the dev +@@ -1829,6 +1849,20 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + */ + dm_list_iterate_items(id, &dev->ids) { + if (id->idtype == du->idtype) { ++ ++ /* ++ * For t10 wwids, remove actual trailing underscores from the dev wwid ++ * (in id->idname), because all trailing underscores were removed from ++ * the du->idname read from the devices file. i.e. no trailing _ are ++ * used in t10 wwid comparisons. ++ */ ++ if ((id->idtype == DEV_ID_TYPE_SYS_WWID) && ++ id->idname && !strncmp(id->idname, "t10", 3) && du_t10[0]) { ++ memset(id_t10, 0, sizeof(id_t10)); ++ strncpy(id_t10, id->idname, DEV_WWID_SIZE-1); ++ _remove_trailing_underscores(id_t10); ++ } ++ + if ((id->idtype == DEV_ID_TYPE_DEVNAME) && _match_dm_devnames(cmd, dev, id, du)) { + /* dm devs can have differing names that we know still match */ + du->dev = dev; +@@ -1846,9 +1880,7 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + idtype_to_str(du->idtype), du->idname, dev_name(dev)); + return 1; + +- } else if ((id->idtype == DEV_ID_TYPE_SYS_WWID) && id->idname && +- !strncmp(id->idname, "t10", 3) && du_t10[0] && !strcmp(id->idname, du_t10)) { +- /* Compare the shorter form du t10 wwid to the dev t10 wwid. */ ++ } else if ((id->idtype == DEV_ID_TYPE_SYS_WWID) && du_t10[0] && id_t10[0] && !strcmp(id_t10, du_t10)) { + du->dev = dev; + dev->id = id; + dev->flags |= DEV_MATCHED_USE_ID; +diff --git a/test/shell/devicesfile-vpd-ids.sh b/test/shell/devicesfile-vpd-ids.sh +index b2042fb9a..52805737b 100644 +--- a/test/shell/devicesfile-vpd-ids.sh ++++ b/test/shell/devicesfile-vpd-ids.sh +@@ -80,6 +80,7 @@ echo $DEV1 + DFDIR="$LVM_SYSTEM_DIR/devices" + mkdir -p "$DFDIR" || true + DF="$DFDIR/system.devices" ++DFTMP="$DFDIR/system.devices_tmp" + touch $DF + + pvcreate "$DEV1" +@@ -243,6 +244,118 @@ vgremove $vg + rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid + cleanup_sysfs + ++# Test t10 wwid with trailing space and line feed at the end ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ ++2048 4152 4444 4953 4b20 2020 2020 2020 \ ++2020 2020 2020 2020 2020 2020 2020 2020 \ ++2020 2020 5642 3963 3130 6433 3138 2d31 \ ++3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++lvmdevices --adddev "$DEV1" ++cat $DF ++vgcreate $vg "$DEV1" ++lvcreate -l1 -an $vg ++cat $DF ++# check wwid string in metadata output ++pvs -o+deviceidtype,deviceid "$DEV1" |tee out ++grep sys_wwid out ++# check wwid string in system.devices ++grep sys_wwid $DF ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cleanup_sysfs ++ ++# Test t10 wwid with trailing space at the end that was created by 9.0/9.1 ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ ++2048 4152 4444 4953 4b20 2020 2020 2020 \ ++2020 2020 2020 2020 2020 2020 2020 2020 \ ++2020 2020 5642 3963 3130 6433 3138 2d31 \ ++3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++lvmdevices --adddev "$DEV1" ++cat $DF ++vgcreate $vg "$DEV1" ++PVID1=`pvs "$DEV1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++T10_WWID_RHEL91="t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9ebc_" ++lvcreate -l1 -an $vg ++cat $DF ++# check wwid string in metadata output ++pvs -o+deviceidtype,deviceid "$DEV1" |tee out ++grep sys_wwid out ++# check wwid string in system.devices ++grep sys_wwid $DF ++# Replace IDNAME with the IDNAME that 9.0/9.1 created from this wwid ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_wwid IDNAME=t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9ebc_ DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs ++pvs -o+deviceidtype,deviceid "$DEV1" ++# Removing the trailing _ which should then work ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_wwid IDNAME=t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9ebc DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs ++pvs -o+deviceidtype,deviceid "$DEV1" ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cleanup_sysfs ++ ++# test a t10 wwid that has actual trailing underscore which ++# is followed by a trailing space. ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ ++2048 4152 4444 4953 4b20 2020 2020 2020 \ ++2020 2020 2020 2020 2020 2020 2020 2020 \ ++2020 2020 5642 3963 3130 6433 3138 2d31 \ ++3838 6439 6562 5f20 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++# The wwid has an actual underscore char (5f) followed by a space char (20) ++# 9.1 converts the trailing space to an underscore ++T10_WWID_RHEL91="t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9eb__" ++# 9.2 ignores the trailing space ++T10_WWID_RHEL92="t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9eb_" ++lvmdevices --adddev "$DEV1" ++cat $DF ++vgcreate $vg "$DEV1" ++PVID1=`pvs "$DEV1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++lvcreate -l1 -an $vg ++cat $DF ++# check wwid string in metadata output ++pvs -o+deviceidtype,deviceid "$DEV1" |tee out ++grep sys_wwid out ++# check wwid string in system.devices ++grep sys_wwid $DF ++# Replace IDNAME with the IDNAME that 9.0/9.1 created from this wwid ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_wwid IDNAME=${T10_WWID_RHEL91} DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs ++pvs -o+deviceidtype,deviceid "$DEV1" ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++cleanup_sysfs ++ ++ + # TODO: lvmdevices --adddev --deviceidtype --deviceid + # This would let the user specify the second naa wwid. + +-- +2.41.0 + diff --git a/0011-device_id-fix-handling-of-non-PV-with-duplicate-seri.patch b/0011-device_id-fix-handling-of-non-PV-with-duplicate-seri.patch new file mode 100644 index 0000000..0ba66c0 --- /dev/null +++ b/0011-device_id-fix-handling-of-non-PV-with-duplicate-seri.patch @@ -0,0 +1,82 @@ +From 499fd37ff0c2bae1c492c3883f063a332e12ac3d Mon Sep 17 00:00:00 2001 +From: David Teigland +Date: Thu, 8 Jun 2023 12:24:05 -0500 +Subject: [PATCH 11/14] device_id: fix handling of non-PV with duplicate serial + number + +Fix in the code that matches devices to system.devices entries when +the devices have the same serial number. A non-PV device in +system.devices has no pvid value, and the code was segfaulting +when checking the null pvid value. + +(cherry picked from commit 74feebdab723c1ea46d4316f8a581750c1d8cda3) +--- + lib/device/device_id.c | 2 ++ + test/shell/devicesfile-serial.sh | 38 ++++++++++++++++++++++++++++++++ + 2 files changed, 40 insertions(+) + +diff --git a/lib/device/device_id.c b/lib/device/device_id.c +index 7db6c9b86..e3d622ecc 100644 +--- a/lib/device/device_id.c ++++ b/lib/device/device_id.c +@@ -2625,6 +2625,8 @@ void device_ids_check_serial(struct cmd_context *cmd, struct dm_list *scan_devs, + * Match du to a dev based on PVID. + */ + dm_list_iterate_items(dul, &dus_check) { ++ if (!dul->du->pvid) ++ continue; + log_debug("Matching suspect serial device id %s PVID %s prev %s", + dul->du->idname, dul->du->pvid, dul->du->devname); + found = 0; +diff --git a/test/shell/devicesfile-serial.sh b/test/shell/devicesfile-serial.sh +index a88c1906a..a4cbd5cb2 100644 +--- a/test/shell/devicesfile-serial.sh ++++ b/test/shell/devicesfile-serial.sh +@@ -851,6 +851,44 @@ grep $PVID4 out4 + vgcreate $vg2 $dev2 $dev3 + vgs | grep $vg2 + ++# 3 devs with duplicate serial, 2 pvs with stale devnames, 1 non-pv device ++ ++aux wipefs_a $dev1 ++aux wipefs_a $dev2 ++aux wipefs_a $dev3 ++ ++echo $SERIAL1 > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/serial ++echo $SERIAL1 > $SYS_DIR/dev/block/$MAJOR2:$MINOR2/device/serial ++echo $SERIAL1 > $SYS_DIR/dev/block/$MAJOR3:$MINOR3/device/serial ++ ++rm $DF ++touch $DF ++vgcreate $vg1 $dev1 $dev2 ++lvmdevices --adddev $dev3 ++cat $DF ++cp $DF $ORIG ++ ++PVID1=`pvs "$dev1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++PVID2=`pvs "$dev2" --noheading -o uuid | tr -d - | awk '{print $1}'` ++OPVID1=`pvs "$dev1" --noheading -o uuid | awk '{print $1}'` ++OPVID2=`pvs "$dev2" --noheading -o uuid | awk '{print $1}'` ++ ++pvs -o+uuid,deviceid ++ ++sed -e "s|DEVNAME=$dev1|DEVNAME=tmp|" $ORIG > tmp1 ++sed -e "s|DEVNAME=$dev2|DEVNAME=$dev1|" tmp1 > tmp2 ++sed -e "s|DEVNAME=tmp|DEVNAME=$dev2|" tmp2 > $DF ++cat $DF ++ ++# pvs should report the correct info and fix the DF ++pvs -o+uuid,deviceid |tee out ++grep $dev1 out |tee out1 ++grep $dev2 out |tee out2 ++grep $OPVID1 out1 ++grep $OPVID2 out2 ++grep $SERIAL1 out1 ++grep $SERIAL1 out2 ++ + remove_base + rmmod brd + +-- +2.41.0 + diff --git a/0012-device_id-ignore-leading-and-trailing-spaces-for-sys.patch b/0012-device_id-ignore-leading-and-trailing-spaces-for-sys.patch new file mode 100644 index 0000000..81152cf --- /dev/null +++ b/0012-device_id-ignore-leading-and-trailing-spaces-for-sys.patch @@ -0,0 +1,693 @@ +From 894ae888233ff5026c981500623f4f829d358405 Mon Sep 17 00:00:00 2001 +From: David Teigland +Date: Thu, 15 Jun 2023 13:58:48 -0500 +Subject: [PATCH 12/14] device_id: ignore leading and trailing spaces for + sys_wwid and sys_serial + +Leading and trailing underscores are also ignored to handle device ids +written by previous versions which replaced all spaces with underscores. + +(cherry picked from commit 228a8e8c1fd8e82a2e31a6060614dc3dd2f8bc51) +--- + lib/device/device.h | 1 + + lib/device/device_id.c | 278 +++++++++++++++++++----------- + lib/device/parse_vpd.c | 50 ++++++ + test/shell/devicesfile-vpd-ids.sh | 101 +++++++++-- + 4 files changed, 312 insertions(+), 118 deletions(-) + +diff --git a/lib/device/device.h b/lib/device/device.h +index 446104218..84d87232b 100644 +--- a/lib/device/device.h ++++ b/lib/device/device.h +@@ -239,6 +239,7 @@ int dev_mpath_init(const char *config_wwids_file); + void dev_mpath_exit(void); + int parse_vpd_ids(const unsigned char *vpd_data, int vpd_datalen, struct dm_list *ids); + int format_t10_id(const unsigned char *in, int in_bytes, unsigned char *out, int out_bytes); ++int format_general_id(const char *in, int in_bytes, unsigned char *out, int out_bytes); + int parse_vpd_serial(const unsigned char *in, char *out, int outsize); + + /* dev_util */ +diff --git a/lib/device/device_id.c b/lib/device/device_id.c +index e3d622ecc..a6fc4a26d 100644 +--- a/lib/device/device_id.c ++++ b/lib/device/device_id.c +@@ -185,6 +185,71 @@ void free_dids(struct dm_list *ids) + } + } + ++/* More than one _ in a row is replaced with one _ */ ++static void _reduce_repeating_underscores(char *buf, int bufsize) ++{ ++ char *tmpbuf; ++ int us = 0, i, j = 0; ++ ++ if (!(tmpbuf = strndup(buf, bufsize-1))) ++ return; ++ ++ memset(buf, 0, bufsize); ++ ++ for (i = 0; i < strlen(tmpbuf); i++) { ++ if (tmpbuf[i] == '_') ++ us++; ++ else ++ us = 0; ++ ++ if (us == 1) ++ buf[j++] = '_'; ++ else if (us > 1) ++ continue; ++ else ++ buf[j++] = tmpbuf[i]; ++ ++ if (j == bufsize) ++ break; ++ } ++ buf[bufsize-1] = '\0'; ++ free(tmpbuf); ++} ++ ++static void _remove_leading_underscores(char *buf, int bufsize) ++{ ++ char *tmpbuf; ++ int i, j = 0; ++ ++ if (buf[0] != '_') ++ return; ++ ++ if (!(tmpbuf = strndup(buf, bufsize-1))) ++ return; ++ ++ memset(buf, 0, bufsize); ++ ++ for (i = 0; i < strlen(tmpbuf); i++) { ++ if (!j && tmpbuf[i] == '_') ++ continue; ++ buf[j++] = tmpbuf[i]; ++ ++ if (j == bufsize) ++ break; ++ } ++ free(tmpbuf); ++} ++ ++static void _remove_trailing_underscores(char *buf, int bufsize) ++{ ++ char *end; ++ ++ end = buf + strlen(buf) - 1; ++ while ((end > buf) && (*end == '_')) ++ end--; ++ end[1] = '\0'; ++} ++ + static int _read_sys_block(struct cmd_context *cmd, struct device *dev, + const char *suffix, char *sysbuf, int sysbufsize, + int binary, int *retlen) +@@ -406,7 +471,7 @@ struct dev_wwid *dev_add_wwid(char *id, int id_type, struct dm_list *ids) + + int dev_read_vpd_wwids(struct cmd_context *cmd, struct device *dev) + { +- unsigned char vpd_data[VPD_SIZE] = { 0 }; ++ char vpd_data[VPD_SIZE] = { 0 }; + int vpd_datalen = 0; + + dev->flags |= DEV_ADDED_VPD_WWIDS; +@@ -417,36 +482,47 @@ int dev_read_vpd_wwids(struct cmd_context *cmd, struct device *dev) + return 0; + + /* adds dev_wwid entry to dev->wwids for each id in vpd data */ +- parse_vpd_ids(vpd_data, vpd_datalen, &dev->wwids); ++ parse_vpd_ids((const unsigned char *)vpd_data, vpd_datalen, &dev->wwids); + return 1; + } + + int dev_read_sys_wwid(struct cmd_context *cmd, struct device *dev, +- char *buf, int bufsize, struct dev_wwid **dw_out) ++ char *outbuf, int outbufsize, struct dev_wwid **dw_out) + { +- char tmpbuf[DEV_WWID_SIZE]; ++ char buf[DEV_WWID_SIZE] = { 0 }; + struct dev_wwid *dw; +- int ret; ++ int is_t10 = 0; ++ int i, ret; + + dev->flags |= DEV_ADDED_SYS_WWID; + +- ret = read_sys_block(cmd, dev, "device/wwid", buf, bufsize); ++ ret = read_sys_block(cmd, dev, "device/wwid", buf, sizeof(buf)); + if (!ret || !buf[0]) { + /* the wwid file is not under device for nvme devs */ +- ret = read_sys_block(cmd, dev, "wwid", buf, bufsize); ++ ret = read_sys_block(cmd, dev, "wwid", buf, sizeof(buf)); + } + if (!ret || !buf[0]) + return 0; + +- /* in t10 id, replace characters like space and quote */ +- if (!strncmp(buf, "t10.", 4)) { +- if (bufsize < DEV_WWID_SIZE) +- return 0; +- memcpy(tmpbuf, buf, DEV_WWID_SIZE); +- memset(buf, 0, bufsize); +- format_t10_id((const unsigned char *)tmpbuf, DEV_WWID_SIZE, (unsigned char *)buf, bufsize); ++ for (i = 0; i < sizeof(buf) - 4; i++) { ++ if (buf[i] == ' ') ++ continue; ++ if (!strncmp(&buf[i], "t10", 3)) ++ is_t10 = 1; ++ break; + } + ++ /* ++ * Remove leading and trailing spaces. ++ * Replace internal spaces with underscores. ++ * t10 wwids have multiple sequential spaces ++ * replaced by a single underscore. ++ */ ++ if (is_t10) ++ format_t10_id((const unsigned char *)buf, sizeof(buf), (unsigned char *)outbuf, outbufsize); ++ else ++ format_general_id((const char *)buf, sizeof(buf), (unsigned char *)outbuf, outbufsize); ++ + /* Note, if wwids are also read from vpd, this same wwid will be added again. */ + + if (!(dw = dev_add_wwid(buf, 0, &dev->wwids))) +@@ -457,9 +533,9 @@ int dev_read_sys_wwid(struct cmd_context *cmd, struct device *dev, + } + + static int _dev_read_sys_serial(struct cmd_context *cmd, struct device *dev, +- char *buf, int bufsize) ++ char *outbuf, int outbufsize) + { +- unsigned char vpd_data[VPD_SIZE] = { 0 }; ++ char buf[VPD_SIZE] = { 0 }; + const char *devname; + int vpd_datalen = 0; + +@@ -471,13 +547,16 @@ static int _dev_read_sys_serial(struct cmd_context *cmd, struct device *dev, + * (Only virtio disks /dev/vdx are known to use /sys/class/block/vdx/serial.) + */ + +- read_sys_block(cmd, dev, "device/serial", buf, bufsize); +- if (buf[0]) +- return 1; ++ read_sys_block(cmd, dev, "device/serial", buf, sizeof(buf)); ++ if (buf[0]) { ++ format_general_id((const char *)buf, sizeof(buf), (unsigned char *)outbuf, outbufsize); ++ if (outbuf[0]) ++ return 1; ++ } + +- if (read_sys_block_binary(cmd, dev, "device/vpd_pg80", (char *)vpd_data, VPD_SIZE, &vpd_datalen) && vpd_datalen) { +- parse_vpd_serial(vpd_data, buf, bufsize); +- if (buf[0]) ++ if (read_sys_block_binary(cmd, dev, "device/vpd_pg80", buf, VPD_SIZE, &vpd_datalen) && vpd_datalen) { ++ parse_vpd_serial((const unsigned char *)buf, outbuf, outbufsize); ++ if (outbuf[0]) + return 1; + } + +@@ -505,12 +584,13 @@ static int _dev_read_sys_serial(struct cmd_context *cmd, struct device *dev, + if (dm_snprintf(path, sizeof(path), "%s/class/block/%s/serial", sysfs_dir, vdx) < 0) + return 0; + +- ret = get_sysfs_value(path, buf, bufsize, 0); ++ ret = get_sysfs_value(path, buf, sizeof(buf), 0); + if (ret && !buf[0]) + ret = 0; + if (ret) { +- buf[bufsize - 1] = '\0'; +- return 1; ++ format_general_id((const char *)buf, sizeof(buf), (unsigned char *)outbuf, outbufsize); ++ if (buf[0]) ++ return 1; + } + } + +@@ -520,6 +600,7 @@ static int _dev_read_sys_serial(struct cmd_context *cmd, struct device *dev, + const char *device_id_system_read(struct cmd_context *cmd, struct device *dev, uint16_t idtype) + { + char sysbuf[PATH_MAX] = { 0 }; ++ char sysbuf2[PATH_MAX] = { 0 }; + const char *idname = NULL; + struct dev_wwid *dw; + int i; +@@ -584,16 +665,45 @@ const char *device_id_system_read(struct cmd_context *cmd, struct device *dev, u + return NULL; + } + +- /* wwids are already munged if needed */ +- if (idtype != DEV_ID_TYPE_SYS_WWID) { ++ /* ++ * Replace all spaces, quotes, control chars with underscores. ++ * sys_wwid, sys_serial, and wwid_* have already been handled, ++ * and with slightly different replacement (see format_t10_id, ++ * format_general_id.) ++ */ ++ if ((idtype != DEV_ID_TYPE_SYS_WWID) && ++ (idtype != DEV_ID_TYPE_SYS_SERIAL) && ++ (idtype != DEV_ID_TYPE_WWID_NAA) && ++ (idtype != DEV_ID_TYPE_WWID_EUI) && ++ (idtype != DEV_ID_TYPE_WWID_T10)) { + for (i = 0; i < strlen(sysbuf); i++) { +- if (sysbuf[i] == '"') +- continue; +- if (isblank(sysbuf[i]) || isspace(sysbuf[i]) || iscntrl(sysbuf[i])) ++ if ((sysbuf[i] == '"') || ++ isblank(sysbuf[i]) || ++ isspace(sysbuf[i]) || ++ iscntrl(sysbuf[i])) + sysbuf[i] = '_'; + } + } + ++ /* ++ * Reduce actual leading and trailing underscores for sys_wwid ++ * and sys_serial, since underscores were previously used as ++ * replacements for leading/trailing spaces which are now ignored. ++ * Also reduce any actual repeated underscores in t10 wwid since ++ * multiple repeated spaces were also once replaced by underscores. ++ */ ++ if ((idtype == DEV_ID_TYPE_SYS_WWID) || ++ (idtype == DEV_ID_TYPE_SYS_SERIAL)) { ++ memcpy(sysbuf2, sysbuf, sizeof(sysbuf2)); ++ _remove_leading_underscores(sysbuf2, sizeof(sysbuf2)); ++ _remove_trailing_underscores(sysbuf2, sizeof(sysbuf2)); ++ if (idtype == DEV_ID_TYPE_SYS_WWID && !strncmp(sysbuf2, "t10", 3) && strstr(sysbuf2, "__")) ++ _reduce_repeating_underscores(sysbuf2, sizeof(sysbuf2)); ++ if (memcmp(sysbuf, sysbuf2, sizeof(sysbuf))) ++ log_debug("device_id_system_read reduced underscores %s to %s", sysbuf, sysbuf2); ++ memcpy(sysbuf, sysbuf2, sizeof(sysbuf)); ++ } ++ + if (!sysbuf[0]) + goto bad; + +@@ -1728,40 +1838,6 @@ static int _match_dm_devnames(struct cmd_context *cmd, struct device *dev, + return 0; + } + +-/* More than one _ in a row is replaced with one _ */ +-static void _reduce_repeating_underscores(char *in, int in_len, char *out, int out_size) +-{ +- int us = 0, i, j = 0; +- +- for (i = 0; i < in_len; i++) { +- if (in[i] == '_') +- us++; +- else +- us = 0; +- +- if (us == 1) +- out[j++] = '_'; +- else if (us > 1) +- continue; +- else +- out[j++] = in[i]; +- +- if (j == out_size) +- break; +- } +-} +- +-/* Remove any _ at the end of the string. */ +-static void _remove_trailing_underscores(char *buf) +-{ +- char *end; +- +- end = buf + strlen(buf) - 1; +- while ((end > buf) && (*end == '_')) +- end--; +- end[1] = '\0'; +-} +- + /* + * du is a devices file entry. dev is any device on the system. + * check if du is for dev by comparing the device's ids to du->idname. +@@ -1775,8 +1851,7 @@ static void _remove_trailing_underscores(char *buf) + + static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct device *dev) + { +- char du_t10[DEV_WWID_SIZE] = { 0 }; +- char id_t10[DEV_WWID_SIZE]; ++ char du_idname[PATH_MAX]; + struct dev_id *id; + const char *idname; + int part; +@@ -1827,20 +1902,30 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + } + + /* +- * Devices file entries with IDTYPE=sys_wwid and a T10 WWID +- * for IDNAME were saved in the past with each space replaced +- * by one _. Now we convert multiple spaces to a single _. +- * So, convert a df entry with the old style to the new shorter +- * style to compare. Also, in past versions, trailing spaces +- * in the wwid would be replaced by _, but now trailing spaces +- * are ignored. This means devices file entries created by +- * past versions may have _ at the end of the IDNAME string. +- * So, exclude trailing underscores when comparing a t10 wwid +- * from a device with a t10 wwid in the devices file. ++ * sys_wwid and sys_serial were saved in the past with leading and ++ * trailing spaces replaced with underscores, and t10 wwids also had ++ * repeated internal spaces replaced with one underscore each. Now we ++ * ignore leading and trailing spaces and replace multiple repeated ++ * spaces with one underscore in t10 wwids. In order to handle ++ * system.devices entries created by older versions, modify the IDNAME ++ * value that's read (du->idname) to remove leading and trailing ++ * underscores, and reduce repeated underscores to one in t10 wwids. ++ * ++ * Example: wwid is reported as " t10.123 456 " (without quotes) ++ * Previous versions would save this in system.devices as: __t10.123__456__ ++ * Current versions will save this in system.devices as: t10.123_456 ++ * device_id_system_read() now returns: t10.123_456 ++ * When this code reads __t10.123__456__ from system.devices, that ++ * string is modified to t10.123_456 so that it will match the value ++ * returned from device_id_system_read(). + */ +- if (du->idtype == DEV_ID_TYPE_SYS_WWID && !strncmp(du->idname, "t10", 3) && strchr(du->idname, '_')) { +- _reduce_repeating_underscores(du->idname, strlen(du->idname), du_t10, sizeof(du_t10) - 1); +- _remove_trailing_underscores(du_t10); ++ strncpy(du_idname, du->idname, PATH_MAX-1); ++ if (((du->idtype == DEV_ID_TYPE_SYS_WWID) || (du->idtype == DEV_ID_TYPE_SYS_SERIAL)) && ++ strchr(du_idname, '_')) { ++ _remove_leading_underscores(du_idname, sizeof(du_idname)); ++ _remove_trailing_underscores(du_idname, sizeof(du_idname)); ++ if (du->idtype == DEV_ID_TYPE_SYS_WWID && !strncmp(du_idname, "t10", 3) && strstr(du_idname, "__")) ++ _reduce_repeating_underscores(du_idname, sizeof(du_idname)); + } + + /* +@@ -1848,21 +1933,14 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + * (and saved on dev->ids to avoid rereading.) + */ + dm_list_iterate_items(id, &dev->ids) { +- if (id->idtype == du->idtype) { ++ if (!id->idname) ++ continue; + ++ if (id->idtype == du->idtype) { + /* +- * For t10 wwids, remove actual trailing underscores from the dev wwid +- * (in id->idname), because all trailing underscores were removed from +- * the du->idname read from the devices file. i.e. no trailing _ are +- * used in t10 wwid comparisons. ++ * dm names can have different forms, so matching names ++ * is not always a direct comparison. + */ +- if ((id->idtype == DEV_ID_TYPE_SYS_WWID) && +- id->idname && !strncmp(id->idname, "t10", 3) && du_t10[0]) { +- memset(id_t10, 0, sizeof(id_t10)); +- strncpy(id_t10, id->idname, DEV_WWID_SIZE-1); +- _remove_trailing_underscores(id_t10); +- } +- + if ((id->idtype == DEV_ID_TYPE_DEVNAME) && _match_dm_devnames(cmd, dev, id, du)) { + /* dm devs can have differing names that we know still match */ + du->dev = dev; +@@ -1871,22 +1949,16 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + log_debug("Match device_id %s %s to %s: dm names", + idtype_to_str(du->idtype), du->idname, dev_name(dev)); + return 1; ++ } + +- } else if (id->idname && !strcmp(id->idname, du->idname)) { ++ if (!strcmp(id->idname, du_idname)) { + du->dev = dev; + dev->id = id; + dev->flags |= DEV_MATCHED_USE_ID; + log_debug("Match device_id %s %s to %s", +- idtype_to_str(du->idtype), du->idname, dev_name(dev)); ++ idtype_to_str(du->idtype), du_idname, dev_name(dev)); + return 1; + +- } else if ((id->idtype == DEV_ID_TYPE_SYS_WWID) && du_t10[0] && id_t10[0] && !strcmp(id_t10, du_t10)) { +- du->dev = dev; +- dev->id = id; +- dev->flags |= DEV_MATCHED_USE_ID; +- log_debug("Match device_id %s %s to %s", +- idtype_to_str(du->idtype), du->idname, dev_name(dev)); +- return 1; + } else { + /* + log_debug("Mismatch device_id %s %s to %s: idname %s", +@@ -1913,12 +1985,12 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + id->dev = dev; + dm_list_add(&dev->ids, &id->list); + +- if (idname && !strcmp(idname, du->idname)) { ++ if (idname && !strcmp(idname, du_idname)) { + du->dev = dev; + dev->id = id; + dev->flags |= DEV_MATCHED_USE_ID; + log_debug("Match device_id %s %s to %s", +- idtype_to_str(du->idtype), du->idname, dev_name(dev)); ++ idtype_to_str(du->idtype), idname, dev_name(dev)); + return 1; + } + +@@ -1944,7 +2016,7 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct + dev_read_vpd_wwids(cmd, dev); + + dm_list_iterate_items(dw, &dev->wwids) { +- if (!strcmp(dw->id, du->idname)) { ++ if (!strcmp(dw->id, du_idname)) { + if (!(id = zalloc(sizeof(struct dev_id)))) + return_0; + /* wwid types are 1,2,3 and idtypes are DEV_ID_TYPE_ */ +diff --git a/lib/device/parse_vpd.c b/lib/device/parse_vpd.c +index c1ac974fd..938277e38 100644 +--- a/lib/device/parse_vpd.c ++++ b/lib/device/parse_vpd.c +@@ -36,7 +36,57 @@ + #include + + /* ++ * Remove leading spaces. ++ * Remove trailing spaces. ++ * Replace each space with underscore. ++ * Skip quotes, non-ascii, non-printable. ++ */ ++int format_general_id(const char *in, int in_bytes, unsigned char *out, int out_bytes) ++{ ++ const char *end; ++ int end_bytes = strlen(in); ++ int retlen = 0; ++ int j = 0; ++ int i; ++ ++ if (!end_bytes) ++ return 0; ++ ++ end = in + end_bytes - 1; ++ while ((end > in) && (*end == ' ')) { ++ end--; ++ end_bytes--; ++ } ++ ++ for (i = 0; i < end_bytes; i++) { ++ if (!in[i]) ++ break; ++ if (j >= (out_bytes - 2)) ++ break; ++ /* skip leading spaces */ ++ if (!retlen && (in[i] == ' ')) ++ continue; ++ /* skip non-ascii non-printable characters */ ++ if (!isascii(in[i]) || !isprint(in[i])) ++ continue; ++ /* skip quote */ ++ if (in[i] == '"') ++ continue; ++ /* replace each space with _ */ ++ if (in[i] == ' ') ++ out[j++] = '_'; ++ else ++ out[j++] = in[i]; ++ retlen++; ++ } ++ return retlen; ++} ++ ++/* ++ * Remove leading spaces. ++ * Remove trailing spaces. + * Replace series of spaces with a single _. ++ * Skip quotes, non-ascii, non-printable. + */ + int format_t10_id(const unsigned char *in, int in_bytes, unsigned char *out, int out_bytes) + { +diff --git a/test/shell/devicesfile-vpd-ids.sh b/test/shell/devicesfile-vpd-ids.sh +index 52805737b..04dbae7d0 100644 +--- a/test/shell/devicesfile-vpd-ids.sh ++++ b/test/shell/devicesfile-vpd-ids.sh +@@ -223,8 +223,8 @@ cleanup_sysfs + # Test t10 wwid containing quote + rm $DF + aux wipefs_a "$DEV1" +-mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ +-echo "t10.ATA_2.5\"_SATA_SSD_1112-A___111111111111" > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device ++echo "t10.ATA_2.5\"_SATA_SSD_1112-A___111111111111" > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + lvmdevices --adddev "$DEV1" + cat $DF + vgcreate $vg "$DEV1" +@@ -241,19 +241,19 @@ grep sys_wwid $DF + grep 2.5_SATA_SSD $DF + lvremove -y $vg + vgremove $vg +-rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + cleanup_sysfs + + # Test t10 wwid with trailing space and line feed at the end + rm $DF + aux wipefs_a "$DEV1" +-mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device + echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ + 2048 4152 4444 4953 4b20 2020 2020 2020 \ + 2020 2020 2020 2020 2020 2020 2020 2020 \ + 2020 2020 5642 3963 3130 6433 3138 2d31 \ +-3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid +-cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + lvmdevices --adddev "$DEV1" + cat $DF + vgcreate $vg "$DEV1" +@@ -266,19 +266,19 @@ grep sys_wwid out + grep sys_wwid $DF + lvremove -y $vg + vgremove $vg +-rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + cleanup_sysfs + + # Test t10 wwid with trailing space at the end that was created by 9.0/9.1 + rm $DF + aux wipefs_a "$DEV1" +-mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device + echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ + 2048 4152 4444 4953 4b20 2020 2020 2020 \ + 2020 2020 2020 2020 2020 2020 2020 2020 \ + 2020 2020 5642 3963 3130 6433 3138 2d31 \ +-3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid +-cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++3838 6439 6562 6320 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + lvmdevices --adddev "$DEV1" + cat $DF + vgcreate $vg "$DEV1" +@@ -311,20 +311,20 @@ pvs + pvs -o+deviceidtype,deviceid "$DEV1" + lvremove -y $vg + vgremove $vg +-rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + cleanup_sysfs + + # test a t10 wwid that has actual trailing underscore which + # is followed by a trailing space. + rm $DF + aux wipefs_a "$DEV1" +-mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/ ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device + echo -n "7431 302e 4154 4120 2020 2020 5642 4f58 \ + 2048 4152 4444 4953 4b20 2020 2020 2020 \ + 2020 2020 2020 2020 2020 2020 2020 2020 \ + 2020 2020 5642 3963 3130 6433 3138 2d31 \ +-3838 6439 6562 5f20 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid +-cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++3838 6439 6562 5f20 0a" | xxd -r -p > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cat $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid + # The wwid has an actual underscore char (5f) followed by a space char (20) + # 9.1 converts the trailing space to an underscore + T10_WWID_RHEL91="t10.ATA_____VBOX_HARDDISK___________________________VB9c10d318-188d9eb__" +@@ -352,9 +352,80 @@ pvs + pvs -o+deviceidtype,deviceid "$DEV1" + lvremove -y $vg + vgremove $vg +-rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/wwid ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cleanup_sysfs ++ ++# ++# Test trailing/leading/center spaces in sys_wwid and sys_serial device ++# ids, and that old system.devices files that have trailing/leading ++# underscores are understood. ++# ++ ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device ++echo -n " s123 456 " > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/serial ++lvmdevices --adddev "$DEV1" ++cat $DF ++grep "IDNAME=s123__456 DEVNAME" $DF ++vgcreate $vg "$DEV1" ++PVID1=`pvs "$DEV1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_serial IDNAME=__s123__456__ DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs -o+deviceidtype,deviceid "$DEV1" ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/serial + cleanup_sysfs + ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device ++echo -n " t10.123 456 " > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++lvmdevices --adddev "$DEV1" ++cat $DF ++grep "IDNAME=t10.123_456 DEVNAME" $DF ++vgcreate $vg "$DEV1" ++PVID1=`pvs "$DEV1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_wwid IDNAME=__t10.123__456__ DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs -o+deviceidtype,deviceid "$DEV1" ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cleanup_sysfs ++ ++rm $DF ++aux wipefs_a "$DEV1" ++mkdir -p $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device ++echo -n " naa.123 456 " > $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++lvmdevices --adddev "$DEV1" ++cat $DF ++grep "IDNAME=naa.123__456 DEVNAME" $DF ++vgcreate $vg "$DEV1" ++PVID1=`pvs "$DEV1" --noheading -o uuid | tr -d - | awk '{print $1}'` ++cat $DF | grep -v IDNAME > $DFTMP ++cat $DFTMP ++echo "IDTYPE=sys_wwid IDNAME=__naa.123__456__ DEVNAME=${DEV1} PVID=${PVID1}" >> $DFTMP ++cp $DFTMP $DF ++cat $DF ++vgs ++pvs -o+deviceidtype,deviceid "$DEV1" ++lvremove -y $vg ++vgremove $vg ++rm $SYS_DIR/dev/block/$MAJOR1:$MINOR1/device/wwid ++cleanup_sysfs ++ ++ ++ + + # TODO: lvmdevices --adddev --deviceidtype --deviceid + # This would let the user specify the second naa wwid. +-- +2.41.0 + diff --git a/0013-Fix-multisegment-RAID1-allocator-uses-one-disk-for-b.patch b/0013-Fix-multisegment-RAID1-allocator-uses-one-disk-for-b.patch new file mode 100644 index 0000000..4d29cc8 --- /dev/null +++ b/0013-Fix-multisegment-RAID1-allocator-uses-one-disk-for-b.patch @@ -0,0 +1,79 @@ +From 14cb9d915270634c364d89918f824c538b28dc80 Mon Sep 17 00:00:00 2001 +From: heinzm +Date: Wed, 10 May 2023 18:22:11 +0200 +Subject: [PATCH 13/14] Fix "multisegment RAID1, allocator uses one disk for + both legs" + +In case of e.g. 3 PVs, creating or extending a RaidLV causes SubLV +collocation thus putting segments of diffent rimage (and potentially +larger rmeta) SubLVs onto the same PV. For redundant RaidLVs this'll +compromise redundancy. Fix by detecting such bogus allocation on +lvcreate/lvextend and reject the request. + +(cherry picked from commit 05c2b10c5d0a99993430ffbcef684a099ba810ad) +--- + lib/metadata/lv_manip.c | 41 +++++++++++++++++++++++++++++++++++++++++ + 1 file changed, 41 insertions(+) + +diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c +index add9512ff..e4799e082 100644 +--- a/lib/metadata/lv_manip.c ++++ b/lib/metadata/lv_manip.c +@@ -4455,6 +4455,38 @@ static int _lv_extend_layered_lv(struct alloc_handle *ah, + return 1; + } + ++/* Check either RAID images and metas are being allocated redundantly. */ ++static int _lv_raid_redundant(struct logical_volume *lv, ++ struct dm_list *allocatable_pvs, int meta) ++{ ++ uint32_t nlvs, s; ++ struct lv_segment *seg = first_seg(lv); ++ struct pv_list *pvl; ++ ++ if (meta && !seg->meta_areas) ++ return 1; ++ ++ dm_list_iterate_items(pvl, allocatable_pvs) { ++ nlvs = 0; ++ ++ for (s = 0; s < seg->area_count; s++) { ++ struct logical_volume *slv = meta ? seg_metalv(seg, s) : seg_lv(seg, s); ++ ++ if (slv && lv_is_on_pv(slv, pvl->pv) && nlvs++) ++ return 0; ++ } ++ } ++ ++ return 1; ++} ++ ++/* Check both RAID images and metas are being allocated redundantly. */ ++static int _lv_raid_redundant_allocation(struct logical_volume *lv, struct dm_list *allocatable_pvs) ++{ ++ return _lv_raid_redundant(lv, allocatable_pvs, 0) && ++ _lv_raid_redundant(lv, allocatable_pvs, 1); ++} ++ + /* + * Entry point for single-step LV allocation + extension. + * Extents is the number of logical extents to append to the LV unless +@@ -4557,6 +4589,15 @@ int lv_extend(struct logical_volume *lv, + mirrors, stripes, stripe_size))) + goto_out; + ++ if (segtype_is_raid(segtype) && ++ alloc != ALLOC_ANYWHERE && ++ !(r = _lv_raid_redundant_allocation(lv, allocatable_pvs))) { ++ log_error("Insufficient suitable allocatable extents for logical volume %s", display_lvname(lv)); ++ if (!lv_remove(lv) || !vg_write(lv->vg) || !vg_commit(lv->vg)) ++ return_0; ++ goto out; ++ } ++ + if (lv_raid_has_integrity(lv)) { + if (!lv_extend_integrity_in_raid(lv, allocatable_pvs)) { + r = 0; +-- +2.41.0 + diff --git a/0014-tests-integrity-caching-ensure-raid-redundancy.patch b/0014-tests-integrity-caching-ensure-raid-redundancy.patch new file mode 100644 index 0000000..67140e1 --- /dev/null +++ b/0014-tests-integrity-caching-ensure-raid-redundancy.patch @@ -0,0 +1,117 @@ +From 4e28d22cc152fd9c753e5584a5ae99e7a5d1ac96 Mon Sep 17 00:00:00 2001 +From: David Teigland +Date: Wed, 17 May 2023 14:15:25 -0500 +Subject: [PATCH 14/14] tests: integrity-caching: ensure raid redundancy + +The recent fix 05c2b10c5d0a9 ensures that raid LV images are not +using the same devices. This was happening in the lvextend commands +used by this test, so fix the test to use more devices to ensue +redundancy. + +(cherry picked from commit 24e4b6df1182d0d41763176c175e98e5fa6153ab) +--- + lib/metadata/lv_manip.c | 5 ++++- + test/shell/integrity-caching.sh | 27 ++++++++++++++++++--------- + 2 files changed, 22 insertions(+), 10 deletions(-) + +diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c +index e4799e082..70c969de5 100644 +--- a/lib/metadata/lv_manip.c ++++ b/lib/metadata/lv_manip.c +@@ -4472,8 +4472,11 @@ static int _lv_raid_redundant(struct logical_volume *lv, + for (s = 0; s < seg->area_count; s++) { + struct logical_volume *slv = meta ? seg_metalv(seg, s) : seg_lv(seg, s); + +- if (slv && lv_is_on_pv(slv, pvl->pv) && nlvs++) ++ if (slv && lv_is_on_pv(slv, pvl->pv) && nlvs++) { ++ log_error("LV %s using PV %s is not redundant.", ++ display_lvname(slv), dev_name(pvl->pv->dev)); + return 0; ++ } + } + } + +diff --git a/test/shell/integrity-caching.sh b/test/shell/integrity-caching.sh +index 5539ac575..06fc04928 100644 +--- a/test/shell/integrity-caching.sh ++++ b/test/shell/integrity-caching.sh +@@ -23,7 +23,7 @@ aux kernel_at_least 5 10 || export LVM_TEST_PREFER_BRD=0 + mnt="mnt" + mkdir -p $mnt + +-aux prepare_devs 6 80 ++aux prepare_devs 9 80 + + # Use awk instead of anoyingly long log out from printf + #printf "%0.sA" {1..16384} >> fileA +@@ -319,7 +319,7 @@ vgremove -ff $vg + # Test lvextend while inactive + + _prepare_vg +-lvcreate --type raid1 -m1 --raidintegrity y -n $lv1 -l 8 $vg ++lvcreate --type raid1 -m1 --raidintegrity y -n $lv1 -l 8 $vg "$dev1" "$dev2" + _wait_recalc $vg/${lv1}_rimage_0 + _wait_recalc $vg/${lv1}_rimage_1 + _wait_recalc $vg/$lv1 +@@ -329,7 +329,11 @@ lvs -a -o name,size,segtype,devices,sync_percent $vg + _add_new_data_to_mnt + umount $mnt + lvchange -an $vg/$lv1 +-lvextend -l 16 $vg/$lv1 ++# use two new devs for raid extend to ensure redundancy ++vgextend $vg "$dev7" "$dev8" ++lvs -a -o name,segtype,devices $vg ++lvextend -l 16 $vg/$lv1 "$dev7" "$dev8" ++lvs -a -o name,segtype,devices $vg + lvchange -ay $vg/$lv1 + mount "$DM_DEV_DIR/$vg/$lv1" $mnt + xfs_growfs $mnt +@@ -346,16 +350,19 @@ vgremove -ff $vg + # Test lvextend while active + + _prepare_vg +-lvcreate --type raid1 -m1 --raidintegrity y -n $lv1 -l 8 $vg ++lvcreate --type raid1 -m1 --raidintegrity y -n $lv1 -l 8 $vg "$dev1" "$dev2" + _wait_recalc $vg/${lv1}_rimage_0 + _wait_recalc $vg/${lv1}_rimage_1 + _wait_recalc $vg/$lv1 + lvcreate --type $create_type -n fast -l 4 -an $vg "$dev6" + lvconvert -y --type $convert_type $convert_option fast $vg/$lv1 ++# use two new devs for raid extend to ensure redundancy ++vgextend $vg "$dev7" "$dev8" + lvs -a -o name,size,segtype,devices,sync_percent $vg + _add_new_data_to_mnt +-lvextend -l 16 $vg/$lv1 +-xfs_growfs $mnt ++lvextend -l 16 $vg/$lv1 "$dev7" "$dev8" ++lvs -a -o name,size,segtype,devices,sync_percent $vg ++resize2fs "$DM_DEV_DIR/$vg/$lv1" + _wait_recalc $vg/${lv1}_${suffix}_rimage_0 + _wait_recalc $vg/${lv1}_${suffix}_rimage_1 + _add_more_data_to_mnt +@@ -367,17 +374,19 @@ lvremove $vg/$lv1 + vgremove -ff $vg + + _prepare_vg +-lvcreate --type raid5 --raidintegrity y -n $lv1 -l 8 $vg ++lvcreate --type raid5 --raidintegrity y -n $lv1 -I4 -l 8 $vg "$dev1" "$dev2" "$dev3" + _wait_recalc $vg/${lv1}_rimage_0 + _wait_recalc $vg/${lv1}_rimage_1 + _wait_recalc $vg/${lv1}_rimage_2 + _wait_recalc $vg/$lv1 + lvcreate --type $create_type -n fast -l 4 -an $vg "$dev6" + lvconvert -y --type $convert_type $convert_option fast $vg/$lv1 ++vgextend $vg "$dev7" "$dev8" "$dev9" + lvs -a -o name,size,segtype,devices,sync_percent $vg + _add_new_data_to_mnt +-lvextend -l 16 $vg/$lv1 +-xfs_growfs $mnt ++lvextend -l 16 $vg/$lv1 "$dev7" "$dev8" "$dev9" ++lvs -a -o name,size,segtype,devices,sync_percent $vg ++resize2fs "$DM_DEV_DIR/$vg/$lv1" + _wait_recalc $vg/${lv1}_${suffix}_rimage_0 + _wait_recalc $vg/${lv1}_${suffix}_rimage_1 + _add_more_data_to_mnt +-- +2.41.0 +