DM-Devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/7] device mapper udev rules rework
@ 2024-03-05 12:05 Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 1/7] 13-dm-disk.rules: import ID_FS_TYPE Martin Wilck
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

All,

here is a rewrite of the device mapper udev rules, as follow up
to the previous thread "About DM_UDEV_DISABLE_OTHER_RULES_FLAG and
DM_NOSCAN" [1]. While I have taken care to minimize the impact on
other udev rule sets, some changes to both the multipath-tools and
systemd rules will be necessary. Patches for these are in preparation.

In short, the effect of this patch set is as follows:

- As before, DM_UDEV_DISABLE_OTHER_RULES_FLAG is used as general flag to
  indicate to follow-up rules that they should avoid probing the device, and
  should import device properties from the udev db if necessary.  The flag
  does not imply that existing device stacks must be destructed.  -
- DM_UDEV_DISABLE_OTHER_RULES_FLAG represents the combination of multiple
  conditions that may make it impossible to probe the device just now.  As
  such, it isn't restored from the udev db, but determined anew for every
  uevent. For saving the DM_COOKIE variable of the same name in the udev db,
  a new variable DM_COOKIE_DISABLE_OTHER_RULES_FLAG is introduced.
- The properties DM_SUSPENDED and DM_NOSCAN, which have always been considered
  internal to device-mapper and its subsystems, are renamed to .DM_SUSPENDED
  and .DM_NOSCAN, respectively. Thus they aren't saved in the udev db any
  more, and it becomes more obvious that they are not supposed to be used
  by non-dm udev rules.
- ID_FS_TYPE is imported from the db for "spurious" events.
- Properties are imported for DISK_RO="?" events, too.

Changes from v1 ("RFC") patch set:

- Instead of removing the DISK_RO clause entirely, just moved it further down
  (after the property import), and apply it for DISK_RO==0, too.
- Updated comments about device properties at the top of 10-dm.rules.

[1] https://lore.kernel.org/linux-lvm/9d50edb0-baa4-4a01-a2f0-136dfdb79937@redhat.com/T/#t

Martin Wilck (7):
  13-dm-disk.rules: import ID_FS_TYPE
  10-dm.rules: test DISK_RO after importing properties
  10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db
  11-dm-lvm.rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from
    db
  dm udev rules: don't export and save DM_SUSPENDED
  dm udev rules: don't export and save DM_NOSCAN
  10-dm.rules: bump DM_UDEV_RULES_VSN to 3

 udev/10-dm.rules.in          | 43 +++++++++++++++++++++++++++---------
 udev/11-dm-lvm.rules.in      | 13 +++++------
 udev/12-dm-permissions.rules |  2 +-
 udev/13-dm-disk.rules.in     |  9 ++++----
 4 files changed, 43 insertions(+), 24 deletions(-)

-- 
2.43.2


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

* [PATCH v2 1/7] 13-dm-disk.rules: import ID_FS_TYPE
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 2/7] 10-dm.rules: test DISK_RO after importing properties Martin Wilck
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

ID_FS_TYPE is the most important udev property for most follow-up
rules. It must be imported from the udev db if blkid can't be run.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/13-dm-disk.rules.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/udev/13-dm-disk.rules.in b/udev/13-dm-disk.rules.in
index dca00bc..eaad972 100644
--- a/udev/13-dm-disk.rules.in
+++ b/udev/13-dm-disk.rules.in
@@ -26,6 +26,7 @@ ENV{DM_NOSCAN}=="1", GOTO="dm_watch"
 GOTO="dm_link"
 
 LABEL="dm_import"
+IMPORT{db}="ID_FS_TYPE"
 IMPORT{db}="ID_FS_USAGE"
 IMPORT{db}="ID_FS_UUID_ENC"
 IMPORT{db}="ID_FS_LABEL_ENC"
-- 
2.43.2


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

* [PATCH v2 2/7] 10-dm.rules: test DISK_RO after importing properties
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 1/7] 13-dm-disk.rules: import ID_FS_TYPE Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 3/7] 10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db Martin Wilck
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

