main - device_id: fix search on filtered device
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=fee39370020424234be...
Commit: fee39370020424234bee1295cac480afeef72fd6
Parent: 8e61c0ad6ecad5e0bcee09e44029e39396c1b59a
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Tue Nov 16 09:29:24 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Tue Nov 16 09:29:24 2021 -0600
device_id: fix search on filtered device
When devnames are used as device ids and devnames change,
then new devices need to be located for the PVs. If the old
devname is now used by a filtered device, this was preventing
the code from searching for the new device, so the PV was
reported as missing.
---
lib/device/device_id.c | 16 +++++++--
test/shell/devicesfile-devname.sh | 69 +++++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 2 deletions(-)
diff --git a/lib/device/device_id.c b/lib/device/device_id.c
index 749acdf89..7bcaf3df4 100644
--- a/lib/device/device_id.c
+++ b/lib/device/device_id.c
@@ -2031,12 +2031,19 @@ void device_ids_find_renamed_devs(struct cmd_context *cmd, struct dm_list *dev_l
search_auto = !strcmp(cmd->search_for_devnames, "auto");
dm_list_iterate_items(du, &cmd->use_devices) {
- if (du->dev)
- continue;
if (!du->pvid)
continue;
if (du->idtype != DEV_ID_TYPE_DEVNAME)
continue;
+
+ /*
+ * if the old incorrect devname is now a device that's
+ * filtered and not scanned, e.g. an mpath component,
+ * then we want to look for the pvid on a new device.
+ */
+ if (du->dev && !du->dev->filtered_flags)
+ continue;
+
if (!(dil = dm_pool_zalloc(cmd->mem, sizeof(*dil))))
continue;
@@ -2061,6 +2068,11 @@ void device_ids_find_renamed_devs(struct cmd_context *cmd, struct dm_list *dev_l
* the searched file, so a subsequent lvm command will do the search
* again. In future perhaps we could add a policy to automatically
* remove a devices file entry that's not been found for some time.
+ *
+ * TODO: like the hint file, add a hash of all devnames to the searched
+ * file so it can be ignored and removed if the devs/hash change.
+ * If hints are enabled, the hints invalidation could also remove the
+ * searched file.
*/
if (_searched_devnames_exists(cmd)) {
log_debug("Search for PVIDs skipped for %s", _searched_file);
diff --git a/test/shell/devicesfile-devname.sh b/test/shell/devicesfile-devname.sh
index 46f66a073..1b881c1cb 100644
--- a/test/shell/devicesfile-devname.sh
+++ b/test/shell/devicesfile-devname.sh
@@ -568,4 +568,73 @@ grep "$PVID2" "$DF" |tee out
grep "$dev2" out
not grep "$dev1" out
+vgchange -an $vg1
+vgchange -an $vg2
+vgremove -ff $vg1
+vgremove -ff $vg2
+
+# devnames change so the new devname now refers to a filtered device,
+# e.g. an mpath or md component, which is not scanned
+
+wait_md_create() {
+ local md=$1
+
+ while :; do
+ if ! grep "$(basename $md)" /proc/mdstat; then
+ echo "$md not ready"
+ cat /proc/mdstat
+ sleep 2
+ else
+ break
+ fi
+ done
+ echo "$md" > WAIT_MD_DEV
+}
+
+aux wipefs_a "$dev1"
+aux wipefs_a "$dev2"
+aux wipefs_a "$dev3"
+aux wipefs_a "$dev4"
+
+mddev="/dev/md33"
+not grep $mddev /proc/mdstat || skip
+
+rm "$DF"
+touch "$DF"
+vgcreate $vg1 "$dev1" "$dev2"
+cat "$DF"
+cp "$DF" "$ORIG"
+
+# PVID with dashes for matching pvs -o+uuid output
+OPVID1=`pvs "$dev1" --noheading -o uuid | awk '{print $1}'`
+OPVID2=`pvs "$dev2" --noheading -o uuid | awk '{print $1}'`
+
+# PVID without dashes for matching devices file fields
+PVID1=`pvs "$dev1" --noheading -o uuid | tr -d - | awk '{print $1}'`
+PVID2=`pvs "$dev2" --noheading -o uuid | tr -d - | awk '{print $1}'`
+
+mdadm --create --metadata=1.0 "$mddev" --level 1 --chunk=64 --raid-devices=2 "$dev3" "$dev4"
+wait_md_create "$mddev"
+
+sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > tmp1.devices
+sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" tmp1.devices > "$DF"
+cat "$DF"
+pvs -o+uuid |tee out
+grep "$dev1" out
+grep "$dev2" out
+grep "$OPVID1" out
+grep "$OPVID2" out
+not grep "$dev3" out
+not grep "$dev4" out
+
+grep "$dev1" "$DF"
+grep "$dev2" "$DF"
+grep "$PVID1" "$DF"
+grep "$PVID2" "$DF"
+not grep "$dev3" "$DF"
+not grep "$dev4" "$DF"
+
+mdadm --stop "$mddev"
+aux udev_wait
+
vgremove -ff $vg1
2 years, 5 months
main - tests devicesfile-devname: remove searched_devnames
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=8e61c0ad6ecad5e0bce...
Commit: 8e61c0ad6ecad5e0bcee09e44029e39396c1b59a
Parent: 89c54db16cfca284d78dd3be5316dc9bca67e8ef
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Mon Nov 15 18:04:10 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 15 18:04:10 2021 -0600
tests devicesfile-devname: remove searched_devnames
remove /run/lvm/searched_devnames when preparing each test
in case it has appeared on the system
---
test/shell/devicesfile-devname.sh | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/test/shell/devicesfile-devname.sh b/test/shell/devicesfile-devname.sh
index f95be52b1..46f66a073 100644
--- a/test/shell/devicesfile-devname.sh
+++ b/test/shell/devicesfile-devname.sh
@@ -100,6 +100,7 @@ lvcreate -l4 -an -i2 -n $lv1 $vg1
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -116,6 +117,7 @@ cat "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvcreate uses correct dev
lvcreate -l1 -n $lv2 -an $vg1 "$dev1"
# lvcreate fixed the DF
@@ -135,6 +137,7 @@ cat "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
not grep "$PVID3" "$DF"
@@ -147,6 +150,7 @@ cat "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev2|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -163,6 +167,7 @@ cat "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev2|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvcreate uses correct dev
lvcreate -l1 -n $lv2 -an $vg1 "$dev1"
# lvcreate fixed the DF
@@ -182,6 +187,7 @@ cat "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev2|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -196,6 +202,7 @@ sed -e "s|IDNAME=$dev1|IDNAME=tmpname|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev2|IDNAME=$dev1|" tmp1.devices > tmp2.devices
sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp2.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -214,6 +221,7 @@ sed -e "s|IDNAME=$dev1|IDNAME=tmpname|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev2|IDNAME=$dev1|" tmp1.devices > tmp2.devices
sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp2.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvcreate uses correct dev
lvcreate -l1 -n $lv2 -an $vg1 "$dev1"
# lvcreate fixed the DF
@@ -235,6 +243,7 @@ sed -e "s|IDNAME=$dev1|IDNAME=tmpname|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev2|IDNAME=$dev1|" tmp1.devices > tmp2.devices
sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp2.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -255,6 +264,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -271,6 +281,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
not grep "$PVID3" "$DF"
@@ -283,6 +294,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev2|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -299,6 +311,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev2|" "$ORIG" > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -313,6 +326,7 @@ sed -e "s|DEVNAME=$dev1|DEVNAME=tmpname|" "$ORIG" > tmp1.devices
sed -e "s|DEVNAME=$dev2|DEVNAME=$dev1|" tmp1.devices > tmp2.devices
sed -e "s|DEVNAME=tmpname|DEVNAME=$dev2|" tmp2.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -331,6 +345,7 @@ sed -e "s|DEVNAME=$dev1|DEVNAME=tmpname|" "$ORIG" > tmp1.devices
sed -e "s|DEVNAME=$dev2|DEVNAME=$dev1|" tmp1.devices > tmp2.devices
sed -e "s|DEVNAME=tmpname|DEVNAME=$dev2|" tmp2.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -352,6 +367,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" tmp1.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -370,6 +386,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" tmp1.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
not grep "$PVID3" "$DF"
@@ -384,6 +401,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev2|" tmp1.devices > "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev2|" tmp1.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -403,6 +421,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev2|" tmp1.devices > "$DF"
sed -e "s|IDNAME=$dev1|IDNAME=$dev2|" tmp1.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -422,6 +441,7 @@ sed -e "s|IDNAME=$dev1|IDNAME=tmpname|" tmp3.devices > tmp4.devices
sed -e "s|IDNAME=$dev2|IDNAME=$dev1|" tmp4.devices > tmp5.devices
sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp5.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# pvs reports correct info
pvs -o+uuid | tee pvs.out
grep $vg1 pvs.out > out
@@ -445,6 +465,7 @@ sed -e "s|IDNAME=$dev1|IDNAME=tmpname|" tmp3.devices > tmp4.devices
sed -e "s|IDNAME=$dev2|IDNAME=$dev1|" tmp4.devices > tmp5.devices
sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp5.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
# lvmdevices fixes the DF
lvmdevices --update
grep "$PVID1" "$DF" |tee out
@@ -468,6 +489,7 @@ cat "$DF"
sed -e "s|DEVNAME=$dev1|DEVNAME=$dev3|" "$ORIG" > tmp1.devices
sed -e "s|IDNAME=$dev1|IDNAME=$dev3|" tmp1.devices > "$DF"
cat "$DF"
+rm /run/lvm/searched_devnames || true
_clear_online_files
pvscan --cache -aay "$dev1"
pvscan --cache -aay "$dev2"
@@ -508,6 +530,7 @@ sed -e "s|IDNAME=tmpname|IDNAME=$dev2|" tmp5.devices > "$DF"
cat "$DF"
_clear_online_files
+rm /run/lvm/searched_devnames || true
# pvscan creates the correct online files and activates correct vg
pvscan --cache -aay "$dev1"
2 years, 5 months
main - vgchange autoactivation: error path cleanup
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=89c54db16cfca284d78...
Commit: 89c54db16cfca284d78dd3be5316dc9bca67e8ef
Parent: c5f998aec4b5e42f7f1b6ddeec6c4544dceaa1b5
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Nov 12 16:46:39 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Mon Nov 15 11:07:11 2021 -0600
vgchange autoactivation: error path cleanup
If the optimized label scan fails (using online files),
then clear the device state prior to falling back to the
standard label_scan. This avoids printing output about
unexpected state.
---
lib/label/label.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
tools/vgchange.c | 36 +++++++++++++++++--------------
2 files changed, 80 insertions(+), 19 deletions(-)
diff --git a/lib/label/label.c b/lib/label/label.c
index d506b81e3..5c77a6923 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -1021,6 +1021,35 @@ int label_scan_for_pvid(struct cmd_context *cmd, char *pvid, struct device **dev
return ret;
}
+/*
+ * Clear state that label_scan_vg_online() created so it will not
+ * confuse the standard label_scan() that the caller falls back to.
+ * the results of filtering (call filter->wipe)
+ * the results of matching device_id (reset dev and du)
+ * the results of scanning in lvmcache
+ */
+static void _clear_scan_state(struct cmd_context *cmd, struct dm_list *devs)
+{
+ struct device_list *devl;
+ struct device *dev;
+ struct dev_use *du;
+
+ dm_list_iterate_items(devl, devs) {
+ dev = devl->dev;
+
+ cmd->filter->wipe(cmd, cmd->filter, dev, NULL);
+ dev->flags &= ~DEV_MATCHED_USE_ID;
+ dev->id = NULL;
+
+ if ((du = get_du_for_dev(cmd, dev)))
+ du->dev = NULL;
+
+ lvmcache_del_dev(dev);
+
+ memset(dev->pvid, 0, ID_LEN);
+ }
+}
+
/*
* Use files under /run/lvm/, created by pvscan --cache autoactivation,
* to optimize device setup/scanning. autoactivation happens during
@@ -1033,6 +1062,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
{
struct dm_list pvs_online;
struct dm_list devs;
+ struct dm_list devs_drop;
struct pv_online *po;
struct device_list *devl, *devl2;
int relax_deviceid_filter = 0;
@@ -1041,6 +1071,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
dm_list_init(&pvs_online);
dm_list_init(&devs);
+ dm_list_init(&devs_drop);
log_debug_devs("Finding online devices to scan");
@@ -1116,6 +1147,15 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
* factor code common to pvscan_cache_args
*/
+ /*
+ * Match devs with the devices file because special/optimized
+ * device setup was used which does not check the devices file.
+ * If a match fails here do not exclude it, that will be done below by
+ * passes_filter() which runs filter-deviceid. The
+ * relax_deviceid_filter case needs to be able to work around
+ * unmatching devs.
+ */
+
if (cmd->enable_devices_file) {
dm_list_iterate_items(devl, &devs)
device_ids_match_dev(cmd, devl->dev);
@@ -1127,6 +1167,8 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
if (cmd->enable_devices_file && device_ids_use_devname(cmd)) {
relax_deviceid_filter = 1;
cmd->filter_deviceid_skip = 1;
+ /* PVIDs read from devs matched to devices file below instead. */
+ log_debug("Skipping device_id filtering due to devname ids.");
}
cmd->filter_nodata_only = 1;
@@ -1136,6 +1178,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
log_print("%s excluded by filters: %s.",
dev_name(devl->dev), dev_filtered_reason(devl->dev));
dm_list_del(&devl->list);
+ dm_list_add(&devs_drop, &devl->list);
}
}
@@ -1160,11 +1203,13 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
label_scan_setup_bcache();
dm_list_iterate_items_safe(devl, devl2, &devs) {
+ struct dev_use *du;
int has_pvid;
if (!label_read_pvid(devl->dev, &has_pvid)) {
log_print("%s cannot read label.", dev_name(devl->dev));
dm_list_del(&devl->list);
+ dm_list_add(&devs_drop, &devl->list);
continue;
}
@@ -1172,6 +1217,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
/* Not an lvm device */
log_print("%s not an lvm device.", dev_name(devl->dev));
dm_list_del(&devl->list);
+ dm_list_add(&devs_drop, &devl->list);
continue;
}
@@ -1180,11 +1226,16 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
* so in place of that check if the pvid is in the devices file.
*/
if (relax_deviceid_filter) {
- if (!get_du_for_pvid(cmd, devl->dev->pvid)) {
+ if (!(du = get_du_for_pvid(cmd, devl->dev->pvid))) {
log_print("%s excluded by devices file (checking PVID).",
dev_name(devl->dev));
dm_list_del(&devl->list);
+ dm_list_add(&devs_drop, &devl->list);
continue;
+ } else {
+ /* Special case matching for devname entries based on pvid. */
+ log_debug("Match device_id %s %s to %s: matching PVID",
+ idtype_to_str(du->idtype), du->idname, dev_name(devl->dev));
}
}
@@ -1193,6 +1244,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
log_print("%s excluded by filters: %s.",
dev_name(devl->dev), dev_filtered_reason(devl->dev));
dm_list_del(&devl->list);
+ dm_list_add(&devs_drop, &devl->list);
}
}
@@ -1202,6 +1254,7 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
free_po_list(&pvs_online);
if (dm_list_empty(&devs)) {
+ _clear_scan_state(cmd, &devs_drop);
*found_none = 1;
return 1;
}
@@ -1227,6 +1280,8 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
if (metadata_pv_count > dm_list_size(&devs)) {
log_debug("Incomplete PV list from online files %d metadata %d.",
dm_list_size(&devs), metadata_pv_count);
+ _clear_scan_state(cmd, &devs_drop);
+ _clear_scan_state(cmd, &devs);
*found_incomplete = 1;
return 1;
}
@@ -1235,6 +1290,8 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
*found_all = 1;
return 1;
bad:
+ _clear_scan_state(cmd, &devs_drop);
+ _clear_scan_state(cmd, &devs);
free_po_list(&pvs_online);
return 0;
}
@@ -1527,7 +1584,7 @@ int label_read_pvid(struct device *dev, int *has_pvid)
lh = (struct label_header *)(buf + 512);
if (memcmp(lh->id, LABEL_ID, sizeof(lh->id))) {
- /* Not an lvm deice */
+ /* Not an lvm device */
label_scan_invalidate(dev);
return 1;
}
@@ -1537,7 +1594,7 @@ int label_read_pvid(struct device *dev, int *has_pvid)
* rest of the label_header intact.
*/
if (memcmp(lh->type, LVM2_LABEL, sizeof(lh->type))) {
- /* Not an lvm deice */
+ /* Not an lvm device */
label_scan_invalidate(dev);
return 1;
}
diff --git a/tools/vgchange.c b/tools/vgchange.c
index 5eebc9ae9..fc076c1d5 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -802,7 +802,7 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
*/
if (!label_scan_vg_online(cmd, vgname, &found_none, &found_all, &found_incomplete)) {
log_print("PVs online error%s%s, using all devices.", vgname ? " for VG " : "", vgname ?: "");
- return 1;
+ goto bad;
}
/*
@@ -815,9 +815,11 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
}
/*
- * The expected and optimal usage, because the udev rule calls
- * vgchange -aay --autoactivation event vgname when all PVs
- * for vgname are found online. Only online pvs are used.
+ * The expected and optimal usage, which is the purpose of
+ * this function. We expect online files to be found for
+ * all PVs because the udev rule calls
+ * vgchange -aay --autoactivation event <vgname>
+ * only after all PVs for vgname are found online.
*/
if (found_all) {
*flags |= PROCESS_SKIP_SCAN;
@@ -825,16 +827,6 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
return 1;
}
- /*
- * The online scanning optimiziation didn't work, so undo the vg
- * locking optimization before falling back to normal processing.
- */
- if (vg_locked) {
- unlock_vg(cmd, NULL, vgname);
- *flags &= ~READ_WITHOUT_LOCK;
- cmd->can_use_one_scan = 0;
- }
-
/*
* Not expected usage, no online pvs for the vgname were found. The
* caller will fall back to process_each doing a full label_scan to
@@ -842,7 +834,7 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
*/
if (found_none) {
log_print("PVs online not found for VG %s, using all devices.", vgname);
- return 1;
+ goto bad;
}
/*
@@ -852,7 +844,7 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
*/
if (found_incomplete) {
log_print("PVs online incomplete for VG %s, using all devicess.", vgname);
- return 1;
+ goto bad;
}
/*
@@ -860,7 +852,19 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
* process_each. (No optimization used.)
*/
log_print("PVs online unknown for VG %s, using all devices.", vgname);
+
+ bad:
+ /*
+ * The online scanning optimization didn't work, so undo the vg
+ * locking optimization before falling back to normal processing.
+ */
+ if (vg_locked) {
+ unlock_vg(cmd, NULL, vgname);
+ *flags &= ~READ_WITHOUT_LOCK;
+ cmd->can_use_one_scan = 0;
+ }
return 1;
+
}
int vgchange(struct cmd_context *cmd, int argc, char **argv)
2 years, 5 months
main - device_id: match different dm device names
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=c5f998aec4b5e42f7f1...
Commit: c5f998aec4b5e42f7f1b6ddeec6c4544dceaa1b5
Parent: 4926061d32f855ffe7a38034b2fe2917e63409a0
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Nov 12 16:42:51 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Nov 12 16:42:51 2021 -0600
device_id: match different dm device names
If a devices file entry for a dm device is using the devname
for the device id, then recognize different dm names as matching.
---
lib/device/device_id.c | 81 ++++++++++++++++++++++++++++++++++++++++++--------
1 file changed, 69 insertions(+), 12 deletions(-)
diff --git a/lib/device/device_id.c b/lib/device/device_id.c
index 18669630c..749acdf89 100644
--- a/lib/device/device_id.c
+++ b/lib/device/device_id.c
@@ -1359,6 +1359,10 @@ void device_id_update_vg_uuid(struct cmd_context *cmd, struct volume_group *vg,
static int _idtype_compatible_with_major_number(struct cmd_context *cmd, int idtype, int major)
{
+ /* devname can be used with any kind of device */
+ if (idtype == DEV_ID_TYPE_DEVNAME)
+ return 1;
+
if (idtype == DEV_ID_TYPE_MPATH_UUID ||
idtype == DEV_ID_TYPE_CRYPT_UUID ||
idtype == DEV_ID_TYPE_LVMLV_UUID)
@@ -1387,6 +1391,43 @@ static int _idtype_compatible_with_major_number(struct cmd_context *cmd, int idt
return 1;
}
+static int _match_dm_devnames(struct cmd_context *cmd, struct device *dev,
+ struct dev_id *id, struct dev_use *du)
+{
+ struct stat buf;
+
+ if (MAJOR(dev->dev) != cmd->dev_types->device_mapper_major)
+ return 0;
+
+ if (id->idname && du->idname && !strcmp(id->idname, du->idname))
+ return 1;
+
+ if (du->idname && !strcmp(du->idname, dev_name(dev))) {
+ log_debug("Match device_id %s %s to %s: ignoring idname %s",
+ idtype_to_str(du->idtype), du->idname, dev_name(dev), id->idname ?: ".");
+ return 1;
+ }
+
+ if (!du->idname)
+ return 0;
+
+ /* detect that a du entry is for a dm device */
+
+ if (!strncmp(du->idname, "/dev/dm-", 8) || !strncmp(du->idname, "/dev/mapper/", 12)) {
+ if (stat(du->idname, &buf))
+ return 0;
+
+ if ((MAJOR(buf.st_rdev) == cmd->dev_types->device_mapper_major) &&
+ (MINOR(buf.st_rdev) == MINOR(dev->dev))) {
+ log_debug("Match device_id %s %s to %s: using other dm name, ignoring %s",
+ idtype_to_str(du->idtype), du->idname, dev_name(dev), id->idname ?: ".");
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
/*
* check for dev->ids entry with du->idtype, if found compare it,
* if not, system_read of this type and add entry to dev->ids, compare it.
@@ -1407,35 +1448,52 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct
* so we can skip trying to match certain du entries based simply on
* the major number of dev.
*/
- if (!_idtype_compatible_with_major_number(cmd, du->idtype, (int)MAJOR(dev->dev)))
+ if (!_idtype_compatible_with_major_number(cmd, du->idtype, (int)MAJOR(dev->dev))) {
+ /*
+ log_debug("Mismatch device_id %s %s to %s: wrong major",
+ idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev));
+ */
return 0;
+ }
if (!dev_get_partition_number(dev, &part)) {
- log_debug("compare %s failed to get dev partition", dev_name(dev));
+ /*
+ log_debug("Mismatch device_id %s %s to %s: no partition",
+ idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev));
+ */
return 0;
}
if (part != du->part) {
/*
- log_debug("compare mis %s %s part %d to %s part %d",
- idtype_to_str(du->idtype), du->idname ?: ".", du->part, dev_name(dev), part);
+ log_debug("Mismatch device_id %s %s to %s: wrong partition %d vs %d",
+ idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev), du->part, part);
*/
return 0;
}
dm_list_iterate_items(id, &dev->ids) {
if (id->idtype == du->idtype) {
- if (id->idname && !strcmp(id->idname, du->idname)) {
+ 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;
+ dev->id = id;
+ dev->flags |= DEV_MATCHED_USE_ID;
+ 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)) {
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("compare mis %s %s to %s %s",
- idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev),
- ((id->idtype != DEV_ID_TYPE_DEVNAME) && id->idname) ? id->idname : "");
+ log_debug("Mismatch device_id %s %s to %s: idname %s",
+ idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev), id->idname ?: ":");
*/
return 0;
}
@@ -1455,7 +1513,7 @@ 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);
/*
- log_debug("compare mis %s %s to %s no idtype",
+ log_debug("Mismatch device_id %s %s to %s: no idtype",
idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev));
*/
return 0;
@@ -1480,9 +1538,8 @@ static int _match_du_to_dev(struct cmd_context *cmd, struct dev_use *du, struct
}
/*
- log_debug("compare mis %s %s to %s %s",
- idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev),
- ((id->idtype != DEV_ID_TYPE_DEVNAME) && id->idname) ? id->idname : "");
+ log_debug("Mismatch device_id %s %s to %s: idname %s",
+ idtype_to_str(du->idtype), du->idname ?: ".", dev_name(dev), idname);
*/
return 0;
}
2 years, 5 months
main - tests: udev-pvscan-vgchange clear services
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=4926061d32f855ffe7a...
Commit: 4926061d32f855ffe7a38034b2fe2917e63409a0
Parent: 66f0fe57c3b376672696e3e7f4fe9e691a06d482
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Nov 12 12:10:26 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Nov 12 12:10:26 2021 -0600
tests: udev-pvscan-vgchange clear services
---
test/shell/udev-pvscan-vgchange.sh | 10 ++++++++++
1 file changed, 10 insertions(+)
diff --git a/test/shell/udev-pvscan-vgchange.sh b/test/shell/udev-pvscan-vgchange.sh
index fcd8e961b..a82bf8876 100644
--- a/test/shell/udev-pvscan-vgchange.sh
+++ b/test/shell/udev-pvscan-vgchange.sh
@@ -400,3 +400,13 @@ mdadm --stop "$mddev"
aux udev_wait
wipe_all
+systemctl stop lvm-activate-$vg1
+systemctl stop lvm-activate-$vg2
+systemctl stop lvm-activate-$vg3
+systemctl stop lvm-activate-$vg4
+systemctl stop lvm-activate-$vg5
+systemctl stop lvm-activate-$vg6
+systemctl stop lvm-activate-$vg7
+systemctl stop lvm-activate-$vg8
+systemctl stop lvm-activate-$vg9
+
2 years, 5 months
main - online files: fix vgname check
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=66f0fe57c3b37667269...
Commit: 66f0fe57c3b376672696e3e7f4fe9e691a06d482
Parent: 5dbf316cee12484ad701367ff519bebe1523ef9a
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Nov 12 11:52:36 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Nov 12 11:52:36 2021 -0600
online files: fix vgname check
The pvs_online file for a PV will not contain a vgname
if the PV has no metadata, so don't require matching
vgname with the pvs_lookup file.
---
lib/device/online.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/lib/device/online.c b/lib/device/online.c
index c67f45001..024485bb3 100644
--- a/lib/device/online.c
+++ b/lib/device/online.c
@@ -415,8 +415,17 @@ int get_pvs_lookup(struct dm_list *pvs_online, const char *vgname)
if (!online_pvid_file_read(path, &file_major, &file_minor, file_vgname, file_devname))
goto_bad;
- if (vgname && strcmp(file_vgname, vgname))
+ /*
+ * PVs without metadata will not have a vgname in their pvid
+ * file, but the purpose of using the lookup file is that we
+ * know the PV is for this VG even without the pvid vgname
+ * field.
+ */
+ if (vgname && file_vgname[0] && strcmp(file_vgname, vgname)) {
+ /* Should never happen */
+ log_error("Incorrect VG lookup file %s PVID %s %s.", vgname, pvid, file_vgname);
goto_bad;
+ }
if (!(po = zalloc(sizeof(*po))))
goto_bad;
2 years, 5 months
main - pvscan: consistent creation of pvs_lookup file
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=5dbf316cee12484ad70...
Commit: 5dbf316cee12484ad701367ff519bebe1523ef9a
Parent: b945ea1c93a1f6ac76a082391652193b475950c7
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Fri Nov 12 10:06:54 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Fri Nov 12 11:40:06 2021 -0600
pvscan: consistent creation of pvs_lookup file
Consistently create the pvs_lookup file for VGs with
more than one PV. Previously the file create would be
skipped if all the PVs happened to already be online.
That led to unpredicatable results in an uncommon case
(when the last PV to come online is the only PV with
metadata.)
---
tools/pvscan.c | 43 ++++++++++++++++++++++++++++++-------------
1 file changed, 30 insertions(+), 13 deletions(-)
diff --git a/tools/pvscan.c b/tools/pvscan.c
index df1722c45..d360ec87f 100644
--- a/tools/pvscan.c
+++ b/tools/pvscan.c
@@ -289,7 +289,7 @@ static int _write_lookup_file(struct cmd_context *cmd, struct volume_group *vg)
line[ID_LEN+1] = '\0';
if (write(fd, &line, ID_LEN+1) < 0)
- log_sys_debug("write", path);
+ log_error_pvscan(cmd, "Failed to write lookup entry %s %s", path, line);
}
if (close(fd))
@@ -1137,22 +1137,39 @@ static int _online_devs(struct cmd_context *cmd, int do_all, struct dm_list *pvs
if (vg) {
/*
- * Use the VG metadata from this PV for a list of all
- * PVIDs. Write a lookup file of PVIDs in case another
- * pvscan needs it. After writing lookup file, recheck
- * pvid files to resolve a possible race with another
- * pvscan reading the lookup file that missed it.
+ * Check if the VG is complete by checking that
+ * pvs_online/<pvid> files exist for all vg->pvs.
*/
log_debug("checking all pvid files from vg %s", vg->name);
_count_pvid_files(vg, &pvs_online, &pvs_offline);
-
- if (pvs_offline && _write_lookup_file(cmd, vg)) {
- log_debug("rechecking all pvid files from vg %s", vg->name);
- _count_pvid_files(vg, &pvs_online, &pvs_offline);
- if (!pvs_offline)
- log_print_pvscan(cmd, "VG %s complete after recheck.", vg->name);
+
+ /*
+ * When there is more than one PV in the VG, write
+ * /run/lvm/pvs_lookup/<vgname> with a list of PVIDs in
+ * the VG. This is used in case a later PV comes
+ * online that has no metadata, in which case pvscan
+ * for that PV needs to use the lookup file to check if
+ * the VG is complete. The lookup file is also used by
+ * vgchange -aay --autoactivation event <vgname>
+ * to check if all pvs_online files for the VG exist.
+ *
+ * For multiple concurrent pvscan's, they will race to
+ * create the lookup file and the first will succeed.
+ *
+ * After writing the lookup file, recheck pvid files to
+ * resolve a possible race with another pvscan reading
+ * the lookup file that missed it.
+ */
+ if (dm_list_size(&vg->pvs) > 1) {
+ if (_write_lookup_file(cmd, vg)) {
+ if (pvs_offline) {
+ log_debug("rechecking all pvid files from vg %s", vg->name);
+ _count_pvid_files(vg, &pvs_online, &pvs_offline);
+ if (!pvs_offline)
+ log_print_pvscan(cmd, "VG %s complete after recheck.", vg->name);
+ }
+ }
}
-
vgname = vg->name;
} else {
/*
2 years, 5 months
main - tests vgchange-pvs-online: clean up with devices file
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=b945ea1c93a1f6ac76a...
Commit: b945ea1c93a1f6ac76a082391652193b475950c7
Parent: 20c550ab10e50d25249ed0b0553ff8a33ed3348f
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Nov 11 16:38:10 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Nov 11 16:38:10 2021 -0600
tests vgchange-pvs-online: clean up with devices file
changing the dev names resulted in stale devices file
entries that created noise in the output.
---
test/shell/vgchange-pvs-online.sh | 59 ++++++++++++++++++++++++++++-----------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/test/shell/vgchange-pvs-online.sh b/test/shell/vgchange-pvs-online.sh
index 1e9c37db8..ced03f4da 100644
--- a/test/shell/vgchange-pvs-online.sh
+++ b/test/shell/vgchange-pvs-online.sh
@@ -19,20 +19,45 @@ _clear_online_files() {
aux prepare_devs 4
+DFDIR="$LVM_SYSTEM_DIR/devices"
+mkdir -p "$DFDIR" || true
+DF="$DFDIR/system.devices"
+
# Because mapping devno to devname gets dm name from sysfs
aux lvmconf 'devices/scan = "/dev"'
base1=$(basename $dev1)
base2=$(basename $dev2)
base3=$(basename $dev3)
base4=$(basename $dev4)
+bd1=/dev/mapper/$base1
+bd2=/dev/mapper/$base2
+bd3=/dev/mapper/$base3
+bd4=/dev/mapper/$base4
aux extend_filter "a|/dev/mapper/$base1|"
aux extend_filter "a|/dev/mapper/$base2|"
aux extend_filter "a|/dev/mapper/$base3|"
aux extend_filter "a|/dev/mapper/$base4|"
-vgcreate $vg1 "$dev1" "$dev2"
-vgcreate $vg2 "$dev3"
-pvcreate "$dev4"
+# Changing names will confuse df based on devname
+if lvmdevices; then
+rm -f "$DF"
+touch "$DF"
+lvmdevices --adddev "$bd1"
+lvmdevices --adddev "$bd2"
+lvmdevices --adddev "$bd3"
+lvmdevices --adddev "$bd4"
+cat "$DF"
+fi
+
+# Using $bd instead of $dev because validation of pvid file content
+# checks that the devname begins with /dev.
+
+# FIXME: test vgchange aay with pvs_online that includes devname in pvid file
+# and the devices file entry uses devname with a stale name.
+
+vgcreate $vg1 "$bd1" "$bd2"
+vgcreate $vg2 "$bd3"
+pvcreate "$bd4"
lvcreate -l1 -n $lv1 -an $vg1
lvcreate -l1 -n $lv2 -an $vg1
@@ -42,14 +67,14 @@ lvcreate -l1 -n $lv1 -an $vg2
_clear_online_files
-pvscan --cache "$dev1"
-pvscan --cache "$dev2"
+pvscan --cache "$bd1"
+pvscan --cache "$bd2"
vgchange -aay --autoactivation event $vg1
check lv_field $vg1/$lv1 lv_active "active"
check lv_field $vg1/$lv2 lv_active "active"
check lv_field $vg2/$lv1 lv_active ""
-pvscan --cache "$dev3"
+pvscan --cache "$bd3"
vgchange -aay --autoactivation event $vg2
check lv_field $vg2/$lv1 lv_active "active"
@@ -60,13 +85,13 @@ if which strace; then
vgchange -an
_clear_online_files
-pvscan --cache "$dev1"
-pvscan --cache "$dev2"
+pvscan --cache "$bd1"
+pvscan --cache "$bd2"
strace -e io_submit vgchange -aay --autoactivation event $vg1 2>&1|tee trace.out
test "$(grep io_submit trace.out | wc -l)" -eq 3
rm trace.out
-strace -e io_submit pvscan --cache "$dev3" 2>&1|tee trace.out
+strace -e io_submit pvscan --cache "$bd3" 2>&1|tee trace.out
test "$(grep io_submit trace.out | wc -l)" -eq 1
rm trace.out
@@ -85,19 +110,19 @@ check lv_field $vg1/$lv1 lv_active ""
check lv_field $vg1/$lv2 lv_active ""
check lv_field $vg2/$lv1 lv_active ""
-pvscan --cache "$dev1"
+pvscan --cache "$bd1"
vgchange -aay --autoactivation event
check lv_field $vg1/$lv1 lv_active ""
check lv_field $vg1/$lv2 lv_active ""
check lv_field $vg2/$lv1 lv_active ""
-pvscan --cache "$dev2"
+pvscan --cache "$bd2"
vgchange -aay --autoactivation event
check lv_field $vg1/$lv1 lv_active "active"
check lv_field $vg1/$lv2 lv_active "active"
check lv_field $vg2/$lv1 lv_active ""
-pvscan --cache "$dev3"
+pvscan --cache "$bd3"
vgchange -aay --autoactivation event
check lv_field $vg2/$lv1 lv_active "active"
@@ -115,7 +140,7 @@ vgchange -an
_clear_online_files
# incomplete pvs_online, vgchange falls back to full label scan
-pvscan --cache "$dev1"
+pvscan --cache "$bd1"
vgchange -aay --autoactivation event $vg1
check lv_field $vg1/$lv1 lv_active "active"
check lv_field $vg1/$lv2 lv_active "active"
@@ -126,7 +151,7 @@ _clear_online_files
# incomplete pvs_online, pvs_online from different vg,
# no pvs_online found for vg arg so vgchange falls back to full label scan
-pvscan --cache "$dev3"
+pvscan --cache "$bd3"
vgchange -aay --autoactivation event $vg1
check lv_field $vg1/$lv1 lv_active "active"
check lv_field $vg1/$lv2 lv_active "active"
@@ -140,14 +165,14 @@ _clear_online_files
# same tests but using command options matching udev rule
-pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$dev1"
-pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$dev2"
+pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$bd1"
+pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$bd2"
vgchange -aay --autoactivation event $vg1
check lv_field $vg1/$lv1 lv_active "active"
check lv_field $vg1/$lv2 lv_active "active"
check lv_field $vg2/$lv1 lv_active ""
-pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$dev3"
+pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$bd3"
vgchange -aay --autoactivation event $vg2
check lv_field $vg2/$lv1 lv_active "active"
2 years, 5 months
main - tests: udev-pvscan-vgchange fix wait
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=20c550ab10e50d25249...
Commit: 20c550ab10e50d25249ed0b0553ff8a33ed3348f
Parent: 0e0faf30e01f78828b7e240f57217755b62650bb
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Thu Nov 11 16:04:24 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Thu Nov 11 16:04:24 2021 -0600
tests: udev-pvscan-vgchange fix wait
the service now remains after completion
---
test/shell/udev-pvscan-vgchange.sh | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/test/shell/udev-pvscan-vgchange.sh b/test/shell/udev-pvscan-vgchange.sh
index f0d637562..fcd8e961b 100644
--- a/test/shell/udev-pvscan-vgchange.sh
+++ b/test/shell/udev-pvscan-vgchange.sh
@@ -71,15 +71,11 @@ wipe_all() {
done
}
-# udevadm trigger runs udev rule which runs systemd-run --no-wait vgchange -aay
-# Because of --no-wait, we need to wait for the transient systemd
-# service to be gone before checking the effects of the vgchange.
-
wait_lvm_activate() {
local vgw=$1
local wait=0
- while systemctl status lvm-activate-$vgw > /dev/null && test "$wait" -le 30; do
+ while systemctl status lvm-activate-$vgw | grep "active (running)" && test "$wait" -le 30; do
sleep .2
wait=$(( wait + 1 ))
done
2 years, 5 months
main - vgchange autoactivation: lock vg early to avoid second label scan
by David Teigland
Gitweb: https://sourceware.org/git/?p=lvm2.git;a=commitdiff;h=0e0faf30e01f78828b7...
Commit: 0e0faf30e01f78828b7e240f57217755b62650bb
Parent: 92e741eda060b4712fab5b37e0bee2cc5e155a57
Author: David Teigland <teigland(a)redhat.com>
AuthorDate: Wed Nov 10 16:43:21 2021 -0600
Committer: David Teigland <teigland(a)redhat.com>
CommitterDate: Wed Nov 10 16:50:50 2021 -0600
vgchange autoactivation: lock vg early to avoid second label scan
Copy another optimization from pvscan -aay to vgchange -aay.
When using the optimized label scan for only one VG, acquire the
VG lock prior to the scan. This allows vg_read to then skip the
repeated label scan that normally happens after locking the vg.
---
lib/label/label.c | 4 ---
test/shell/vgchange-pvs-online.sh | 4 ++-
tools/vgchange.c | 58 +++++++++++++++++++++------------------
3 files changed, 35 insertions(+), 31 deletions(-)
diff --git a/lib/label/label.c b/lib/label/label.c
index 324cfd034..d506b81e3 100644
--- a/lib/label/label.c
+++ b/lib/label/label.c
@@ -1044,10 +1044,6 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname,
log_debug_devs("Finding online devices to scan");
- /* reads devices file, does not populate dev-cache */
- if (!setup_devices_for_online_autoactivation(cmd))
- return 0;
-
/*
* First attempt to use /run/lvm/pvs_lookup/vgname which should be
* used in cases where all PVs in a VG do not contain metadata.
diff --git a/test/shell/vgchange-pvs-online.sh b/test/shell/vgchange-pvs-online.sh
index c2ebe7327..1e9c37db8 100644
--- a/test/shell/vgchange-pvs-online.sh
+++ b/test/shell/vgchange-pvs-online.sh
@@ -56,13 +56,14 @@ check lv_field $vg2/$lv1 lv_active "active"
# Count io to check the pvs_online optimization
# is working to limit scanning.
+if which strace; then
vgchange -an
_clear_online_files
pvscan --cache "$dev1"
pvscan --cache "$dev2"
strace -e io_submit vgchange -aay --autoactivation event $vg1 2>&1|tee trace.out
-test "$(grep io_submit trace.out | wc -l)" -eq 4
+test "$(grep io_submit trace.out | wc -l)" -eq 3
rm trace.out
strace -e io_submit pvscan --cache "$dev3" 2>&1|tee trace.out
@@ -72,6 +73,7 @@ rm trace.out
strace -e io_submit vgchange -aay --autoactivation event $vg2 2>&1|tee trace.out
test "$(grep io_submit trace.out | wc -l)" -eq 2
rm trace.out
+fi
# non-standard usage: no VG name arg, vgchange will only used pvs_online files
diff --git a/tools/vgchange.c b/tools/vgchange.c
index bbb565643..5eebc9ae9 100644
--- a/tools/vgchange.c
+++ b/tools/vgchange.c
@@ -743,6 +743,7 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
{
const char *aa;
char *vgname = NULL;
+ int vg_locked = 0;
int found_none = 0, found_all = 0, found_incomplete = 0;
if (!(aa = arg_str_value(cmd, autoactivation_ARG, NULL)))
@@ -763,29 +764,35 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
vp->vg_complete_to_activate = 1;
cmd->use_hints = 0;
- get_single_vgname_cmd_arg(cmd, NULL, &vgname);
-
/*
- * Special label scan optimized for autoactivation that is based on
- * info read from /run/lvm/ files created by pvscan --cache during
- * autoactivation. Add an option to disable this optimization? e.g.
+ * Add an option to skip the pvs_online optimization? e.g.
* "online_skip" in --autoactivation / auto_activation_settings
*
- * In some cases it might be useful to strictly follow the online
- * files, and not fall back to a standard label scan when no pvs or
- * incomplete pvs are found from the online files. Add option for
- * that? e.g.
- * "online_only" in --autoactivation / auto_activation_settings
- *
- * Generally the way that vgchange -aay --autoactivation event is
- * currently used, it will not be called until pvscan --cache has found
- * the VG is complete, so it will not generally be following the paths
- * that fall back to standard label_scan.
- *
- * TODO: Like pvscan_aa_quick, this could do lock_vol(vgname) before
- * label_scan_vg_online, then set cmd->can_use_one_scan=1 to avoid
- * rescanning in _vg_read called by process_each_vg.
+ * if (online_skip)
+ * return 1;
+ */
+
+ /* reads devices file, does not populate dev-cache */
+ if (!setup_devices_for_online_autoactivation(cmd))
+ return_0;
+
+ get_single_vgname_cmd_arg(cmd, NULL, &vgname);
+
+ /*
+ * Lock the VG before scanning the PVs so _vg_read can avoid the normal
+ * lock_vol+rescan (READ_WITHOUT_LOCK avoids the normal lock_vol and
+ * can_use_one_scan avoids the normal rescan.) If this early lock_vol
+ * fails, continue and use the normal lock_vol in _vg_read.
*/
+ if (vgname) {
+ if (!lock_vol(cmd, vgname, LCK_VG_WRITE, NULL)) {
+ log_debug("Failed early VG locking for autoactivation.");
+ } else {
+ *flags |= READ_WITHOUT_LOCK;
+ cmd->can_use_one_scan = 1;
+ vg_locked = 1;
+ }
+ }
/*
* Perform label_scan on PVs that are online (per /run/lvm files)
@@ -819,15 +826,14 @@ static int _vgchange_autoactivation_setup(struct cmd_context *cmd,
}
/*
- * Possible option to only activate from only online pvs even if they
- * are not all found, and not fall back to a full label_scan.
+ * The online scanning optimiziation didn't work, so undo the vg
+ * locking optimization before falling back to normal processing.
*/
- /*
- if (online_only) {
- log_print("PVs online %s.", found_none ? "not found" : "incomplete");
- return vgname ? 0 : 1;
+ if (vg_locked) {
+ unlock_vg(cmd, NULL, vgname);
+ *flags &= ~READ_WITHOUT_LOCK;
+ cmd->can_use_one_scan = 0;
}
- */
/*
* Not expected usage, no online pvs for the vgname were found. The
2 years, 5 months