| Message ID | 20240301224011.11117-8-mwilck@suse.com (mailing list archive) |
|---|---|
| State | Not Applicable, archived |
| Delegated to: | christophe varoqui |
| Headers | show |
| Series | device mapper udev rules rework | expand |
On 3/1/24 23:40, Martin Wilck wrote: > 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. > > Signed-off-by: Martin Wilck <mwilck@suse.com> > --- > udev/10-dm.rules.in | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in > index d30f663..21bbcb0 100644 > --- a/udev/10-dm.rules.in > +++ b/udev/10-dm.rules.in > @@ -136,7 +136,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}" > One thing that comes to my mind here is cooperation between the rules from initrd/initramfs and rootfs - the initrd/initramfs can have different versions of the rules installed. This was actually the original reason for introducing such versioning so we can still try to do our best even if the rules are mixed (to not cause a hang at boot just because a proper symlink was not found undev /dev). I haven't tested this with the new rules yet...
On Mon, 2024-03-04 at 12:09 +0100, Peter Rajnoha wrote: > On 3/1/24 23:40, Martin Wilck wrote: > > 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. > > > > Signed-off-by: Martin Wilck <mwilck@suse.com> > > --- > > udev/10-dm.rules.in | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in > > index d30f663..21bbcb0 100644 > > --- a/udev/10-dm.rules.in > > +++ b/udev/10-dm.rules.in > > @@ -136,7 +136,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}" > > > > One thing that comes to my mind here is cooperation between the rules > from initrd/initramfs and rootfs - the initrd/initramfs can have > different versions of the rules installed. Yes, that's a source of pain. Are there current initramfs tools that user DM_UDEV_RULES_VSN!=2? I think "2" should be the standard today, given that it has existed since 2009. dracut just installs the upstream rules it finds, at least for dm, AFAICT. I've reviewed other rule sets I'm aware of, and the only one in which I needed to check DM_UDEV_RULES_VSN was 11-dm-mpath.rules. I didn't have a close look at the rule sets that dracut ships yet, let alone other tools for initramfs maintenance. Regardless, my patch set changes the availability and semantics of the device-mapper udev properties, and thus we should bump the version, no? Thanks, Martin
On 3/4/24 17:46, Martin Wilck wrote: > On Mon, 2024-03-04 at 12:09 +0100, Peter Rajnoha wrote: >> One thing that comes to my mind here is cooperation between the rules >> from initrd/initramfs and rootfs - the initrd/initramfs can have >> different versions of the rules installed. > > Yes, that's a source of pain. Are there current initramfs tools that > user DM_UDEV_RULES_VSN!=2? I think "2" should be the standard today, > given that it has existed since 2009. dracut just installs the upstream > rules it finds, at least for dm, AFAICT. > > I've reviewed other rule sets I'm aware of, and the only one in which I > needed to check DM_UDEV_RULES_VSN was 11-dm-mpath.rules. I didn't have > a close look at the rule sets that dracut ships yet, let alone other > tools for initramfs maintenance. > > Regardless, my patch set changes the availability and semantics of the > device-mapper udev properties, and thus we should bump the version, no? Sure, that is expected if we do such changes to rules. I just meant to be cautious about a situation where we have initramfs running a different version of rules, then keeping the udev database over the pivot-to-rootfs (which happens with dracut, not sure about others). Then, on coldplug running from rootfs, refreshing the state with the other version of rules. Important here is that the rules running from rootfs do not get mislead with the state that was taken over from the initrams.
On Tue, 2024-03-05 at 09:26 +0100, Peter Rajnoha wrote: > On 3/4/24 17:46, Martin Wilck wrote: > > On Mon, 2024-03-04 at 12:09 +0100, Peter Rajnoha wrote: > > > One thing that comes to my mind here is cooperation between the > > > rules > > > from initrd/initramfs and rootfs - the initrd/initramfs can have > > > different versions of the rules installed. > > > > Yes, that's a source of pain. Are there current initramfs tools > > that > > user DM_UDEV_RULES_VSN!=2? I think "2" should be the standard > > today, > > given that it has existed since 2009. dracut just installs the > > upstream > > rules it finds, at least for dm, AFAICT. > > > > I've reviewed other rule sets I'm aware of, and the only one in > > which I > > needed to check DM_UDEV_RULES_VSN was 11-dm-mpath.rules. I didn't > > have > > a close look at the rule sets that dracut ships yet, let alone > > other > > tools for initramfs maintenance. > > > > Regardless, my patch set changes the availability and semantics of > > the > > device-mapper udev properties, and thus we should bump the version, > > no? > > Sure, that is expected if we do such changes to rules. > > I just meant to be cautious about a situation where we have initramfs > running a different version of rules, then keeping the udev database > over the pivot-to-rootfs (which happens with dracut, not sure about > others). Then, on coldplug running from rootfs, refreshing the state > with the other version of rules. > > Important here is that the rules running from rootfs do not get > mislead > with the state that was taken over from the initrams. For udev rules, that can't happen. If any udev rules were running after switching root, they'd be running in the context of a new uevent, which means that the device-mapper rules from the root FS would already be in place. We don't import DM_SUSPENDED from the db, so this property wouldn't survive switching root, even if it had been set pre-pivot. Other (non-udev) system components might be confused if they read properties directly from udev data base using udev_device_get_property_value(). But we can't do anything about this. For multipathd, which is one of the prime suspects in this area, I can confirm that it doesn't use libudev to access any device–mapper internal properties. In general, I can't conceive any danger arising from an older dm ruleset (e.g. in the initramfs) that still sets DM_SUSPENDED or DM_NOSCAN. In the worst case, some rule would be triggered that's also triggered with today's rule set. That's not optimal, but it can hardly be fatal. FTR, summarizing the effect of my patch set for follow-up rules: - it slightly changes the meaning of DM_UDEV_DISABLE_OTHER_RULES FLAG, - it renames DM_SUSPENDED to .DM_SUSPENDED, - it renames DM_NOSCAN to .DM_NOSCAN. Technically, the renames just have the effect that these variables aren't saved in the udev db. "Psychologically", the intention is that people realize they are meant to be "dm internal", knowing that unless we add more rules to unset these properties, we can't prevent later rules from reading and using them. With this patch set, later rules that depend on DM_SUSPENDED or DM_NOSCAN won't trigger any more. The only rule outside of dm and multipath that I am aware of is the broken rule in 99-systemd.rules. Regards Martin
diff --git a/udev/10-dm.rules.in b/udev/10-dm.rules.in index d30f663..21bbcb0 100644 --- a/udev/10-dm.rules.in +++ b/udev/10-dm.rules.in @@ -136,7 +136,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}"
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. Signed-off-by: Martin Wilck <mwilck@suse.com> --- udev/10-dm.rules.in | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)