DISK_RO is set in the environment of a block-device uevent if and only if
the read-only (ro) attribute of the device just changed (the kernel
function set_disk_ro() was called). It is not synoymous with the "ro" sysfs
attribute; the device could very well be write-protected if DISK_RO is not
set. Device mapper-level probing is possible for DISK_RO events, but it makes
little sense, because the device propreties haven't changed as far as dm is
concerned. But we should import possible previously set device properties
to avoid confusing follow-up rules. We should do this for both DISK_RO=1
and DISK_RO=0 events.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/10-dm.rules.in | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
index 4ffd3e2..f83fbec 100644
--- a/udev/10-dm.rules.in
+++ b/udev/10-dm.rules.in
@@ -50,9 +50,6 @@ ACTION!="add|change", GOTO="dm_end"
 # kernels >= 2.6.31 only. Cookie is not decoded for remove event.
 ENV{DM_COOKIE}=="?*", IMPORT{program}="(DM_EXEC)/dmsetup udevflags $env{DM_COOKIE}"
 
-# Rule out easy-to-detect inappropriate events first.
-ENV{DISK_RO}=="1", GOTO="dm_disable"
-
 # There is no cookie set nor any flags encoded in events not originating
 # in libdevmapper so we need to detect this and try to behave correctly.
 # For such spurious events, regenerate all flags from current udev database content
@@ -69,6 +66,10 @@ IMPORT{db}="DM_UDEV_FLAG7"
 IMPORT{db}="DM_UDEV_RULES_VSN"
 LABEL="dm_flags_done"
 
+# If DISK_RO is set, it's an uevent that changes the ro attribute of the device.
+# The event should be ignored as far as dm is concerned.
+ENV{DISK_RO}=="0|1", GOTO="dm_disable"
+
 # Normally, we operate on "change" events. But when coldplugging, there's an
 # "add" event present. We have to recognize this and do our actions in this
 # particular situation, too. Also, we don't want the nodes to be created
-- 
2.43.2


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

* [PATCH v2 3/7] 10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 1/7] 13-dm-disk.rules: import ID_FS_TYPE Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 2/7] 10-dm.rules: test DISK_RO after importing properties Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 4/7] 11-dm-lvm.rules: " Martin Wilck
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

We use DM_UDEV_DISABLE_OTHER_RULES_FLAG to tell upper non-DM layers
to keep their hands off the device in question, for any reason.
One possible reason is that the device is supended; another is that
the cookie carries the flag of the same name.

DM_SUSPENDED is not restored from the db, but evaluated anew for every
uevent. Therefore DM_UDEV_DISABLE_OTHER_RULES_FLAG shouldn't be
restored, either. Use a new variable DM_COOKIE_DISABLE_OTHER_RULES_FLAG
to save and restore the original value from the cookie.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/10-dm.rules.in | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
index f83fbec..e5cc10f 100644
--- a/udev/10-dm.rules.in
+++ b/udev/10-dm.rules.in
@@ -48,7 +48,14 @@ ACTION!="add|change", GOTO="dm_end"
 # These flags are encoded in DM_COOKIE variable that was introduced in
 # kernel version 2.6.31. Therefore, we can use this feature with
 # kernels >= 2.6.31 only. Cookie is not decoded for remove event.
-ENV{DM_COOKIE}=="?*", IMPORT{program}="(DM_EXEC)/dmsetup udevflags $env{DM_COOKIE}"
+ENV{DM_COOKIE}!="?*", GOTO="dm_no_cookie"
+IMPORT{program}="(DM_EXEC)/dmsetup udevflags $env{DM_COOKIE}"
+
+# Store the original flag from the cookie as DM_COOKIE_DISABLE_OTHER_RULES_FLAG
+# in the udev db. DM_UDEV_DISABLE_OTHER_RULES_FLAG will be or'd with other
+# conditions for use by upper, non-dm layers.
+ENV{DM_COOKIE_DISABLE_OTHER_RULES_FLAG}="%E{DM_UDEV_DISABLE_OTHER_RULES_FLAG}"
+LABEL="dm_no_cookie"
 
 # There is no cookie set nor any flags encoded in events not originating
 # in libdevmapper so we need to detect this and try to behave correctly.
