diff --git a/lib/device/dev-cache.c b/lib/device/dev-cache.c index e71cef38d..525dae31e 100644 --- a/lib/device/dev-cache.c +++ b/lib/device/dev-cache.c @@ -2164,7 +2164,7 @@ static char *_get_devname_from_devno(struct cmd_context *cmd, dev_t devno) } if (devname[0]) { - log_debug("Found %s for %d:%d from sys", devname, major, minor); + log_debug("Found %s for %d:%d from sys dm", devname, major, minor); return _strdup(devname); } return NULL; diff --git a/lib/device/online.c b/lib/device/online.c index cd89d72e3..58696871e 100644 --- a/lib/device/online.c +++ b/lib/device/online.c @@ -134,8 +134,12 @@ int get_pvs_online(struct dm_list *pvs_online, const char *vgname) dm_list_add(pvs_online, &po->list); } + if (closedir(dir)) log_sys_debug("closedir", PVS_ONLINE_DIR); + + log_debug("PVs online found %d for %s", dm_list_size(pvs_online), vgname ?: "all"); + return 1; } @@ -355,6 +359,8 @@ int get_pvs_lookup(struct dm_list *pvs_online, const char *vgname) dm_list_add(pvs_online, &po->list); } + log_debug("PVs online lookup found %d for %s", dm_list_size(pvs_online), vgname); + fclose(fp); return 1; diff --git a/lib/label/label.c b/lib/label/label.c index 2f9b5c371..9875b5f02 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -1023,13 +1023,13 @@ int label_scan_for_pvid(struct cmd_context *cmd, char *pvid, struct device **dev /* * Use files under /run/lvm/, created by pvscan --cache autoactivation, - * to optimize device setup/scanning for a command that is run for a - * specific vg name. autoactivation happens during system startup - * when the hints file is not useful, so this uses the online files as - * an alternative. - */ - -int label_scan_vg_online(struct cmd_context *cmd, const char *vgname) + * to optimize device setup/scanning. autoactivation happens during + * system startup when the hints file is not useful, but he pvs_online + * files can provide a similar optimization to the hints file. + */ + +int label_scan_vg_online(struct cmd_context *cmd, const char *vgname, + int *found_none, int *found_all, int *found_incomplete) { struct dm_list pvs_online; struct dm_list devs; @@ -1042,6 +1042,8 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname) dm_list_init(&pvs_online); dm_list_init(&devs); + 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; @@ -1055,11 +1057,21 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname) * info from the online files tell us which devices those PVs are * located on. */ - if (!get_pvs_lookup(&pvs_online, vgname)) { - if (!get_pvs_online(&pvs_online, vgname)) + if (vgname) { + if (!get_pvs_lookup(&pvs_online, vgname)) { + if (!get_pvs_online(&pvs_online, vgname)) + goto bad; + } + } else { + if (!get_pvs_online(&pvs_online, NULL)) goto bad; } + if (dm_list_empty(&pvs_online)) { + *found_none = 1; + return 1; + } + /* * For each po devno add a struct dev to dev-cache. This is a faster * alternative to the usual dev_cache_scan() which looks at all @@ -1201,8 +1213,10 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname) free_po_list(&pvs_online); - if (dm_list_empty(&devs)) + if (dm_list_empty(&devs)) { + *found_none = 1; return 1; + } /* * Scan devs to populate lvmcache info, which includes the mda info that's @@ -1220,13 +1234,17 @@ int label_scan_vg_online(struct cmd_context *cmd, const char *vgname) * be able to fall back to a standard label scan if the online hints * gave fewer PVs than listed in VG metadata. */ - metadata_pv_count = lvmcache_pvsummary_count(vgname); - if (metadata_pv_count != dm_list_size(&devs)) { - log_debug("Incorrect PV list from online files %d metadata %d.", - dm_list_size(&devs), metadata_pv_count); - return 0; + if (vgname) { + metadata_pv_count = lvmcache_pvsummary_count(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); + *found_incomplete = 1; + return 1; + } } + *found_all = 1; return 1; bad: free_po_list(&pvs_online); diff --git a/lib/label/label.h b/lib/label/label.h index 55ebfde45..3cda1818c 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -118,7 +118,9 @@ int label_scan_open_excl(struct device *dev); int label_scan_open_rw(struct device *dev); int label_scan_reopen_rw(struct device *dev); int label_read_pvid(struct device *dev, int *has_pvid); -int label_scan_vg_online(struct cmd_context *cmd, const char *vgname); +int label_scan_vg_online(struct cmd_context *cmd, const char *vgname, + int *found_none, int *found_all, int *found_incomplete); + int label_scan_for_pvid(struct cmd_context *cmd, char *pvid, struct device **dev_out); diff --git a/test/shell/vgchange-pvs-online.sh b/test/shell/vgchange-pvs-online.sh index bdc481ced..c2ebe7327 100644 --- a/test/shell/vgchange-pvs-online.sh +++ b/test/shell/vgchange-pvs-online.sh @@ -19,62 +19,136 @@ _clear_online_files() { aux prepare_devs 4 +# 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) +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" lvcreate -l1 -n $lv1 -an $vg1 +lvcreate -l1 -n $lv2 -an $vg1 lvcreate -l1 -n $lv1 -an $vg2 -# With no pv online files, vgchange that uses online files -# will find no PVs to activate from. +# Expected use, with vg name and all online files exist for vgchange. _clear_online_files -not vgchange -aay --autoactivation event $vg1 -not vgchange -aay --autoactivation event $vg2 -vgchange -aay --autoactivation event - -check lv_field $vg1/$lv1 lv_active "" -check lv_field $vg2/$lv1 lv_active "" - -# incomplete vg will not be activated - pvscan --cache "$dev1" +pvscan --cache "$dev2" vgchange -aay --autoactivation event $vg1 -# VG foo is incomplete -check lv_field $vg1/$lv1 lv_active "" - -# complete vg is activated +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" vgchange -aay --autoactivation event $vg2 check lv_field $vg2/$lv1 lv_active "active" -pvscan --cache "$dev2" -vgchange -aay --autoactivation event $vg1 -check lv_field $vg1/$lv1 lv_active "active" - -vgchange -an $vg1 -vgchange -an $vg2 - -# the same tests but using command options matching udev rule +# Count io to check the pvs_online optimization +# is working to limit scanning. +vgchange -an _clear_online_files -pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$dev1" -vgchange -aay --autoactivation event $vg1 -# VG foo is incomplete +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 +rm trace.out + +strace -e io_submit pvscan --cache "$dev3" 2>&1|tee trace.out +test "$(grep io_submit trace.out | wc -l)" -eq 1 +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 + +# non-standard usage: no VG name arg, vgchange will only used pvs_online files + +vgchange -an +_clear_online_files + +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 "$dev1" +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" +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" +vgchange -aay --autoactivation event +check lv_field $vg2/$lv1 lv_active "active" + +# non-standard usage: include VG name arg, but missing or incomplete pvs_online files + +vgchange -an +_clear_online_files + +# all missing pvs_online, vgchange falls back to full label scan +vgchange -aay --autoactivation event $vg1 +check lv_field $vg1/$lv1 lv_active "active" +check lv_field $vg1/$lv2 lv_active "active" + +vgchange -an +_clear_online_files + +# incomplete pvs_online, vgchange falls back to full label scan +pvscan --cache "$dev1" +vgchange -aay --autoactivation event $vg1 +check lv_field $vg1/$lv1 lv_active "active" +check lv_field $vg1/$lv2 lv_active "active" + +vgchange -an +_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" +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 "" + +vgchange -aay --autoactivation event $vg2 +check lv_field $vg2/$lv1 lv_active "active" + +vgchange -an +_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" +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" vgchange -aay --autoactivation event $vg2 check lv_field $vg2/$lv1 lv_active "active" -pvscan --cache --listvg --checkcomplete --vgonline --autoactivation event --udevoutput --journal=output "$dev2" -vgchange -aay --autoactivation event $vg1 -check lv_field $vg1/$lv1 lv_active "active" - vgchange -an $vg1 vgchange -an $vg2 diff --git a/tools/vgchange.c b/tools/vgchange.c index e20026e4d..acdde97a8 100644 --- a/tools/vgchange.c +++ b/tools/vgchange.c @@ -879,7 +879,9 @@ int vgchange(struct cmd_context *cmd, int argc, char **argv) } if (arg_is_set(cmd, autoactivation_ARG)) { + int found_none = 0, found_all = 0, found_incomplete = 0; int skip_command = 0; + if (!_check_autoactivation(cmd, &vp, &skip_command)) return ECMD_FAILED; if (skip_command) @@ -889,14 +891,49 @@ int vgchange(struct cmd_context *cmd, int argc, char **argv) * 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?) + * Add an option to disable this 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. */ get_single_vgname_cmd_arg(cmd, NULL, &vgname); - if (vgname) { - if (!label_scan_vg_online(cmd, vgname)) - log_debug("Standard label_scan required in place of online scan."); - else + 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 ?: ""); + } else { + if (!vgname) { + /* Not expected usage, activate any VGs that are complete based on pvs_online. */ flags |= PROCESS_SKIP_SCAN; + } else if (found_all) { + /* The expected and optimal usage, only online PVs are read. */ + flags |= PROCESS_SKIP_SCAN; + /* + } else if (online_only) { + log_print("PVs online %s.", found_none ? "not found" : "incomplete"); + return vgname ? ECMD_FAILED : ECMD_PROCESSED; + */ + } else if (found_none) { + /* Not expected usage, use full label_scan in process_each */ + log_print("PVs online not found for VG %s, using all devices.", vgname); + } else if (found_incomplete) { + /* Not expected usage, use full label_scan in process_each */ + log_print("PVs online incomplete for VG %s, using all devicess.", vgname); + } else { + /* Shouldn't happen */ + log_print("PVs online unknown for VG %s, using all devices.", vgname); + } } }