[PATCH] 11-dm-mpath.rules: Fix udev rule processing during coldplug

[PATCH] 11-dm-mpath.rules: Fix udev rule processing during coldplug

[mwilck]

From: Martin Wilck <mwilck@suse.com>

DM_SUBSYSTEM_UDEV_FLAG0 is used to indicate that upper layer
udev rules don't need to be processed for multipath maps, e.g.
for map reloads.

However, this can fail if maps were created during initrd processing,
but udev processing for the change events of these maps didn't
complete because udevd was killed to switch root. So, in the coldplug
case, play safe and process the rules once more.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/11-dm-mpath.rules | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/multipath/11-dm-mpath.rules b/multipath/11-dm-mpath.rules
index cd522e8c..34c21fd1 100644
--- a/multipath/11-dm-mpath.rules
+++ b/multipath/11-dm-mpath.rules
@@ -52,8 +52,13 @@ LABEL="mpath_action"
 # something that should be reacted upon since it would be useless extra work.
 # It's exactly mpath's job to provide *seamless* device access to any of the
 # paths that are available underneath.
+# Exception: On cold-plug, if the map had already been set up before,
+# do process upper layers, because rule processing may not necessarily have
+# completed during initrd processing.
+ACTION=="add", ENV{.MPATH_DEVICE_READY_OLD}=="1", GOTO="process_reload"
 ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", \
 	ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1"
+LABEL="process_reload"
 
 # For path failed or reinstated events, unset DM_ACTIVATION.
 # This is similar to the DM_SUBSYSTEM_UDEV_FLAG0 case above.
-- 
2.26.2

Re: [PATCH] 11-dm-mpath.rules: Fix udev rule processing during coldplug

[Benjamin Marzinski]

On Tue, Jun 02, 2020 at 05:54:43PM +0200, mwilck@suse.com wrote:
> From: Martin Wilck <mwilck@suse.com>
> 
> DM_SUBSYSTEM_UDEV_FLAG0 is used to indicate that upper layer
> udev rules don't need to be processed for multipath maps, e.g.
> for map reloads.
> 
> However, this can fail if maps were created during initrd processing,
> but udev processing for the change events of these maps didn't
> complete because udevd was killed to switch root. So, in the coldplug
> case, play safe and process the rules once more.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> ---
>  multipath/11-dm-mpath.rules | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/multipath/11-dm-mpath.rules b/multipath/11-dm-mpath.rules
> index cd522e8c..34c21fd1 100644
> --- a/multipath/11-dm-mpath.rules
> +++ b/multipath/11-dm-mpath.rules
> @@ -52,8 +52,13 @@ LABEL="mpath_action"
>  # something that should be reacted upon since it would be useless extra work.
>  # It's exactly mpath's job to provide *seamless* device access to any of the
>  # paths that are available underneath.
> +# Exception: On cold-plug, if the map had already been set up before,
> +# do process upper layers, because rule processing may not necessarily have
> +# completed during initrd processing.
> +ACTION=="add", ENV{.MPATH_DEVICE_READY_OLD}=="1", GOTO="process_reload"
>  ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", \
>  	ENV{DM_ACTIVATION}="0", ENV{MPATH_UNCHANGED}="1"
> +LABEL="process_reload"
>  
>  # For path failed or reinstated events, unset DM_ACTIVATION.
>  # This is similar to the DM_SUBSYSTEM_UDEV_FLAG0 case above.
> -- 
> 2.26.2

Re: [PATCH] 11-dm-mpath.rules: Fix udev rule processing during coldplug

[Martin Wilck]

Hi Ben, Hi Christophe,

On Tue, 2020-06-02 at 16:21 -0500, Benjamin Marzinski wrote:
> On Tue, Jun 02, 2020 at 05:54:43PM +0200, mwilck@suse.com wrote:
> > From: Martin Wilck <mwilck@suse.com>
> > 
> > DM_SUBSYSTEM_UDEV_FLAG0 is used to indicate that upper layer
> > udev rules don't need to be processed for multipath maps, e.g.
> > for map reloads.
> > 
> > However, this can fail if maps were created during initrd
> > processing,
> > but udev processing for the change events of these maps didn't
> > complete because udevd was killed to switch root. So, in the
> > coldplug
> > case, play safe and process the rules once more.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

This patch was wrong. It doesn't hurt but it also doesn't help; it
simply has no effect because DM_SUBSYSTEM_UDEV_FLAG0
is never set on "add" uevents.

Christophe, please don't apply it.

Martin