@@ -58,12 +65,13 @@ ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", ENV{DM_ACTIVATION}="1", GOTO="dm_flags_do
 IMPORT{db}="DM_UDEV_DISABLE_DM_RULES_FLAG"
 IMPORT{db}="DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG"
 IMPORT{db}="DM_UDEV_DISABLE_DISK_RULES_FLAG"
-IMPORT{db}="DM_UDEV_DISABLE_OTHER_RULES_FLAG"
+IMPORT{db}="DM_COOKIE_DISABLE_OTHER_RULES_FLAG"
 IMPORT{db}="DM_UDEV_LOW_PRIORITY_FLAG"
 IMPORT{db}="DM_UDEV_DISABLE_LIBRARY_FALLBACK_FLAG"
 IMPORT{db}="DM_UDEV_PRIMARY_SOURCE_FLAG"
 IMPORT{db}="DM_UDEV_FLAG7"
 IMPORT{db}="DM_UDEV_RULES_VSN"
+ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="%E{DM_COOKIE_DISABLE_OTHER_RULES_FLAG}"
 LABEL="dm_flags_done"
 
 # If DISK_RO is set, it's an uevent that changes the ro attribute of the device.
-- 
2.43.2


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

* [PATCH v2 4/7] 11-dm-lvm.rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
                   ` (2 preceding siblings ...)
  2024-03-05 12:05 ` [PATCH v2 3/7] 10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 5/7] dm udev rules: don't export and save DM_SUSPENDED Martin Wilck
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

DM_UDEV_DISABLE_OTHER_RULES_FLAG is used as the "output" flag of the
device-mapper rules, to be consumed by non-dm rules. It is a logical OR of
several conditions that might make dm devices inaccessible. 10-dm.rules
calculates it for every uevent, whether it's genuine or spurious.

DM_SUBSYSTEM_UDEV_FLAG0 is just another flag that needs to be or'd in. We
don't need to restore the previous state of DM_UDEV_DISABLE_OTHER_RULES_FLAG.
Actually, doing so is wrong if the flag has previously been set because the
device was suspended, and the device isn't suspended anymore.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/11-dm-lvm.rules.in | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/udev/11-dm-lvm.rules.in b/udev/11-dm-lvm.rules.in
index 7c58994..0b77fe2 100644
--- a/udev/11-dm-lvm.rules.in
+++ b/udev/11-dm-lvm.rules.in
@@ -26,14 +26,11 @@ IMPORT{program}="(DM_EXEC)/dmsetup splitname --nameprefixes --noheadings --rows
 # to zero any stale metadata found within the LV data area. Such stale
 # metadata could cause false claim of the LV device, keeping it open etc.
 #
-# If the NOSCAN flag is present, backup selected existing flags used to
-# disable rules, then set them firmly so those selected rules are surely skipped.
-# Restore these flags once the NOSCAN flag is dropped (which is normally any
-# uevent that follows for this LV, even an artificially generated one).
-ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{DM_NOSCAN}="1", ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
-ENV{DM_SUBSYSTEM_UDEV_FLAG0}!="1", IMPORT{db}="DM_NOSCAN", IMPORT{db}="DM_DISABLE_OTHER_RULES_FLAG_OLD"
-ENV{DM_SUBSYSTEM_UDEV_FLAG0}!="1", ENV{DM_NOSCAN}=="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="$env{DM_DISABLE_OTHER_RULES_FLAG_OLD}", \
-				   ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}="", ENV{DM_NOSCAN}=""
+# If the NOSCAN flag is present, set DM_UDEV_DISABLE_OTHER_RULES_FLAG
+# so those selected rules are surely skipped.
+# We don't need to save and restore the previous of DM_UDEV_DISABLE_OTHER_RULES_FLAG,
+# that's taken care of in 10-dm.rules.
+ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
 
 ENV{DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG}=="1", GOTO="lvm_end"
 
