Hello community, here is the log from the commit of package lvm2 for openSUSE:Leap:15.2 checked in at 2020-03-15 07:11:09 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Leap:15.2/lvm2 (Old) and /work/SRC/openSUSE:Leap:15.2/.lvm2.new.3160 (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "lvm2" Sun Mar 15 07:11:09 2020 rev:54 rq:784477 version:2.03.05 Changes: -------- --- /work/SRC/openSUSE:Leap:15.2/lvm2/lvm2.changes 2020-01-15 15:27:51.470658242 +0100 +++ /work/SRC/openSUSE:Leap:15.2/.lvm2.new.3160/lvm2.changes 2020-03-15 07:11:29.068967780 +0100 @@ -1,0 +2,67 @@ +Thu Mar 12 09:53:00 UTC 2020 - [email protected] + +- fix patch name typo + - bug-1158628-04-pvmove-correcting-read_ahead-setting.patch + + bug-1158628_04-pvmove-correcting-read_ahead-setting.patch + +------------------------------------------------------------------- +Mon Feb 27 09:36:00 UTC 2020 - [email protected] + +- vgreduce --removemissing with cache devices will remove lvs (bsc#1157736) + + bug-1157736-add-suggestion-message-for-mirror-LVs.patch + +------------------------------------------------------------------- +Thu Jan 9 10:00:30 UTC 2020 - [email protected] + +- Update lvm.conf file (bsc#1159238) + - enable issue_discards by default + +------------------------------------------------------------------- +Mon Dec 23 07:22:00 UTC 2019 - [email protected] + +- LVM Metadata Error: Error writing device at 4096 length 512 (bsc#1150021) + + bug-1150021_01-scanning-open-devs-rw-when-rescanning-for-write.patch + + bug-1150021_02-bcache-add-bcache_abort.patch + + bug-1150021_03-label-Use-bcache_abort_fd-to-ensure-blocks-are-no-lo.patch + + bug-1150021_04-bcache-add-unit-test.patch + + bug-1150021_05-bcache-bcache_invalidate_fd-only-remove-prefixes-on.patch + + bug-1150021_06-fix-dev_unset_last_byte-after-write-error.patch +- Update patch, according to bug-1150021_01-scanning-xxx.patch + + bug-1158861_06-fix-segfault-for-invalid-characters-in-vg-name.patch + +------------------------------------------------------------------- +Tue Dec 10 08:26:00 UTC 2019 - [email protected] + +- backport patches for lvm2 to avoid software abnormal work (bsc#1158861) + + bug-1158861_01-config-remove-filter-typo.patch + + bug-1158861_02-config-Fix-default-option-which-makes-no-sense.patch + + bug-1158861_03-vgchange-don-t-fail-monitor-command-if-vg-is-exporte.patch + + bug-1158861_04-fix-duplicate-pv-size-check.patch + + bug-1158861_05-hints-fix-copy-of-filter.patch + + bug-1158861_06-fix-segfault-for-invalid-characters-in-vg-name.patch + + bug-1158861_07-vgck-let-updatemetadata-repair-mismatched-metadata.patch + + bug-1158861_08-hints-fix-mem-leaking-buffers.patch + + bug-1158861_09-pvcreate-pvremove-fix-reacquiring-global-lock-after.patch + +------------------------------------------------------------------- +Tue Dec 10 08:11:00 UTC 2019 - [email protected] + +- backport upstream patches for passing lvm2 testsuite (bsc#1158628) + + bug-1158628_01-tests-replaces-grep-q-usage.patch + + bug-1158628_02-tests-fix-ra-checking.patch + + bug-1158628_03-tests-simplify-some-var-settings.patch + + bug-1158628-04-pvmove-correcting-read_ahead-setting.patch + + bug-1158628_05-activation-add-synchronization-point.patch + + bug-1158628_06-pvmove-add-missing-synchronization.patch + + bug-1158628_07-activation-extend-handling-of-pending_delete.patch + + bug-1158628_08-lv_manip-add-synchronizations.patch + + bug-1158628_09-lvconvert-improve-validation-thin-and-cache-pool-con.patch + + bug-1158628_10-thin-activate-layer-pool-aas-read-only-LV.patch + + bug-1158628_11-tests-mdadm-stop-in-test-cleanup.patch + + bug-1158628_12-test-increase-size-of-raid10-LV-allowing-tests-to-su.patch + + bug-1158628_13-lvconvert-fix-return-value-when-zeroing-fails.patch + + bug-1158628_14-tests-add-extra-settle.patch + + bug-1158628_15-test-Fix-handling-leftovers-from-previous-tests.patch + - bug-1043040_test-fix-read-ahead-issues-in-test-scripts.patch + +------------------------------------------------------------------- Old: ---- bug-1043040_test-fix-read-ahead-issues-in-test-scripts.patch New: ---- bug-1150021_01-scanning-open-devs-rw-when-rescanning-for-write.patch bug-1150021_02-bcache-add-bcache_abort.patch bug-1150021_03-label-Use-bcache_abort_fd-to-ensure-blocks-are-no-lo.patch bug-1150021_04-bcache-add-unit-test.patch bug-1150021_05-bcache-bcache_invalidate_fd-only-remove-prefixes-on.patch bug-1150021_06-fix-dev_unset_last_byte-after-write-error.patch bug-1157736-add-suggestion-message-for-mirror-LVs.patch bug-1158628_01-tests-replaces-grep-q-usage.patch bug-1158628_02-tests-fix-ra-checking.patch bug-1158628_03-tests-simplify-some-var-settings.patch bug-1158628_04-pvmove-correcting-read_ahead-setting.patch bug-1158628_05-activation-add-synchronization-point.patch bug-1158628_06-pvmove-add-missing-synchronization.patch bug-1158628_07-activation-extend-handling-of-pending_delete.patch bug-1158628_08-lv_manip-add-synchronizations.patch bug-1158628_09-lvconvert-improve-validation-thin-and-cache-pool-con.patch bug-1158628_10-thin-activate-layer-pool-aas-read-only-LV.patch bug-1158628_11-tests-mdadm-stop-in-test-cleanup.patch bug-1158628_12-test-increase-size-of-raid10-LV-allowing-tests-to-su.patch bug-1158628_13-lvconvert-fix-return-value-when-zeroing-fails.patch bug-1158628_14-tests-add-extra-settle.patch bug-1158628_15-test-Fix-handling-leftovers-from-previous-tests.patch bug-1158861_01-config-remove-filter-typo.patch bug-1158861_02-config-Fix-default-option-which-makes-no-sense.patch bug-1158861_03-vgchange-don-t-fail-monitor-command-if-vg-is-exporte.patch bug-1158861_04-fix-duplicate-pv-size-check.patch bug-1158861_05-hints-fix-copy-of-filter.patch bug-1158861_06-fix-segfault-for-invalid-characters-in-vg-name.patch bug-1158861_07-vgck-let-updatemetadata-repair-mismatched-metadata.patch bug-1158861_08-hints-fix-mem-leaking-buffers.patch bug-1158861_09-pvcreate-pvremove-fix-reacquiring-global-lock-after.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ lvm2.spec ++++++ --- /var/tmp/diff_new_pack.dK2TUm/_old 2020-03-15 07:11:29.872968261 +0100 +++ /var/tmp/diff_new_pack.dK2TUm/_new 2020-03-15 07:11:29.876968264 +0100 @@ -1,7 +1,7 @@ # # spec file for package lvm2 # -# Copyright (c) 2019 SUSE LINUX GmbH, Nuernberg, Germany. +# Copyright (c) 2020 SUSE LLC # # All modifications and additions to the file contributed by third parties # remain the property of their copyright owners, unless otherwise agreed @@ -56,23 +56,53 @@ Source99: baselibs.conf # Upstream patches Patch0001: bug-1122666_devices-drop-open-error-message.patch -Patch0002: bug-1149408_Fix-rounding-writes-up-to-sector-size.patch -Patch0003: bug-1149408_vgcreate-vgextend-restrict-PVs-with-mixed-block-size.patch -Patch0004: bug-1152378-md-component-detection-for-differing-PV-and-device-s.patch -Patch0005: bug-1152378-pvscan-fix-PV-online-when-device-has-a-different-siz.patch -Patch0006: jcs-SLE5498_pvscan-allow-use-of-noudevsync-option.patch -Patch0007: bug-1154655_udev-remove-unsupported-OPTIONS-event_timeout-rule.patch +Patch0002: bug-1150021_01-scanning-open-devs-rw-when-rescanning-for-write.patch +Patch0003: bug-1149408_Fix-rounding-writes-up-to-sector-size.patch +Patch0004: bug-1149408_vgcreate-vgextend-restrict-PVs-with-mixed-block-size.patch +Patch0005: bug-1152378-md-component-detection-for-differing-PV-and-device-s.patch +Patch0006: bug-1152378-pvscan-fix-PV-online-when-device-has-a-different-siz.patch +Patch0007: jcs-SLE5498_pvscan-allow-use-of-noudevsync-option.patch +Patch0008: bug-1154655_udev-remove-unsupported-OPTIONS-event_timeout-rule.patch +Patch0009: bug-1158628_01-tests-replaces-grep-q-usage.patch +Patch0010: bug-1158628_02-tests-fix-ra-checking.patch +Patch0011: bug-1158628_03-tests-simplify-some-var-settings.patch +Patch0012: bug-1158628_04-pvmove-correcting-read_ahead-setting.patch +Patch0013: bug-1158628_05-activation-add-synchronization-point.patch +Patch0014: bug-1158628_06-pvmove-add-missing-synchronization.patch +Patch0015: bug-1158628_07-activation-extend-handling-of-pending_delete.patch +Patch0016: bug-1158628_08-lv_manip-add-synchronizations.patch +Patch0017: bug-1158628_09-lvconvert-improve-validation-thin-and-cache-pool-con.patch +Patch0018: bug-1158628_10-thin-activate-layer-pool-aas-read-only-LV.patch +Patch0019: bug-1158628_11-tests-mdadm-stop-in-test-cleanup.patch +Patch0020: bug-1158628_12-test-increase-size-of-raid10-LV-allowing-tests-to-su.patch +Patch0021: bug-1158628_13-lvconvert-fix-return-value-when-zeroing-fails.patch +Patch0022: bug-1158628_14-tests-add-extra-settle.patch +Patch0023: bug-1158628_15-test-Fix-handling-leftovers-from-previous-tests.patch +Patch0024: bug-1158861_01-config-remove-filter-typo.patch +Patch0025: bug-1158861_02-config-Fix-default-option-which-makes-no-sense.patch +Patch0026: bug-1158861_03-vgchange-don-t-fail-monitor-command-if-vg-is-exporte.patch +Patch0027: bug-1158861_04-fix-duplicate-pv-size-check.patch +Patch0028: bug-1158861_05-hints-fix-copy-of-filter.patch +Patch0029: bug-1158861_06-fix-segfault-for-invalid-characters-in-vg-name.patch +Patch0030: bug-1158861_07-vgck-let-updatemetadata-repair-mismatched-metadata.patch +Patch0031: bug-1158861_08-hints-fix-mem-leaking-buffers.patch +Patch0032: bug-1158861_09-pvcreate-pvremove-fix-reacquiring-global-lock-after.patch +Patch0033: bug-1150021_02-bcache-add-bcache_abort.patch +Patch0034: bug-1150021_03-label-Use-bcache_abort_fd-to-ensure-blocks-are-no-lo.patch +Patch0035: bug-1150021_04-bcache-add-unit-test.patch +Patch0036: bug-1150021_05-bcache-bcache_invalidate_fd-only-remove-prefixes-on.patch +Patch0037: bug-1150021_06-fix-dev_unset_last_byte-after-write-error.patch +Patch0038: bug-1157736-add-suggestion-message-for-mirror-LVs.patch # SUSE patches: 1000+ for LVM # Never upstream Patch1001: cmirrord_remove_date_time_from_compilation.patch Patch1002: fate-309425_display-dm-name-for-lv-name.patch Patch1003: fate-31841_fsadm-add-support-for-btrfs.patch Patch1004: bug-935623_dmeventd-fix-dso-name-wrong-compare.patch -#SUSE patches 2000+ for device mapper, udev rules +# SUSE patches 2000+ for device mapper, udev rules Patch2001: bug-1012973_simplify-special-case-for-md-in-69-dm-lvm-metadata.patch -# 3000+ for test code -Patch3001: bug-1043040_test-fix-read-ahead-issues-in-test-scripts.patch -# patches specif for lvm2.spec +# SUSE patches 3000+ for test code +# SUSE patches 4000+ for lvm2.spec Patch4001: bug-1037309_Makefile-skip-compliling-daemons-lvmlockd-directory.patch # To detect modprobe during build BuildRequires: kmod-compat @@ -123,13 +153,43 @@ %patch0005 -p1 %patch0006 -p1 %patch0007 -p1 +%patch0008 -p1 +%patch0009 -p1 +%patch0010 -p1 +%patch0011 -p1 +%patch0012 -p1 +%patch0013 -p1 +%patch0014 -p1 +%patch0015 -p1 +%patch0016 -p1 +%patch0017 -p1 +%patch0018 -p1 +%patch0019 -p1 +%patch0020 -p1 +%patch0021 -p1 +%patch0022 -p1 +%patch0023 -p1 +%patch0024 -p1 +%patch0025 -p1 +%patch0026 -p1 +%patch0027 -p1 +%patch0028 -p1 +%patch0029 -p1 +%patch0030 -p1 +%patch0031 -p1 +%patch0032 -p1 +%patch0033 -p1 +%patch0034 -p1 +%patch0035 -p1 +%patch0036 -p1 +%patch0037 -p1 +%patch0038 -p1 %patch1001 -p1 %patch1002 -p1 %patch1003 -p1 %patch1004 -p1 %patch2001 -p1 -%patch3001 -p1 %if !%{with lockd} %patch4001 -p1 %endif ++++++ bug-1150021_01-scanning-open-devs-rw-when-rescanning-for-write.patch ++++++ >From d16142f90fdcf2aef42a51ecabd0c4ff11733d7c Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Tue, 11 Jun 2019 16:17:24 -0500 Subject: [PATCH] scanning: open devs rw when rescanning for write When vg_read rescans devices with the intention of writing the VG, the label rescan can open the devs RW so they do not need to be closed and reopened RW in dev_write_bytes. --- lib/cache/lvmcache.c | 17 ++++++++-- lib/cache/lvmcache.h | 1 + lib/label/label.c | 32 +++++++++++++++++- lib/label/label.h | 1 + lib/metadata/metadata-exported.h | 5 +-- lib/metadata/metadata.c | 72 ++++++++++++++++++++++++++-------------- tools/lvchange.c | 2 +- tools/pvscan.c | 2 +- tools/vgchange.c | 4 ++- 9 files changed, 103 insertions(+), 33 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 1d92e0198d..e2d19e8996 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -772,7 +772,7 @@ next: * to that VG after a scan. */ -int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid) +static int _label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid, int rw) { struct dm_list devs; struct device_list *devl, *devl2; @@ -804,7 +804,10 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const /* FIXME: should we also rescan unused_duplicate_devs for devs being rescanned here and then repeat resolving the duplicates? */ - label_scan_devs(cmd, cmd->filter, &devs); + if (rw) + label_scan_devs_rw(cmd, cmd->filter, &devs); + else + label_scan_devs(cmd, cmd->filter, &devs); dm_list_iterate_items_safe(devl, devl2, &devs) { dm_list_del(&devl->list); @@ -819,6 +822,16 @@ int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const return 1; } +int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid) +{ + return _label_rescan_vg(cmd, vgname, vgid, 0); +} + +int lvmcache_label_rescan_vg_rw(struct cmd_context *cmd, const char *vgname, const char *vgid) +{ + return _label_rescan_vg(cmd, vgname, vgid, 1); +} + /* * Uses label_scan to populate lvmcache with 'vginfo' struct for each VG * and associated 'info' structs for those VGs. Only VG summary information diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 5b78d7afb2..e2d967c625 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -71,6 +71,7 @@ void lvmcache_destroy(struct cmd_context *cmd, int retain_orphans, int reset); int lvmcache_label_scan(struct cmd_context *cmd); int lvmcache_label_rescan_vg(struct cmd_context *cmd, const char *vgname, const char *vgid); +int lvmcache_label_rescan_vg_rw(struct cmd_context *cmd, const char *vgname, const char *vgid); /* Add/delete a device */ struct lvmcache_info *lvmcache_add(struct labeller *labeller, const char *pvid, diff --git a/lib/label/label.c b/lib/label/label.c index a8d87ec16c..185e51b0c3 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -1118,7 +1118,37 @@ int label_scan_devs(struct cmd_context *cmd, struct dev_filter *f, struct dm_lis _scan_list(cmd, f, devs, NULL); - /* FIXME: this function should probably fail if any devs couldn't be scanned */ + return 1; +} + +/* + * This function is used when the caller plans to write to the devs, so opening + * them RW during rescan avoids needing to close and reopen with WRITE in + * dev_write_bytes. + */ + +int label_scan_devs_rw(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs) +{ + struct device_list *devl; + + if (!scan_bcache) { + if (!_setup_bcache(0)) + return 0; + } + + dm_list_iterate_items(devl, devs) { + if (_in_bcache(devl->dev)) { + bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _scan_dev_close(devl->dev); + } + /* + * With this flag set, _scan_dev_open() done by + * _scan_list() will do open RW + */ + devl->dev->flags |= DEV_BCACHE_WRITE; + } + + _scan_list(cmd, f, devs, NULL); return 1; } diff --git a/lib/label/label.h b/lib/label/label.h index 07bb77d884..f06b7df639 100644 --- a/lib/label/label.h +++ b/lib/label/label.h @@ -104,6 +104,7 @@ extern struct bcache *scan_bcache; int label_scan(struct cmd_context *cmd); int label_scan_devs(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs); +int label_scan_devs_rw(struct cmd_context *cmd, struct dev_filter *f, struct dm_list *devs); int label_scan_devs_excl(struct dm_list *devs); void label_scan_invalidate(struct device *dev); void label_scan_invalidate_lv(struct cmd_context *cmd, struct logical_volume *lv); diff --git a/lib/metadata/metadata-exported.h b/lib/metadata/metadata-exported.h index 9029d3f640..966c88f95e 100644 --- a/lib/metadata/metadata-exported.h +++ b/lib/metadata/metadata-exported.h @@ -184,8 +184,9 @@ #define READ_ALLOW_EXPORTED 0x00020000U #define READ_OK_NOTFOUND 0x00040000U #define READ_WARN_INCONSISTENT 0x00080000U -#define READ_FOR_UPDATE 0x00100000U /* A meta-flag, useful with toollib for_each_* functions. */ -#define PROCESS_SKIP_SCAN 0x00200000U /* skip lvmcache_label_scan in process_each_pv */ +#define READ_FOR_UPDATE 0x00100000U /* command tells vg_read it plans to write the vg */ +#define PROCESS_SKIP_SCAN 0x00200000U /* skip lvmcache_label_scan in process_each_pv */ +#define READ_FOR_ACTIVATE 0x00400000U /* command tells vg_read it plans to activate the vg */ /* vg_read returns these in error_flags */ #define FAILED_NOT_ENABLED 0x00000001U diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 039a7d690d..7b0d6ce923 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -4476,7 +4476,8 @@ void vg_write_commit_bad_mdas(struct cmd_context *cmd, struct volume_group *vg) static struct volume_group *_vg_read(struct cmd_context *cmd, const char *vgname, const char *vgid, - unsigned precommitted) + unsigned precommitted, + int writing) { const struct format_type *fmt = cmd->fmt; struct format_instance *fid = NULL; @@ -4530,8 +4531,11 @@ static struct volume_group *_vg_read(struct cmd_context *cmd, * we can also skip the rescan in that case. */ if (!cmd->can_use_one_scan || lvmcache_scan_mismatch(cmd, vgname, vgid)) { - log_debug_metadata("Rescanning devices for %s", vgname); - lvmcache_label_rescan_vg(cmd, vgname, vgid); + log_debug_metadata("Rescanning devices for %s %s", vgname, writing ? "rw" : ""); + if (writing) + lvmcache_label_rescan_vg_rw(cmd, vgname, vgid); + else + lvmcache_label_rescan_vg(cmd, vgname, vgid); } else { log_debug_metadata("Skipped rescanning devices for %s", vgname); } @@ -4752,6 +4756,7 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const int missing_pv_flag = 0; uint32_t failure = 0; int writing = (read_flags & READ_FOR_UPDATE); + int activating = (read_flags & READ_FOR_ACTIVATE); if (is_orphan_vg(vg_name)) { log_very_verbose("Reading orphan VG %s", vg_name); @@ -4766,13 +4771,23 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const return NULL; } - if (!lock_vol(cmd, vg_name, writing ? LCK_VG_WRITE : LCK_VG_READ, NULL)) { + /* + * When a command is reading the VG with the intention of eventually + * writing it, it passes the READ_FOR_UPDATE flag. This causes vg_read + * to acquire an exclusive VG lock, and causes vg_read to do some more + * checks, e.g. that the VG is writable and not exported. It also + * means that when the label scan is repeated on the VG's devices, the + * VG's PVs can be reopened read-write when rescanning in anticipation + * of needing to write to them. + */ + + if (!lock_vol(cmd, vg_name, (writing || activating) ? LCK_VG_WRITE : LCK_VG_READ, NULL)) { log_error("Can't get lock for %s", vg_name); failure |= FAILED_LOCKING; goto_bad; } - if (!(vg = _vg_read(cmd, vg_name, vgid, 0))) { + if (!(vg = _vg_read(cmd, vg_name, vgid, 0, writing))) { /* Some callers don't care if the VG doesn't exist and don't want an error message. */ if (!(read_flags & READ_OK_NOTFOUND)) log_error("Volume group \"%s\" not found", vg_name); @@ -4883,29 +4898,36 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const goto_bad; } - if (writing && !(read_flags & READ_ALLOW_EXPORTED) && vg_is_exported(vg)) { - log_error("Volume group %s is exported", vg->name); - failure |= FAILED_EXPORTED; - goto_bad; - } + /* + * If the command intends to write or activate the VG, there are + * additional restrictions. FIXME: These restrictions should + * probably be checked/applied after vg_read returns. + */ + if (writing || activating) { + if (!(read_flags & READ_ALLOW_EXPORTED) && vg_is_exported(vg)) { + log_error("Volume group %s is exported", vg->name); + failure |= FAILED_EXPORTED; + goto_bad; + } - if (writing && !(vg->status & LVM_WRITE)) { - log_error("Volume group %s is read-only", vg->name); - failure |= FAILED_READ_ONLY; - goto_bad; - } + if (!(vg->status & LVM_WRITE)) { + log_error("Volume group %s is read-only", vg->name); + failure |= FAILED_READ_ONLY; + goto_bad; + } - if (!cmd->handles_missing_pvs && (missing_pv_dev || missing_pv_flag) && writing) { - log_error("Cannot change VG %s while PVs are missing.", vg->name); - log_error("See vgreduce --removemissing and vgextend --restoremissing."); - failure |= FAILED_NOT_ENABLED; - goto_bad; - } + if (!cmd->handles_missing_pvs && (missing_pv_dev || missing_pv_flag)) { + log_error("Cannot change VG %s while PVs are missing.", vg->name); + log_error("See vgreduce --removemissing and vgextend --restoremissing."); + failure |= FAILED_NOT_ENABLED; + goto_bad; + } - if (!cmd->handles_unknown_segments && vg_has_unknown_segments(vg) && writing) { - log_error("Cannot change VG %s with unknown segments in it!", vg->name); - failure |= FAILED_NOT_ENABLED; /* FIXME new failure code here? */ - goto_bad; + if (!cmd->handles_unknown_segments && vg_has_unknown_segments(vg)) { + log_error("Cannot change VG %s with unknown segments in it!", vg->name); + failure |= FAILED_NOT_ENABLED; /* FIXME new failure code here? */ + goto_bad; + } } /* diff --git a/tools/lvchange.c b/tools/lvchange.c index 7bdf99742a..e7fb57d1d3 100644 --- a/tools/lvchange.c +++ b/tools/lvchange.c @@ -1435,7 +1435,7 @@ int lvchange_activate_cmd(struct cmd_context *cmd, int argc, char **argv) } else /* Component LVs might be active, support easy deactivation */ cmd->process_component_lvs = 1; - ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_UPDATE, + ret = process_each_lv(cmd, argc, argv, NULL, NULL, READ_FOR_ACTIVATE, NULL, &_lvchange_activate_check, &_lvchange_activate_single); if (ret != ECMD_PROCESSED) diff --git a/tools/pvscan.c b/tools/pvscan.c index d41345fac2..12711cb0ae 100644 --- a/tools/pvscan.c +++ b/tools/pvscan.c @@ -900,7 +900,7 @@ static int _pvscan_aa(struct cmd_context *cmd, struct pvscan_aa_params *pp, return ECMD_PROCESSED; } - ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, READ_FOR_UPDATE, 0, handle, _pvscan_aa_single); + ret = process_each_vg(cmd, 0, NULL, NULL, vgnames, READ_FOR_ACTIVATE, 0, handle, _pvscan_aa_single); destroy_processing_handle(cmd, handle); diff --git a/tools/vgchange.c b/tools/vgchange.c index a17f4566ff..aad6db32fb 100644 --- a/tools/vgchange.c +++ b/tools/vgchange.c @@ -800,8 +800,10 @@ int vgchange(struct cmd_context *cmd, int argc, char **argv) cmd->lockd_vg_enforce_sh = 1; } - if (update || arg_is_set(cmd, activate_ARG)) + if (update) flags |= READ_FOR_UPDATE; + else if (arg_is_set(cmd, activate_ARG)) + flags |= READ_FOR_ACTIVATE; if (!(handle = init_processing_handle(cmd, NULL))) { log_error("Failed to initialize processing handle."); -- 2.16.4 ++++++ bug-1150021_02-bcache-add-bcache_abort.patch ++++++ >From 2938b4dcca0a1df661758abfab7f402ea7aab018 Mon Sep 17 00:00:00 2001 From: Joe Thornber <[email protected]> Date: Mon, 28 Oct 2019 14:29:47 +0000 Subject: [PATCH] [bcache] add bcache_abort() This gives us a way to cope with write failures. --- lib/device/bcache.c | 33 ++++++++++++++++++++++++++ lib/device/bcache.h | 7 ++++++ test/unit/bcache_t.c | 67 +++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 1 deletion(-) diff --git a/lib/device/bcache.c b/lib/device/bcache.c index d100419770..b0edf28062 100644 --- a/lib/device/bcache.c +++ b/lib/device/bcache.c @@ -1382,6 +1382,39 @@ bool bcache_invalidate_fd(struct bcache *cache, int fd) //---------------------------------------------------------------- +static bool _abort_v(struct radix_tree_iterator *it, + uint8_t *kb, uint8_t *ke, union radix_value v) +{ + struct block *b = v.ptr; + + if (b->ref_count) { + log_fatal("bcache_abort: block (%d, %llu) still held", + b->fd, (unsigned long long) b->index); + return true; + } + + _unlink_block(b); + _free_block(b); + + // We can't remove the block from the radix tree yet because + // we're in the middle of an iteration. + return true; +} + +void bcache_abort_fd(struct bcache *cache, int fd) +{ + union key k; + struct radix_tree_iterator it; + + k.parts.fd = fd; + + it.visit = _abort_v; + radix_tree_iterate(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd), &it); + radix_tree_remove_prefix(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd)); +} + +//---------------------------------------------------------------- + void bcache_set_last_byte(struct bcache *cache, int fd, uint64_t offset, int sector_size) { _last_byte_fd = fd; diff --git a/lib/device/bcache.h b/lib/device/bcache.h index 8c16caa199..7622fd354f 100644 --- a/lib/device/bcache.h +++ b/lib/device/bcache.h @@ -144,6 +144,13 @@ bool bcache_invalidate(struct bcache *cache, int fd, block_address index); */ bool bcache_invalidate_fd(struct bcache *cache, int fd); +/* + * Call this function if flush, or invalidate fail and you do not + * wish to retry the writes. This will throw away any dirty data + * not written. If any blocks for fd are held, then it will call + * abort(). + */ +void bcache_abort_fd(struct bcache *cache, int fd); //---------------------------------------------------------------- // The next four functions are utilities written in terms of the above api. diff --git a/test/unit/bcache_t.c b/test/unit/bcache_t.c index 92c2d57d4d..668d24d776 100644 --- a/test/unit/bcache_t.c +++ b/test/unit/bcache_t.c @@ -793,7 +793,6 @@ static void test_invalidate_after_write_error(void *context) static void test_invalidate_held_block(void *context) { - struct fixture *f = context; struct mock_engine *me = f->me; struct bcache *cache = f->cache; @@ -809,6 +808,67 @@ static void test_invalidate_held_block(void *context) bcache_put(b); } +//---------------------------------------------------------------- +// abort tests + +static void test_abort_no_blocks(void *context) +{ + struct fixture *f = context; + struct bcache *cache = f->cache; + int fd = 17; + + // We have no expectations + bcache_abort_fd(cache, fd); +} + +static void test_abort_single_block(void *context) +{ + struct fixture *f = context; + struct bcache *cache = f->cache; + struct block *b; + int fd = 17; + + T_ASSERT(bcache_get(cache, fd, 0, GF_ZERO, &b)); + bcache_put(b); + + bcache_abort_fd(cache, fd); + + // no write should be issued + T_ASSERT(bcache_flush(cache)); +} + +static void test_abort_only_specific_fd(void *context) +{ + struct fixture *f = context; + struct mock_engine *me = f->me; + struct bcache *cache = f->cache; + struct block *b; + int fd1 = 17, fd2 = 18; + + T_ASSERT(bcache_get(cache, fd1, 0, GF_ZERO, &b)); + bcache_put(b); + + T_ASSERT(bcache_get(cache, fd1, 1, GF_ZERO, &b)); + bcache_put(b); + + T_ASSERT(bcache_get(cache, fd2, 0, GF_ZERO, &b)); + bcache_put(b); + + T_ASSERT(bcache_get(cache, fd2, 1, GF_ZERO, &b)); + bcache_put(b); + + bcache_abort_fd(cache, fd2); + + // writes for fd1 should still be issued + _expect_write(me, fd1, 0); + _expect_write(me, fd1, 1); + + _expect(me, E_WAIT); + _expect(me, E_WAIT); + + T_ASSERT(bcache_flush(cache)); +} + //---------------------------------------------------------------- // Chasing a bug reported by dct @@ -897,6 +957,11 @@ static struct test_suite *_small_tests(void) T("invalidate-read-error", "invalidate a block that errored", test_invalidate_after_read_error); T("invalidate-write-error", "invalidate a block that errored", test_invalidate_after_write_error); T("invalidate-fails-in-held", "invalidating a held block fails", test_invalidate_held_block); + + T("abort-with-no-blocks", "you can call abort, even if there are no blocks in the cache", test_abort_no_blocks); + T("abort-single-block", "single block get silently discarded", test_abort_single_block); + T("abort-specific-fd", "abort doesn't effect other fds", test_abort_only_specific_fd); + T("concurrent-reads-after-invalidate", "prefetch should still issue concurrent reads after invalidate", test_concurrent_reads_after_invalidate); -- 2.16.4 ++++++ bug-1150021_03-label-Use-bcache_abort_fd-to-ensure-blocks-are-no-lo.patch ++++++ diff -Nupr a/lib/label/label.c b/lib/label/label.c --- a/lib/label/label.c 2019-12-23 16:01:02.144729254 +0800 +++ b/lib/label/label.c 2019-12-23 16:01:25.752772110 +0800 @@ -619,6 +619,14 @@ static void _drop_bad_aliases(struct dev } } +// Like bcache_invalidate, only it throws any dirty data away if the +// write fails. +static void _invalidate_fd(struct bcache *cache, int fd) +{ + if (!bcache_invalidate_fd(cache, fd)) + bcache_abort_fd(cache, fd); +} + /* * Read or reread label/metadata from selected devs. * @@ -730,7 +738,7 @@ static int _scan_list(struct cmd_context * drop it from bcache. */ if (scan_failed || !is_lvm_device) { - bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _invalidate_fd(scan_bcache, devl->dev->bcache_fd); _scan_dev_close(devl->dev); } @@ -935,7 +943,7 @@ int label_scan(struct cmd_context *cmd) * so this will usually not be true. */ if (_in_bcache(dev)) { - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); } @@ -1111,7 +1119,7 @@ int label_scan_devs(struct cmd_context * dm_list_iterate_items(devl, devs) { if (_in_bcache(devl->dev)) { - bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _invalidate_fd(scan_bcache, devl->dev->bcache_fd); _scan_dev_close(devl->dev); } } @@ -1138,7 +1146,7 @@ int label_scan_devs_rw(struct cmd_contex dm_list_iterate_items(devl, devs) { if (_in_bcache(devl->dev)) { - bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _invalidate_fd(scan_bcache, devl->dev->bcache_fd); _scan_dev_close(devl->dev); } /* @@ -1160,7 +1168,7 @@ int label_scan_devs_excl(struct dm_list dm_list_iterate_items(devl, devs) { if (_in_bcache(devl->dev)) { - bcache_invalidate_fd(scan_bcache, devl->dev->bcache_fd); + _invalidate_fd(scan_bcache, devl->dev->bcache_fd); _scan_dev_close(devl->dev); } /* @@ -1180,7 +1188,7 @@ int label_scan_devs_excl(struct dm_list void label_scan_invalidate(struct device *dev) { if (_in_bcache(dev)) { - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); } } @@ -1262,7 +1270,7 @@ int label_read(struct device *dev) dm_list_add(&one_dev, &devl->list); if (_in_bcache(dev)) { - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); } @@ -1304,7 +1312,7 @@ int label_scan_open_excl(struct device * if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_EXCL)) { /* FIXME: avoid tossing out bcache blocks just to replace fd. */ log_debug("Close and reopen excl %s", dev_name(dev)); - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); } dev->flags |= DEV_BCACHE_EXCL; @@ -1317,7 +1325,7 @@ int label_scan_open_rw(struct device *de if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_WRITE)) { /* FIXME: avoid tossing out bcache blocks just to replace fd. */ log_debug("Close and reopen rw %s", dev_name(dev)); - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); } dev->flags |= DEV_BCACHE_WRITE; @@ -1365,7 +1373,7 @@ bool dev_write_bytes(struct device *dev, if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_WRITE)) { /* FIXME: avoid tossing out bcache blocks just to replace fd. */ log_debug("Close and reopen to write %s", dev_name(dev)); - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); dev->flags |= DEV_BCACHE_WRITE; @@ -1411,7 +1419,7 @@ bool dev_write_zeros(struct device *dev, if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_WRITE)) { /* FIXME: avoid tossing out bcache blocks just to replace fd. */ log_debug("Close and reopen to write %s", dev_name(dev)); - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); dev->flags |= DEV_BCACHE_WRITE; @@ -1462,7 +1470,7 @@ bool dev_set_bytes(struct device *dev, u if (_in_bcache(dev) && !(dev->flags & DEV_BCACHE_WRITE)) { /* FIXME: avoid tossing out bcache blocks just to replace fd. */ log_debug("Close and reopen to write %s", dev_name(dev)); - bcache_invalidate_fd(scan_bcache, dev->bcache_fd); + _invalidate_fd(scan_bcache, dev->bcache_fd); _scan_dev_close(dev); /* goes to label_scan_open() since bcache_fd < 0 */ } ++++++ bug-1150021_04-bcache-add-unit-test.patch ++++++ >From 5fdebf9bbf68a53b9d2153dbd8bf27a36e0ef3cd Mon Sep 17 00:00:00 2001 From: Joe Thornber <[email protected]> Date: Tue, 29 Oct 2019 10:33:31 +0000 Subject: [PATCH] [bcache] add unit test abort-forces-read --- test/unit/bcache_t.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/unit/bcache_t.c b/test/unit/bcache_t.c index 668d24d776..2a8f931e45 100644 --- a/test/unit/bcache_t.c +++ b/test/unit/bcache_t.c @@ -837,6 +837,29 @@ static void test_abort_single_block(void *context) T_ASSERT(bcache_flush(cache)); } +static void test_abort_forces_reread(void *context) +{ + struct fixture *f = context; + struct mock_engine *me = f->me; + struct bcache *cache = f->cache; + struct block *b; + int fd = 17; + + _expect_read(me, fd, 0); + _expect(me, E_WAIT); + T_ASSERT(bcache_get(cache, fd, 0, GF_DIRTY, &b)); + bcache_put(b); + + bcache_abort_fd(cache, fd); + T_ASSERT(bcache_flush(cache)); + + // Check the block is re-read + _expect_read(me, fd, 0); + _expect(me, E_WAIT); + T_ASSERT(bcache_get(cache, fd, 0, 0, &b)); + bcache_put(b); +} + static void test_abort_only_specific_fd(void *context) { struct fixture *f = context; @@ -960,6 +983,7 @@ static struct test_suite *_small_tests(void) T("abort-with-no-blocks", "you can call abort, even if there are no blocks in the cache", test_abort_no_blocks); T("abort-single-block", "single block get silently discarded", test_abort_single_block); + T("abort-forces-read", "if a block has been discarded then another read is necc.", test_abort_forces_reread); T("abort-specific-fd", "abort doesn't effect other fds", test_abort_only_specific_fd); T("concurrent-reads-after-invalidate", "prefetch should still issue concurrent reads after invalidate", -- 2.16.4 ++++++ bug-1150021_05-bcache-bcache_invalidate_fd-only-remove-prefixes-on.patch ++++++ >From 25e7bf021a4e7c5ad5f925b86605bf025ff1a949 Mon Sep 17 00:00:00 2001 From: Joe Thornber <[email protected]> Date: Tue, 29 Oct 2019 15:21:11 +0000 Subject: [PATCH] [bcache] bcache_invalidate_fd, only remove prefixes on success. --- lib/device/bcache.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/device/bcache.c b/lib/device/bcache.c index b0edf28062..04d49f1656 100644 --- a/lib/device/bcache.c +++ b/lib/device/bcache.c @@ -1376,7 +1376,10 @@ bool bcache_invalidate_fd(struct bcache *cache, int fd) it.success = true; it.it.visit = _invalidate_v; radix_tree_iterate(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd), &it.it); - radix_tree_remove_prefix(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd)); + + if (it.success) + radix_tree_remove_prefix(cache->rtree, k.bytes, k.bytes + sizeof(k.parts.fd)); + return it.success; } -- 2.16.4 ++++++ bug-1150021_06-fix-dev_unset_last_byte-after-write-error.patch ++++++ >From 13c254fc05386d05ab6bbda2806f9ca4d3358a0c Mon Sep 17 00:00:00 2001 From: Heming Zhao <[email protected]> Date: Wed, 13 Nov 2019 09:15:07 -0600 Subject: [PATCH] fix dev_unset_last_byte after write error dev_unset_last_byte() must be called while the fd is still valid. After a write error, dev_unset_last_byte() must be called before closing the dev and resetting the fd. In the write error path, dev_unset_last_byte() was being called after label_scan_invalidate() which meant that it would not unset the last_byte values. After a write error, dev_unset_last_byte() is now called in dev_write_bytes() before label_scan_invalidate(), instead of by the caller of dev_write_bytes(). In the common case of a successful write, the sequence is still: dev_set_last_byte(); dev_write_bytes(); dev_unset_last_byte(); Signed-off-by: Zhao Heming <[email protected]> --- lib/format_text/format-text.c | 3 --- lib/label/label.c | 8 ++++---- lib/metadata/mirror.c | 1 - 3 files changed, 4 insertions(+), 8 deletions(-) diff --git a/lib/format_text/format-text.c b/lib/format_text/format-text.c index 6ec47bfcef..894a71007d 100644 --- a/lib/format_text/format-text.c +++ b/lib/format_text/format-text.c @@ -277,7 +277,6 @@ static int _raw_write_mda_header(const struct format_type *fmt, dev_set_last_byte(dev, start_byte + MDA_HEADER_SIZE); if (!dev_write_bytes(dev, start_byte, MDA_HEADER_SIZE, mdah)) { - dev_unset_last_byte(dev); log_error("Failed to write mda header to %s fd %d", dev_name(dev), dev->bcache_fd); return 0; } @@ -769,7 +768,6 @@ static int _vg_write_raw(struct format_i if (!dev_write_bytes(mdac->area.dev, mdac->area.start + rlocn_new->offset, (size_t) (rlocn_new->size - new_wrap), new_buf)) { log_error("Failed to write metadata to %s fd %d", dev_name(mdac->area.dev), mdac->area.dev->bcache_fd); - dev_unset_last_byte(mdac->area.dev); goto out; } @@ -782,7 +780,6 @@ static int _vg_write_raw(struct format_i if (!dev_write_bytes(mdac->area.dev, mdac->area.start + MDA_HEADER_SIZE, (size_t) new_wrap, new_buf + rlocn_new->size - new_wrap)) { log_error("Failed to write metadata wrap to %s fd %d", dev_name(mdac->area.dev), mdac->area.dev->bcache_fd); - dev_unset_last_byte(mdac->area.dev); goto out; } } diff --git a/lib/label/label.c b/lib/label/label.c index 7dcaf8afd5..05986cbe70 100644 --- a/lib/label/label.c +++ b/lib/label/label.c @@ -218,7 +218,7 @@ int label_write(struct device *dev, struct label *label) if (!dev_write_bytes(dev, offset, LABEL_SIZE, buf)) { log_debug_devs("Failed to write label to %s", dev_name(dev)); - r = 0; + return 0; } dev_unset_last_byte(dev); @@ -655,7 +655,6 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, int submit_count; int scan_failed; int is_lvm_device; - int error; int ret; dm_list_init(&wait_devs); @@ -702,12 +701,11 @@ static int _scan_list(struct cmd_context *cmd, struct dev_filter *f, dm_list_iterate_items_safe(devl, devl2, &wait_devs) { bb = NULL; - error = 0; scan_failed = 0; is_lvm_device = 0; if (!bcache_get(scan_bcache, devl->dev->bcache_fd, 0, 0, &bb)) { - log_debug_devs("Scan failed to read %s error %d.", dev_name(devl->dev), error); + log_debug_devs("Scan failed to read %s.", dev_name(devl->dev)); scan_failed = 1; scan_read_errors++; scan_failed_count++; @@ -1451,6 +1449,7 @@ bool dev_write_bytes(struct device *dev, uint64_t start, size_t len, void *data) if (!bcache_write_bytes(scan_bcache, dev->bcache_fd, start, len, data)) { log_error("Error writing device %s at %llu length %u.", dev_name(dev), (unsigned long long)start, (uint32_t)len); + dev_unset_last_byte(dev); label_scan_invalidate(dev); return false; } @@ -1458,6 +1457,7 @@ bool dev_write_bytes(struct device *dev, uint64_t start, size_t len, void *data) if (!bcache_flush(scan_bcache)) { log_error("Error writing device %s at %llu length %u.", dev_name(dev), (unsigned long long)start, (uint32_t)len); + dev_unset_last_byte(dev); label_scan_invalidate(dev); return false; } diff --git a/lib/metadata/mirror.c b/lib/metadata/mirror.c index 01f0246b90..d8803b3e3f 100644 --- a/lib/metadata/mirror.c +++ b/lib/metadata/mirror.c @@ -266,7 +266,6 @@ static int _write_log_header(struct cmd_context *cmd, struct logical_volume *lv) dev_set_last_byte(dev, sizeof(log_header)); if (!dev_write_bytes(dev, UINT64_C(0), sizeof(log_header), &log_header)) { - dev_unset_last_byte(dev); log_error("Failed to write log header to %s.", name); return 0; } -- 2.16.4 ++++++ bug-1157736-add-suggestion-message-for-mirror-LVs.patch ++++++ >From d53bfae273677975bd805bfaa3d7e7cd995aaa52 Mon Sep 17 00:00:00 2001 From: Heming Zhao <[email protected]> Date: Wed, 15 Jan 2020 02:44:30 +0000 Subject: [PATCH] add suggestion message for mirror LVs Currently the error messages are not clear. This very easy to guide user to execute "--removemissing --force", it is dangerous and will make the LVs to be destroied. Signed-off-by: Zhao Heming <[email protected]> --- tools/vgreduce.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/vgreduce.c b/tools/vgreduce.c index b001ccb..4a4202e 100644 --- a/tools/vgreduce.c +++ b/tools/vgreduce.c @@ -67,6 +67,8 @@ static int _consolidate_vg(struct cmd_context *cmd, struct volume_group *vg) cmd->handles_missing_pvs = 1; log_error("There are still partial LVs in VG %s.", vg->name); log_error("To remove them unconditionally use: vgreduce --removemissing --force."); + log_error("To remove them unconditionally from mirror LVs use: vgreduce" + " --removemissing --mirrorsonly --force."); log_warn("WARNING: Proceeding to remove empty missing PVs."); } -- 1.8.3.1 ++++++ bug-1158628_01-tests-replaces-grep-q-usage.patch ++++++ >From 3596210e02452f3785f0639115f6744f8f7e2dfc Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Mon, 17 Jun 2019 22:47:35 +0200 Subject: [PATCH] tests: replaces grep -q usage Since we use 'set -euE -o pipefail' for shell execution, any failure of any command in the 'piped' shell can result in failure of whole executed chain - resulting in typically unsually test skip, that was left unnoticed. Since checked command have usually short output, the simplest fix seems to be to let grep parse whole output instead of quiting after first match. --- test/lib/aux.sh | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/test/lib/aux.sh b/test/lib/aux.sh index e3f624cda8..81e8f91c98 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -26,7 +26,7 @@ expect_failure() { check_daemon_in_builddir() { # skip if we don't have our own deamon... if test -z "${installed_testsuite+varset}"; then - (which "$1" 2>/dev/null | grep -q "$abs_builddir") || skip "$1 is not in executed path." + (which "$1" 2>/dev/null | grep "$abs_builddir" >/dev/null ) || skip "$1 is not in executed path." fi rm -f debug.log strace.log } @@ -167,7 +167,7 @@ prepare_clvmd() { test -e "$DM_DEV_DIR/control" || dmsetup table >/dev/null # create control node # skip if singlenode is not compiled in - (clvmd --help 2>&1 | grep "Available cluster managers" | grep -q "singlenode") || \ + (clvmd --help 2>&1 | grep "Available cluster managers" | grep "singlenode" >/dev/null) || \ skip "Compiled clvmd does not support singlenode for testing." # lvmconf "activation/monitoring = 1" @@ -531,7 +531,7 @@ teardown() { dm_table | not grep -E -q "$vg|$vg1|$vg2|$vg3|$vg4" || { # Avoid activation of dmeventd if there is no pid cfg=$(test -s LOCAL_DMEVENTD || echo "--config activation{monitoring=0}") - if dm_info suspended,name | grep -q "^Suspended:.*$PREFIX" ; then + if dm_info suspended,name | grep "^Suspended:.*$PREFIX" >/dev/null ; then echo "## skipping vgremove, suspended devices detected." else vgremove -ff "$cfg" \ @@ -662,7 +662,7 @@ prepare_scsi_debug_dev() { # Skip test if scsi_debug module is unavailable or is already in use modprobe --dry-run scsi_debug || skip - lsmod | not grep -q scsi_debug || skip + lsmod | not grep scsi_debug >/dev/null || skip # Create the scsi_debug device and determine the new scsi device's name # NOTE: it will _never_ make sense to pass num_tgts param; @@ -1447,7 +1447,7 @@ driver_at_least() { } have_thin() { - lvm segtypes 2>/dev/null | grep -q thin$ || { + lvm segtypes 2>/dev/null | grep thin$ >/dev/null || { echo "Thin is not built-in." >&2 return 1 } @@ -1471,7 +1471,7 @@ have_thin() { } have_vdo() { - lvm segtypes 2>/dev/null | grep -q vdo$ || { + lvm segtypes 2>/dev/null | grep vdo$ >/dev/null || { echo "VDO is not built-in." >&2 return 1 } @@ -1507,7 +1507,7 @@ have_raid4 () { } have_cache() { - lvm segtypes 2>/dev/null | grep -q cache$ || { + lvm segtypes 2>/dev/null | grep cache$ >/dev/null || { echo "Cache is not built-in." >&2 return 1 } -- 2.24.0 ++++++ bug-1158628_02-tests-fix-ra-checking.patch ++++++ >From d3903d94e910b7ca673ef090c6fe9f79b04b7dd5 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Tue, 18 Jun 2019 13:20:27 +0200 Subject: [PATCH] tests: fix ra checking Since with some installed package like 'tuned' the value of 'RA' on PV origin device can be different, adapting tests to count with this. --- test/shell/lvcreate-usage.sh | 14 ++++++++++---- test/shell/read-ahead.sh | 6 +++++- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/test/shell/lvcreate-usage.sh b/test/shell/lvcreate-usage.sh index 6d424014e2..ca8694e8e6 100644 --- a/test/shell/lvcreate-usage.sh +++ b/test/shell/lvcreate-usage.sh @@ -175,15 +175,21 @@ check lv_field $vg/$lv2 lv_kernel_read_ahead "0" lvcreate -L 8 -n $lv3 --readahead 8k $vg check lv_field $vg/$lv3 lv_read_ahead "8.00k" check lv_field $vg/$lv3 lv_kernel_read_ahead "8.00k" -lvcreate -L 8 -n $lv4 --readahead auto $vg +lvcreate -L 8 -n $lv4 --readahead auto $vg "$dev1" check lv_field $vg/$lv4 lv_read_ahead "auto" -check lv_field $vg/$lv4 lv_kernel_read_ahead "128.00k" +# figure RA value of a PV origin device +DEVICE=$(dmsetup deps -o blkdevname "$dev1" | sed -e "s,.*:\ (\(.*\)),/dev/\1,") +RASZ=$(( $(blockdev --getra "$DEVICE" ) / 2 )) +SZ="$RASZ.00k" +test "$RASZ" -ge 128 || SZ="128.00k" +check lv_field $vg/$lv4 lv_kernel_read_ahead "$SZ" --units k lvcreate -L 8 -n $lv5 -i2 --stripesize 16k --readahead auto $vg check lv_field $vg/$lv5 lv_read_ahead "auto" -check lv_field $vg/$lv5 lv_kernel_read_ahead "128.00k" +check lv_field $vg/$lv5 lv_kernel_read_ahead "$SZ" --units k lvcreate -L 8 -n $lv6 -i2 --stripesize 128k --readahead auto $vg check lv_field $vg/$lv6 lv_read_ahead "auto" -check lv_field $vg/$lv6 lv_kernel_read_ahead "512.00k" +test "$RASZ" -ge 512 || SZ="512.00k" +check lv_field $vg/$lv6 lv_kernel_read_ahead "$SZ" --units k lvremove -ff $vg # diff --git a/test/shell/read-ahead.sh b/test/shell/read-ahead.sh index 60e5912f2f..53cc572b9a 100644 --- a/test/shell/read-ahead.sh +++ b/test/shell/read-ahead.sh @@ -35,8 +35,12 @@ lvremove -ff $vg #COMM "read ahead is properly inherited from underlying PV" blockdev --setra 768 "$dev1" vgscan +DEVICE=$(dmsetup deps -o blkdevname "$dev1" | sed -e "s,.*:\ (\(.*\)),/dev/\1,") +RASZ=$(blockdev --getra "$DEVICE") +SZ=$RASZ +test "$RASZ" -ge 768 || SZ=768 lvcreate -n $lv -L4m $vg "$dev1" -test "$(blockdev --getra "$DM_DEV_DIR/$vg/$lv")" -eq 768 +test "$(blockdev --getra "$DM_DEV_DIR/$vg/$lv")" -eq "$SZ" lvremove -ff $vg # Check default, active/inactive values for read_ahead / kernel_read_ahead -- 2.24.0 ++++++ bug-1158628_03-tests-simplify-some-var-settings.patch ++++++ >From e653f43732fd61f8dc272b157f323663abce3daa Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Thu, 20 Jun 2019 00:03:18 +0200 Subject: [PATCH] tests: simplify some var settings scan_lvs now automatically comes with extend_filter_LVMTEST --- test/lib/aux.sh | 2 +- test/shell/discards-thin.sh | 2 -- test/shell/lvconvert-thin.sh | 2 -- test/shell/lvcreate-large-raid10.sh | 1 - test/shell/lvcreate-large.sh | 2 -- test/shell/pvcreate-operation-md.sh | 3 +-- test/shell/pvcreate-restore.sh | 2 -- test/shell/pvremove-thin.sh | 2 -- test/shell/scan-lvs.sh | 4 +--- test/shell/snapshot-remove-dmsetup.sh | 2 -- test/shell/snapshot-usage-exa.sh | 2 -- test/shell/snapshot-usage.sh | 2 -- test/shell/vgsplit-stacked.sh | 2 -- 13 files changed, 3 insertions(+), 25 deletions(-) diff --git a/test/lib/aux.sh b/test/lib/aux.sh index 81e8f91c98..32d5a0ba7b 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -1164,7 +1164,7 @@ devices/default_data_alignment = 1 devices/dir = "$DM_DEV_DIR" devices/filter = "a|.*|" devices/global_filter = [ "a|$DM_DEV_DIR/mapper/${PREFIX}.*pv[0-9_]*$|", "r|.*|" ] -devices/md_component_detection = 0 +devices/md_component_detection = 0 devices/scan = "$DM_DEV_DIR" devices/sysfs_scan = 1 devices/write_cache_state = 0 diff --git a/test/shell/discards-thin.sh b/test/shell/discards-thin.sh index 3564a8793e..f27d4c3d14 100644 --- a/test/shell/discards-thin.sh +++ b/test/shell/discards-thin.sh @@ -25,8 +25,6 @@ export LVM_TEST_THIN_REPAIR_CMD=${LVM_TEST_THIN_REPAIR_CMD-/bin/false} # aux have_thin 1 1 0 || skip -aux lvmconf 'devices/scan_lvs = 1' - aux prepare_vg 2 64 get_devs diff --git a/test/shell/lvconvert-thin.sh b/test/shell/lvconvert-thin.sh index 731c45bd5e..1f8d2edb9a 100644 --- a/test/shell/lvconvert-thin.sh +++ b/test/shell/lvconvert-thin.sh @@ -17,8 +17,6 @@ export LVM_TEST_THIN_REPAIR_CMD=${LVM_TEST_THIN_REPAIR_CMD-/bin/false} . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - prepare_lvs() { lvremove -f $vg lvcreate -L10M -n $lv1 $vg diff --git a/test/shell/lvcreate-large-raid10.sh b/test/shell/lvcreate-large-raid10.sh index 1ad09aac52..a46be374f6 100644 --- a/test/shell/lvcreate-large-raid10.sh +++ b/test/shell/lvcreate-large-raid10.sh @@ -20,7 +20,6 @@ SKIP_WITH_LVMPOLLD=1 # FIXME update test to make something useful on <16T aux can_use_16T || skip aux have_raid 1 3 0 || skip -aux lvmconf 'devices/scan_lvs = 1' aux prepare_vg 5 diff --git a/test/shell/lvcreate-large.sh b/test/shell/lvcreate-large.sh index b1cb0b0831..473b0edb36 100644 --- a/test/shell/lvcreate-large.sh +++ b/test/shell/lvcreate-large.sh @@ -17,8 +17,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - # FIXME update test to make something useful on <16T aux can_use_16T || skip diff --git a/test/shell/pvcreate-operation-md.sh b/test/shell/pvcreate-operation-md.sh index 11f08877f4..4b51932374 100644 --- a/test/shell/pvcreate-operation-md.sh +++ b/test/shell/pvcreate-operation-md.sh @@ -22,8 +22,7 @@ test -f /proc/mdstat && grep -q raid0 /proc/mdstat || \ modprobe raid0 || skip aux lvmconf 'devices/md_component_detection = 1' -aux extend_filter_LVMTEST -aux extend_filter "a|/dev/md.*|" +aux extend_filter_LVMTEST "a|/dev/md|" aux prepare_devs 2 diff --git a/test/shell/pvcreate-restore.sh b/test/shell/pvcreate-restore.sh index 789f45c16f..d0b46eb2bf 100644 --- a/test/shell/pvcreate-restore.sh +++ b/test/shell/pvcreate-restore.sh @@ -15,8 +15,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - aux prepare_vg 4 lvcreate --type snapshot -s -L10 -n $lv1 $vg --virtualsize 2T diff --git a/test/shell/pvremove-thin.sh b/test/shell/pvremove-thin.sh index 84a2a558b1..9859b6cadb 100644 --- a/test/shell/pvremove-thin.sh +++ b/test/shell/pvremove-thin.sh @@ -18,8 +18,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - aux prepare_vg aux have_thin 1 8 0 || skip diff --git a/test/shell/scan-lvs.sh b/test/shell/scan-lvs.sh index 8e8a77d3e9..f49bf4df5c 100644 --- a/test/shell/scan-lvs.sh +++ b/test/shell/scan-lvs.sh @@ -15,10 +15,9 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest +# Sets 'scan_lvs = 1' aux extend_filter_LVMTEST -aux lvmconf 'devices/scan_lvs = 1' - aux prepare_pvs 1 vgcreate $SHARED $vg1 "$dev1" @@ -44,4 +43,3 @@ lvchange -an "$vg1/$lv1" lvremove "$vg1/$lv1" vgremove $vg1 - diff --git a/test/shell/snapshot-remove-dmsetup.sh b/test/shell/snapshot-remove-dmsetup.sh index 916aec9265..19411ce93f 100644 --- a/test/shell/snapshot-remove-dmsetup.sh +++ b/test/shell/snapshot-remove-dmsetup.sh @@ -17,8 +17,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - which mkfs.ext2 || skip aux prepare_vg 5 diff --git a/test/shell/snapshot-usage-exa.sh b/test/shell/snapshot-usage-exa.sh index f537f0dbf8..5d666bf439 100644 --- a/test/shell/snapshot-usage-exa.sh +++ b/test/shell/snapshot-usage-exa.sh @@ -18,8 +18,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - aux can_use_16T || skip aux prepare_pvs 1 diff --git a/test/shell/snapshot-usage.sh b/test/shell/snapshot-usage.sh index 5cfdae6e4c..bcfa16a3e9 100644 --- a/test/shell/snapshot-usage.sh +++ b/test/shell/snapshot-usage.sh @@ -17,8 +17,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - MKFS=mkfs.ext2 which $MKFS || skip diff --git a/test/shell/vgsplit-stacked.sh b/test/shell/vgsplit-stacked.sh index 09af3f76c6..331ee8e86b 100644 --- a/test/shell/vgsplit-stacked.sh +++ b/test/shell/vgsplit-stacked.sh @@ -15,8 +15,6 @@ SKIP_WITH_LVMPOLLD=1 . lib/inittest -aux lvmconf 'devices/scan_lvs = 1' - aux extend_filter_LVMTEST aux prepare_pvs 3 -- 2.24.0 ++++++ bug-1158628_04-pvmove-correcting-read_ahead-setting.patch ++++++ >From 0451225c1997273ffaa36c3eb595eae7737b7dda Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Tue, 20 Aug 2019 12:30:25 +0200 Subject: [PATCH] pvmove: correcting read_ahead setting When pvmove is finished, we do a tricky operation since we try to resume multiple different device that were all joined into 1 big tree. Currently we use the infromation from existing live DM table, where we can get list of all holders of pvmove device. We look for these nodes (by uuid) in new metadata, and we do now a full regular device add into dm tree structure. All devices should be already PRELOAD with correct table before entering suspend state, however for correctly working readahead we need to put correct info also into RESUME tree. Since table are preloaded, the same table is skip and resume, but correct read ahead is now set. --- lib/activate/dev_manager.c | 47 +++++++++++++++++++++++++++----------- 1 files changed, 34 insertions(+), 13 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index 981f4674ac..b218225bb2 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -2034,10 +2034,20 @@ static uint16_t _get_udev_flags(struct dev_manager *dm, const struct logical_vol static int _add_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, const struct logical_volume *lv, int origin_only); - +static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, + const struct logical_volume *lv, + struct lv_activate_opts *laopts, + const char *layer); +/* + * Check for device holders (ATM used only for removed pvmove targets) + * and add them into dtree structures. + * When 'laopts != NULL' add them as new nodes - which also corrects READ_AHEAD. + * Note: correct table are already explicitelly PRELOADED. + */ static int _check_holder(struct dev_manager *dm, struct dm_tree *dtree, - const struct logical_volume *lv, uint32_t major, - const char *d_name) + const struct logical_volume *lv, + struct lv_activate_opts *laopts, + uint32_t major, const char *d_name) { const char *default_uuid_prefix = dm_uuid_prefix(); const size_t default_uuid_prefix_len = strlen(default_uuid_prefix); @@ -2089,8 +2099,11 @@ static int _check_holder(struct dev_manager *dm, struct dm_tree *dtree, log_debug_activation("Found holder %s of %s.", display_lvname(lv_det), display_lvname(lv)); - if (!_add_lv_to_dtree(dm, dtree, lv_det, 0)) - goto_out; + if (!laopts) { + if (!_add_lv_to_dtree(dm, dtree, lv_det, 0)) + goto_out; + } else if (!_add_new_lv_to_dtree(dm, dtree, lv_det, laopts, 0)) + goto_out; } } @@ -2107,7 +2120,9 @@ out: * i.e. PVMOVE is being finished and final table is going to be resumed. */ static int _add_holders_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, - const struct logical_volume *lv, struct dm_info *info) + const struct logical_volume *lv, + struct lv_activate_opts *laopts, + const struct dm_info *info) { const char *sysfs_dir = dm_sysfs_dir(); char sysfs_path[PATH_MAX]; @@ -2130,7 +2145,7 @@ static int _add_holders_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, while ((dirent = readdir(d))) /* Expects minor is added to 'dm-' prefix */ if (!strncmp(dirent->d_name, "dm-", 3) && - !_check_holder(dm, dtree, lv, info->major, dirent->d_name)) + !_check_holder(dm, dtree, lv, laopts, info->major, dirent->d_name)) goto_out; r = 1; @@ -2202,7 +2217,7 @@ static int _add_dev_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, */ if (info.exists && !lv_is_pvmove(lv) && !strchr(lv->name, '_') && !strncmp(lv->name, "pvmove", 6)) - if (!_add_holders_to_dtree(dm, dtree, lv, &info)) + if (!_add_holders_to_dtree(dm, dtree, lv, NULL, &info)) return_0; return 1; @@ -3000,11 +3015,6 @@ static int _add_target_to_dtree(struct dev_manager *dm, &dm->pvmove_mirror_count); } -static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, - const struct logical_volume *lv, - struct lv_activate_opts *laopts, - const char *layer); - static int _add_new_external_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, struct logical_volume *external_lv, @@ -3439,6 +3449,17 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree, !_pool_register_callback(dm, dnode, lv)) return_0; + /* + * Update tables for ANY PVMOVE holders for active LV where the name starts with 'pvmove', + * but it's not anymore PVMOVE LV and also it's not a PVMOVE _mimage LV. + * When resume happens, tables MUST be already preloaded with correct entries! + * (since we can't preload different table while devices are suspended) + */ + if (!lv_is_pvmove(lv) && !strncmp(lv->name, "pvmove", 6) && !strchr(lv->name, '_') && + (dinfo = _cached_dm_info(dm->mem, dtree, lv, NULL))) + if (!_add_holders_to_dtree(dm, dtree, lv, laopts, dinfo)) + return_0; + if (read_ahead == DM_READ_AHEAD_AUTO) { /* we need RA at least twice a whole stripe - see the comment in md/raid0.c */ read_ahead = max_stripe_size * 2; -- 2.24.0 ++++++ bug-1158628_05-activation-add-synchronization-point.patch ++++++ >From 30a98e4d6710a543692d40d11428ae4baea11b7b Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Fri, 16 Aug 2019 23:49:59 +0200 Subject: [PATCH] activation: add synchronization point Resuming of 'error' table entry followed with it's dirrect removal is now troublesame with latest udev as it may skip processing of udev rules for already 'dropped' device nodes. As we cannot 'synchronize' with udev while we know we have devices in suspended state - rework 'cleanup' so it collects nodes for removal into pending_delete list and process the list with synchronization once we are without any suspended nodes. --- lib/activate/dev_manager.c | 20 ++++++++++++-------- 1 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index b218225bb2..3168e88031 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -3579,13 +3579,6 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root, const const char *name, *uuid; struct dm_str_list *dl; - /* Deactivate any tracked pending delete nodes */ - dm_list_iterate_items(dl, &dm->pending_delete) { - log_debug_activation("Deleting tracked UUID %s.", dl->str); - if (!dm_tree_deactivate_children(root, dl->str, strlen(dl->str))) - return_0; - } - while ((child = dm_tree_next_child(&handle, root, 0))) { if (!(name = dm_tree_node_get_name(child))) continue; @@ -3606,10 +3599,21 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root, const if (non_toplevel_tree_dlid && !strcmp(non_toplevel_tree_dlid, uuid)) continue; - if (!dm_tree_deactivate_children(root, uuid, strlen(uuid))) + if (!str_list_add(dm->mem, &dm->pending_delete, uuid)) return_0; } + /* Deactivate any tracked pending delete nodes */ + if (!dm_list_empty(&dm->pending_delete) && !dm_get_suspended_counter()) { + fs_unlock(); + dm_tree_set_cookie(root, fs_get_cookie()); + dm_list_iterate_items(dl, &dm->pending_delete) { + log_debug_activation("Deleting tracked UUID %s.", dl->str); + if (!dm_tree_deactivate_children(root, dl->str, strlen(dl->str))) + return_0; + } + } + return 1; } -- 2.24.0 ++++++ bug-1158628_06-pvmove-add-missing-synchronization.patch ++++++ >From 0bdd6d6240251996694a8581432f726e5442c4a2 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Tue, 20 Aug 2019 12:23:08 +0200 Subject: [PATCH] pvmove: add missing synchronization Between 'resume' and 'remove' we need to wait for udev to synchronize, otherwise udev may 'skip' resume event processing if the udev node is already gone. --- tools/pvmove_poll.c | 2 ++ 1 files changed, 1 insertions(+) diff --git a/tools/pvmove_poll.c b/tools/pvmove_poll.c index e50747cf46..d379596f2f 100644 --- a/tools/pvmove_poll.c +++ b/tools/pvmove_poll.c @@ -97,6 +97,8 @@ int pvmove_finish(struct cmd_context *cmd, struct volume_group *vg, if (!lv_update_and_reload(lv_mirr)) return_0; + sync_local_dev_names(cmd); + /* Deactivate mirror LV */ if (!deactivate_lv(cmd, lv_mirr)) { log_error("ABORTING: Unable to deactivate temporary logical " -- 2.24.0 ++++++ bug-1158628_07-activation-extend-handling-of-pending_delete.patch ++++++ >From 7833c45fbe79e49ac22e50b90917b7d7ff2d78ac Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Fri, 23 Aug 2019 13:08:34 +0200 Subject: [PATCH] activation: extend handling of pending_delete With previous patch 30a98e4d6710a543692d40d11428ae4baea11b7b we started to put devices one pending_delete list instead of directly scheduling their removal. However we have operations like 'snapshot merge' where we are resuming device tree in 2 subsequent activation calls - so 1st such call will still have suspened devices and no chance to push 'remove' ioctl. Since we curently cannot easily solve this by doing just single activation call (which would be preferred solution) - we introduce a preservation of pending_delete via command structure and then restore it on next activation call. This way we keep to remove devices later - although it might be not the best moment - this may need futher tunning. Also we don't keep the list of operation in 1 trasaction (unless we do verify udev symlinks) - this could probably also make it more correct in terms of which 'remove' can be combined we already running 'resume'. --- lib/activate/dev_manager.c | 24 +++++++++++------------- lib/commands/toolcontext.c | 8 ++++++++ lib/commands/toolcontext.h | 1 + 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index 3168e88031..74dbc609c3 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -3612,6 +3612,7 @@ static int _clean_tree(struct dev_manager *dm, struct dm_tree_node *root, const if (!dm_tree_deactivate_children(root, dl->str, strlen(dl->str))) return_0; } + dm_list_init(&dm->pending_delete); } return 1; @@ -3738,25 +3739,22 @@ out_no_root: int dev_manager_activate(struct dev_manager *dm, const struct logical_volume *lv, struct lv_activate_opts *laopts) { + dm_list_splice(&dm->pending_delete, &lv->vg->cmd->pending_delete); + if (!_tree_action(dm, lv, laopts, ACTIVATE)) return_0; - /* - * When lvm2 resumes a device and shortly after that it removes it, - * udevd rule will try to blindly call 'dmsetup info' on already removed - * device leaving the trace inside syslog about failing operation. - * - * TODO: It's not completely clear this call here is the best fix. - * Maybe there can be a better sequence, but ATM we do usually resume - * error device i.e. on cache deletion and remove it. - * TODO2: there could be more similar cases! - */ - if (!dm_list_empty(&dm->pending_delete)) - fs_unlock(); - if (!_tree_action(dm, lv, laopts, CLEAN)) return_0; + if (!dm_list_empty(&dm->pending_delete)) { + log_debug("Preserving %d device(s) for removal while being suspended.", + dm_list_size(&dm->pending_delete)); + if (!(str_list_dup(lv->vg->cmd->mem, &lv->vg->cmd->pending_delete, + &dm->pending_delete))) + return_0; + } + return 1; } diff --git a/lib/commands/toolcontext.c b/lib/commands/toolcontext.c index 1e03ea2357..0a93553617 100644 --- a/lib/commands/toolcontext.c +++ b/lib/commands/toolcontext.c @@ -1734,6 +1734,8 @@ struct cmd_context *create_toolcontext(unsigned is_clvmd, cmd->current_settings = cmd->default_settings; cmd->initialized.config = 1; + + dm_list_init(&cmd->pending_delete); out: if (!cmd->initialized.config) { destroy_toolcontext(cmd); @@ -1922,6 +1924,12 @@ int refresh_toolcontext(struct cmd_context *cmd) cmd->initialized.config = 1; + if (!dm_list_empty(&cmd->pending_delete)) { + log_debug(INTERNAL_ERROR "Unprocessed pending delete for %d devices.", + dm_list_size(&cmd->pending_delete)); + dm_list_init(&cmd->pending_delete); + } + if (cmd->initialized.connections && !init_connections(cmd)) return_0; diff --git a/lib/commands/toolcontext.h b/lib/commands/toolcontext.h index 655d9f2978..8a20d1f14d 100644 --- a/lib/commands/toolcontext.h +++ b/lib/commands/toolcontext.h @@ -239,6 +239,7 @@ struct cmd_context { const char *report_list_item_separator; const char *time_format; unsigned rand_seed; + struct dm_list pending_delete; /* list of LVs for removal */ }; /* -- 2.24.0 ++++++ bug-1158628_08-lv_manip-add-synchronizations.patch ++++++ >From 4b1dcc2eebb27cae54b4c618ab31072bdb230ea9 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Mon, 26 Aug 2019 13:28:17 +0200 Subject: [PATCH] lv_manip: add synchronizations New udev in rawhide seems to be 'dropping' udev rule operations for devices that are no longer existing - while this is 'probably' a bug - it's revealing moments in lvm2 that likely should not run in a single transaction and we should wait for a cookie before submitting more work. TODO: it seem more 'error' paths should always include synchronization before starting deactivating 'just activated' devices. We should probably figure out some 'automatic' solution for this instead of placing sync_local_dev_name() all over the place... --- lib/metadata/lv_manip.c | 14 +++++++++++--- lib/metadata/snapshot_manip.c | 5 +++++ lib/metadata/thin_manip.c | 6 ++++++ tools/lvconvert.c | 12 ++++++++++++ 4 files changed, 34 insertions(+), 3 deletions(-) diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index af3a16fe2d..6451368772 100644 --- a/lib/metadata/lv_manip.c +++ b/lib/metadata/lv_manip.c @@ -5830,9 +5830,17 @@ out: ret = 1; bad: - if (activated && !deactivate_lv(cmd, lock_lv)) { - log_error("Problem deactivating %s.", display_lvname(lock_lv)); - ret = 0; + if (activated) { + if (!sync_local_dev_names(lock_lv->vg->cmd)) { + log_error("Failed to sync local devices before deactivating LV %s.", + display_lvname(lock_lv)); + return 0; + } + + if (!deactivate_lv(cmd, lock_lv)) { + log_error("Problem deactivating %s.", display_lvname(lock_lv)); + ret = 0; + } } return ret; diff --git a/lib/metadata/snapshot_manip.c b/lib/metadata/snapshot_manip.c index 65d8dbd13f..d105942c0f 100644 --- a/lib/metadata/snapshot_manip.c +++ b/lib/metadata/snapshot_manip.c @@ -292,6 +292,11 @@ int vg_remove_snapshot(struct logical_volume *cow) if (is_origin_active && lv_is_virtual_origin(origin)) { + if (!sync_local_dev_names(origin->vg->cmd)) { + log_error("Failed to sync local devices before deactivating origin LV %s.", + display_lvname(origin)); + return 0; + } if (!deactivate_lv(origin->vg->cmd, origin)) { log_error("Failed to deactivate logical volume \"%s\"", origin->name); diff --git a/lib/metadata/thin_manip.c b/lib/metadata/thin_manip.c index b9c01ee215..f94797620f 100644 --- a/lib/metadata/thin_manip.c +++ b/lib/metadata/thin_manip.c @@ -529,6 +529,12 @@ int update_pool_lv(struct logical_volume *lv, int activate) } } + if (!sync_local_dev_names(lv->vg->cmd)) { + log_error("Failed to sync local devices LV %s.", + display_lvname(lv)); + return 0; + } + if (activate && !deactivate_lv(lv->vg->cmd, lv)) { log_error("Failed to deactivate %s.", display_lvname(lv)); diff --git a/tools/lvconvert.c b/tools/lvconvert.c index ebc22433f8..31f9296769 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -2513,6 +2513,12 @@ static int _lvconvert_cache_repair(struct cmd_context *cmd, /* TODO: any active validation of cache-pool metadata? */ deactivate_mlv: + if (!sync_local_dev_names(cmd)) { + log_error("Failed to sync local devices before deactivating LV %s.", + display_lvname(mlv)); + return 0; + } + if (!deactivate_lv(cmd, mlv)) { log_error("Cannot deactivate pool metadata volume %s.", display_lvname(mlv)); @@ -2520,6 +2526,12 @@ deactivate_mlv: } deactivate_pmslv: + if (!sync_local_dev_names(cmd)) { + log_error("Failed to sync local devices before deactivating LV %s.", + display_lvname(pmslv)); + return 0; + } + if (!deactivate_lv(cmd, pmslv)) { log_error("Cannot deactivate pool metadata spare volume %s.", display_lvname(pmslv)); -- 2.24.0 ++++++ bug-1158628_09-lvconvert-improve-validation-thin-and-cache-pool-con.patch ++++++ >From 7612c21f5511c58bac81fc46e304bd9f4cd2cd75 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Fri, 6 Sep 2019 18:09:40 +0200 Subject: [PATCH] lvconvert: improve validation thin and cache pool conversion Limit convertible LVs to thin-pool and cache-pools. Also fix return code on interal error path to return ECMD_FAILED. --- tools/lvconvert.c | 52 ++++++++++++++++++++++++++++++++++++++++++----- 1 files changed, 47 insertions(+), 5 deletions(-) diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 31f9296769..d64ab224aa 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -4455,24 +4455,66 @@ static int _lvconvert_to_pool_or_swap_metadata_single(struct cmd_context *cmd, struct dm_list *use_pvh = NULL; int to_thinpool = 0; int to_cachepool = 0; + int lvt_enum = get_lvt_enum(lv); + struct lv_type *lvtype; switch (cmd->command->command_enum) { case lvconvert_to_thinpool_or_swap_metadata_CMD: + if (lv_is_cache(lv)) + /* For cached LV check the cache origin LV type */ + lvt_enum = get_lvt_enum(seg_lv(first_seg(lv), 0)); to_thinpool = 1; break; case lvconvert_to_cachepool_or_swap_metadata_CMD: + if (lv_is_cache(lv)) + goto_bad; /* Cache over cache is not supported */ to_cachepool = 1; break; default: - log_error(INTERNAL_ERROR "Invalid lvconvert pool command"); - return 0; - }; + log_error(INTERNAL_ERROR "Invalid lvconvert pool command."); + return ECMD_FAILED; + } + + switch (lvt_enum) { + case thinpool_LVT: + if (!to_thinpool) + goto_bad; /* can't accept cache-pool */ + break; /* swap thin-pool */ + case cachepool_LVT: + if (!to_cachepool) + goto_bad; /* can't accept thin-pool */ + break; /* swap cache-pool */ + case linear_LVT: + case raid_LVT: + case striped_LVT: + case zero_LVT: + break; + default: +bad: + lvtype = get_lv_type(lvt_enum); + log_error("LV %s with type %s cannot be used as a %s pool LV.", + display_lvname(lv), lvtype ? lvtype->name : "unknown", + to_thinpool ? "thin" : "cache"); + return ECMD_FAILED; + } if (lv_is_origin(lv)) { log_error("Cannot convert logical volume %s under snapshot.", display_lvname(lv)); - return 0; - }; + return ECMD_FAILED; + } + + if (!lv_is_visible(lv)) { + log_error("Can't convert internal LV %s.", + display_lvname(lv)); + return ECMD_FAILED; + } + + if (lv_is_locked(lv)) { + log_error("Can't convert locked LV %s.", + display_lvname(lv)); + return ECMD_FAILED; + } if (cmd->position_argc > 1) { /* First pos arg is required LV, remaining are optional PVs. */ -- 2.24.0 ++++++ bug-1158628_10-thin-activate-layer-pool-aas-read-only-LV.patch ++++++ >From 66f69e766e576692ea32328c1921acbacb69ed14 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Sat, 14 Sep 2019 01:08:14 +0200 Subject: [PATCH] thin: activate layer pool aas read-only LV When lvm2 is activating layered pool LV (to basically keep pool opened, the other function used to be 'locking' be in sync with DM table) use this LV in read-only mode - this prevents 'write' access into data volume content of thin-pool. Note: since EMPTY/unused thin-pool is created as 'public LV' for generic use by any user who i.e. wish to maintain thin-pool and thins himself. At this moment, thin-pool appears as writable LV. As soon as the 1st. thinLV is created, layer volume will appear is 'read-only' LV from this moment. --- lib/activate/dev_manager.c | 5 +++++ 1 files changed, 5 insertions(+) diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index c780deaa02..32fdcb94f7 100644 --- a/lib/activate/dev_manager.c +++ b/lib/activate/dev_manager.c @@ -85,6 +85,11 @@ int read_only_lv(const struct logical_volume *lv, const struct lv_activate_opts if (lv_is_raid_image(lv) || lv_is_raid_metadata(lv)) return 0; /* Keep RAID SubLvs writable */ + if (!layer) { + if (lv_is_thin_pool(lv)) + return 1; + } + return (laopts->read_only || !(lv->status & LVM_WRITE)); } -- 2.24.0 ++++++ bug-1158628_11-tests-mdadm-stop-in-test-cleanup.patch ++++++ >From fd5b8b72da79e2f0a10785d055a27643d9eaaf19 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Fri, 27 Sep 2019 12:51:34 -0500 Subject: [PATCH] tests: mdadm stop in test cleanup try to clear any existing md devs remaining after a test --- test/lib/aux.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/test/lib/aux.sh b/test/lib/aux.sh index 9c9e1fda39..7b0ef22348 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -421,9 +421,14 @@ teardown_devs() { teardown_udev_cookies test ! -f MD_DEV || cleanup_md_dev + udev_wait + mdadm --stop --scan || true + udev_wait test ! -f DEVICES || teardown_devs_prefixed "$PREFIX" test ! -f RAMDISK || { modprobe -r brd || true ; } + mdadm --stop --scan || true + # NOTE: SCSI_DEBUG_DEV test must come before the LOOP test because # prepare_scsi_debug_dev() also sets LOOP to short-circuit prepare_loop() if test -f SCSI_DEBUG_DEV; then -- 2.24.0 ++++++ bug-1158628_12-test-increase-size-of-raid10-LV-allowing-tests-to-su.patch ++++++ >From b138a87f43bcfdd3f523556d54f0a482ecf8c40b Mon Sep 17 00:00:00 2001 From: Heinz Mauelshagen <[email protected]> Date: Wed, 2 Oct 2019 14:59:17 +0200 Subject: [PATCH] test: increase size of raid10 LV allowing tests to succeed on fast storage Also add health char check. --- test/shell/lvchange-rebuild-raid.sh | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/shell/lvchange-rebuild-raid.sh b/test/shell/lvchange-rebuild-raid.sh index 22acb925da..d2bb723435 100644 --- a/test/shell/lvchange-rebuild-raid.sh +++ b/test/shell/lvchange-rebuild-raid.sh @@ -43,8 +43,8 @@ done ############################################## # Create an 8-way striped raid10 with 4 mirror # groups and rebuild selected PVs. -lvcreate --type raid10 -m 1 -i 4 -l 2 -n $lv1 $vg -_sync +lvcreate --type raid10 -m 1 -i 4 -l 64 -n $lv1 $vg +_sync "AAAAAAAA" # Rebuild 1st and 2nd device would rebuild a # whole mirror group and needs to be rejected. -- 2.24.0 ++++++ bug-1158628_13-lvconvert-fix-return-value-when-zeroing-fails.patch ++++++ >From 76a9a86fd359681a598a6509eb90940c3a10f1f3 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Mon, 14 Oct 2019 11:54:49 +0200 Subject: [PATCH] lvconvert: fix return value when zeroing fails Use correct error return code for fail path. --- tools/lvconvert.c | 2 +- 1 files changed, 1 insertions(+), 1 deletion(-) diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 60ab956614..caef5d5a54 100644 --- a/tools/lvconvert.c +++ b/tools/lvconvert.c @@ -5565,7 +5565,7 @@ static int _lvconvert_writecache_attach_single(struct cmd_context *cmd, if (!_writecache_zero(cmd, lv_fast)) { log_error("LV %s could not be zeroed.", display_lvname(lv_fast)); - return 0; + return ECMD_FAILED; } /* -- 2.24.0 ++++++ bug-1158628_14-tests-add-extra-settle.patch ++++++ >From 3b05fd4d072d94bfead8c2d188ecf704fe57e2a8 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Fri, 8 Nov 2019 15:09:17 +0100 Subject: [PATCH] tests: add extra settle To avoid removing, while 'add' might not have been processed yet. (when emulating reboot in pvmove-restart) --- test/lib/aux.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/lib/aux.sh b/test/lib/aux.sh index 36e1c2bb3e..6822d7e3e4 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -1652,6 +1652,10 @@ wait_pvmove_lv_ready() { retries=$((retries-1)) done fi + + # Adding settle here, to avoid remove, before processing of 'add' is finished + # (masking systemd-udevd issue) + udevadm settle --timeout=2 || true } # Holds device open with sleep which automatically expires after given timeout -- 2.24.0 ++++++ bug-1158628_15-test-Fix-handling-leftovers-from-previous-tests.patch ++++++ >From 1e669ab315c32aba3f47c37771f26ab7c8e151dd Mon Sep 17 00:00:00 2001 From: Marian Csontos <[email protected]> Date: Wed, 20 Nov 2019 15:23:48 +0100 Subject: [PATCH] test: Fix handling leftovers from previous tests teardown fails current PREFIX is prefix of previously failed test with leftovers in dmtable. --- test/lib/aux.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/lib/aux.sh b/test/lib/aux.sh index 6822d7e3e4..566e9b52e1 100644 --- a/test/lib/aux.sh +++ b/test/lib/aux.sh @@ -520,7 +520,7 @@ teardown() { if test ! -f SKIP_THIS_TEST ; then # Evaluate left devices only for non-skipped tests - TEST_LEAKED_DEVICES=$(dmsetup table | grep "$PREFIX" | grep -v "${PREFIX}pv") || true + TEST_LEAKED_DEVICES=$(dmsetup table | grep "$PREFIX" | grep -Ev "${PREFIX}(pv|[0-9])") || true fi kill_tagged_processes -- 2.24.0 ++++++ bug-1158861_01-config-remove-filter-typo.patch ++++++ >From 7c697c1058ed32ca6311c3fd4e05f1b444e72aa1 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Mon, 17 Jun 2019 09:38:24 -0500 Subject: [PATCH] config: remove filter typo Remove unnecessary but harmless / in the filter string "a|.*/|". --- lib/config/config_settings.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/config/config_settings.h b/lib/config/config_settings.h index e718decd9b..7851a76fc9 100644 --- a/lib/config/config_settings.h +++ b/lib/config/config_settings.h @@ -288,7 +288,7 @@ cfg_array(devices_preferred_names_CFG, "preferred_names", devices_CFG_SECTION, C "preferred_names = [ \"^/dev/mpath/\", \"^/dev/mapper/mpath\", \"^/dev/[hs]d\" ]\n" "#\n") -cfg_array(devices_filter_CFG, "filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, "#Sa|.*/|", vsn(1, 0, 0), NULL, 0, NULL, +cfg_array(devices_filter_CFG, "filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, "#Sa|.*|", vsn(1, 0, 0), NULL, 0, NULL, "Limit the block devices that are used by LVM commands.\n" "This is a list of regular expressions used to accept or reject block\n" "device path names. Each regex is delimited by a vertical bar '|'\n" @@ -306,7 +306,7 @@ cfg_array(devices_filter_CFG, "filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENT "#\n" "Example\n" "Accept every block device:\n" - "filter = [ \"a|.*/|\" ]\n" + "filter = [ \"a|.*|\" ]\n" "Reject the cdrom drive:\n" "filter = [ \"r|/dev/cdrom|\" ]\n" "Work with just loopback devices, e.g. for testing:\n" @@ -314,10 +314,10 @@ cfg_array(devices_filter_CFG, "filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENT "Accept all loop devices and ide drives except hdc:\n" "filter = [ \"a|loop|\", \"r|/dev/hdc|\", \"a|/dev/ide|\", \"r|.*|\" ]\n" "Use anchors to be very specific:\n" - "filter = [ \"a|^/dev/hda8$|\", \"r|.*/|\" ]\n" + "filter = [ \"a|^/dev/hda8$|\", \"r|.*|\" ]\n" "#\n") -cfg_array(devices_global_filter_CFG, "global_filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, "#Sa|.*/|", vsn(2, 2, 98), NULL, 0, NULL, +cfg_array(devices_global_filter_CFG, "global_filter", devices_CFG_SECTION, CFG_DEFAULT_COMMENTED, CFG_TYPE_STRING, "#Sa|.*|", vsn(2, 2, 98), NULL, 0, NULL, "Limit the block devices that are used by LVM system components.\n" "Because devices/filter may be overridden from the command line, it is\n" "not suitable for system-wide device filtering, e.g. udev.\n" -- 2.24.0 ++++++ bug-1158861_02-config-Fix-default-option-which-makes-no-sense.patch ++++++ >From 556dcd2c6b82ead3a5aa50211f08f9d69be13fe1 Mon Sep 17 00:00:00 2001 From: Marian Csontos <[email protected]> Date: Mon, 17 Jun 2019 19:08:28 +0200 Subject: [PATCH] config: Fix default option which makes no sense Default value is either undefined or commented, never both. --- lib/config/config_settings.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/config/config_settings.h b/lib/config/config_settings.h index 7851a76fc9..527d5bd070 100644 --- a/lib/config/config_settings.h +++ b/lib/config/config_settings.h @@ -1694,7 +1694,7 @@ cfg(metadata_vgmetadatacopies_CFG, "vgmetadatacopies", metadata_CFG_SECTION, CFG "and allows you to control which metadata areas are used at the\n" "individual PV level using pvchange --metadataignore y|n.\n") -cfg_runtime(metadata_pvmetadatasize_CFG, "pvmetadatasize", metadata_CFG_SECTION, CFG_DEFAULT_COMMENTED | CFG_DEFAULT_UNDEFINED, CFG_TYPE_INT, vsn(1, 0, 0), 0, NULL, +cfg_runtime(metadata_pvmetadatasize_CFG, "pvmetadatasize", metadata_CFG_SECTION, CFG_DEFAULT_UNDEFINED, CFG_TYPE_INT, vsn(1, 0, 0), 0, NULL, "The default size of the metadata area in units of 512 byte sectors.\n" "The metadata area begins at an offset of the page size from the start\n" "of the device. The first PE is by default at 1 MiB from the start of\n" -- 2.24.0 ++++++ bug-1158861_03-vgchange-don-t-fail-monitor-command-if-vg-is-exporte.patch ++++++ >From 82b137ef2f7f1b6fc1bbf83918750037835a9568 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Thu, 20 Jun 2019 15:59:36 -0500 Subject: [PATCH] vgchange: don't fail monitor command if vg is exported When monitoring, skip exported VGs without causing a command failure. The lvm2-monitor service runs 'vgchange --monitor y', so any exported VG on the system would cause the service to fail. --- tools/vgchange.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/vgchange.c b/tools/vgchange.c index d6d4f9175f..a17f4566ff 100644 --- a/tools/vgchange.c +++ b/tools/vgchange.c @@ -631,6 +631,8 @@ static int _vgchange_single(struct cmd_context *cmd, const char *vg_name, }; if (vg_is_exported(vg)) { + if (cmd->command->command_enum == vgchange_monitor_CMD) + return ECMD_PROCESSED; log_error("Volume group \"%s\" is exported", vg_name); return ECMD_FAILED; } -- 2.24.0 ++++++ bug-1158861_04-fix-duplicate-pv-size-check.patch ++++++ >From dcbed38b3339ce4da722bccec8eaf7b8d775a6c2 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Tue, 27 Aug 2019 15:40:24 -0500 Subject: [PATCH] fix duplicate pv size check Fixes a segfault in the recent commit e01fddc57: "improve duplicate pv handling for md components" While choosing between duplicates, the info struct is not always valid; it may have been dropped already. Remove the code that was still using the info struct for size comparisons. The size comparisons were a bogus check anyway because it was just preferring the dev that had already been chosen, it wasn't actually comparing the dev size to the PV size. It would be good to use a dev/PV size comparison in the duplicate handling code, but the PV size is not available until after vg_read, not from the scan. --- lib/cache/lvmcache.c | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 87c0021ad8..29d6446a64 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -491,12 +491,10 @@ static void _choose_duplicates(struct cmd_context *cmd, struct lvmcache_info *info; struct device *dev1, *dev2; uint32_t dev1_major, dev1_minor, dev2_major, dev2_minor; - uint64_t info_size, dev1_size, dev2_size; int in_subsys1, in_subsys2; int is_dm1, is_dm2; int has_fs1, has_fs2; int has_lv1, has_lv2; - int same_size1, same_size2; int prev_unchosen1, prev_unchosen2; int change; @@ -613,11 +611,6 @@ next: dev2_major = MAJOR(dev2->dev); dev2_minor = MINOR(dev2->dev); - if (!dev_get_size(dev1, &dev1_size)) - dev1_size = 0; - if (!dev_get_size(dev2, &dev2_size)) - dev2_size = 0; - has_lv1 = (dev1->flags & DEV_USED_FOR_LV) ? 1 : 0; has_lv2 = (dev2->flags & DEV_USED_FOR_LV) ? 1 : 0; @@ -630,21 +623,11 @@ next: has_fs1 = dm_device_has_mounted_fs(dev1_major, dev1_minor); has_fs2 = dm_device_has_mounted_fs(dev2_major, dev2_minor); - info_size = info->device_size >> SECTOR_SHIFT; - same_size1 = (dev1_size == info_size); - same_size2 = (dev2_size == info_size); - log_debug_cache("PV %s compare duplicates: %s %u:%u. %s %u:%u.", devl->dev->pvid, dev_name(dev1), dev1_major, dev1_minor, dev_name(dev2), dev2_major, dev2_minor); - log_debug_cache("PV %s: wants size %llu. %s is %llu. %s is %llu.", - devl->dev->pvid, - (unsigned long long)info_size, - dev_name(dev1), (unsigned long long)dev1_size, - dev_name(dev2), (unsigned long long)dev2_size); - log_debug_cache("PV %s: %s was prev %s. %s was prev %s.", devl->dev->pvid, dev_name(dev1), prev_unchosen1 ? "not chosen" : "<none>", @@ -686,13 +669,6 @@ next: /* change to 2 */ change = 1; reason = "device is used by LV"; - } else if (same_size1 && !same_size2) { - /* keep 1 */ - reason = "device size is correct"; - } else if (same_size2 && !same_size1) { - /* change to 2 */ - change = 1; - reason = "device size is correct"; } else if (has_fs1 && !has_fs2) { /* keep 1 */ reason = "device has fs mounted"; -- 2.24.0 ++++++ bug-1158861_05-hints-fix-copy-of-filter.patch ++++++ >From 12707adac8ba9e3a58175616bcd59b0e229a6705 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Wed, 28 Aug 2019 12:33:04 -0500 Subject: [PATCH] hints: fix copy of filter Only the first entry of the filter array was being included in the copy of the filter, rather than the entire thing. The result is that hints would not be refreshed if the filter was changed but the first entry was unchanged. --- lib/label/hints.c | 79 ++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 68 insertions(+), 11 deletions(-) diff --git a/lib/label/hints.c b/lib/label/hints.c index 6de54bc60b..580304dee0 100644 --- a/lib/label/hints.c +++ b/lib/label/hints.c @@ -579,6 +579,52 @@ static void _apply_hints(struct cmd_context *cmd, struct dm_list *hints, } } +static void _filter_to_str(struct cmd_context *cmd, int filter_cfg, char **strp) +{ + const struct dm_config_node *cn; + const struct dm_config_value *cv; + char *str; + int pos = 0; + int len = 0; + int ret; + + *strp = NULL; + + if (!(cn = find_config_tree_array(cmd, filter_cfg, NULL))) { + /* shouldn't happen because default is a|*| */ + return; + } + + for (cv = cn->v; cv; cv = cv->next) { + if (cv->type != DM_CFG_STRING) + continue; + + len += (strlen(cv->v.str) + 1); + } + len++; + + if (len == 1) { + /* shouldn't happen because default is a|*| */ + return; + } + + str = malloc(len); + memset(str, 0, len); + + for (cv = cn->v; cv; cv = cv->next) { + if (cv->type != DM_CFG_STRING) + continue; + + ret = snprintf(str + pos, len - pos, "%s", cv->v.str); + + if (ret >= len - pos) + break; + pos += ret; + } + + *strp = str; +} + /* * Return 1 and needs_refresh 0: the hints can be used * Return 1 and needs_refresh 1: the hints can't be used and should be updated @@ -590,12 +636,11 @@ static int _read_hint_file(struct cmd_context *cmd, struct dm_list *hints, int * { char devpath[PATH_MAX]; FILE *fp; - const struct dm_config_node *cn; struct dev_iter *iter; struct hint *hint; struct device *dev; char *split[HINT_LINE_WORDS]; - char *name, *pvid, *devn, *vgname, *p; + char *name, *pvid, *devn, *vgname, *p, *filter_str = NULL; uint32_t read_hash = 0; uint32_t calc_hash = INITIAL_CRC; uint32_t read_count = 0; @@ -655,23 +700,31 @@ static int _read_hint_file(struct cmd_context *cmd, struct dm_list *hints, int * keylen = strlen("global_filter:"); if (!strncmp(_hint_line, "global_filter:", keylen)) { - cn = find_config_tree_array(cmd, devices_global_filter_CFG, NULL); - if (strcmp(cn->v->v.str, _hint_line + keylen)) { + _filter_to_str(cmd, devices_global_filter_CFG, &filter_str); + if (!filter_str || strcmp(filter_str, _hint_line + keylen)) { log_debug("ignore hints with different global_filter"); + if (filter_str) + free(filter_str); *needs_refresh = 1; break; } + if (filter_str) + free(filter_str); continue; } keylen = strlen("filter:"); if (!strncmp(_hint_line, "filter:", keylen)) { - cn = find_config_tree_array(cmd, devices_filter_CFG, NULL); - if (strcmp(cn->v->v.str, _hint_line + keylen)) { + _filter_to_str(cmd, devices_filter_CFG, &filter_str); + if (!filter_str || strcmp(filter_str, _hint_line + keylen)) { log_debug("ignore hints with different filter"); + if (filter_str) + free(filter_str); *needs_refresh = 1; break; } + if (filter_str) + free(filter_str); continue; } @@ -800,11 +853,11 @@ int write_hint_file(struct cmd_context *cmd, int newhints) { char devpath[PATH_MAX]; FILE *fp; - const struct dm_config_node *cn; struct lvmcache_info *info; struct dev_iter *iter; struct device *dev; const char *vgname; + char *filter_str = NULL; uint32_t hash = INITIAL_CRC; uint32_t count = 0; time_t t; @@ -855,11 +908,15 @@ int write_hint_file(struct cmd_context *cmd, int newhints) fprintf(fp, "# Created by %s pid %d %s", cmd->name, getpid(), ctime(&t)); fprintf(fp, "hints_version: %d.%d\n", HINTS_VERSION_MAJOR, HINTS_VERSION_MINOR); - cn = find_config_tree_array(cmd, devices_global_filter_CFG, NULL); - fprintf(fp, "global_filter:%s\n", cn->v->v.str); + _filter_to_str(cmd, devices_global_filter_CFG, &filter_str); + fprintf(fp, "global_filter:%s\n", filter_str ?: "-"); + if (filter_str) + free(filter_str); - cn = find_config_tree_array(cmd, devices_filter_CFG, NULL); - fprintf(fp, "filter:%s\n", cn->v->v.str); + _filter_to_str(cmd, devices_filter_CFG, &filter_str); + fprintf(fp, "filter:%s\n", filter_str ?: "-"); + if (filter_str) + free(filter_str); fprintf(fp, "scan_lvs:%d\n", cmd->scan_lvs); -- 2.24.0 ++++++ bug-1158861_06-fix-segfault-for-invalid-characters-in-vg-name.patch ++++++ >From 7cfbf3a394c2663fbeed17705320b83e69781720 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Thu, 29 Aug 2019 11:35:46 -0500 Subject: [PATCH] fix segfault for invalid characters in vg name Fixes a regression from commit ba7ff96faff0 "improve reading and repairing vg metadata" where the error path for a vg name with invalid charaters was missing an error flag, which led to the caller not recognizing an error occured. Previously, an error flag was hidden in the old _vg_make_handle function. --- lib/metadata/metadata.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 2c61bdeca2..6d21ff99cc 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -4900,7 +4900,8 @@ struct volume_group *vg_read(struct cmd_context *cmd, const char *vg_name, const if (!validate_name(vg_name)) { log_error("Volume group name \"%s\" has invalid characters.", vg_name); - return NULL; + failure |= FAILED_NOTFOUND; + goto_bad; } /* -- 2.16.4 ++++++ bug-1158861_07-vgck-let-updatemetadata-repair-mismatched-metadata.patch ++++++ >From bd21736e8b082319e1a9a29e75badd906ee277f6 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Tue, 8 Oct 2019 14:44:24 -0500 Subject: [PATCH] vgck: let updatemetadata repair mismatched metadata Let vgck --updatemetadata repair cases where different mdas hold indepedently valid but unmatching copies of the metadata, i.e. different text metadata checksums or text metadata sizes. --- lib/cache/lvmcache.c | 1 + lib/cache/lvmcache.h | 1 + lib/format_text/text_label.c | 25 +++++++++++++++++++------ lib/metadata/metadata.c | 3 +++ lib/metadata/metadata.h | 1 + 5 files changed, 25 insertions(+), 6 deletions(-) [email protected] NOTE: lib/cache/lvmcache.h patch lines had been modified for patch happy. diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c index 316624fe52..f6e792459b 100644 --- a/lib/cache/lvmcache.c +++ b/lib/cache/lvmcache.c @@ -1649,6 +1649,7 @@ int lvmcache_update_vgname_and_id(struct lvmcache_info *info, struct lvmcache_vg vgsummary->mda_checksum, vgsummary->mda_size, vginfo->mda_checksum, vginfo->mda_size); vginfo->scan_summary_mismatch = true; + vgsummary->mismatch = 1; return 0; } diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h index 1401974be6..d614e5469e 100644 --- a/lib/cache/lvmcache.h +++ b/lib/cache/lvmcache.h @@ -63,6 +63,7 @@ struct lvmcache_vgsummary { int mda_num; /* 1 = summary from mda1, 2 = summary from mda2 */ unsigned mda_ignored:1; unsigned zero_offset:1; + unsigned mismatch:1; /* lvmcache sets if this summary differs from previous values */ }; int lvmcache_init(struct cmd_context *cmd); diff --git a/lib/format_text/text_label.c b/lib/format_text/text_label.c index 41276be731..246fb7b4ad 100644 --- a/lib/format_text/text_label.c +++ b/lib/format_text/text_label.c @@ -507,10 +507,17 @@ static int _text_read(struct labeller *labeller, struct device *dev, void *label if (rv1 && !vgsummary.zero_offset && !vgsummary.mda_ignored) { if (!lvmcache_update_vgname_and_id(info, &vgsummary)) { /* I believe this is only an internal error. */ - log_warn("WARNING: Scanning %s mda1 failed to save internal summary.", dev_name(dev)); dm_list_del(&mda1->list); - bad_fields |= BAD_MDA_INTERNAL; + + /* Are there other cases besides mismatch and internal error? */ + if (vgsummary.mismatch) { + log_warn("WARNING: Scanning %s mda1 found mismatch with other metadata.", dev_name(dev)); + bad_fields |= BAD_MDA_MISMATCH; + } else { + log_warn("WARNING: Scanning %s mda1 failed to save internal summary.", dev_name(dev)); + bad_fields |= BAD_MDA_INTERNAL; + } mda1->bad_fields = bad_fields; lvmcache_save_bad_mda(info, mda1); mda1 = NULL; @@ -550,11 +557,17 @@ static int _text_read(struct labeller *labeller, struct device *dev, void *label if (rv2 && !vgsummary.zero_offset && !vgsummary.mda_ignored) { if (!lvmcache_update_vgname_and_id(info, &vgsummary)) { - /* I believe this is only an internal error. */ - log_warn("WARNING: Scanning %s mda2 failed to save internal summary.", dev_name(dev)); - dm_list_del(&mda2->list); - bad_fields |= BAD_MDA_INTERNAL; + + /* Are there other cases besides mismatch and internal error? */ + if (vgsummary.mismatch) { + log_warn("WARNING: Scanning %s mda2 found mismatch with other metadata.", dev_name(dev)); + bad_fields |= BAD_MDA_MISMATCH; + } else { + log_warn("WARNING: Scanning %s mda2 failed to save internal summary.", dev_name(dev)); + bad_fields |= BAD_MDA_INTERNAL; + } + mda2->bad_fields = bad_fields; lvmcache_save_bad_mda(info, mda2); mda2 = NULL; diff --git a/lib/metadata/metadata.c b/lib/metadata/metadata.c index 39544e66a8..b09f4b35ed 100644 --- a/lib/metadata/metadata.c +++ b/lib/metadata/metadata.c @@ -4518,6 +4518,9 @@ void vg_write_commit_bad_mdas(struct cmd_context *cmd, struct volume_group *vg) * above. * * TEXT: general error related to text metadata, we can repair. + * + * MISMATCH: different values between instances of metadata, + * can repair. */ if (!mda->bad_fields || (mda->bad_fields & BAD_MDA_READ) || diff --git a/lib/metadata/metadata.h b/lib/metadata/metadata.h index 6516e627c0..ac18879b0b 100644 --- a/lib/metadata/metadata.h +++ b/lib/metadata/metadata.h @@ -179,6 +179,7 @@ struct metadata_area_ops { #define BAD_MDA_MAGIC 0x00000020 #define BAD_MDA_VERSION 0x00000040 #define BAD_MDA_START 0x00000080 +#define BAD_MDA_MISMATCH 0x00000100 /* lvmcache found difference from prev metadata */ struct metadata_area { struct dm_list list; -- 2.24.0 ++++++ bug-1158861_08-hints-fix-mem-leaking-buffers.patch ++++++ >From c38be0653111e3d63efbbf2f8914c83f15a73e70 Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac <[email protected]> Date: Thu, 14 Nov 2019 18:00:54 +0100 Subject: [PATCH] hints: fix mem leaking buffers --- lib/label/hints.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/label/hints.c b/lib/label/hints.c index df1327a10b..8739f337df 100644 --- a/lib/label/hints.c +++ b/lib/label/hints.c @@ -1200,6 +1200,8 @@ check: return; } } + + free(name); } /* @@ -1353,6 +1355,9 @@ int get_hints(struct cmd_context *cmd, struct dm_list *hints_out, int *newhints, dm_list_size(devs_out), dm_list_size(devs_in)); dm_list_splice(hints_out, &hints_list); + + free(vgname); + return 1; } -- 2.24.0 ++++++ bug-1158861_09-pvcreate-pvremove-fix-reacquiring-global-lock-after.patch ++++++ >From 2037476008ea42e79388a407355c7f285656a5d9 Mon Sep 17 00:00:00 2001 From: David Teigland <[email protected]> Date: Tue, 26 Nov 2019 14:34:43 -0600 Subject: [PATCH] pvcreate,pvremove: fix reacquiring global lock after prompt When pvcreate/pvremove prompt the user, they first release the global lock, then acquire it again after the prompt, to avoid blocking other commands while waiting for a user response. This release/reacquire changes the locking order with respect to the hints flock (and potentially other locks). So, to avoid deadlock, use a nonblocking request when reacquiring the global lock. --- lib/locking/locking.c | 14 +++++++++++--- lib/locking/locking.h | 1 + lib/misc/lvm-flock.c | 2 +- tools/toollib.c | 5 +++-- 4 files changed, 16 insertions(+), 6 deletions(-) diff --git a/lib/locking/locking.c b/lib/locking/locking.c index 3058a8ba01..65ff8c2211 100644 --- a/lib/locking/locking.c +++ b/lib/locking/locking.c @@ -338,7 +338,7 @@ int sync_local_dev_names(struct cmd_context* cmd) * an explicitly acquired ex global lock to sh in process_each. */ -static int _lockf_global(struct cmd_context *cmd, const char *mode, int convert) +static int _lockf_global(struct cmd_context *cmd, const char *mode, int convert, int nonblock) { uint32_t flags = 0; int ret; @@ -346,6 +346,9 @@ static int _lockf_global(struct cmd_context *cmd, const char *mode, int convert) if (convert) flags |= LCK_CONVERT; + if (nonblock) + flags |= LCK_NONBLOCK; + if (!strcmp(mode, "ex")) { flags |= LCK_WRITE; @@ -379,7 +382,7 @@ static int _lockf_global(struct cmd_context *cmd, const char *mode, int convert) int lockf_global(struct cmd_context *cmd, const char *mode) { - return _lockf_global(cmd, mode, 0); + return _lockf_global(cmd, mode, 0, 0); } int lockf_global_convert(struct cmd_context *cmd, const char *mode) @@ -388,7 +391,12 @@ int lockf_global_convert(struct cmd_context *cmd, const char *mode) if (cmd->lockf_global_ex && !strcmp(mode, "ex")) return 1; - return _lockf_global(cmd, mode, 1); + return _lockf_global(cmd, mode, 1, 0); +} + +int lockf_global_nonblock(struct cmd_context *cmd, const char *mode) +{ + return _lockf_global(cmd, mode, 0, 1); } int lock_global(struct cmd_context *cmd, const char *mode) diff --git a/lib/locking/locking.h b/lib/locking/locking.h index 746667a9bf..3e8ae6f0c2 100644 --- a/lib/locking/locking.h +++ b/lib/locking/locking.h @@ -75,6 +75,7 @@ int activate_lvs(struct cmd_context *cmd, struct dm_list *lvs, unsigned exclusiv int lockf_global(struct cmd_context *cmd, const char *mode); int lockf_global_convert(struct cmd_context *cmd, const char *mode); +int lockf_global_nonblock(struct cmd_context *cmd, const char *mode); int lock_global(struct cmd_context *cmd, const char *mode); int lock_global_convert(struct cmd_context *cmd, const char *mode); diff --git a/lib/misc/lvm-flock.c b/lib/misc/lvm-flock.c index d65601d940..d48ff22e19 100644 --- a/lib/misc/lvm-flock.c +++ b/lib/misc/lvm-flock.c @@ -164,7 +164,7 @@ static int _do_write_priority_flock(const char *file, int *fd, int operation, ui strcpy(file_aux, file); strcat(file_aux, AUX_LOCK_SUFFIX); - if ((r = _do_flock(file_aux, &fd_aux, LOCK_EX, 0))) { + if ((r = _do_flock(file_aux, &fd_aux, LOCK_EX, nonblock))) { if (operation == LOCK_EX) { r = _do_flock(file, fd, operation, nonblock); _undo_flock(file_aux, fd_aux); diff --git a/tools/toollib.c b/tools/toollib.c index ee2419b8c4..a5304bf63f 100644 --- a/tools/toollib.c +++ b/tools/toollib.c @@ -5577,10 +5577,11 @@ int pvcreate_each_device(struct cmd_context *cmd, * Reacquire the lock that was released above before waiting, then * check again that the devices can still be used. If the second loop * finds them changed, or can't find them any more, then they aren't - * used. + * used. Use a non-blocking request when reacquiring to avoid + * potential deadlock since this is not the normal locking sequence. */ - if (!lockf_global(cmd, "ex")) { + if (!lockf_global_nonblock(cmd, "ex")) { log_error("Failed to reacquire global lock after prompt."); goto_out; } -- 2.24.0 ++++++ lvm.conf ++++++ --- /var/tmp/diff_new_pack.dK2TUm/_old 2020-03-15 07:11:30.140968422 +0100 +++ /var/tmp/diff_new_pack.dK2TUm/_new 2020-03-15 07:11:30.144968425 +0100 @@ -278,7 +278,7 @@ # benefit from discards, but SSDs and thinly provisioned LUNs # generally do. If enabled, discards will only be issued if both the # storage and kernel provide support. - issue_discards = 0 + issue_discards = 1 # Configuration option devices/allow_changes_with_duplicate_pvs. # Allow VG modification while a PV appears on multiple devices.
