* [PATCH 00/11] multipath udev rules changes for dm rules V3
@ 2024-03-24 21:12 Martin Wilck
2024-03-24 21:12 ` [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices Martin Wilck
` (10 more replies)
0 siblings, 11 replies; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
This patch set adapts the multipath rules to the pending changes
for device mapper rules that update DM_UDEV_RULES_VSN to 3, and
applies some additional clean-ups to the rules files.
Comments welcome.
Martin Wilck (11):
11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended
devices
11-dm-mpath.rules: don't import DM_NOSCAN from udev db
11-dm-mpath.rules: don't import ID_FS_VERSION from udev db
11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules
update
11-dm-mpath.rules: adapt coldplug event handling ro 10-dm.rules update
11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules
11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED
11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN
11-dm-mpath.rules: simplify PATH_FAILED case
11-dm-mpath.rules: make label names more intuitive
kpartx.rules: ignore DM_SUSPENDED
kpartx/kpartx.rules | 2 +-
multipath/11-dm-mpath.rules.in | 84 ++++++++++++++++++++--------------
2 files changed, 50 insertions(+), 36 deletions(-)
--
2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 21:52 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db Martin Wilck
` (9 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
MPATH_DEVICE_READY is an internal property, not to be consumed by
later rules. It means that there are no usable paths. Being suspended
is a different, independent property of a map.
Don't set MPATH_DEVICE_READY=0 for suspended devices.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index d9585ab..3f23108 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -100,7 +100,7 @@ ENV{MPATH_DEVICE_READY}=="0", \
# we have to postpone the activation until the next event.
ENV{MPATH_DEVICE_READY}=="0", GOTO="dont_activate"
ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
-ENV{DM_SUSPENDED}=="1", ENV{MPATH_DEVICE_READY}="0", GOTO="dont_activate"
+ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
LABEL="dont_activate"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
2024-03-24 21:12 ` [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 22:03 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION " Martin Wilck
` (8 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
DM_NOSCAN is our "output" flag for 13-dm-disk.rules, and it should
be treated the same way as DM_UDEV_DISABLE_OTHER_RULES_FLAG, which
isn't imported from the udev database. The state that we need to
remember is MPATH_DEVICE_READY, which we've already imported above,
and we will set the "output" flags accordingly in the "force_activation"
code path further down.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 3f23108..11b5adf 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -18,9 +18,11 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1
# If this uevent didn't come from dm, don't try to update the
# device state
+# Note that .MPATH_DEVICE_READY_OLD=="" here. Thus we won't activate the
+# device below at mpath_is_ready, which is correct.
ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
- IMPORT{db}="DM_NOSCAN", IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
- GOTO="scan_import"
+ IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
+ GOTO="force_activation"
ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION from udev db
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
2024-03-24 21:12 ` [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices Martin Wilck
2024-03-24 21:12 ` [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 22:04 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update Martin Wilck
` (7 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
Use the same set of properties to import as 13-dm-disk.rules.
ID_FS_VERSION isn't used in any udev rule I am aware of.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 1 -
1 file changed, 1 deletion(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 11b5adf..5259b86 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -124,7 +124,6 @@ IMPORT{db}="ID_FS_TYPE"
IMPORT{db}="ID_FS_USAGE"
IMPORT{db}="ID_FS_UUID_ENC"
IMPORT{db}="ID_FS_LABEL_ENC"
-IMPORT{db}="ID_FS_VERSION"
IMPORT{db}="ID_PART_ENTRY_NAME"
IMPORT{db}="ID_PART_ENTRY_UUID"
IMPORT{db}="ID_PART_ENTRY_SCHEME"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (2 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION " Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 22:17 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro " Martin Wilck
` (6 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
With the late patches for 10-dm.rules, DM_UDEV_DISABLE_OTHER_RULES_FLAG isn't
restored from the udev database any more, so we don't need to restore
the flag to its original state before it is saved to the db.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 20 +++++++++++---------
1 file changed, 11 insertions(+), 9 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 5259b86..997e3e7 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -84,23 +84,25 @@ ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", \
LABEL="force_activation"
+ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
# Do not initiate scanning if no path is available,
# otherwise there would be a hang or IO error on access.
# We'd like to avoid this, especially within udev processing.
-ENV{MPATH_DEVICE_READY}=="0", ENV{DM_NOSCAN}="1"
-
-# Skip all foreign rules if no path is available.
+# This is communicated to later rules in DM_NOSCAN.
+# Likewise, skip all foreign rules if no path is available.
# Use DM_UDEV_DISABLE_OTHER_RULES_FLAG to communicate this
-# to upper layers. The original value will be restored in a late
-# udev rule.
-ENV{MPATH_DEVICE_READY}=="0", \
- ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}", \
- ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
+# to upper layers. With dm rules < v3, save the original value here;
+# it will be restored in a late udev rule.
+ENV{DM_UDEV_RULES_VSN}!="3",
+ ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
+ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
+GOTO="dont_activate"
+
+LABEL="mpath_is_ready"
# If the device comes back online, set DM_ACTIVATION so that
# upper layers do a rescan. If the device is currently suspended,
# we have to postpone the activation until the next event.
-ENV{MPATH_DEVICE_READY}=="0", GOTO="dont_activate"
ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro 10-dm.rules update
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (3 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 22:44 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules Martin Wilck
` (5 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
With late late patches for 10-dm.rules, DM_UDEV_DISABLE_OTHER_RULES_FLAG is
never restored from the udev db. Thus we don't need to clear it here
any more for coldplug events. Also, we must use .DM_SUSPENDED instead of
DM_SUSPENDED as input flag with the v3 rule set (other occurences of
DM_SUSPENDED will be replaced in a follow-up patch).
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 997e3e7..cf004fd 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -9,12 +9,17 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="1", \
PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.warning \"Coldplug event for suspended device\"", \
ENV{DM_COLDPLUG_SUSPENDED}="1", GOTO="scan_import"
-# Coldplug event. DM_UDEV_DISABLE_OTHER_RULES_FLAG has been restored
+# Coldplug event. Import previously set properties.
+ACTION!="add", GOTO="mpath_coldplug_end"
+ENV{DM_ACTIVATION}!="1", GOTO="mpath_coldplug_end"
+ENV{DM_UDEV_RULES_VSN}=="3", ENV{.DM_SUSPENDED}!="1", GOTO="scan_import"
+# With DM rules < v3, DM_UDEV_DISABLE_OTHER_RULES_FLAG has been restored
# from DB in 10-dm.rules. If the device is not suspended, clear the flag.
# This is safe for multipath where DM_UDEV_DISABLE_OTHER_RULES_FLAG is basically
# equivalent to DM_SUSPENDED==1 || DISK_RO==1
-ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
+ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="", GOTO="scan_import"
+LABEL="mpath_coldplug_end"
# If this uevent didn't come from dm, don't try to update the
# device state
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (4 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro " Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 22:58 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED Martin Wilck
` (4 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
With the late changes to 13-dm-disk.rules, we don't need to import any
blkid-generated properties from the udev database, because they will
be imported later.
Except for ID_FS_TYPE, this actually holds since lvm2 commit 94f77a4 ("udev:
import previous results of blkid when in suspended state"), included in lvm2
2.03.19, but we have no simple way to detect the version of the lvm2 rules.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index cf004fd..8f22954 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -124,6 +124,10 @@ LABEL="scan_import"
# have never been properly set. Don't import them.
ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", GOTO="import_end"
+# DM rules v3 will import missing properties on 13-dm-disk.rules.
+# No need to do it here.
+ENV{DM_UDEV_RULES_VSN}=="3", GOTO="import_end"
+
# Don't import the properties from db if we will run blkid later.
ENV{DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (5 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-26 23:36 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN Martin Wilck
` (3 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
With the late changes to the device mapper rules, DM_SUSPENDED
is not exported any more. Use .DM_SUSPENDED instead.
Note that although 11-dm-mpath.rules is not a part of lvm2, it
can be considered as part of the device-mapper layer (everything
before 13-dm-disk.rules can), and is thus allowed to use
.DM_SUSPENDED. In practice .DM_SUSPENDED is equivalent to
DM_UDEV_DISABLE_OTHER_RULES_FLAG for multipath devices, but
using .DM_SUSPENDED here makes the intention more obvious.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 8f22954..c4b5685 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -4,8 +4,11 @@ ENV{DM_UUID}!="mpath-?*", GOTO="mpath_end"
IMPORT{db}="MPATH_DEVICE_READY"
+# device-mapper rules v2 compatibility
+ENV{.DM_SUSPENDED}!="?*", ENV{.DM_SUSPENDED}="$env{DM_SUSPENDED}"
+
# Coldplug event while device is suspended (e.g. during a reload)
-ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="1", \
+ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{.DM_SUSPENDED}=="1", \
PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.warning \"Coldplug event for suspended device\"", \
ENV{DM_COLDPLUG_SUSPENDED}="1", GOTO="scan_import"
@@ -17,7 +20,7 @@ ENV{DM_UDEV_RULES_VSN}=="3", ENV{.DM_SUSPENDED}!="1", GOTO="scan_import"
# from DB in 10-dm.rules. If the device is not suspended, clear the flag.
# This is safe for multipath where DM_UDEV_DISABLE_OTHER_RULES_FLAG is basically
# equivalent to DM_SUSPENDED==1 || DISK_RO==1
-ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
+ENV{DM_UDEV_RULES_VSN}!="3", ENV{.DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="", GOTO="scan_import"
LABEL="mpath_coldplug_end"
@@ -68,7 +71,7 @@ LABEL="mpath_action"
# Activation might have been partially skipped. Activate the device now,
# i.e. disable the MPATH_UNCHANGED logic and set DM_ACTIVATION=1.
IMPORT{db}="DM_COLDPLUG_SUSPENDED"
-ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{DM_SUSPENDED}!="1", \
+ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \
ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \
PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \
GOTO="force_activation"
@@ -109,7 +112,7 @@ LABEL="mpath_is_ready"
# upper layers do a rescan. If the device is currently suspended,
# we have to postpone the activation until the next event.
ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
-ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
+ENV{.DM_SUSPENDED}=="1", GOTO="dont_activate"
ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
LABEL="dont_activate"
@@ -143,7 +146,7 @@ IMPORT{db}="ID_PART_GPT_AUTO_ROOT"
LABEL="import_end"
# Reset previous DM_COLDPLUG_SUSPENDED if activation happens now
-ENV{DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
+ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
# Multipath maps should take precedence over their members.
ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (6 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-27 0:03 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case Martin Wilck
` (2 subsequent siblings)
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
We don't need to restore DM_NOSCAN from the db anymore, so we can rename
it to .DM_NOSCAN, making it a temporary property.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index c4b5685..8833e08 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -96,14 +96,14 @@ ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
# Do not initiate scanning if no path is available,
# otherwise there would be a hang or IO error on access.
# We'd like to avoid this, especially within udev processing.
-# This is communicated to later rules in DM_NOSCAN.
+# This is communicated to later rules in .DM_NOSCAN.
# Likewise, skip all foreign rules if no path is available.
# Use DM_UDEV_DISABLE_OTHER_RULES_FLAG to communicate this
# to upper layers. With dm rules < v3, save the original value here;
# it will be restored in a late udev rule.
ENV{DM_UDEV_RULES_VSN}!="3",
ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
-ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
+ENV{.DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
GOTO="dont_activate"
LABEL="mpath_is_ready"
@@ -132,7 +132,7 @@ ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", GOTO="import_end"
ENV{DM_UDEV_RULES_VSN}=="3", GOTO="import_end"
# Don't import the properties from db if we will run blkid later.
-ENV{DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
+ENV{.DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
IMPORT{db}="ID_FS_TYPE"
IMPORT{db}="ID_FS_USAGE"
@@ -148,6 +148,9 @@ LABEL="import_end"
# Reset previous DM_COLDPLUG_SUSPENDED if activation happens now
ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
+# device-mapper rules v2 compatibility for 13-dm-disk.rules
+ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_NOSCAN}="$env{.DM_NOSCAN}"
+
# Multipath maps should take precedence over their members.
ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (7 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN Martin Wilck
@ 2024-03-24 21:12 ` Martin Wilck
2024-03-27 0:04 ` Benjamin Marzinski
2024-03-24 21:13 ` [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive Martin Wilck
2024-03-24 21:13 ` [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED Martin Wilck
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:12 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
This combination of a GOTO and a simple rule can be combined
into a single rule.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 12 ++++--------
1 file changed, 4 insertions(+), 8 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 8833e08..2285853 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -56,14 +56,10 @@ PROGRAM=="@BINDIR@/multipath -U -v1 %k", GOTO="paths_ok"
ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
LABEL="paths_ok"
-# Don't mark a device ready on a PATH_FAILED event. even if
-# DM_NR_VALID_PATHS is greater than 0. Just keep the existing
-# value
-ENV{DM_ACTION}=="PATH_FAILED", GOTO="mpath_action"
-
-# This event is either a PATH_REINSTATED or a table reload where
-# there are active paths. Mark the device ready
-ENV{MPATH_DEVICE_READY}="1"
+# For PATH_FAILED events, keep the existing value of MPATH_DEVICE_READY.
+# If it's not PATH_FAILED, this event is either a PATH_REINSTATED or a
+# table reload where there are active paths. Mark the device ready.
+ENV{DM_ACTION}!="PATH_FAILED", ENV{MPATH_DEVICE_READY}="1"
LABEL="mpath_action"
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (8 preceding siblings ...)
2024-03-24 21:12 ` [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case Martin Wilck
@ 2024-03-24 21:13 ` Martin Wilck
2024-03-27 0:06 ` Benjamin Marzinski
2024-03-24 21:13 ` [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED Martin Wilck
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:13 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
The labels "dont_activate" and "scan_import" denote the same code line.
Remove "dont_activate". Improve two misleading label names.
Substitutions:
dont_activate -> scan_import
force_activation -> check_mpath_ready
mpath_action -> check_mpath_unchanged
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
multipath/11-dm-mpath.rules.in | 22 +++++++++++-----------
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 2285853..526cca8 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -30,7 +30,7 @@ LABEL="mpath_coldplug_end"
# device below at mpath_is_ready, which is correct.
ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
- GOTO="force_activation"
+ GOTO="check_mpath_ready"
ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
@@ -38,13 +38,13 @@ ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
# table with no active devices. If this happens, mark the
# device not ready
ENV{DM_SUBSYSTEM_UDEV_FLAG2}=="1", ENV{MPATH_DEVICE_READY}="0", \
- GOTO="mpath_action"
+ GOTO="check_mpath_unchanged"
# If the last path has failed mark the device not ready
# Note that DM_NR_VALID_PATHS is only set for PATH_FAILED|PATH_REINSTATED
# events.
# This may not be reliable, as events aren't necessarily received in order.
-ENV{DM_NR_VALID_PATHS}=="0", ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
+ENV{DM_NR_VALID_PATHS}=="0", ENV{MPATH_DEVICE_READY}="0", GOTO="check_mpath_unchanged"
# Don't run multipath -U during "coldplug" after switching root,
# because paths are just being added to the udev db.
@@ -53,7 +53,8 @@ ACTION=="add", ENV{.MPATH_DEVICE_READY_OLD}=="1", GOTO="paths_ok"
# Check the map state directly with multipath -U.
# This doesn't attempt I/O on the device.
PROGRAM=="@BINDIR@/multipath -U -v1 %k", GOTO="paths_ok"
-ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
+ENV{MPATH_DEVICE_READY}="0", GOTO="check_mpath_unchanged"
+
LABEL="paths_ok"
# For PATH_FAILED events, keep the existing value of MPATH_DEVICE_READY.
@@ -61,7 +62,7 @@ LABEL="paths_ok"
# table reload where there are active paths. Mark the device ready.
ENV{DM_ACTION}!="PATH_FAILED", ENV{MPATH_DEVICE_READY}="1"
-LABEL="mpath_action"
+LABEL="check_mpath_unchanged"
# A previous coldplug event occurred while the device was suspended.
# Activation might have been partially skipped. Activate the device now,
@@ -70,7 +71,7 @@ IMPORT{db}="DM_COLDPLUG_SUSPENDED"
ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \
ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \
PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \
- GOTO="force_activation"
+ GOTO="check_mpath_ready"
# DM_SUBSYSTEM_UDEV_FLAG0 is the "RELOAD" flag for multipath subsystem.
# Drop the DM_ACTIVATION flag here as mpath reloads tables if any of its
@@ -86,7 +87,7 @@ ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", \
ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", \
ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1"
-LABEL="force_activation"
+LABEL="check_mpath_ready"
ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
# Do not initiate scanning if no path is available,
@@ -100,18 +101,17 @@ ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
ENV{DM_UDEV_RULES_VSN}!="3",
ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
ENV{.DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
-GOTO="dont_activate"
+GOTO="scan_import"
LABEL="mpath_is_ready"
# If the device comes back online, set DM_ACTIVATION so that
# upper layers do a rescan. If the device is currently suspended,
# we have to postpone the activation until the next event.
-ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
-ENV{.DM_SUSPENDED}=="1", GOTO="dont_activate"
+ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="scan_import"
+ENV{.DM_SUSPENDED}=="1", GOTO="scan_import"
ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
-LABEL="dont_activate"
# The code to check multipath state ends here. We need to set
# properties and symlinks regardless whether the map is usable or
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
` (9 preceding siblings ...)
2024-03-24 21:13 ` [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive Martin Wilck
@ 2024-03-24 21:13 ` Martin Wilck
2024-03-27 0:07 ` Benjamin Marzinski
10 siblings, 1 reply; 28+ messages in thread
From: Martin Wilck @ 2024-03-24 21:13 UTC (permalink / raw
To: Christophe Varoqui, Benjamin Marzinski
Cc: dm-devel, Peter Rajnoha, Martin Wilck
DM_SUSPENDED=1 implies DM_UDEV_DISABLE_OTHER_RULES_FLAG=1, no
need to check it again. The DM_NOSCAN check needs to remain in
order to keep compatibility with dm rules v2.
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
kpartx/kpartx.rules | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
index 1969dee..8dd3369 100644
--- a/kpartx/kpartx.rules
+++ b/kpartx/kpartx.rules
@@ -27,7 +27,7 @@ ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="mpath_kpartx_end"
ENV{MPATH_UNCHANGED}=="1", GOTO="mpath_kpartx_end"
# Don't run kpartx now if we know it will fail or hang.
-ENV{DM_SUSPENDED}=="1", GOTO="mpath_kpartx_end"
+# This is required for device mapper rules v2 compatibility.
ENV{DM_NOSCAN}=="1", GOTO="mpath_kpartx_end"
# Run kpartx
--
2.43.2
^ permalink raw reply related [flat|nested] 28+ messages in thread
* Re: [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices
2024-03-24 21:12 ` [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices Martin Wilck
@ 2024-03-26 21:52 ` Benjamin Marzinski
2024-04-04 14:06 ` Martin Wilck
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 21:52 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:51PM +0100, Martin Wilck wrote:
> MPATH_DEVICE_READY is an internal property, not to be consumed by
> later rules. It means that there are no usable paths. Being suspended
> is a different, independent property of a map.
>
> Don't set MPATH_DEVICE_READY=0 for suspended devices.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index d9585ab..3f23108 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -100,7 +100,7 @@ ENV{MPATH_DEVICE_READY}=="0", \
> # we have to postpone the activation until the next event.
> ENV{MPATH_DEVICE_READY}=="0", GOTO="dont_activate"
> ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
> -ENV{DM_SUSPENDED}=="1", ENV{MPATH_DEVICE_READY}="0", GOTO="dont_activate"
> +ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
I need to clarify this one. I thought that the point of setting
ENV{MPATH_DEVICE_READY} to "0" here was that
ENV{.MPATH_DEVICE_READY_OLD} must be "0" and ENV{MPATH_DEVICE_READY}
must be "1" here, so we want to set DM_ACTIVATE. Unfortunately, the
device is suspended, so we want to postpone the activation till the next
event, like the command above this codeblock says. To do that, we set
ENV{MPATH_DEVICE_READY} back to "0", so that on the next event,
ENV{.MPATH_DEVICE_READY_OLD} will be "0".
Is that not necessary for some reason? Perhaps because we're going to
get an actual DM generated event when we resume, so DM_ACTIVATE will
already be set when we get the next event where DM_SUSPENDED != "1".
If that's the case we should probably change the comment over this code
block.
-Ben
>
> ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
> LABEL="dont_activate"
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db
2024-03-24 21:12 ` [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db Martin Wilck
@ 2024-03-26 22:03 ` Benjamin Marzinski
2024-03-27 0:14 ` Benjamin Marzinski
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 22:03 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:52PM +0100, Martin Wilck wrote:
> DM_NOSCAN is our "output" flag for 13-dm-disk.rules, and it should
> be treated the same way as DM_UDEV_DISABLE_OTHER_RULES_FLAG, which
> isn't imported from the udev database. The state that we need to
> remember is MPATH_DEVICE_READY, which we've already imported above,
> and we will set the "output" flags accordingly in the "force_activation"
> code path further down.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 3f23108..11b5adf 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -18,9 +18,11 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1
>
> # If this uevent didn't come from dm, don't try to update the
> # device state
> +# Note that .MPATH_DEVICE_READY_OLD=="" here. Thus we won't activate the
> +# device below at mpath_is_ready, which is correct.
Do you mean "force_activation" instead of "mpath_is_ready"?
-Ben
> ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
> - IMPORT{db}="DM_NOSCAN", IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> - GOTO="scan_import"
> + IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> + GOTO="force_activation"
>
> ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
>
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION from udev db
2024-03-24 21:12 ` [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION " Martin Wilck
@ 2024-03-26 22:04 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 22:04 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:53PM +0100, Martin Wilck wrote:
> Use the same set of properties to import as 13-dm-disk.rules.
> ID_FS_VERSION isn't used in any udev rule I am aware of.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> multipath/11-dm-mpath.rules.in | 1 -
> 1 file changed, 1 deletion(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 11b5adf..5259b86 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -124,7 +124,6 @@ IMPORT{db}="ID_FS_TYPE"
> IMPORT{db}="ID_FS_USAGE"
> IMPORT{db}="ID_FS_UUID_ENC"
> IMPORT{db}="ID_FS_LABEL_ENC"
> -IMPORT{db}="ID_FS_VERSION"
> IMPORT{db}="ID_PART_ENTRY_NAME"
> IMPORT{db}="ID_PART_ENTRY_UUID"
> IMPORT{db}="ID_PART_ENTRY_SCHEME"
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update
2024-03-24 21:12 ` [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update Martin Wilck
@ 2024-03-26 22:17 ` Benjamin Marzinski
2024-04-04 13:45 ` Martin Wilck
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 22:17 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:54PM +0100, Martin Wilck wrote:
> With the late patches for 10-dm.rules, DM_UDEV_DISABLE_OTHER_RULES_FLAG isn't
> restored from the udev database any more, so we don't need to restore
> the flag to its original state before it is saved to the db.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 20 +++++++++++---------
> 1 file changed, 11 insertions(+), 9 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 5259b86..997e3e7 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -84,23 +84,25 @@ ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", \
>
> LABEL="force_activation"
>
> +ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
> # Do not initiate scanning if no path is available,
> # otherwise there would be a hang or IO error on access.
> # We'd like to avoid this, especially within udev processing.
> -ENV{MPATH_DEVICE_READY}=="0", ENV{DM_NOSCAN}="1"
> -
> -# Skip all foreign rules if no path is available.
> +# This is communicated to later rules in DM_NOSCAN.
> +# Likewise, skip all foreign rules if no path is available.
> # Use DM_UDEV_DISABLE_OTHER_RULES_FLAG to communicate this
> -# to upper layers. The original value will be restored in a late
> -# udev rule.
> -ENV{MPATH_DEVICE_READY}=="0", \
> - ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}", \
> - ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
> +# to upper layers. With dm rules < v3, save the original value here;
> +# it will be restored in a late udev rule.
> +ENV{DM_UDEV_RULES_VSN}!="3",
Can't we do something like:
ENV{DM_UDEV_RULES_VSN}=="1|2",
instead, that wouldn't break on future version number bumps.
> + ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
> +ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
> +GOTO="dont_activate"
> +
> +LABEL="mpath_is_ready"
>
> # If the device comes back online, set DM_ACTIVATION so that
> # upper layers do a rescan. If the device is currently suspended,
> # we have to postpone the activation until the next event.
> -ENV{MPATH_DEVICE_READY}=="0", GOTO="dont_activate"
> ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
> ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
>
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro 10-dm.rules update
2024-03-24 21:12 ` [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro " Martin Wilck
@ 2024-03-26 22:44 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 22:44 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:55PM +0100, Martin Wilck wrote:
> With late late patches for 10-dm.rules, DM_UDEV_DISABLE_OTHER_RULES_FLAG is
> never restored from the udev db. Thus we don't need to clear it here
> any more for coldplug events. Also, we must use .DM_SUSPENDED instead of
> DM_SUSPENDED as input flag with the v3 rule set (other occurences of
> DM_SUSPENDED will be replaced in a follow-up patch).
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 997e3e7..cf004fd 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -9,12 +9,17 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="1", \
> PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.warning \"Coldplug event for suspended device\"", \
> ENV{DM_COLDPLUG_SUSPENDED}="1", GOTO="scan_import"
>
> -# Coldplug event. DM_UDEV_DISABLE_OTHER_RULES_FLAG has been restored
> +# Coldplug event. Import previously set properties.
> +ACTION!="add", GOTO="mpath_coldplug_end"
> +ENV{DM_ACTIVATION}!="1", GOTO="mpath_coldplug_end"
> +ENV{DM_UDEV_RULES_VSN}=="3", ENV{.DM_SUSPENDED}!="1", GOTO="scan_import"
> +# With DM rules < v3, DM_UDEV_DISABLE_OTHER_RULES_FLAG has been restored
> # from DB in 10-dm.rules. If the device is not suspended, clear the flag.
> # This is safe for multipath where DM_UDEV_DISABLE_OTHER_RULES_FLAG is basically
> # equivalent to DM_SUSPENDED==1 || DISK_RO==1
> -ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
> +ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
Again, I'd prefer ENV{DM_UDEV_RULES_VSN}="1|2"
> ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="", GOTO="scan_import"
> +LABEL="mpath_coldplug_end"
>
> # If this uevent didn't come from dm, don't try to update the
> # device state
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules
2024-03-24 21:12 ` [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules Martin Wilck
@ 2024-03-26 22:58 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 22:58 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:56PM +0100, Martin Wilck wrote:
> With the late changes to 13-dm-disk.rules, we don't need to import any
> blkid-generated properties from the udev database, because they will
> be imported later.
>
> Except for ID_FS_TYPE, this actually holds since lvm2 commit 94f77a4 ("udev:
> import previous results of blkid when in suspended state"), included in lvm2
> 2.03.19, but we have no simple way to detect the version of the lvm2 rules.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index cf004fd..8f22954 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -124,6 +124,10 @@ LABEL="scan_import"
> # have never been properly set. Don't import them.
> ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", GOTO="import_end"
>
> +# DM rules v3 will import missing properties on 13-dm-disk.rules.
> +# No need to do it here.
> +ENV{DM_UDEV_RULES_VSN}=="3", GOTO="import_end"
Again, I'd rather
ENV{DM_UDEV_RULES_VSN}!="1|2"
> +
> # Don't import the properties from db if we will run blkid later.
> ENV{DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
>
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED
2024-03-24 21:12 ` [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED Martin Wilck
@ 2024-03-26 23:36 ` Benjamin Marzinski
2024-04-04 15:17 ` Martin Wilck
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-26 23:36 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:57PM +0100, Martin Wilck wrote:
> With the late changes to the device mapper rules, DM_SUSPENDED
> is not exported any more. Use .DM_SUSPENDED instead.
>
> Note that although 11-dm-mpath.rules is not a part of lvm2, it
> can be considered as part of the device-mapper layer (everything
> before 13-dm-disk.rules can), and is thus allowed to use
> .DM_SUSPENDED. In practice .DM_SUSPENDED is equivalent to
> DM_UDEV_DISABLE_OTHER_RULES_FLAG for multipath devices, but
> using .DM_SUSPENDED here makes the intention more obvious.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 8f22954..c4b5685 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -4,8 +4,11 @@ ENV{DM_UUID}!="mpath-?*", GOTO="mpath_end"
>
> IMPORT{db}="MPATH_DEVICE_READY"
>
> +# device-mapper rules v2 compatibility
> +ENV{.DM_SUSPENDED}!="?*", ENV{.DM_SUSPENDED}="$env{DM_SUSPENDED}"
> +
> # Coldplug event while device is suspended (e.g. during a reload)
> -ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="1", \
> +ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{.DM_SUSPENDED}=="1", \
> PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.warning \"Coldplug event for suspended device\"", \
> ENV{DM_COLDPLUG_SUSPENDED}="1", GOTO="scan_import"
>
> @@ -17,7 +20,7 @@ ENV{DM_UDEV_RULES_VSN}=="3", ENV{.DM_SUSPENDED}!="1", GOTO="scan_import"
> # from DB in 10-dm.rules. If the device is not suspended, clear the flag.
> # This is safe for multipath where DM_UDEV_DISABLE_OTHER_RULES_FLAG is basically
> # equivalent to DM_SUSPENDED==1 || DISK_RO==1
> -ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
> +ENV{DM_UDEV_RULES_VSN}!="3", ENV{.DM_SUSPENDED}!="1", ENV{DISK_RO}!="1", \
> ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="", GOTO="scan_import"
> LABEL="mpath_coldplug_end"
>
> @@ -68,7 +71,7 @@ LABEL="mpath_action"
> # Activation might have been partially skipped. Activate the device now,
> # i.e. disable the MPATH_UNCHANGED logic and set DM_ACTIVATION=1.
> IMPORT{db}="DM_COLDPLUG_SUSPENDED"
> -ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{DM_SUSPENDED}!="1", \
> +ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \
When I reviewed patch 0001, I wondered if it was o.k. to not force
DM_ACTIVATION to "1" if a device had previously been suspended when
MPATH_DEVICE_READY switched to "1". I thought it might it might be o.k.
because there would be another event when the device resumed, which
would have DM_ACTIVATION set to "1". But if that's the case then we
shouldn't need to worry about a device being suspended when a coldplug
happens. We will get an event when the device resumes, and there's not
much we can do until it gets resumed at any rate. So either patch 0001
is wrong or we can remove this DM_COLDPLUG_SUSPENDED forced activation
code or I'm missing some important difference between the two.
-Ben
> ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \
> PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \
> GOTO="force_activation"
> @@ -109,7 +112,7 @@ LABEL="mpath_is_ready"
> # upper layers do a rescan. If the device is currently suspended,
> # we have to postpone the activation until the next event.
> ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
> -ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
> +ENV{.DM_SUSPENDED}=="1", GOTO="dont_activate"
>
> ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
> LABEL="dont_activate"
> @@ -143,7 +146,7 @@ IMPORT{db}="ID_PART_GPT_AUTO_ROOT"
> LABEL="import_end"
>
> # Reset previous DM_COLDPLUG_SUSPENDED if activation happens now
> -ENV{DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
> +ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
>
> # Multipath maps should take precedence over their members.
> ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN
2024-03-24 21:12 ` [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN Martin Wilck
@ 2024-03-27 0:03 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-27 0:03 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:58PM +0100, Martin Wilck wrote:
> We don't need to restore DM_NOSCAN from the db anymore, so we can rename
> it to .DM_NOSCAN, making it a temporary property.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
> multipath/11-dm-mpath.rules.in | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index c4b5685..8833e08 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -96,14 +96,14 @@ ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
> # Do not initiate scanning if no path is available,
> # otherwise there would be a hang or IO error on access.
> # We'd like to avoid this, especially within udev processing.
> -# This is communicated to later rules in DM_NOSCAN.
> +# This is communicated to later rules in .DM_NOSCAN.
> # Likewise, skip all foreign rules if no path is available.
> # Use DM_UDEV_DISABLE_OTHER_RULES_FLAG to communicate this
> # to upper layers. With dm rules < v3, save the original value here;
> # it will be restored in a late udev rule.
> ENV{DM_UDEV_RULES_VSN}!="3",
> ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
> -ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
> +ENV{.DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
> GOTO="dont_activate"
>
> LABEL="mpath_is_ready"
> @@ -132,7 +132,7 @@ ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}!="1", GOTO="import_end"
> ENV{DM_UDEV_RULES_VSN}=="3", GOTO="import_end"
>
> # Don't import the properties from db if we will run blkid later.
> -ENV{DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
> +ENV{.DM_NOSCAN}!="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}!="1", GOTO="import_end"
>
> IMPORT{db}="ID_FS_TYPE"
> IMPORT{db}="ID_FS_USAGE"
> @@ -148,6 +148,9 @@ LABEL="import_end"
> # Reset previous DM_COLDPLUG_SUSPENDED if activation happens now
> ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""
>
> +# device-mapper rules v2 compatibility for 13-dm-disk.rules
> +ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_NOSCAN}="$env{.DM_NOSCAN}"
> +
Again, I'd prefer
ENV{DM_UDEV_RULES_VSN}="1|2"
> # Multipath maps should take precedence over their members.
> ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
>
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case
2024-03-24 21:12 ` [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case Martin Wilck
@ 2024-03-27 0:04 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-27 0:04 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:12:59PM +0100, Martin Wilck wrote:
> This combination of a GOTO and a simple rule can be combined
> into a single rule.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> multipath/11-dm-mpath.rules.in | 12 ++++--------
> 1 file changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 8833e08..2285853 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -56,14 +56,10 @@ PROGRAM=="@BINDIR@/multipath -U -v1 %k", GOTO="paths_ok"
> ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
> LABEL="paths_ok"
>
> -# Don't mark a device ready on a PATH_FAILED event. even if
> -# DM_NR_VALID_PATHS is greater than 0. Just keep the existing
> -# value
> -ENV{DM_ACTION}=="PATH_FAILED", GOTO="mpath_action"
> -
> -# This event is either a PATH_REINSTATED or a table reload where
> -# there are active paths. Mark the device ready
> -ENV{MPATH_DEVICE_READY}="1"
> +# For PATH_FAILED events, keep the existing value of MPATH_DEVICE_READY.
> +# If it's not PATH_FAILED, this event is either a PATH_REINSTATED or a
> +# table reload where there are active paths. Mark the device ready.
> +ENV{DM_ACTION}!="PATH_FAILED", ENV{MPATH_DEVICE_READY}="1"
>
> LABEL="mpath_action"
>
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive
2024-03-24 21:13 ` [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive Martin Wilck
@ 2024-03-27 0:06 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-27 0:06 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:13:00PM +0100, Martin Wilck wrote:
> The labels "dont_activate" and "scan_import" denote the same code line.
> Remove "dont_activate". Improve two misleading label names.
>
> Substitutions:
> dont_activate -> scan_import
> force_activation -> check_mpath_ready
> mpath_action -> check_mpath_unchanged
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> multipath/11-dm-mpath.rules.in | 22 +++++++++++-----------
> 1 file changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> index 2285853..526cca8 100644
> --- a/multipath/11-dm-mpath.rules.in
> +++ b/multipath/11-dm-mpath.rules.in
> @@ -30,7 +30,7 @@ LABEL="mpath_coldplug_end"
> # device below at mpath_is_ready, which is correct.
> ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
> IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> - GOTO="force_activation"
> + GOTO="check_mpath_ready"
>
> ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
>
> @@ -38,13 +38,13 @@ ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
> # table with no active devices. If this happens, mark the
> # device not ready
> ENV{DM_SUBSYSTEM_UDEV_FLAG2}=="1", ENV{MPATH_DEVICE_READY}="0", \
> - GOTO="mpath_action"
> + GOTO="check_mpath_unchanged"
>
> # If the last path has failed mark the device not ready
> # Note that DM_NR_VALID_PATHS is only set for PATH_FAILED|PATH_REINSTATED
> # events.
> # This may not be reliable, as events aren't necessarily received in order.
> -ENV{DM_NR_VALID_PATHS}=="0", ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
> +ENV{DM_NR_VALID_PATHS}=="0", ENV{MPATH_DEVICE_READY}="0", GOTO="check_mpath_unchanged"
>
> # Don't run multipath -U during "coldplug" after switching root,
> # because paths are just being added to the udev db.
> @@ -53,7 +53,8 @@ ACTION=="add", ENV{.MPATH_DEVICE_READY_OLD}=="1", GOTO="paths_ok"
> # Check the map state directly with multipath -U.
> # This doesn't attempt I/O on the device.
> PROGRAM=="@BINDIR@/multipath -U -v1 %k", GOTO="paths_ok"
> -ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
> +ENV{MPATH_DEVICE_READY}="0", GOTO="check_mpath_unchanged"
> +
> LABEL="paths_ok"
>
> # For PATH_FAILED events, keep the existing value of MPATH_DEVICE_READY.
> @@ -61,7 +62,7 @@ LABEL="paths_ok"
> # table reload where there are active paths. Mark the device ready.
> ENV{DM_ACTION}!="PATH_FAILED", ENV{MPATH_DEVICE_READY}="1"
>
> -LABEL="mpath_action"
> +LABEL="check_mpath_unchanged"
>
> # A previous coldplug event occurred while the device was suspended.
> # Activation might have been partially skipped. Activate the device now,
> @@ -70,7 +71,7 @@ IMPORT{db}="DM_COLDPLUG_SUSPENDED"
> ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \
> ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0", \
> PROGRAM="/bin/logger -t 11-dm-mpath.rules -p daemon.notice \"Forcing activation of previously suspended device\"", \
> - GOTO="force_activation"
> + GOTO="check_mpath_ready"
>
> # DM_SUBSYSTEM_UDEV_FLAG0 is the "RELOAD" flag for multipath subsystem.
> # Drop the DM_ACTIVATION flag here as mpath reloads tables if any of its
> @@ -86,7 +87,7 @@ ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", \
> ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", \
> ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1"
>
> -LABEL="force_activation"
> +LABEL="check_mpath_ready"
>
> ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
> # Do not initiate scanning if no path is available,
> @@ -100,18 +101,17 @@ ENV{MPATH_DEVICE_READY}!="0", GOTO="mpath_is_ready"
> ENV{DM_UDEV_RULES_VSN}!="3",
> ENV{.MPATH_SAVE_DISABLE_OTHER_RULES_FLAG}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
> ENV{.DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
> -GOTO="dont_activate"
> +GOTO="scan_import"
>
> LABEL="mpath_is_ready"
>
> # If the device comes back online, set DM_ACTIVATION so that
> # upper layers do a rescan. If the device is currently suspended,
> # we have to postpone the activation until the next event.
> -ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
> -ENV{.DM_SUSPENDED}=="1", GOTO="dont_activate"
> +ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="scan_import"
> +ENV{.DM_SUSPENDED}=="1", GOTO="scan_import"
>
> ENV{DM_ACTIVATION}="1", ENV{MPATH_UNCHANGED}="0"
> -LABEL="dont_activate"
>
> # The code to check multipath state ends here. We need to set
> # properties and symlinks regardless whether the map is usable or
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED
2024-03-24 21:13 ` [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED Martin Wilck
@ 2024-03-27 0:07 ` Benjamin Marzinski
0 siblings, 0 replies; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-27 0:07 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Sun, Mar 24, 2024 at 10:13:01PM +0100, Martin Wilck wrote:
> DM_SUSPENDED=1 implies DM_UDEV_DISABLE_OTHER_RULES_FLAG=1, no
> need to check it again. The DM_NOSCAN check needs to remain in
> order to keep compatibility with dm rules v2.
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
> kpartx/kpartx.rules | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kpartx/kpartx.rules b/kpartx/kpartx.rules
> index 1969dee..8dd3369 100644
> --- a/kpartx/kpartx.rules
> +++ b/kpartx/kpartx.rules
> @@ -27,7 +27,7 @@ ENV{DM_SUBSYSTEM_UDEV_FLAG1}=="1", GOTO="mpath_kpartx_end"
> ENV{MPATH_UNCHANGED}=="1", GOTO="mpath_kpartx_end"
>
> # Don't run kpartx now if we know it will fail or hang.
> -ENV{DM_SUSPENDED}=="1", GOTO="mpath_kpartx_end"
> +# This is required for device mapper rules v2 compatibility.
> ENV{DM_NOSCAN}=="1", GOTO="mpath_kpartx_end"
>
> # Run kpartx
> --
> 2.43.2
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db
2024-03-26 22:03 ` Benjamin Marzinski
@ 2024-03-27 0:14 ` Benjamin Marzinski
2024-04-04 13:49 ` Martin Wilck
0 siblings, 1 reply; 28+ messages in thread
From: Benjamin Marzinski @ 2024-03-27 0:14 UTC (permalink / raw
To: Martin Wilck; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha, Martin Wilck
On Tue, Mar 26, 2024 at 06:03:42PM -0400, Benjamin Marzinski wrote:
> On Sun, Mar 24, 2024 at 10:12:52PM +0100, Martin Wilck wrote:
> > DM_NOSCAN is our "output" flag for 13-dm-disk.rules, and it should
> > be treated the same way as DM_UDEV_DISABLE_OTHER_RULES_FLAG, which
> > isn't imported from the udev database. The state that we need to
> > remember is MPATH_DEVICE_READY, which we've already imported above,
> > and we will set the "output" flags accordingly in the "force_activation"
> > code path further down.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> > multipath/11-dm-mpath.rules.in | 6 ++++--
> > 1 file changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
> > index 3f23108..11b5adf 100644
> > --- a/multipath/11-dm-mpath.rules.in
> > +++ b/multipath/11-dm-mpath.rules.in
> > @@ -18,9 +18,11 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1
> >
> > # If this uevent didn't come from dm, don't try to update the
> > # device state
> > +# Note that .MPATH_DEVICE_READY_OLD=="" here. Thus we won't activate the
> > +# device below at mpath_is_ready, which is correct.
>
> Do you mean "force_activation" instead of "mpath_is_ready"?
I see that this label is added two commits later. I don't have a problem
with a comment being confusing for two commits, so if you don't feel
like changing it,
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
>
> -Ben
>
> > ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
> > - IMPORT{db}="DM_NOSCAN", IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> > - GOTO="scan_import"
> > + IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> > + GOTO="force_activation"
> >
> > ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
> >
> > --
> > 2.43.2
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update
2024-03-26 22:17 ` Benjamin Marzinski
@ 2024-04-04 13:45 ` Martin Wilck
0 siblings, 0 replies; 28+ messages in thread
From: Martin Wilck @ 2024-04-04 13:45 UTC (permalink / raw
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha
On Tue, 2024-03-26 at 18:17 -0400, Benjamin Marzinski wrote:
> On Sun, Mar 24, 2024 at 10:12:54PM +0100, Martin Wilck wrote:
> >
> > +ENV{DM_UDEV_RULES_VSN}!="3",
>
> Can't we do something like:
>
> ENV{DM_UDEV_RULES_VSN}=="1|2",
>
> instead, that wouldn't break on future version number bumps.
>
Excellent idea, thanks!
Martin
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db
2024-03-27 0:14 ` Benjamin Marzinski
@ 2024-04-04 13:49 ` Martin Wilck
0 siblings, 0 replies; 28+ messages in thread
From: Martin Wilck @ 2024-04-04 13:49 UTC (permalink / raw
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha
On Tue, 2024-03-26 at 20:14 -0400, Benjamin Marzinski wrote:
> On Tue, Mar 26, 2024 at 06:03:42PM -0400, Benjamin Marzinski wrote:
> > On Sun, Mar 24, 2024 at 10:12:52PM +0100, Martin Wilck wrote:
> > > DM_NOSCAN is our "output" flag for 13-dm-disk.rules, and it
> > > should
> > > be treated the same way as DM_UDEV_DISABLE_OTHER_RULES_FLAG,
> > > which
> > > isn't imported from the udev database. The state that we need to
> > > remember is MPATH_DEVICE_READY, which we've already imported
> > > above,
> > > and we will set the "output" flags accordingly in the
> > > "force_activation"
> > > code path further down.
> > >
> > > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > > ---
> > > multipath/11-dm-mpath.rules.in | 6 ++++--
> > > 1 file changed, 4 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-
> > > mpath.rules.in
> > > index 3f23108..11b5adf 100644
> > > --- a/multipath/11-dm-mpath.rules.in
> > > +++ b/multipath/11-dm-mpath.rules.in
> > > @@ -18,9 +18,11 @@ ACTION=="add", ENV{DM_ACTIVATION}=="1",
> > > ENV{DM_SUSPENDED}!="1", ENV{DISK_RO}!="1
> > >
> > > # If this uevent didn't come from dm, don't try to update the
> > > # device state
> > > +# Note that .MPATH_DEVICE_READY_OLD=="" here. Thus we won't
> > > activate the
> > > +# device below at mpath_is_ready, which is correct.
> >
> > Do you mean "force_activation" instead of "mpath_is_ready"?
>
> I see that this label is added two commits later. I don't have a
> problem
> with a comment being confusing for two commits, so if you don't feel
> like changing it,
Good catch, thanks. A few too many rebases, I guess. I will fix it.
Martin
>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
>
> >
> > -Ben
> >
> > > ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", \
> > > - IMPORT{db}="DM_NOSCAN",
> > > IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> > > - GOTO="scan_import"
> > > + IMPORT{db}="DM_COLDPLUG_SUSPENDED", \
> > > + GOTO="force_activation"
> > >
> > > ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
> > >
> > > --
> > > 2.43.2
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices
2024-03-26 21:52 ` Benjamin Marzinski
@ 2024-04-04 14:06 ` Martin Wilck
0 siblings, 0 replies; 28+ messages in thread
From: Martin Wilck @ 2024-04-04 14:06 UTC (permalink / raw
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha
On Tue, 2024-03-26 at 17:52 -0400, Benjamin Marzinski wrote:
> On Sun, Mar 24, 2024 at 10:12:51PM +0100, Martin Wilck wrote:
> > MPATH_DEVICE_READY is an internal property, not to be consumed by
> > later rules. It means that there are no usable paths. Being
> > suspended
> > is a different, independent property of a map.
> >
> > Don't set MPATH_DEVICE_READY=0 for suspended devices.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> > multipath/11-dm-mpath.rules.in | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-
> > mpath.rules.in
> > index d9585ab..3f23108 100644
> > --- a/multipath/11-dm-mpath.rules.in
> > +++ b/multipath/11-dm-mpath.rules.in
> > @@ -100,7 +100,7 @@ ENV{MPATH_DEVICE_READY}=="0", \
> > # we have to postpone the activation until the next event.
> > ENV{MPATH_DEVICE_READY}=="0", GOTO="dont_activate"
> > ENV{.MPATH_DEVICE_READY_OLD}!="0", GOTO="dont_activate"
> > -ENV{DM_SUSPENDED}=="1", ENV{MPATH_DEVICE_READY}="0",
> > GOTO="dont_activate"
> > +ENV{DM_SUSPENDED}=="1", GOTO="dont_activate"
>
> I need to clarify this one. I thought that the point of setting
> ENV{MPATH_DEVICE_READY} to "0" here was that
> ENV{.MPATH_DEVICE_READY_OLD} must be "0" and ENV{MPATH_DEVICE_READY}
> must be "1" here, so we want to set DM_ACTIVATE. Unfortunately, the
> device is suspended, so we want to postpone the activation till the
> next
> event, like the command above this codeblock says. To do that, we
> set
> ENV{MPATH_DEVICE_READY} back to "0", so that on the next event,
> ENV{.MPATH_DEVICE_READY_OLD} will be "0".
>
> Is that not necessary for some reason? Perhaps because we're going to
> get an actual DM generated event when we resume, so DM_ACTIVATE will
> already be set when we get the next event where DM_SUSPENDED != "1".
> If that's the case we should probably change the comment over this
> block.
You are right. We must set ENV{MPATH_DEVICE_READY}="0" here to
remember the state until the next event is received.
Thanks for catching this.
Martin
^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED
2024-03-26 23:36 ` Benjamin Marzinski
@ 2024-04-04 15:17 ` Martin Wilck
0 siblings, 0 replies; 28+ messages in thread
From: Martin Wilck @ 2024-04-04 15:17 UTC (permalink / raw
To: Benjamin Marzinski; +Cc: Christophe Varoqui, dm-devel, Peter Rajnoha
On Tue, 2024-03-26 at 19:36 -0400, Benjamin Marzinski wrote:
> On Sun, Mar 24, 2024 at 10:12:57PM +0100, Martin Wilck wrote:
> > With the late changes to the device mapper rules, DM_SUSPENDED
> > is not exported any more. Use .DM_SUSPENDED instead.
> >
> > Note that although 11-dm-mpath.rules is not a part of lvm2, it
> > can be considered as part of the device-mapper layer (everything
> > before 13-dm-disk.rules can), and is thus allowed to use
> > .DM_SUSPENDED. In practice .DM_SUSPENDED is equivalent to
> > DM_UDEV_DISABLE_OTHER_RULES_FLAG for multipath devices, but
> > using .DM_SUSPENDED here makes the intention more obvious.
> >
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> > multipath/11-dm-mpath.rules.in | 13 ++++++++-----
> > 1 file changed, 8 insertions(+), 5 deletions(-)
> >
> > diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-
> > mpath.rules.in
> > index 8f22954..c4b5685 100644
> > --- a/multipath/11-dm-mpath.rules.in
> > +++ b/multipath/11-dm-mpath.rules.in
> > @@ -4,8 +4,11 @@ ENV{DM_UUID}!="mpath-?*", GOTO="mpath_end"
> >
> > IMPORT{db}="MPATH_DEVICE_READY"
> >
> > +# device-mapper rules v2 compatibility
> > +ENV{.DM_SUSPENDED}!="?*", ENV{.DM_SUSPENDED}="$env{DM_SUSPENDED}"
> > +
> > # Coldplug event while device is suspended (e.g. during a reload)
> > -ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{DM_SUSPENDED}=="1", \
> > +ACTION=="add", ENV{DM_ACTIVATION}=="1", ENV{.DM_SUSPENDED}=="1", \
> > PROGRAM="/bin/logger -t 11-dm-mpath.rules -p
> > daemon.warning \"Coldplug event for suspended device\"", \
> > ENV{DM_COLDPLUG_SUSPENDED}="1", GOTO="scan_import"
> >
> > @@ -17,7 +20,7 @@ ENV{DM_UDEV_RULES_VSN}=="3",
> > ENV{.DM_SUSPENDED}!="1", GOTO="scan_import"
> > # from DB in 10-dm.rules. If the device is not suspended, clear
> > the flag.
> > # This is safe for multipath where
> > DM_UDEV_DISABLE_OTHER_RULES_FLAG is basically
> > # equivalent to DM_SUSPENDED==1 || DISK_RO==1
> > -ENV{DM_UDEV_RULES_VSN}!="3", ENV{DM_SUSPENDED}!="1",
> > ENV{DISK_RO}!="1", \
> > +ENV{DM_UDEV_RULES_VSN}!="3", ENV{.DM_SUSPENDED}!="1",
> > ENV{DISK_RO}!="1", \
> > ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="",
> > GOTO="scan_import"
> > LABEL="mpath_coldplug_end"
> >
> > @@ -68,7 +71,7 @@ LABEL="mpath_action"
> > # Activation might have been partially skipped. Activate the
> > device now,
> > # i.e. disable the MPATH_UNCHANGED logic and set DM_ACTIVATION=1.
> > IMPORT{db}="DM_COLDPLUG_SUSPENDED"
> > -ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{DM_SUSPENDED}!="1", \
> > +ENV{DM_COLDPLUG_SUSPENDED}=="1", ENV{.DM_SUSPENDED}!="1", \
>
> When I reviewed patch 0001, I wondered if it was o.k. to not force
> DM_ACTIVATION to "1" if a device had previously been suspended when
> MPATH_DEVICE_READY switched to "1". I thought it might it might be
> o.k.
> because there would be another event when the device resumed, which
> would have DM_ACTIVATION set to "1". But if that's the case then we
> shouldn't need to worry about a device being suspended when a
> coldplug
> happens. We will get an event when the device resumes, and there's
> not
> much we can do until it gets resumed at any rate. So either patch
> 0001
> is wrong or we can remove this DM_COLDPLUG_SUSPENDED forced
> activation
> code or I'm missing some important difference between the two.
I tried to explain this in 02bbc17 ("11-dm-mpath.rules: handle reloads
during coldplug events"). But I just needed to recall the logic again
myself, it's subtle.
We will indeed get another change event when the device is resumed.
But because of our MPATH_UNCHANGED logic, we may see that the
the properties of the device haven't changed, and unset DM_ACTIVATION.
Thus follow-up rules won't get executed, neither in coldplug event
itself (device suspended) nor in the follow-up change event (device
unchanged). I hope this makes sense.
The problem that you pointed out for 0001 for this series is similar,
but it handles a different case where the device switched from 0 to one
usable path while the device was suspended. Patch 0001 of this series
is really broken, and it breaks a patch that I'd just created a month
ago, which is truly embarrassing. udev rules are confusing :-/
Regards
Martin
^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2024-04-04 15:17 UTC | newest]
Thread overview: 28+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-24 21:12 [PATCH 00/11] multipath udev rules changes for dm rules V3 Martin Wilck
2024-03-24 21:12 ` [PATCH 01/11] 11-dm-mpath.rules: don't set MPATH_DEVICE_READY=0 for suspended devices Martin Wilck
2024-03-26 21:52 ` Benjamin Marzinski
2024-04-04 14:06 ` Martin Wilck
2024-03-24 21:12 ` [PATCH 02/11] 11-dm-mpath.rules: don't import DM_NOSCAN from udev db Martin Wilck
2024-03-26 22:03 ` Benjamin Marzinski
2024-03-27 0:14 ` Benjamin Marzinski
2024-04-04 13:49 ` Martin Wilck
2024-03-24 21:12 ` [PATCH 03/11] 11-dm-mpath.rules: don't import ID_FS_VERSION " Martin Wilck
2024-03-26 22:04 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 04/11] 11-dm-mpath.rules: adapt MPATH_DEVICE_READY=0 logic to 10-dm.rules update Martin Wilck
2024-03-26 22:17 ` Benjamin Marzinski
2024-04-04 13:45 ` Martin Wilck
2024-03-24 21:12 ` [PATCH 05/11] 11-dm-mpath.rules: adapt coldplug event handling ro " Martin Wilck
2024-03-26 22:44 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 06/11] 11-dm-mpath.rules: don't import properties with new 13-dm-disk.rules Martin Wilck
2024-03-26 22:58 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 07/11] 11-dm-mpath.rules: replace DM_SUSPENDED by .DM_SUSPENDED Martin Wilck
2024-03-26 23:36 ` Benjamin Marzinski
2024-04-04 15:17 ` Martin Wilck
2024-03-24 21:12 ` [PATCH 08/11] 11-dm-mpath.rules: replace DM_NOSCAN by .DM_NOSCAN Martin Wilck
2024-03-27 0:03 ` Benjamin Marzinski
2024-03-24 21:12 ` [PATCH 09/11] 11-dm-mpath.rules: simplify PATH_FAILED case Martin Wilck
2024-03-27 0:04 ` Benjamin Marzinski
2024-03-24 21:13 ` [PATCH 10/11] 11-dm-mpath.rules: make label names more intuitive Martin Wilck
2024-03-27 0:06 ` Benjamin Marzinski
2024-03-24 21:13 ` [PATCH 11/11] kpartx.rules: ignore DM_SUSPENDED Martin Wilck
2024-03-27 0:07 ` Benjamin Marzinski
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.