-- 
2.43.2


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

* [PATCH v2 5/7] dm udev rules: don't export and save DM_SUSPENDED
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
                   ` (3 preceding siblings ...)
  2024-03-05 12:05 ` [PATCH v2 4/7] 11-dm-lvm.rules: " Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 6/7] dm udev rules: don't export and save DM_NOSCAN Martin Wilck
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

DM_SUSPENDED is a device-mapper internal flag, which is not intended to be
used by other rules, and which is determined by 10-dm.rules from sysfs for
every uevent. Rename it to ".DM_SUSPENDED", so that it won't be saved in the
udev database.

Known consumers of DM_SUSPENDED are 66-kpartx.rules (from multipath-tools) and
99-systemd.rules (from systemd). These will have to be adapted.
11-dm-mpath.rules will be changed to use .DM_SUSPENDED.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/10-dm.rules.in          | 14 ++++++++------
 udev/12-dm-permissions.rules |  2 +-
 udev/13-dm-disk.rules.in     |  4 ++--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
index e5cc10f..fccf3bc 100644
--- a/udev/10-dm.rules.in
+++ b/udev/10-dm.rules.in
@@ -11,7 +11,6 @@
 # for use in later rules:
 #   DM_NAME - actual DM device's name
 #   DM_UUID - UUID set for DM device (blank if not specified)
-#   DM_SUSPENDED - suspended state of DM device (0 or 1)
 #   DM_UDEV_RULES_VSN - DM udev rules version
 #
 # These rules cover only basic device-mapper functionality in udev.
@@ -118,15 +117,18 @@ LABEL="dm_no_coldplug"
 # The "suspended" item was added even later (kernels >= 2.6.31),
 # so we also have to call dmsetup if the kernel version used
 # is in between these releases.
-TEST=="dm", ENV{DM_NAME}="$attr{dm/name}", ENV{DM_UUID}="$attr{dm/uuid}", ENV{DM_SUSPENDED}="$attr{dm/suspended}"
+TEST=="dm", ENV{DM_NAME}="$attr{dm/name}", ENV{DM_UUID}="$attr{dm/uuid}", ENV{.DM_SUSPENDED}="$attr{dm/suspended}"
 TEST!="dm", IMPORT{program}="(DM_EXEC)/dmsetup info -j %M -m %m -c --nameprefixes --noheadings --rows -o name,uuid,suspended"
-ENV{DM_SUSPENDED}!="?*", IMPORT{program}="(DM_EXEC)/dmsetup info -j %M -m %m -c --nameprefixes --noheadings --rows -o suspended"
 
+ENV{.DM_SUSPENDED}=="?*", GOTO="dm_suspended_set"
+TEST=="dm", IMPORT{program}="(DM_EXEC)/dmsetup info -j %M -m %m -c --nameprefixes --noheadings --rows -o suspended"
 # dmsetup tool provides suspended state information in textual
 # form with values "Suspended"/"Active". We translate it to
 # 0/1 respectively to be consistent with sysfs values.
-ENV{DM_SUSPENDED}=="Active", ENV{DM_SUSPENDED}="0"
-ENV{DM_SUSPENDED}=="Suspended", ENV{DM_SUSPENDED}="1"
+ENV{DM_SUSPENDED}=="Active", ENV{.DM_SUSPENDED}="0"
+ENV{DM_SUSPENDED}=="Suspended", ENV{.DM_SUSPENDED}="1"
+ENV{DM_SUSPENDED}=""
+LABEL="dm_suspended_set"
 
 # This variable provides a reliable way to check that device-mapper
 # rules were installed. It means that all needed variables are set
@@ -144,7 +146,7 @@ ENV{DM_UDEV_DISABLE_DM_RULES_FLAG}!="1", ENV{DM_NAME}=="?*", SYMLINK+="(DM_DIR)/
 # Avoid processing and scanning a DM device in the other (foreign)
 # rules if it is in suspended state. However, we still keep 'disk'
 # and 'DM subsystem' related rules enabled in this case.
