All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
@ 2017-12-21 11:57 Martin Wilck
  2017-12-21 11:57 ` [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule Martin Wilck
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Martin Wilck @ 2017-12-21 11:57 UTC (permalink / raw)
  To: lvm-devel

The current logic in 69-dm-lvm-metad.rules is broken for the default
"enable-udev-systemd-background-jobs" case. Detailed information about the
problem can be found in the commit message of the 2nd patch in the set.
That patch also contains the tiny actual change of this patch set: if systemd
background jobs are active, the variables SYSTEMD_ALIAS and SYSTEMD_WANTS are
also set for CHANGE events, not only for ADD.

The reason that the patch set is quite large nonetheless is that I wanted
the comments in the rules file to match the actual behavior. Substitution of
multi-line comments is very hard, if not impossible, with the string
substitution approach in the current Makefile. That necessitates the first
patch, which introduces no functional change.

Martin Wilck (2):
  lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule
  lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"

 udev/69-dm-lvm-metad.rules.in | 53 +++++++++++++++++++++++++++++++++++++++----
 udev/Makefile.in              |  9 +++++---
 2 files changed, 54 insertions(+), 8 deletions(-)

-- 
2.15.1



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule
  2017-12-21 11:57 [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
@ 2017-12-21 11:57 ` Martin Wilck
  2017-12-21 11:57 ` [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" Martin Wilck
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Martin Wilck @ 2017-12-21 11:57 UTC (permalink / raw)
  To: lvm-devel

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 <mwilck@suse.com>
---
 udev/69-dm-lvm-metad.rules.in | 19 ++++++++++++++++++-
 udev/Makefile.in              |  7 ++++---
 2 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/udev/69-dm-lvm-metad.rules.in b/udev/69-dm-lvm-metad.rules.in
index bd75fc8efcd5..38687f443e0c 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 c498aa8388c5..9b2e2c34c518 100644
--- a/udev/Makefile.in
+++ b/udev/Makefile.in
@@ -25,6 +25,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\"
@@ -46,13 +47,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)/$(<F)
-- 
2.15.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"
  2017-12-21 11:57 [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
  2017-12-21 11:57 ` [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule Martin Wilck
@ 2017-12-21 11:57 ` Martin Wilck
  2018-02-02 14:56   ` Eric Ren
  2017-12-22 11:54 ` [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
       [not found] ` <1516050733.5699.48.camel@suse.com>
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Wilck @ 2017-12-21 11:57 UTC (permalink / raw)
  To: lvm-devel

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 at .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 at .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 at .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 <mwilck@suse.com>
---
 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 38687f443e0c..2ff8ddc3162e 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 at .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 9b2e2c34c518..6f57d46125ce 100644
--- a/udev/Makefile.in
+++ b/udev/Makefile.in
@@ -48,12 +48,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)/$(<F)
-- 
2.15.1



