Re: [RFC PATCH 5/7] dm udev rules: don't export and save DM_SUSPENDED

[Peter Rajnoha <prajnoha@redhat.com>]

On 3/1/24 23:40, Martin Wilck wrote:
> 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          | 15 +++++++++------
>  udev/12-dm-permissions.rules |  2 +-
>  udev/13-dm-disk.rules.in     |  4 ++--
>  3 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in
> index ef36209..d30f663 100644
> --- a/udev/10-dm.rules.in
> +++ b/udev/10-dm.rules.in
> @@ -11,7 +11,7 @@
>  # 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_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.
> @@ -114,15 +114,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
> @@ -140,7 +143,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)

OK, makes sense. But I haven't looked at what implications this might
have for 99-systemd.rules yet, but we surely need to have that covered
somehow.

Maybe, now, I would probably even remove the mention about DM_SUSPENDED
in 12-dm-permissions.rules, it looks superfluous there. We normally set
perms based on names, not on DM_SUSPENDED state. I'm not sure why we
mentioned it there before.

Do mpath rules still need to look at DM_SUSPENDED?

-- 
Peter