-ENV{DM_SUSPENDED}=="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
+ENV{.DM_SUSPENDED}=="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
 
 GOTO="dm_end"
 
diff --git a/udev/12-dm-permissions.rules b/udev/12-dm-permissions.rules
index a9d4c32..6a69d2f 100644
--- a/udev/12-dm-permissions.rules
+++ b/udev/12-dm-permissions.rules
@@ -14,7 +14,7 @@
 #   DM_UDEV_RULES_VSN - DM udev rules version
 #   DM_NAME - actual DM device's name
 #   DM_UUID - UUID set for DM device (blank if not specified)
-#   DM_SUSPENDED - suspended state of DM device (0 or 1)
+#   .DM_SUSPENDED - suspended state of DM device (0 or 1)
 #   DM_LV_NAME - logical volume name (not set if LVM device not present)
 #   DM_VG_NAME - volume group name (not set if LVM device not present)
 #   DM_LV_LAYER - logical volume layer (not set if LVM device not present)
diff --git a/udev/13-dm-disk.rules.in b/udev/13-dm-disk.rules.in
index eaad972..cb2ce2d 100644
--- a/udev/13-dm-disk.rules.in
+++ b/udev/13-dm-disk.rules.in
@@ -17,9 +17,9 @@ ENV{DM_UDEV_DISABLE_DISK_RULES_FLAG}=="1", GOTO="dm_end"
 SYMLINK+="disk/by-id/dm-name-$env{DM_NAME}"
 ENV{DM_UUID}=="?*", SYMLINK+="disk/by-id/dm-uuid-$env{DM_UUID}"
 
-ENV{DM_SUSPENDED}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
+ENV{.DM_SUSPENDED}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
 ENV{DM_NOSCAN}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
-ENV{DM_SUSPENDED}=="1", GOTO="dm_end"
+ENV{.DM_SUSPENDED}=="1", GOTO="dm_end"
 ENV{DM_NOSCAN}=="1", GOTO="dm_watch"
 
 (BLKID_RULE)
-- 
2.43.2


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

