From 628729af6b9c3f6be32119dc20da7d60d6684fc5ca9bc81e9f62bf294a6b53ca Mon Sep 17 00:00:00 2001 From: Martin Wilck Date: Thu, 19 Apr 2018 09:49:14 +0000 Subject: [PATCH] Accepting request 598472 from home:ganghe:branches:openSUSE:Factory - Fix handling of udev CHANGE events with systemd (bsc#1067312) + lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch + lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch - Fix handling of udev CHANGE events with systemd (bsc#1067312) + lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch + lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch - Fix handling of udev CHANGE events with systemd (bsc#1067312) + lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch + lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch OBS-URL: https://build.opensuse.org/request/show/598472 OBS-URL: https://build.opensuse.org/package/show/Base:System/lvm2?expand=0&rev=221 --- device-mapper.changes | 7 + device-mapper.spec | 6 +- ...lvm-metad.rules-explicit-pvscan-rule.patch | 83 ++++++++++ ...etad.rules-set-systemd-vars-on-chang.patch | 153 ++++++++++++++++++ lvm2-clvm.changes | 7 + lvm2-clvm.spec | 6 +- lvm2.changes | 7 + lvm2.spec | 6 +- 8 files changed, 272 insertions(+), 3 deletions(-) create mode 100644 lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch create mode 100644 lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch diff --git a/device-mapper.changes b/device-mapper.changes index a6d4c4b..fb345cc 100644 --- a/device-mapper.changes +++ b/device-mapper.changes @@ -1,3 +1,10 @@ +------------------------------------------------------------------- +Thu Apr 19 10:59:59 UTC 2018 - mwilck@suse.com + +- Fix handling of udev CHANGE events with systemd (bsc#1067312) + + lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch + + lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch + ------------------------------------------------------------------- Mon Feb 12 09:01:29 UTC 2018 - tchvatal@suse.com diff --git a/device-mapper.spec b/device-mapper.spec index 5d79d3e..37b4440 100644 --- a/device-mapper.spec +++ b/device-mapper.spec @@ -27,7 +27,7 @@ Name: device-mapper Version: %{device_mapper_version} Release: 0 Summary: Device Mapper Tools -License: GPL-2.0+ and LGPL-2.1+ +License: GPL-2.0-or-later AND LGPL-2.1-or-later Group: System/Base Url: http://www.sourceware.org/lvm2/ Source: ftp://sources.redhat.com/pub/lvm2/LVM2.%{lvm2_version}.tgz @@ -59,6 +59,8 @@ Patch1005: bsc1080299-detect-clvm-properly.patch #SUSE patches 2000+ for device mapper, udev rules Patch2001: bug-1012973_simplify-special-case-for-md-in-69-dm-lvm-metadata.patch +Patch2002: lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch +Patch2003: lvm2-69-dm-lvm-metad.rules-set-systemd-vars-on-chang.patch ### COMMON-PATCH-END ### %description @@ -74,6 +76,8 @@ Programs and man pages for configuring and using the device mapper. %patch1004 -p1 %patch1005 -p1 %patch2001 -p1 +%patch2002 -p1 +%patch2003 -p1 ### COMMON-PREP-END ### %build diff --git a/lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch b/lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch new file mode 100644 index 0000000..1b99706 --- /dev/null +++ b/lvm2-69-dm-lvm-metad.rules-explicit-pvscan-rule.patch @@ -0,0 +1,83 @@ +From fe1af8d113e8b0f5a8aa9397b05eb00341aea332 Mon Sep 17 00:00:00 2001 +From: Martin Wilck +Date: Thu, 21 Dec 2017 09:30:16 +0100 +Subject: [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule + +Make the distinction between the cases with and without systemd +background jobs explicit in 69-dm-lvm-metad.rules rather than +substituting the rule from the Makefile. At this stage, +this improves only readibility, at the cost of one GOTO statement. + +The next patch will add more differences between the two cases (mostly +comments), which are practically impossible to generate with the current +string subsitution approach. + +This patch introduces no functional change to the udev rules. + +Signed-off-by: Martin Wilck +--- + udev/69-dm-lvm-metad.rules.in | 19 ++++++++++++++++++- + udev/Makefile.in | 9 +++++---- + 2 files changed, 23 insertions(+), 5 deletions(-) + +diff --git a/udev/69-dm-lvm-metad.rules.in b/udev/69-dm-lvm-metad.rules.in +index fff063f6dafd..e8d9a64c29f5 100644 +--- a/udev/69-dm-lvm-metad.rules.in ++++ b/udev/69-dm-lvm-metad.rules.in +@@ -88,6 +88,23 @@ LABEL="lvm_scan" + # loop | | X | X* | | + # other | X | | X | | X + ENV{SYSTEMD_READY}="1" +-(PVSCAN_RULE) ++ ++# The method for invoking pvscan is selected at build time with the option ++# --(enable|disable)-udev-systemd-background-jobs to "configure". ++# On modern distributions with recent systemd, it's "systemd_background"; ++# on others, "direct_pvscan". ++GOTO="(PVSCAN_RULE)" ++ ++LABEL="systemd_background" ++ ++ACTION!="remove", ENV{LVM_PV_GONE}=="1", RUN+="(BINDIR)/systemd-run (LVM_EXEC)/lvm pvscan --cache $major:$minor", GOTO="lvm_end" ++ENV{SYSTEMD_ALIAS}="/dev/block/$major:$minor" ++ENV{ID_MODEL}="LVM PV $env{ID_FS_UUID_ENC} on /dev/$name" ++ENV{SYSTEMD_WANTS}+="lvm2-pvscan@$major:$minor.service" ++GOTO="lvm_end" ++ ++LABEL="direct_pvscan" ++ ++RUN+="(LVM_EXEC)/lvm pvscan --background --cache --activate ay --major $major --minor $minor", ENV{LVM_SCANNED}="1" + + LABEL="lvm_end" +diff --git a/udev/Makefile.in b/udev/Makefile.in +index cd031aefa084..5797ee699b95 100644 +--- a/udev/Makefile.in ++++ b/udev/Makefile.in +@@ -26,6 +26,7 @@ endif + + DM_DIR=$(shell $(GREP) "\#define DM_DIR" $(top_srcdir)/libdm/misc/dm-ioctl.h | $(AWK) '{print $$3}') + ++BINDIR=@bindir@ + ifeq ("@UDEV_RULE_EXEC_DETECTION@", "yes") + SBIN=\$$env{DM_SBIN_PATH} + DM_EXEC_RULE=ENV{DM_SBIN_PATH}=\"\/sbin\"\\nTEST!=\"\$$env{DM_SBIN_PATH}\/dmsetup\", ENV{DM_SBIN_PATH}=\"\/usr\/sbin\" +@@ -47,13 +48,13 @@ BLKID_RULE=IMPORT{program}=\"${SBIN}\/blkid -o udev -p \$$tempnode\" + endif + + ifeq ("@UDEV_SYSTEMD_BACKGROUND_JOBS@", "yes") +-PVSCAN_RULE=ACTION\!=\"remove\", ENV{LVM_PV_GONE}==\"1\", RUN\+=\"@bindir@/systemd-run $(LVM_EXEC)\/lvm pvscan --cache \$$major\:\$$minor\", GOTO=\"lvm_end\"\nENV{SYSTEMD_ALIAS}=\"\/dev\/block\/\$$major:\$$minor\"\nENV{ID_MODEL}=\"LVM PV \$$env{ID_FS_UUID_ENC} on \/dev\/\$$name\"\nENV{SYSTEMD_WANTS}\+=\"lvm2-pvscan@\$$major:\$$minor.service\" ++PVSCAN_RULE=systemd_background + else +-PVSCAN_RULE=RUN\+\=\"$(LVM_EXEC)/lvm pvscan --background --cache --activate ay --major \$$major --minor \$$minor\", ENV{LVM_SCANNED}=\"1\" ++PVSCAN_RULE=direct_pvscan + endif + + %.rules: $(srcdir)/%.rules.in +- $(SED) -e "s+(DM_DIR)+$(DM_DIR)+;s+(BLKID_RULE)+$(BLKID_RULE)+;s+(PVSCAN_RULE)+$(PVSCAN_RULE)+;s+(DM_EXEC_RULE)+$(DM_EXEC_RULE)+;s+(DM_EXEC)+$(DM_EXEC)+;s+(LVM_EXEC_RULE)+$(LVM_EXEC_RULE)+;s+(LVM_EXEC)+$(LVM_EXEC)+;" $< >$@ ++ $(SED) -e "s+(DM_DIR)+$(DM_DIR)+;s+(BINDIR)+$(BINDIR)+;s+(BLKID_RULE)+$(BLKID_RULE)+;s+(PVSCAN_RULE)+$(PVSCAN_RULE)+;s+(DM_EXEC_RULE)+$(DM_EXEC_RULE)+;s+(DM_EXEC)+$(DM_EXEC)+;s+(LVM_EXEC_RULE)+$(LVM_EXEC_RULE)+;s+(LVM_EXEC)+$(LVM_EXEC)+;" $< >$@ + + %_install: %.rules + $(INSTALL_DATA) -D $< $(udevdir)/$( +Date: Thu, 21 Dec 2017 10:06:38 +0100 +Subject: [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" + +The current logic that avoids setting SYSTEMD_ALIAS and SYSTEMD_WANTS +on "change" events is flawed in the default "systemd background job" +configuration. For systemd, it's important that device properties don't +change spuriously. + +If an "add" event starts lvm2-pvscan@.service for a device, and a +"change" event follows, removing SYSTEMD_ALIAS and SYSTEMD_WANTS from the +udev db, information about unit dependencies between the device and the +pvscan service can be lost in systemd, in particular if the daemon +configuration is reloaded. + +Steps to reproduce problem: + +- create a device with an LVM PV +- remove device +- add device (generates "add" and "change" uevents for the device) + (at this point SYSTEMD_ALIAS and SYSTEMD_WANTS are clear in udev db) +- systemctl daemon-reload + (systemd reloads udev db) +- vgchange -a n +- remove device + +=> the lvm2-pvscan@.service for the device is still active although the +device is gone. + +- add device again + +=> the PV is not detected, because systemd sees the lvm2-pvscan@.service +as active and thus doesn't restart it. + +The original purpose of this logic was to avoid volumes being scanned +over and over again. With systemd background jobs, that isn't necessary, +because systemd will not restart the job as long as it's active. + +Signed-off-by: Martin Wilck +--- + udev/69-dm-lvm-metad.rules.in | 56 +++++++++++++++++++++++++++++++------------ + udev/Makefile.in | 4 +++- + 2 files changed, 44 insertions(+), 16 deletions(-) + +diff --git a/udev/69-dm-lvm-metad.rules.in b/udev/69-dm-lvm-metad.rules.in +index e8d9a64c29f5..203cb479f592 100644 +--- a/udev/69-dm-lvm-metad.rules.in ++++ b/udev/69-dm-lvm-metad.rules.in +@@ -68,25 +68,15 @@ ACTION=="change", ENV{LVM_LOOP_PV_ACTIVATED}!="1", TEST=="loop/backing_file", EN + ENV{LVM_LOOP_PV_ACTIVATED}!="1", ENV{SYSTEMD_READY}="0" + GOTO="lvm_end" + +-# If the PV is not a special device listed above, scan only after device addition (ADD event) ++# If the PV is not a special device listed above, scan only if necessary. ++# For "direct_pvscan" mode (see below), this means run rules only an ADD events. ++# For "systemd_background" mode, systemd takes care of this by activating ++# the lvm2-pvscan@.service only once. + LABEL="next" +-ACTION!="add", GOTO="lvm_end" ++ACTION!="(PVSCAN_ACTION)", GOTO="lvm_end" + + LABEL="lvm_scan" + +-# The table below summarises the situations in which we reach the LABEL="lvm_scan". +-# Marked by X, X* means only if the special dev is properly set up. +-# The artificial ADD is supported for coldplugging. We avoid running the pvscan +-# on artificial CHANGE so there's no unexpected autoactivation when WATCH rule fires. +-# N.B. MD and loop never actually reaches lvm_scan on REMOVE as the PV label is gone +-# within a CHANGE event (these are caught by the "LVM_PV_GONE" rule at the beginning). +-# +-# | real ADD | real CHANGE | artificial ADD | artificial CHANGE | REMOVE +-# ============================================================================= +-# DM | | X | X* | | X +-# MD | | X | X* | | +-# loop | | X | X* | | +-# other | X | | X | | X + ENV{SYSTEMD_READY}="1" + + # The method for invoking pvscan is selected at build time with the option +@@ -97,6 +87,27 @@ GOTO="(PVSCAN_RULE)" + + LABEL="systemd_background" + ++# The table below summarises the situations in which we reach the LABEL="lvm_scan" ++# in the "systemd_background" case. ++# Marked by X, X* means only if the special dev is properly set up. ++# The artificial ADD is supported for coldplugging. We avoid running the pvscan ++# on artificial CHANGE so there's no unexpected autoactivation when WATCH rule fires. ++# N.B. MD and loop never actually reaches lvm_scan on REMOVE as the PV label is gone ++# within a CHANGE event (these are caught by the "LVM_PV_GONE" rule at the beginning). ++# ++# In this case, we simply set up the dependency between the device and the pvscan ++# job using SYSTEMD_ALIAS (which sets up a simplified device identifier that ++# allows using "BindsTo" in the sytemd unit file) and SYSTEMD_WANTS (which tells ++# systemd to start the pvscan job once the device is ready). ++# We need to set these variables for both "add" and "change" events, otherwise ++# systemd may loose information about the device/unit dependencies. ++# ++# | real ADD | real CHANGE | artificial ADD | artificial CHANGE | REMOVE ++# ============================================================================= ++# DM | | X | X* | | X ++# MD | | X | X* | | ++# loop | | X | X* | | ++# other | X | X | X | | X + ACTION!="remove", ENV{LVM_PV_GONE}=="1", RUN+="(BINDIR)/systemd-run (LVM_EXEC)/lvm pvscan --cache $major:$minor", GOTO="lvm_end" + ENV{SYSTEMD_ALIAS}="/dev/block/$major:$minor" + ENV{ID_MODEL}="LVM PV $env{ID_FS_UUID_ENC} on /dev/$name" +@@ -105,6 +116,21 @@ GOTO="lvm_end" + + LABEL="direct_pvscan" + ++# The table below summarises the situations in which we reach the LABEL="lvm_scan" ++# for the "direct_pvscan" case. ++# Marked by X, X* means only if the special dev is properly set up. ++# The artificial ADD is supported for coldplugging. We avoid running the pvscan ++# on artificial CHANGE so there's no unexpected autoactivation when WATCH rule fires. ++# ++# In this case, we need to make sure that pvscan is not invoked spuriously, therefore ++# we invoke it only for "add" events for "other" devices. ++# ++# | real ADD | real CHANGE | artificial ADD | artificial CHANGE | REMOVE ++# ============================================================================= ++# DM | | X | X* | | X ++# MD | | X | X* | | ++# loop | | X | X* | | ++# other | X | | X | | X + RUN+="(LVM_EXEC)/lvm pvscan --background --cache --activate ay --major $major --minor $minor", ENV{LVM_SCANNED}="1" + + LABEL="lvm_end" +diff --git a/udev/Makefile.in b/udev/Makefile.in +index 5797ee699b95..3e274736d145 100644 +--- a/udev/Makefile.in ++++ b/udev/Makefile.in +@@ -49,12 +49,14 @@ endif + + ifeq ("@UDEV_SYSTEMD_BACKGROUND_JOBS@", "yes") + PVSCAN_RULE=systemd_background ++PVSCAN_ACTION=add|change + else + PVSCAN_RULE=direct_pvscan ++PVSCAN_ACTION=add + endif + + %.rules: $(srcdir)/%.rules.in +- $(SED) -e "s+(DM_DIR)+$(DM_DIR)+;s+(BINDIR)+$(BINDIR)+;s+(BLKID_RULE)+$(BLKID_RULE)+;s+(PVSCAN_RULE)+$(PVSCAN_RULE)+;s+(DM_EXEC_RULE)+$(DM_EXEC_RULE)+;s+(DM_EXEC)+$(DM_EXEC)+;s+(LVM_EXEC_RULE)+$(LVM_EXEC_RULE)+;s+(LVM_EXEC)+$(LVM_EXEC)+;" $< >$@ ++ $(SED) -e "s+(DM_DIR)+$(DM_DIR)+;s+(BINDIR)+$(BINDIR)+;s+(BLKID_RULE)+$(BLKID_RULE)+;s+(PVSCAN_RULE)+$(PVSCAN_RULE)+;s+(PVSCAN_ACTION)+$(PVSCAN_ACTION)+;s+(DM_EXEC_RULE)+$(DM_EXEC_RULE)+;s+(DM_EXEC)+$(DM_EXEC)+;s+(LVM_EXEC_RULE)+$(LVM_EXEC_RULE)+;s+(LVM_EXEC)+$(LVM_EXEC)+;" $< >$@ + + %_install: %.rules + $(INSTALL_DATA) -D $< $(udevdir)/$(