Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=e2ecc6c763557462a4bc3…
Commit: e2ecc6c763557462a4bc3d88bbc883917eb0f010
Parent: 0591131a539fb668412fc38807686d2a89decd27
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue May 30 21:14:37 2023 +0200
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Wed May 31 09:38:11 2023 +0200
reporter: restore report type to initial value after processing report_for_selection
If we are executing 'report_for_selection' to do an internal report
just for the selection itself (not for display), we can call it more
than once. In that case, we are reusing the same selection handle
(e.g. inside 'process_each_lv_in_vg').
The selection handle has 'report_type' field which is a union of all
report types needed for the report based on selection fields we actually
use.
The 'report_type' is further clarified based on checks and rules inside
'_get_final_report_type' which 'report_for_selection' calls. Then, this
final report type unambiguously identifies proper branch to take in
'report_all_in_{pv,vg,lv}' that is called next.
If the 'report_for_selection' is called more than once with the same
selection handle, we need to make sure that we always restore the report
type to its initial value, so all the rules inside 'report_for_selection'
are applied correctly next time.
This patch fixes the missing restoration of the 'report_type' value in
'selection_handle' that is reused for recurring 'report_for_selection'
calls.
An example scenario where this failed was with selecting an LV for
removal with "lvremove --select" while using a field in the selection
that required extra DM info or DM status call for the LV (that is,
"Logical Volume Device Info Fields" and "Logical Volume Device Status Fields"
as visible in 'lvs -S help').
---
WHATS_NEW | 1 +
tools/reporter.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index ad870317d..493567c5d 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
version 2.03.22 -
=================================
+ Fix failing -S|--select for non-reporting cmds if using LV info/status fields.
version 2.03.21 - 21st April 2023
=================================
diff --git a/tools/reporter.c b/tools/reporter.c
index e62858f42..45273c0a4 100644
--- a/tools/reporter.c
+++ b/tools/reporter.c
@@ -633,6 +633,7 @@ int report_for_selection(struct cmd_context *cmd,
struct logical_volume *lv)
{
struct selection_handle *sh = parent_handle->selection_handle;
+ report_type_t initial_report_type = sh->report_type;
struct report_args args = {0};
struct single_report_args *single_args = &args.single_args[REPORT_IDX_SINGLE];
int do_lv_info, do_lv_seg_status;
@@ -648,8 +649,10 @@ int report_for_selection(struct cmd_context *cmd,
&sh->report_type))
return_0;
- if (!(handle = init_processing_handle(cmd, parent_handle)))
+ if (!(handle = init_processing_handle(cmd, parent_handle))) {
+ sh->report_type = initial_report_type;
return_0;
+ }
/*
* We're already reporting for select so override
@@ -695,6 +698,8 @@ int report_for_selection(struct cmd_context *cmd,
break;
}
+ sh->report_type = initial_report_type;
+
/*
* Keep the selection handle provided from the caller -
* do not destroy it - the caller will still use it to
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=031f7a4639eb808e2a86e…
Commit: 031f7a4639eb808e2a86e57a215dd9def7519ac4
Parent: 0591131a539fb668412fc38807686d2a89decd27
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue May 30 21:14:37 2023 +0200
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Wed May 31 09:28:11 2023 +0200
reporter: restore report type to initial value after processing report_for_selection
If we are executing 'report_for_selection' to do an internal report
just for the selection itself (not for display), we call
'report_for_selection'. We can call this more than once, in which case
we are reusing the same selection handle (e.g. inside 'process_each_lv_in_vg').
The selection handle has 'report_type' field which is a union of all
report types needed for the report based on selection fields we actually
use.
The 'report_type' is further clarified based on checks and rules inside
'_get_final_report_type' which 'report_for_selection' calls. Then, this
final report type unambiguously identifies proper branch to take in
'report_all_in_{pv,vg,lv}' that is called next.
If the 'report_for_selection' is called more than once with the same
selection handle, we need to make sure that we always restore the report
type to its initial value, so all the rules inside 'report_for_selection'
are applied correctly next time.
This patch fixes the missing restoration of the 'report_type' value in
'selection_handle' that is reused for recurring 'report_for_selection'
calls.
An example scenario where this failed was with selecting an LV for
removal with "lvremove --select" while using a field in the selection
that required extra DM info or DM status call for the LV (that is,
"Logical Volume Device Info Fields" and "Logical Volume Device Status Fields"
as visible in 'lvs -S help').
---
WHATS_NEW | 1 +
tools/reporter.c | 7 ++++++-
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/WHATS_NEW b/WHATS_NEW
index ad870317d..493567c5d 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
version 2.03.22 -
=================================
+ Fix failing -S|--select for non-reporting cmds if using LV info/status fields.
version 2.03.21 - 21st April 2023
=================================
diff --git a/tools/reporter.c b/tools/reporter.c
index e62858f42..45273c0a4 100644
--- a/tools/reporter.c
+++ b/tools/reporter.c
@@ -633,6 +633,7 @@ int report_for_selection(struct cmd_context *cmd,
struct logical_volume *lv)
{
struct selection_handle *sh = parent_handle->selection_handle;
+ report_type_t initial_report_type = sh->report_type;
struct report_args args = {0};
struct single_report_args *single_args = &args.single_args[REPORT_IDX_SINGLE];
int do_lv_info, do_lv_seg_status;
@@ -648,8 +649,10 @@ int report_for_selection(struct cmd_context *cmd,
&sh->report_type))
return_0;
- if (!(handle = init_processing_handle(cmd, parent_handle)))
+ if (!(handle = init_processing_handle(cmd, parent_handle))) {
+ sh->report_type = initial_report_type;
return_0;
+ }
/*
* We're already reporting for select so override
@@ -695,6 +698,8 @@ int report_for_selection(struct cmd_context *cmd,
break;
}
+ sh->report_type = initial_report_type;
+
/*
* Keep the selection handle provided from the caller -
* do not destroy it - the caller will still use it to
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=4cdb178968b44125c41de…
Commit: 4cdb178968b44125c41dee6dd28997283c0afefa
Parent: 24e4b6df1182d0d41763176c175e98e5fa6153ab
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri May 19 12:52:48 2023 -0500
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri May 19 14:05:10 2023 -0500
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.
---
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 38677d1fe..3e05a40a8 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 <dev> --deviceidtype <type> --deviceid <val>
# This would let the user specify the second naa wwid.
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=3b4e7d1625ddc48dd9393…
Commit: 3b4e7d1625ddc48dd9393f03a59cc6b74113275a
Parent: fd6e113bba5fed5ee41152cde33220294c24ce2b
Author: Peter Rajnoha <prajnoha(a)redhat.com>
AuthorDate: Tue May 16 17:17:55 2023 +0200
Committer: Peter Rajnoha <prajnoha(a)redhat.com>
CommitterDate: Tue May 16 17:17:55 2023 +0200
toollib: provide proper hint for referencing VG uuid in case of duplicate VG names
vgrename does not support -S|--select, so do not provide a hint about
using it. Instead, provide a hint about using VG uuid directly.
��� vgs
WARNING: VG name vg1 is used by VGs DXjcSK-gWfu-5gLh-9Kbg-sG49-dtRr-GqXzGL and MVMfyM-sjOa-M2xV-AT4Y-JddR-h4SP-UO5Ttk.
Fix duplicate VG names with vgrename uuid, a device filter, or system IDs.
VG #PV #LV #SN Attr VSize VFree
vg1 1 0 0 wz--n- 124.00m 124.00m
vg1 1 0 0 wz--n- 124.00m 124.00m
(vgrename does not support -S|--select)
��� vgrename vg1 vg2
WARNING: VG name vg1 is used by VGs DXjcSK-gWfu-5gLh-9Kbg-sG49-dtRr-GqXzGL and MVMfyM-sjOa-M2xV-AT4Y-JddR-h4SP-UO5Ttk.
Fix duplicate VG names with vgrename uuid, a device filter, or system IDs.
Multiple VGs found with the same name: skipping vg1
Use VG uuid in place of the VG name.
(vgchange does support -S|--select)
��� vgchange --addtag a vg1
WARNING: VG name vg1 is used by VGs DXjcSK-gWfu-5gLh-9Kbg-sG49-dtRr-GqXzGL and MVMfyM-sjOa-M2xV-AT4Y-JddR-h4SP-UO5Ttk.
Fix duplicate VG names with vgrename uuid, a device filter, or system IDs.
Multiple VGs found with the same name: skipping vg1
Use --select vg_uuid=<uuid> in place of the VG name.
---
tools/lvmcmdline.c | 12 ++++++++++++
tools/toollib.c | 7 ++++++-
tools/tools.h | 1 +
3 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c
index a5bb6a5c5..6bbf1af26 100644
--- a/tools/lvmcmdline.c
+++ b/tools/lvmcmdline.c
@@ -179,6 +179,18 @@ static const struct command_function _command_functions[CMD_COUNT] = {
/* Command line args */
+int arg_is_valid_for_command(const struct cmd_context *cmd, int a)
+{
+ int i;
+
+ for (i = 0; i < cmd->cname->num_args; i++) {
+ if (cmd->cname->valid_args[i] == a)
+ return 1;
+ }
+
+ return 0;
+}
+
unsigned arg_count(const struct cmd_context *cmd, int a)
{
return cmd->opt_arg_values ? cmd->opt_arg_values[a].count : 0;
diff --git a/tools/toollib.c b/tools/toollib.c
index 6b590189b..2f4756b6b 100644
--- a/tools/toollib.c
+++ b/tools/toollib.c
@@ -2313,7 +2313,12 @@ static int _resolve_duplicate_vgnames(struct cmd_context *cmd,
* is unknown.
*/
log_error("Multiple VGs found with the same name: skipping %s", sl->str);
- log_error("Use --select vg_uuid=<uuid> in place of the VG name.");
+
+ if (arg_is_valid_for_command(cmd, select_ARG))
+ log_error("Use --select vg_uuid=<uuid> in place of the VG name.");
+ else
+ log_error("Use VG uuid in place of the VG name.");
+
dm_list_del(&sl->list);
ret = ECMD_FAILED;
}
diff --git a/tools/tools.h b/tools/tools.h
index 36da3bc7e..60952a2aa 100644
--- a/tools/tools.h
+++ b/tools/tools.h
@@ -193,6 +193,7 @@ int repairtype_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_v
int dumptype_arg(struct cmd_context *cmd __attribute__((unused)), struct arg_values *av);
/* we use the enums to access the switches */
+int arg_is_valid_for_command(const struct cmd_context *cmd, int a);
unsigned arg_count(const struct cmd_context *cmd, int a);
unsigned arg_is_set(const struct cmd_context *cmd, int a);
int arg_from_list_is_set(const struct cmd_context *cmd, const char *err_found, ...);