* [PATCH v2 6/7] dm udev rules: don't export and save DM_NOSCAN
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
                   ` (4 preceding siblings ...)
  2024-03-05 12:05 ` [PATCH v2 5/7] dm udev rules: don't export and save DM_SUSPENDED Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 12:05 ` [PATCH v2 7/7] 10-dm.rules: bump DM_UDEV_RULES_VSN to 3 Martin Wilck
  2024-03-05 14:42 ` [PATCH v2 0/7] device mapper udev rules rework Peter Rajnoha
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

DM_NOSCAN is not an official API any more and doesn't have to be
restored from the udev db. Rename it to .DM_NOSCAN.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/11-dm-lvm.rules.in  | 2 +-
 udev/13-dm-disk.rules.in | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/udev/11-dm-lvm.rules.in b/udev/11-dm-lvm.rules.in
index 0b77fe2..d0a5637 100644
--- a/udev/11-dm-lvm.rules.in
+++ b/udev/11-dm-lvm.rules.in
@@ -30,7 +30,7 @@ IMPORT{program}="(DM_EXEC)/dmsetup splitname --nameprefixes --noheadings --rows
 # so those selected rules are surely skipped.
 # We don't need to save and restore the previous of DM_UDEV_DISABLE_OTHER_RULES_FLAG,
 # that's taken care of in 10-dm.rules.
-ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
+ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{.DM_NOSCAN}="1", ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
 
 ENV{DM_UDEV_DISABLE_SUBSYSTEM_RULES_FLAG}=="1", GOTO="lvm_end"
 
diff --git a/udev/13-dm-disk.rules.in b/udev/13-dm-disk.rules.in
index cb2ce2d..7989871 100644
--- a/udev/13-dm-disk.rules.in
+++ b/udev/13-dm-disk.rules.in
@@ -18,9 +18,9 @@ SYMLINK+="disk/by-id/dm-name-$env{DM_NAME}"
 ENV{DM_UUID}=="?*", SYMLINK+="disk/by-id/dm-uuid-$env{DM_UUID}"
 
 ENV{.DM_SUSPENDED}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
-ENV{DM_NOSCAN}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
+ENV{.DM_NOSCAN}=="1", ENV{DM_UDEV_PRIMARY_SOURCE_FLAG}=="1", GOTO="dm_import"
 ENV{.DM_SUSPENDED}=="1", GOTO="dm_end"
-ENV{DM_NOSCAN}=="1", GOTO="dm_watch"
+ENV{.DM_NOSCAN}=="1", GOTO="dm_watch"
 
 (BLKID_RULE)
 GOTO="dm_link"
-- 
2.43.2


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

* [PATCH v2 7/7] 10-dm.rules: bump DM_UDEV_RULES_VSN to 3
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
                   ` (5 preceding siblings ...)
  2024-03-05 12:05 ` [PATCH v2 6/7] dm udev rules: don't export and save DM_NOSCAN Martin Wilck
@ 2024-03-05 12:05 ` Martin Wilck
  2024-03-05 14:42 ` [PATCH v2 0/7] device mapper udev rules rework Peter Rajnoha
  7 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 12:05 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

Bump the rules version in order to indicate that upper level rules
should consume DM_UDEV_DISABLE_OTHER_RULES_FLAG rather than DM_NOSCAN
and DM_SUSPENDED.

Also update the comments at the top of the file that describe the
exported properties, and add a note about internal device-mapper
properties.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 udev/10-dm.rules.in | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
index fccf3bc..0fb2033 100644
--- a/udev/10-dm.rules.in
+++ b/udev/10-dm.rules.in
@@ -12,6 +12,9 @@
 #   DM_NAME - actual DM device's name
 #   DM_UUID - UUID set for DM device (blank if not specified)
 #   DM_UDEV_RULES_VSN - DM udev rules version
+#   DM_UDEV_DISABLE_OTHER_RULES_FLAG - a flag that indicates that
+#      stacked layers shouldn't attempt to probe the device, and
+#      should try to import relevant properties from the udev db.
 #
 # These rules cover only basic device-mapper functionality in udev.
 #
@@ -22,6 +25,11 @@
 #   11-dm-lvm.rules for LVM subsystem
 #   11-dm-mpath.rules for multipath subsystem (since version 0.6.0, recommended!)
 #
+# 11-dm<subsystem_name>.rules may use other DM related properties besides
+# those listed above, like .DM_SUSPENDED. These properties are considered
+# internal to device mapper, and subject to change without notice.
+# Rules that are executed after 13-dm-disk.rules shouldn't use them.
+#
 # Even more specific rules may be required by subsystems so always
 # check subsystem's upstream repository for recent set of rules.
 # Also, keep in mind that recent rules may also require recent
@@ -139,7 +147,9 @@ LABEL="dm_suspended_set"
 # possible future changes.
 # VSN 1 - original rules
 # VSN 2 - add support for synthesized events
-ENV{DM_UDEV_RULES_VSN}="2"
+# VSN 3 - use DM_UDEV_DISABLE_OTHER_RULES_FLAG as the only "API"
+#         to be consumed by non-dm rules.
+ENV{DM_UDEV_RULES_VSN}="3"
 
 ENV{DM_UDEV_DISABLE_DM_RULES_FLAG}!="1", ENV{DM_NAME}=="?*", SYMLINK+="(DM_DIR)/$env{DM_NAME}"
 
-- 
2.43.2


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

* Re: [PATCH v2 0/7] device mapper udev rules rework
  2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
                   ` (6 preceding siblings ...)
  2024-03-05 12:05 ` [PATCH v2 7/7] 10-dm.rules: bump DM_UDEV_RULES_VSN to 3 Martin Wilck
@ 2024-03-05 14:42 ` Peter Rajnoha
  2024-03-05 21:17   ` Martin Wilck
  7 siblings, 1 reply; 10+ messages in thread
From: Peter Rajnoha @ 2024-03-05 14:42 UTC (permalink / raw
  To: Martin Wilck, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Martin Wilck, Hannes Reinecke

On 3/5/24 13:05, Martin Wilck wrote:
> Martin Wilck (7):
>   13-dm-disk.rules: import ID_FS_TYPE
>   10-dm.rules: test DISK_RO after importing properties
>   10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db
>   11-dm-lvm.rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from
>     db
>   dm udev rules: don't export and save DM_SUSPENDED
>   dm udev rules: don't export and save DM_NOSCAN
>   10-dm.rules: bump DM_UDEV_RULES_VSN to 3
> 
>  udev/10-dm.rules.in          | 43 +++++++++++++++++++++++++++---------
>  udev/11-dm-lvm.rules.in      | 13 +++++------
>  udev/12-dm-permissions.rules |  2 +-
>  udev/13-dm-disk.rules.in     |  9 ++++----
>  4 files changed, 43 insertions(+), 24 deletions(-)
> 

Only really minor and cosmetic things:
  - we're using $env instead of %E to reference the variables,
  - maybe let's dump the mention about DM_SUSPENDED in
12-dm-permissions.rules if we're at it

But functionality-wise looks good as already discussed in the RFC
version of the patchset. Thank you!

(You can also create an MR in lvm2's gitlab upstream repo
https://gitlab.com/lvmteam/lvm2 if you have an account there.)

For the whole patchset:
Reviewed-by: Peter Rajnoha <prajnoha@redhat.com>

-- 
Peter


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

* Re: [PATCH v2 0/7] device mapper udev rules rework
  2024-03-05 14:42 ` [PATCH v2 0/7] device mapper udev rules rework Peter Rajnoha
