On 2/9/24 22:58, Martin Wilck wrote:
> Hi Zdenek, Peter, David, Ben,
>
> I have been pondering the device-mapper udev rules a lot lately, and I
> believe I have found glitches in the logic of the device mapper udev
> flags that I'd like to bring to your attention.
>
> TL;DR: change I propose:
>
> * use DM_DISABLE_OTHER_RULES_FLAG consistently as a flag meaning
> "upper layers should leave this device alone until told otherwise",
> saved between uevents in the udev db
> * use DM_SUSPENDED consistently as a flag meaning "upper layers
> should keep their hands off this device temporarily", not saved
> in the udev db.
> * don't use any other flags in upper layers, including 13-dm-
> disk.rules.
>
> This implies:
>
> * stop setting DM_DISABLE_OTHER_RULES_FLAG from DM_SUSPENDED
> in 10-dm.rules
> * check DM_DISABLE_OTHER_RULES_FLAG instead of DM_NOSCAN in
> 13-dm-disk.rules
> * check DM_SUSPENDED in 69-dm-lvm.rules, 80-udisks2.rules
> * stop using DM_NOSCAN in 11-dm-mpath.rules and 66-kpartx.rules
Hi Martin!
It is surely possible we could do some improvements and optimizations
here - all the rules are a result of long evolution where we were fixing
various issues on the way. So if we could shoot down some complexities,
that would be great...
On the other hand, we need to be really careful, because even a tiny
change here can cause troubles if we omit some case.
I'll surely go through your suggestions, thanks for diving deep into
that! Just please give me some time so I'll remap all the paths through
the rules in my head :)
Note that we're working with 3 levels of logical rule separation here:
1) DM base (10-dm.rules, 13-dm-disk.rules, 95-dm-notify.rules)
2) DM subsystem (11-dm-lvm.rules, the mpath rules, ...)
3) all the "other"
As for the very original intentions of the
DM_UDEV_DISABLE_OTHER_RULES_FLAG - this one was supposed to be the *only
one* to check for in "other" rules so they don't need to bother about
checking other states/variables ("other" here means other than DM and
any DM subsystem). One simple variable to check for others - an ideal -
otherwise, we would be having a hard time to persuade others why they
need to add complex checks in their rules/applications.
Yes, there's the unlucky exception in the 99-systemd.rules which needs
to check the DM_SUSPENDED to import the "SYSTEMD_READY" from udev db,
otherwise we had been running into an issue with stopping systemd device
units prematurely (actually, this was an issue spotted much much later
on after years of using the rules without this rule). And we haven't
found a better way to check for this condition. This is mainly because
systemd is also a "device" manager/tracker in some way through its
"device" units, besides udev itself. I simply gave up there and admitted
the check for DM_SUSPENDED case.
The DM_NOSCAN was actually just a helper and a more human readable name
for "DM_SUSBYSTEM_UDEV_FLAG0" within LVM subsystem *only* at first.
It is used during LV initialization - the wiping/zeroing of the LV
before it is pronounced as usable - that's why, when it is set, we
signal to "others" the DM_UDEV_DISABLE_OTHER_RULES_FLAG based on this
flag. However, since we have the 13-dm-disk.rules which manages the
blkid call for DM devices (and which is ours - owned by DM), we need to
signal these rules to avoid calling blkid (as it could see
uninitiliazed/stale data). In this case, we import any previous scan
results from udev db. It's not like "completely ignore and skip rules
for this event". Other DM subsystems may get into exactly the same
situation with initialization as LVM, so that's why it ended up as
generic "DM_NOSCAN" so any 11-dm-<subsystem>.rules can use it to signal
13-dm-disk.rules to avoid the blkid scan this way (the "SCAN" here
actually means "blkid scan" so the better name would be
"DM_NO_BLKID_SCAN" probably). The "DM_NOSCAN" is not meant to be
consumed by "others" at all.
The other way round, we also don't want (ideally) to check for
DM_UDEV_DISABLE_OTHER_RULES flag in DM and DM subysystem rules. This
flag is meant for "others" to check. So we have a clear separation of
what is signalled withing "DM world" and "the other world".
The DM_SUSPENDED flag - well, this was supposed to be just internal to
DM and DM subystem rules. All the "others" have the
DM_UDEV_DISABLE_OTHER_RULES_FLAG - they simply don't need to bother for
what exact reason the device is not usable, whether it is a suspended
case or whatever else...
The DM udev rules are designed in a way that all the DM_UDEV_DISABLE_*
can still be changed in DM/DM-subsystem rules based on further findings
and processing of the state. So the original flags from the DM_COOKIE is
just a starting point here.
--
Peter