^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
  2017-12-21 11:57 [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
  2017-12-21 11:57 ` [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule Martin Wilck
  2017-12-21 11:57 ` [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" Martin Wilck
@ 2017-12-22 11:54 ` Martin Wilck
  2017-12-27  8:03   ` Eric Ren
       [not found] ` <1516050733.5699.48.camel@suse.com>
  3 siblings, 1 reply; 9+ messages in thread
From: Martin Wilck @ 2017-12-22 11:54 UTC (permalink / raw)
  To: lvm-devel

On Thu, 2017-12-21 at 12:57 +0100, Martin Wilck wrote:
> The current logic in 69-dm-lvm-metad.rules is broken for the default
> "enable-udev-systemd-background-jobs" case. Detailed information
> about the
> problem can be found in the commit message of the 2nd patch in the
> set.

A few more remarks may be in order here. 

Firstly, in addition to the patch I sent, it might make sense to call
"systemctl stop lvm-pvscan@$major.minor.service" in the LVM_PV_REMOVED
case in 69-dm-lvm-metad.rules rather than just "pvscan --cache
$major:minor". By doing the latter, udev informs lvmetad about the
removed PV, but not systemd. Comments welcome.

Secondly, you may have a hard time reproducing the issue on a typical
x86/SCSI setup. The problem has been reported to us for s390x DASDs,
using this command sequence:

chccwdev -e $CCW  # generates ADD, then CHANGE event
vgchange -an $VG
chccwdev -d $CCW  # generates CHANGE+LVM_PV_REMOVED, then REMOVE event
chccwdev -e $CCW
=> PV is not seen by lvmetad.

Doing the same thing with SCSI is quite tricky. Maybe someone else
finds a more elegant reproduce. Below is what I came up with.

Martin

Situation after booting, PV (sdf1) is live and registered in udev,
systemd, and lvmetad. I start "udevadm monitor" in the background to
see the uevents. Note the "Following" property below, which represents
the effect of SYSTEMD_ALIAS for the PV.

> # udevadm monitor -u -s block &
> # pvs
>   PV         VG Fmt  Attr PSize PFree  
>   /dev/sdf1  vg lvm2 a--  1.70g 712.00m
> 
> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
> BindsTo=dev-block-8:81.device
> ActiveState=active
> 
> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
8:81.device
> Following=sys-devices-pci0000:00-0000:00:14.0-host18-target18:0:4-
18:0:4:0-block-sdf-sdf1.device
> BoundBy=lvm2-pvscan at 8:81.service
> ActiveState=active
> 
> # udevadm info /dev/sdf1  | grep SYSTEMD
> E: SYSTEMD_ALIAS=/dev/block/8:81
> E: SYSTEMD_READY=1
> E: SYSTEMD_WANTS=lvm2-pvscan at 8:81.service

Now we generate a CHANGE event by simulating a close-after-write. The
SYSTEMD variables are lost in the udev db, but systemd state is still
ok:

> # python -c 'open("/dev/sdf1", "w")'
> UDEV  [507.991529]
change   /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
block/sdf/sdf1 (block)
> 
> # udevadm info /dev/sdf1  | grep SYSTEMD
> (no output)
> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
8:81.device
> Following=sys-devices-pci0000:00-0000:00:14.0-host18-target18:0:4-
18:0:4:0-block-sdf-sdf1.device
> BoundBy=lvm2-pvscan at 8:81.service
> ActiveState=active

systemctl daemon-reload makes systemd loose the connection between the
"alias"  dev-block-8:81.device and the real device unit "sys-devices-
pci0000:00-0000:00:14.0-host18-target18:0:4-18:0:4:0-block-sdf-
sdf1.device". The pvscan unit is unaffected. pvs still works.

> # systemctl daemon-reload
> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
8:81.device
> Following=
> BoundBy=lvm2-pvscan at 8:81.service
> ActiveState=inactive
> 
> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
> BindsTo=dev-block-8:81.device
> ActiveState=active
> 
> # pvs
>   PV         VG Fmt  Attr PSize PFree  
>   /dev/sdf1  vg lvm2 a--  1.70g 712.00m

Next we simulate device removal. It's a bit tricky to do exactly as
chccwdev does it.
  - deactivate LVs (not strictly necessary, but customer did it)
  - make sure blkid can't read from page cache
  - make disk unreadable
  - synthesize CHANGE event (this triggers the LVM_PV_GONE case in 69-
dm-lvmetad.rules, and removes the PV from lvmetad's cache)
  - make disk readable again

> # vgchange -an vg
> # echo 3 >/proc/sys/vm/drop_caches # 
> # echo transport-offline  >/sys/block/sdd/device/state
> # python -c 'open("/dev/sdf1", "w")'
> UDEV  [863.996418]
change   /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
block/sdf/sdf1 (block)
> # echo running  >/sys/block/sdf/device/state
> # pvs
> (no output)

But the pvscan service is still active (of course, nothing happened
that would change its state).

> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
> BindsTo=dev-block-8:81.device
> ActiveState=active

Now we delete the device, and see the pvscan service is *still* active.
THIS IS THE ERROR.
> # echo 1 >/sys/block/sdf/device/delete
> 
> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
> BindsTo=dev-block-8:81.device
> ActiveState=active

We add the disk again. After that we see the pvscan still running (but
with start date 1/2h in the past), and no physical volumes shown by
"pvs".

[ Remark: on my test system, the kernel would sometimes assign a
different block device to the disk at this point, breaking the
reproducer. I didn't investigate this further. The problem is only
reproduced if the dev_t remains the same after re-adding the disk. 
This is always the case with s390 DASDs, AFAICS. ]

> # echo - - - >/sys/class/scsi_host/host18/scan 
> UDEV  [1524.967155]
add      /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
block/sdf (block)
> UDEV  [1524.973179]
add      /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
block/sdf/sdf1 (block)
> # pvs
> (no output)
> # systemctl show -p ActiveState -p BindsTo -p ExecMainStartTimestamp 
lvm2-pvscan at 8:81.service
> BindsTo=dev-block-8:81.device
> ActiveState=active
> ExecMainStartTimestamp=Fri 2017-12-22 11:56:35 CET
> # date
> Fri Dec 22 12:22:21 CET 2017

Restarting the pvscan service brings back the PV.

> # systemctl restart lvm2-pvscan at 8:81.service
> # pvs
>   PV         VG Fmt  Attr PSize PFree  
>   /dev/sdf1  vg lvm2 a--  1.70g 712.00m

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
  2017-12-22 11:54 ` [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
@ 2017-12-27  8:03   ` Eric Ren
  2017-12-27  8:48     ` Eric Ren
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Ren @ 2017-12-27  8:03 UTC (permalink / raw)
  To: lvm-devel

Hi Martin and all,

Thanks for you reproducing guide. I have reproduce this problem on the very
recent openSUSE tumbleweed and fedora distro. Please see the attachments 
for the
details.

- reproducer-on-s390.sh
The patches passed the reproducer on s390.

- reproduced-on-fedora28.txt on x86_64
Reproduced, but not tried the patches on fedora yet.

- very-confusing-result-with-patches.txt on x86_64
This patches fixed the issue on the second testing, but re-occurs at the
third testing. After that, I cannot get the PV back again.

@Martin, can you try to repeat the steps more than 2 times on your testing
machine?

Some special notice to reproduce:

1. reboot after you partitioned the disk and create the LV on it, 
otherwise the following
command will output nothing:

"""

# udevadm info /dev/sdf1  | grep SYSTEMD
(no output)

"""

2. after booting up, the first round testing is always fine, but the problem
will 100% happen at the second round.

I don't know why, do you have an explanation on that?

Thanks,
Eric


On 12/22/2017 07:54 PM, Martin Wilck wrote:
> On Thu, 2017-12-21 at 12:57 +0100, Martin Wilck wrote:
>> The current logic in 69-dm-lvm-metad.rules is broken for the default
>> "enable-udev-systemd-background-jobs" case. Detailed information
>> about the
>> problem can be found in the commit message of the 2nd patch in the
>> set.
> A few more remarks may be in order here.
>
> Firstly, in addition to the patch I sent, it might make sense to call
> "systemctl stop lvm-pvscan@$major.minor.service" in the LVM_PV_REMOVED
> case in 69-dm-lvm-metad.rules rather than just "pvscan --cache
> $major:minor". By doing the latter, udev informs lvmetad about the
> removed PV, but not systemd. Comments welcome.
>
> Secondly, you may have a hard time reproducing the issue on a typical
> x86/SCSI setup. The problem has been reported to us for s390x DASDs,
> using this command sequence:
>
> chccwdev -e $CCW  # generates ADD, then CHANGE event
> vgchange -an $VG
> chccwdev -d $CCW  # generates CHANGE+LVM_PV_REMOVED, then REMOVE event
> chccwdev -e $CCW
> => PV is not seen by lvmetad.
>
> Doing the same thing with SCSI is quite tricky. Maybe someone else
> finds a more elegant reproduce. Below is what I came up with.
>
> Martin
>
> Situation after booting, PV (sdf1) is live and registered in udev,
> systemd, and lvmetad. I start "udevadm monitor" in the background to
> see the uevents. Note the "Following" property below, which represents
> the effect of SYSTEMD_ALIAS for the PV.
>
>> # udevadm monitor -u -s block &
>> # pvs
>>    PV         VG Fmt  Attr PSize PFree
>>    /dev/sdf1  vg lvm2 a--  1.70g 712.00m
>>
>> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
>> BindsTo=dev-block-8:81.device
>> ActiveState=active
>>
>> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
> 8:81.device
>> Following=sys-devices-pci0000:00-0000:00:14.0-host18-target18:0:4-
> 18:0:4:0-block-sdf-sdf1.device
>> BoundBy=lvm2-pvscan at 8:81.service
>> ActiveState=active
>>
>> # udevadm info /dev/sdf1  | grep SYSTEMD
>> E: SYSTEMD_ALIAS=/dev/block/8:81
>> E: SYSTEMD_READY=1
>> E: SYSTEMD_WANTS=lvm2-pvscan at 8:81.service
> Now we generate a CHANGE event by simulating a close-after-write. The
> SYSTEMD variables are lost in the udev db, but systemd state is still
> ok:
>
>> # python -c 'open("/dev/sdf1", "w")'
>> UDEV  [507.991529]
> change   /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
> block/sdf/sdf1 (block)
>> # udevadm info /dev/sdf1  | grep SYSTEMD
>> (no output)
>> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
> 8:81.device
>> Following=sys-devices-pci0000:00-0000:00:14.0-host18-target18:0:4-
> 18:0:4:0-block-sdf-sdf1.device
>> BoundBy=lvm2-pvscan at 8:81.service
>> ActiveState=active
> systemctl daemon-reload makes systemd loose the connection between the
> "alias"  dev-block-8:81.device and the real device unit "sys-devices-
> pci0000:00-0000:00:14.0-host18-target18:0:4-18:0:4:0-block-sdf-
> sdf1.device". The pvscan unit is unaffected. pvs still works.
>
>> # systemctl daemon-reload
>> # systemctl show -p ActiveState -p BoundBy -p Following dev-block-
> 8:81.device
>> Following=
>> BoundBy=lvm2-pvscan at 8:81.service
>> ActiveState=inactive
>>
>> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
>> BindsTo=dev-block-8:81.device
>> ActiveState=active
>>
>> # pvs
>>    PV         VG Fmt  Attr PSize PFree
>>    /dev/sdf1  vg lvm2 a--  1.70g 712.00m
> Next we simulate device removal. It's a bit tricky to do exactly as
> chccwdev does it.
>    - deactivate LVs (not strictly necessary, but customer did it)
>    - make sure blkid can't read from page cache
>    - make disk unreadable
>    - synthesize CHANGE event (this triggers the LVM_PV_GONE case in 69-
> dm-lvmetad.rules, and removes the PV from lvmetad's cache)
>    - make disk readable again
>
>> # vgchange -an vg
>> # echo 3 >/proc/sys/vm/drop_caches #
>> # echo transport-offline  >/sys/block/sdd/device/state
>> # python -c 'open("/dev/sdf1", "w")'
>> UDEV  [863.996418]
> change   /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
> block/sdf/sdf1 (block)
>> # echo running  >/sys/block/sdf/device/state
>> # pvs
>> (no output)
> But the pvscan service is still active (of course, nothing happened
> that would change its state).
>
>> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
>> BindsTo=dev-block-8:81.device
>> ActiveState=active
> Now we delete the device, and see the pvscan service is *still* active.
> THIS IS THE ERROR.
>> # echo 1 >/sys/block/sdf/device/delete
>>
>> # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:81.service
>> BindsTo=dev-block-8:81.device
>> ActiveState=active
> We add the disk again. After that we see the pvscan still running (but
> with start date 1/2h in the past), and no physical volumes shown by
> "pvs".
>
> [ Remark: on my test system, the kernel would sometimes assign a
> different block device to the disk at this point, breaking the
> reproducer. I didn't investigate this further. The problem is only
> reproduced if the dev_t remains the same after re-adding the disk.
> This is always the case with s390 DASDs, AFAICS. ]
>
>> # echo - - - >/sys/class/scsi_host/host18/scan
>> UDEV  [1524.967155]
> add      /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
> block/sdf (block)
>> UDEV  [1524.973179]
> add      /devices/pci0000:00/0000:00:14.0/host18/target18:0:4/18:0:4:0/
> block/sdf/sdf1 (block)
>> # pvs
>> (no output)
>> # systemctl show -p ActiveState -p BindsTo -p ExecMainStartTimestamp
> lvm2-pvscan at 8:81.service
>> BindsTo=dev-block-8:81.device
>> ActiveState=active
>> ExecMainStartTimestamp=Fri 2017-12-22 11:56:35 CET
>> # date
>> Fri Dec 22 12:22:21 CET 2017
> Restarting the pvscan service brings back the PV.
>
>> # systemctl restart lvm2-pvscan at 8:81.service
>> # pvs
>>    PV         VG Fmt  Attr PSize PFree
>>    /dev/sdf1  vg lvm2 a--  1.70g 712.00m

-------------- next part --------------
A non-text attachment was scrubbed...
Name: reproducer-on-s390.sh
Type: application/x-shellscript
Size: 360 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20171227/4bf6c26a/attachment.bin>
-------------- next part --------------
[root at localhost ~]# cat /etc/redhat-release 
Fedora release 28 (Rawhide)
[root at localhost ~]# rpm -qa|grep lvm2
lvm2-2.02.176-1.fc28.x86_64

========== 1st testing on fedora==========
[root at localhost ~]# udevadm monitor -u -s block &
[1] 3043
monitor will print the received events for:
UDEV - the event which udev sends out after rule processing

[root at localhost ~]# udevadm info /dev/sdb1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:17
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:17.service

[root at localhost ~]# python -c 'open("/dev/sdb1", "w")'
[root at localhost ~]# UDEV  [1704.670470] change   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb/sdb1 (block)

[root at localhost ~]# udevadm info /dev/sdb1 | grep SYSTEMD

[root at localhost ~]# systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:17.device
Following=sys-devices-pci0000:00-0000:00:01.1-ata2-host1-target1:0:1-1:0:1:0-block-sdb-sdb1.device
BoundBy=lvm2-pvscan at 8:17.service
ActiveState=active

[root at localhost ~]# systemctl daemon-reload
[root at localhost ~]# systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:17.device
Following=
BoundBy=lvm2-pvscan at 8:17.service
ActiveState=inactive
[root at localhost ~]# systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:17.service
BindsTo=dev-block-8:17.device
ActiveState=active
[root at localhost ~]# pvs
  PV         VG     Fmt  Attr PSize   PFree   
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m
  /dev/sdb1  vg1    lvm2 a--   <2.00g 1020.00m

[root@localhost ~]# echo 3 >/proc/sys/vm/drop_caches 
[root at localhost ~]# echo transport-offline >/sys/block/sdb/device/state 
[root at localhost ~]# python -c 'open("/dev/sdb1", "w")'
[root at localhost ~]# UDEV  [1781.479578] change   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb/sdb1 (block)

[root at localhost ~]# echo running >/sys/block/sdb/device/state 
[root at localhost ~]# pvs
  PV         VG     Fmt  Attr PSize   PFree   
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m
[root at localhost ~]# systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:17.service
BindsTo=dev-block-8:17.device
ActiveState=active

[root@localhost ~]# echo 1 >/sys/block/sdb/device/delete 
[root at localhost ~]# UDEV  [1803.200393] remove   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb/sdb1 (block)
UDEV  [1803.208113] remove   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb (block)

[root at localhost ~]# systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:17.service
BindsTo=dev-block-8:17.device
ActiveState=active
[root at localhost ~]#  echo - - - >/sys/class/scsi_host/host1/scan 
[root at localhost ~]# UDEV  [1817.722955] add      /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc (block)
UDEV  [1817.768257] add      /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc/sdc1 (block)
UDEV  [1818.042588] change   /devices/virtual/block/dm-2 (block)

[root at localhost ~]# pvs
  PV         VG     Fmt  Attr PSize   PFree   
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m
  /dev/sdc1  vg1    lvm2 a--   <2.00g 1020.00m

========== END of 1st testing on fedora=================

============== 2st testing on fedora=====================
[root at localhost ~]# lsblk
NAME            MAJ:MIN RM  SIZE RO TYPE MOUNTPOINT
sda               8:0    0   20G  0 disk
??sda1            8:1    0    1G  0 part /boot
??sda2            8:2    0   19G  0 part
  ??fedora-root 253:0    0   17G  0 lvm  /
  ??fedora-swap 253:1    0    1G  0 lvm
sdc               8:32   0   10G  0 disk
??sdc1            8:33   0    2G  0 part
  ??vg1-lv1     253:2    0    1G  0 lvm

[root at localhost ~]# udevadm info /dev/sdc1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:33
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:33.service
[root at localhost ~]# python -c 'open("/dev/sdc1", "w")'
[root at localhost ~]# UDEV  [1925.268635] change   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc/sdc1 (block)
[root at localhost ~]# udevadm info /dev/sdc1 | grep SYSTEMD

[root at localhost ~]# systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:33.device
Following=sys-devices-pci0000:00-0000:00:01.1-ata2-host1-target1:0:1-1:0:1:0-block-sdc-sdc1.device
BoundBy=lvm2-pvscan at 8:33.service
ActiveState=active
[root at localhost ~]# systemctl daemon-reload
[root at localhost ~]# systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:33.device
Following=
BoundBy=lvm2-pvscan at 8:33.service
ActiveState=inactive
[root at localhost ~]# pvs
  PV         VG     Fmt  Attr PSize   PFree
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m
  /dev/sdc1  vg1    lvm2 a--   <2.00g 1020.00m

[root@localhost ~]# echo 3 >/proc/sys/vm/drop_caches
[root at localhost ~]# echo transport-offline >/sys/block/sdc/device/state
[root at localhost ~]# udevadm info /dev/sdc1 | grep SYSTEMD
[root at localhost ~]# python -c 'open("/dev/sdc1", "w")'
[root at localhost ~]# UDEV  [1975.250949] change   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc/sdc1 (block)
echo transport-offline >/sys/block/sdc/device/state
[root at localhost ~]# echo running >/sys/block/sdc/device/state
[root at localhost ~]# pvs
  PV         VG     Fmt  Attr PSize   PFree
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m
[root at localhost ~]# systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:33.device
Following=
BoundBy=lvm2-pvscan at 8:33.service
ActiveState=inactive
[root at localhost ~]# systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:33.service
BindsTo=dev-block-8:33.device
ActiveState=active

[root@localhost ~]# echo 1 >/sys/block/sdc/device/delete
UDEV  [2035.593916] remove   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc/sdc1 (block)
UDEV  [2035.602933] remove   /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdc (block)
[root at localhost ~]# systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:33.service
BindsTo=dev-block-8:33.device
ActiveState=active
[root at localhost ~]# echo - - - >/sys/class/scsi_host/host1/scan
[root at localhost ~]# UDEV  [2060.525945] add      /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb (block)
UDEV  [2060.566225] add      /devices/pci0000:00/0000:00:01.1/ata2/host1/target1:0:1/1:0:1:0/block/sdb/sdb1 (block)
[root at localhost ~]# pvs
  /dev/vg1/lv1: read failed after 0 of 4096 at 0: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073676288: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073733632: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 4096: Input/output error
  PV         VG     Fmt  Attr PSize   PFree
  /dev/sda2  fedora lvm2 a--  <19.00g 1020.00m

============== END of 2st testing on fedora=====================
-------------- next part --------------
=========1st testing with patches ============

linux-gahs:~ # udevadm info /dev/sdb1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:17
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:17.service
linux-gahs:~ # python -c 'open("/dev/sdb1", "w")'
linux-gahs:~ # UDEV  [436.216943] change   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb/sdb1 (block)
linux-gahs:~ # udevadm info /dev/sdb1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:17
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:17.service

linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:17.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdb-sdb1.device
BoundBy=lvm2-pvscan at 8:17.service
ActiveState=active
linux-gahs:~ # systemctl daemon-reload
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:17.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdb-sdb1.device
BoundBy=lvm2-pvscan at 8:17.service
ActiveState=active

linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan@8:17.service
BindsTo=dev-block-8:17.device
ActiveState=active
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
  /dev/sdb1  vg1      lvm2 a--   10.00g   9.00g

linux-gahs:~ # echo 3 >/proc/sys/vm/drop_caches 
linux-gahs:~ # echo transport-offline >/sys/block/sdb/device/state 
linux-gahs:~ #  python -c 'open("/dev/sdb1", "w")'
linux-gahs:~ # UDEV  [529.237372] change   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb/sdb1 (block)

linux-gahs:~ # echo running >/sys/block/sdb/device/state 
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:17.service
BindsTo=dev-block-8:17.device
ActiveState=active
linux-gahs:~ # echo 1 >/sys/block/sdb/device/delete
UDEV  [564.155452] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb/sdb1 (block)
UDEV  [564.156012] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb (block)
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:17.service
BindsTo=dev-block-8:17.device
ActiveState=inactive
linux-gahs:~ # echo - - - >/sys/class/scsi_host/host1/scan 
linux-gahs:~ # UDEV  [586.438757] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd (block)
UDEV  [586.729698] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)
UDEV  [586.909397] change   /devices/virtual/block/dm-6 (block)
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
  /dev/sdd1  vg1      lvm2 a--   10.00g   9.00g

========================== END ==============================

===========2st testing with patches==========================

linux-gahs:~ # udevadm info /dev/sdd1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:49
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:49.service
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:49.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdd-sdd1.device
BoundBy=lvm2-pvscan at 8:49.service
ActiveState=active
linux-gahs:~ #  systemctl daemon-reload
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:49.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdd-sdd1.device
BoundBy=lvm2-pvscan at 8:49.service
ActiveState=active
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=active
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
  /dev/sdd1  vg1      lvm2 a--   10.00g   9.00g
linux-gahs:~ # echo 3 >/proc/sys/vm/drop_caches
linux-gahs:~ # echo transport-offline >/sys/block/sdd/device/state 
linux-gahs:~ # python -c 'open("/dev/sdd1", "w")'
linux-gahs:~ # UDEV  [799.844660] change   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)

linux-gahs:~ # echo running >/sys/block/sdd/device/state
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=active
linux-gahs:~ # echo 1 >/sys/block/sdd/device/delete
UDEV  [825.385230] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)
UDEV  [825.385532] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd (block)
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=inactive
linux-gahs:~ #  echo - - - >/sys/class/scsi_host/host1/scan 
linux-gahs:~ # UDEV  [845.103884] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb (block)
UDEV  [845.394752] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb/sdb1 (block)
UDEV  [845.566998] change   /devices/virtual/block/dm-6 (block)

linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree  
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
  /dev/sdb1  vg1      lvm2 a--   10.00g   9.00g

========================== END ==============================

========= 3st testing with patches ============
linux-gahs:~ # udevadm info /dev/sdd1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:49
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:49.service
linux-gahs:~ # python -c 'open("/dev/sdd1", "w")'
linux-gahs:~ # UDEV  [5388.897557] change   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)
linux-gahs:~ # udevadm info /dev/sdd1 | grep SYSTEMD
E: SYSTEMD_ALIAS=/dev/block/8:49
E: SYSTEMD_READY=1
E: SYSTEMD_WANTS=lvm2-pvscan at 8:49.service

linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:49.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdd-sdd1.device
BoundBy=lvm2-pvscan at 8:49.service
ActiveState=active
linux-gahs:~ # systemctl daemon-reload
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:49.device
Following=sys-devices-pci0000:00-0000:00:1f.2-ata2-host1-target1:0:0-1:0:0:0-block-sdd-sdd1.device
BoundBy=lvm2-pvscan at 8:49.service
ActiveState=active

linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=active
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
  /dev/sdd1  vg1      lvm2 a--   10.00g   9.00g
linux-gahs:~ # echo 3 >/proc/sys^Cm/drop_caches
linux-gahs:~ # echo transport-offline >/sys/block/sdd/device/state
linux-gahs:~ # python -c 'open("/dev/sdd1", "w")'
linux-gahs:~ # UDEV  [5504.004925] change   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)
linux-gahs:~ #  echo running >/sys/block/sdd/device/state
linux-gahs:~ # pvs
  PV         VG       Fmt  Attr PSize   PFree
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g
linux-gahs:~ #  systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=active

linux-gahs:~ # echo 1 >/sys/block/sdd/device/delete
UDEV  [5531.152051] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd/sdd1 (block)
UDEV  [5531.152464] remove   /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdd (block)
linux-gahs:~ # systemctl show -p ActiveState -p BindsTo lvm2-pvscan at 8:49.service
BindsTo=dev-block-8:49.device
ActiveState=inactive
linux-gahs:~ # echo - - - >/sys/class/scsi_host/host1/scan
linux-gahs:~ # UDEV  [5561.620805] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb (block)
UDEV  [5561.622683] add      /devices/pci0000:00/0000:00:1f.2/ata2/host1/target1:0:0/1:0:0:0/block/sdb/sdb1 (block)

linux-gahs:~ # pvs
  /dev/vg1/lv1: read failed after 0 of 4096 at 0: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073676288: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073733632: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 4096: Input/output error
  PV         VG       Fmt  Attr PSize   PFree
  /dev/sda2  systemvg lvm2 a--  425.76g 295.76g

  linux-gahs:~ # lsblk
NAME                        MAJ:MIN RM   SIZE RO TYPE MOUNTPOINT
...
sdb                           8:16   0   149G  0 disk
??sdb1                        8:17   0    10G  0 part
linux-gahs:~ # udevadm info /dev/sdb1 | grep SYSTEMD
Unknown device, --name=, --path=, or absolute path in /dev/ or /sys expected.
....
linux-gahs:~ # pvscan --cache
  /dev/vg1/lv1: read failed after 0 of 4096 at 0: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073676288: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 1073733632: Input/output error
  /dev/vg1/lv1: read failed after 0 of 4096 at 4096: Input/output error
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:49.device
Following=
BoundBy=lvm2-pvscan at 8:49.service
ActiveState=inactive
linux-gahs:~ # systemctl show -p ActiveState -p BoundBy -p Following dev-block-8:17.device
Following=
BoundBy=lvm2-pvscan at 8:17.service
ActiveState=inactive
================== END ===============================================

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
  2017-12-27  8:03   ` Eric Ren
@ 2017-12-27  8:48     ` Eric Ren
  2017-12-28  2:42       ` Eric Ren
  0 siblings, 1 reply; 9+ messages in thread
From: Eric Ren @ 2017-12-27  8:48 UTC (permalink / raw)
  To: lvm-devel

Hi Martin,

> - very-confusing-result-with-patches.txt on x86_64
> This patches fixed the issue on the second testing, but re-occurs at the
> third testing. After that, I cannot get the PV back again.
>
> @Martin, can you try to repeat the steps more than 2 times on your 
> testing
> machine?

I tried more times. The problem doesn't occur any more. I suspect maybe it's
because there's something wrong during my manual testing - it's easy to 
mistake
following all those steps. I will try to put all steps into a testing 
script for more testing.

Thanks,
Eric



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
  2017-12-27  8:48     ` Eric Ren
@ 2017-12-28  2:42       ` Eric Ren
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Ren @ 2017-12-28  2:42 UTC (permalink / raw)
  To: lvm-devel

Hi all,


> I tried more times. The problem doesn't occur any more. I suspect 
> maybe it's
> because there's something wrong during my manual testing - it's easy 
> to mistake
> following all those steps. I will try to put all steps into a testing 
> script for more testing.

Based on Martin's reproducing steps for x86 arch, I scripted those steps 
together into
reproducer_x86.sh in attachment. The reproducer passed 100 loops on my 
testing
machine with Martin's patches.

Please adjust some variables for your testing machine when running 
reproducer_x86.sh.
There are some ugly hard code settings inside - the device name/dev_t, 
/sys/class/scsi_host/host1/scan/,etc.

Regards,
Eric
-------------- next part --------------
A non-text attachment was scrubbed...
Name: reproducer_x86.sh
Type: application/x-shellscript
Size: 2291 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/lvm-devel/attachments/20171228/7eefcdd1/attachment.bin>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events
       [not found] ` <1516050733.5699.48.camel@suse.com>
@ 2018-02-02 10:55   ` Martin Wilck
  0 siblings, 0 replies; 9+ messages in thread
From: Martin Wilck @ 2018-02-02 10:55 UTC (permalink / raw)
  To: lvm-devel

On Mon, 2018-01-15 at 22:12 +0100, Martin Wilck wrote:
> Dear LVM2 maintainers,
> 
> On Thu, 2017-12-21 at 12:57 +0100, Martin Wilck wrote:
> > The current logic in 69-dm-lvm-metad.rules is broken for the
> > default
> > "enable-udev-systemd-background-jobs" case. Detailed information
> > about the
> > problem can be found in the commit message of the 2nd patch in the
> > set.
> > That patch also contains the tiny actual change of this patch set:
> > if
> > systemd
> > background jobs are active, the variables SYSTEMD_ALIAS and
> > SYSTEMD_WANTS are
> > also set for CHANGE events, not only for ADD.
> > 
> > The reason that the patch set is quite large nonetheless is that I
> > wanted
> > the comments in the rules file to match the actual behavior.
> > Substitution of
> > multi-line comments is very hard, if not impossible, with the
> > string
> > substitution approach in the current Makefile. That necessitates
> > the
> > first
> > patch, which introduces no functional change.
> > 
> > Martin Wilck (2):
> >   lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule
> >   lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"
> > 
> >  udev/69-dm-lvm-metad.rules.in | 53
> > +++++++++++++++++++++++++++++++++++++++----
> >  udev/Makefile.in              |  9 +++++---
> >  2 files changed, 54 insertions(+), 8 deletions(-)
> 
> A review of this patch would be highly appreciated.
> 

a gentle reminder, could someone please have a look?

Regards,
Martin


-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imend?rffer, Jane Smithard, Graham Norton
HRB 21284 (AG N?rnberg)



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change"
  2017-12-21 11:57 ` [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" Martin Wilck
@ 2018-02-02 14:56   ` Eric Ren
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Ren @ 2018-02-02 14:56 UTC (permalink / raw)
  To: lvm-devel

Hi,


On 12/21/2017 07:57 PM, Martin Wilck wrote:
> 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 at .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 at .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 at .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 <mwilck@suse.com>

These changes looks reasonable to? me:

1. The non-functional changes make the difference between 
"systemd-backgroud"
and "direct pvscan" methods more clear/easier to understand. For the 
"systemd-background"
method, `man systemd.device` can give further explanation.

2. It fixes a real problem, which is easily reproduced on s390, and we 
also managed to reproduce
it on x86.

Tested-by: Eric Ren <zren@suse.com>

Thanks,
Eric

> ---
>   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 38687f443e0c..2ff8ddc3162e 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 at .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 9b2e2c34c518..6f57d46125ce 100644
> --- a/udev/Makefile.in
> +++ b/udev/Makefile.in
> @@ -48,12 +48,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)/$(<F)



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-02-02 14:56 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-12-21 11:57 [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
2017-12-21 11:57 ` [PATCH 1/2] lvm2: 69-dm-lvm-metad.rules: explicit pvscan rule Martin Wilck
2017-12-21 11:57 ` [PATCH 2/2] lvm2: 69-dm-lvm-metad.rules: set systemd vars on "change" Martin Wilck
2018-02-02 14:56   ` Eric Ren
2017-12-22 11:54 ` [PATCH 0/2] LVM2: fix lvmetad udev rules for CHANGE events Martin Wilck
2017-12-27  8:03   ` Eric Ren
2017-12-27  8:48     ` Eric Ren
2017-12-28  2:42       ` Eric Ren
     [not found] ` <1516050733.5699.48.camel@suse.com>
2018-02-02 10:55   ` Martin Wilck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.