@ 2024-03-05 21:17   ` Martin Wilck
  0 siblings, 0 replies; 10+ messages in thread
From: Martin Wilck @ 2024-03-05 21:17 UTC (permalink / raw
  To: Peter Rajnoha, Zdenek Kabelac, Benjamin Marzinski, David Teigland
  Cc: linux-lvm, dm-devel, Hannes Reinecke

On Tue, 2024-03-05 at 15:42 +0100, Peter Rajnoha wrote:
> 
> (You can also create an MR in lvm2's gitlab upstream repo
> https://gitlab.com/lvmteam/lvm2 if you have an account there.)
> 
> For the whole patchset:
> Reviewed-by: Peter Rajnoha <prajnoha@redhat.com>
> 

https://gitlab.com/lvmteam/lvm2/-/merge_requests/9

Thanks!
Martin


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

end of thread, other threads:[~2024-03-05 21:17 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-05 12:05 [PATCH v2 0/7] device mapper udev rules rework Martin Wilck
2024-03-05 12:05 ` [PATCH v2 1/7] 13-dm-disk.rules: import ID_FS_TYPE Martin Wilck
2024-03-05 12:05 ` [PATCH v2 2/7] 10-dm.rules: test DISK_RO after importing properties Martin Wilck
2024-03-05 12:05 ` [PATCH v2 3/7] 10-dm-rules: don't restore DM_UDEV_DISABLE_OTHER_RULES_FLAG from db Martin Wilck
2024-03-05 12:05 ` [PATCH v2 4/7] 11-dm-lvm.rules: " Martin Wilck
2024-03-05 12:05 ` [PATCH v2 5/7] dm udev rules: don't export and save DM_SUSPENDED Martin Wilck
2024-03-05 12:05 ` [PATCH v2 6/7] dm udev rules: don't export and save DM_NOSCAN Martin Wilck
2024-03-05 12:05 ` [PATCH v2 7/7] 10-dm.rules: bump DM_UDEV_RULES_VSN to 3 Martin Wilck
2024-03-05 14:42 ` [PATCH v2 0/7] device mapper udev rules rework Peter Rajnoha
2024-03-05 21:17   ` Martin Wilck

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).