[dm-devel] Changes in DM_MULTIPATH_DEVICE_PATH in multipath-tools 0.7.7

[Martin Wilck]

On Thu, 2018-09-13 at 18:16 -0700, Adam Williamson wrote:

> [...]
> That basically means "run multipath -u (device), expect its output to
> be lines of 'KEY=value' pairs, and import those pairs as device
> properties". At the same time, multipath -u itself was changed so
> that
> if the device *is* a valid multipath device, it outputs this to
> stdout:
> 
> DM_MULTIPATH_DEVICE_PATH="1"
> 
> if the device is *not* a valid multipath device, it outputs this:
> 
> DM_MULTIPATH_DEVICE_PATH="0"
> 
> so it's pretty easy to see that the behaviour has changed: now, if
> the
> device is not a valid multipath device, DM_MULTIPATH_DEVICE_PATH is
> set
> to the value "0", instead of not being set at all.

That's right. 

> This turns out to have consequences, because two *other* udev rules
> files (in Fedora at least) use this check:
> 
> ENV{DM_MULTIPATH_DEVICE_PATH}=="?*"
> 
> that check will match if DM_MULTIPATH_DEVICE_PATH is set to *any one
> or
> more characters at all*...including '0'.

That test is wrong. A better test for skipping the device would be:

ENV{DM_MULTIPATH_DEVICE_PATH}=="1" 

but actually, the "right" test in almost all cases is (see below):

ENV{SYSTEMD_READY}=="0"

> The upshot of this is that 65-md-incremental.rules basically gets
> skipped entirely any time this happens, and our firmware RAID set
> doesn't get initialized by that rule set as it should. 

That file seems to be Fedora-specific, upstream mdadm rules check
SYSTEMD_READY exclusively.

> > The other rules
> file that uses this check is 80-udisks2.rules; the impact there is
> that
> various UDISKS_MD_(FOO) device properties may not get set. I'm not
> sure
> what the consequences of that are.

I overlooked that one. I'll have to file a change request to udisks.

> 
> (In passing I'll note that the commit message claims "The exit status
> remains as usual.", but that is not true. The old 'multipath -u'
> exited
> 1 if the device was not a valid multipath device. The new one exits 0
> in this case. I don't know if this is intentional or an oversight).

It's intentional. It's important for multipath to differentiate the
situation where the device has been checked already, with negative
result (that's what DM_MULTIPATH_DEVICE_PATH=0 means) from the
situation where it had never been checked. See commit b28c4063 for
details.

> The *second* commit introduces a new possible value for
> DM_MULTIPATH_DEVICE_PATH - "2", meaning "this *might* be a multipath
> device, but we're not sure yet." This *may* also be significant to
> other rules, because there are several that use this check:
> 
> ENV{DM_MULTIPATH_DEVICE_PATH}=="1"

If you look at the multipath rules, you can see that the value "2" is
never passed on to later rules. If the value "2" is seen by the rules, 
processing continues along the "maybe" path, and eventually reaches
this code: 

LABEL="pretend_mpath"
ENV{DM_MULTIPATH_DEVICE_PATH}="1"
ENV{SYSTEMD_READY}="0"
GOTO="end_mpath"

That could be better documented by comments, I suppose.

In short, code that tests for ENV{DM_MULTIPATH_DEVICE_PATH}=="1" to
skip actions is fine. Code that checks for empty
ENV{DM_MULTIPATH_DEVICE_PATH} should be fixed.

Regards,
Martin

PS: This touches a very general topic about udev: to which extent are
udev variables "global" in the sense that other, independent rules are
allowed to inspect (and maybe even change) them? I don't think there
are any clear conventions about this. Very few variables are obviously
intended to be global, like "SYSTEMD_READY", but others are rather
confined to the code that creates them, and maybe dedicated consumers.
IOW, there is no "API" or standard describing how rule sets from
different packages maintained by different people should interact.

At least I'm not aware of such a thing.

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