diff mbox series

[v2,5/5] 11-dm-mpath.rules.in: set .DM_NOSCAN if MPATH_UNCHANGED is set

Message ID 20241103224349.42582-6-mwilck@suse.com (mailing list archive)
State Not Applicable, archived
Delegated to: Benjamin Marzinski
Headers show
Series multipath-tools: udev rule fixes | expand

Commit Message

Martin Wilck Nov. 3, 2024, 10:43 p.m. UTC
When multipath reloads a device or fails or restores a path, the udev
rules disable LVM scanning, but since .DM_NOSCAN isn't set, blkid is
still run on the device. When multipath devices that are set to
queue_if_no_path lose all their paths at close to the same time, udev
workers can hang trying to run blkid. The blkid results shouldn't
change when multipathd is adding, removing, failing or reinstating
paths, aside from avoiding hanging udev processes, we're skipping
unnecessary work.

Hence, set .DM_NOSCAN if MPATH_UNCHANGED is set, to avoid blkid from
being called in 13-dm.rules.

Suggested-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/11-dm-mpath.rules.in | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

Zdenek Kabelac Nov. 4, 2024, 2 p.m. UTC | #1
Dne 03. 11. 24 v 23:43 Martin Wilck napsal(a):
> When multipath reloads a device or fails or restores a path, the udev
> rules disable LVM scanning, but since .DM_NOSCAN isn't set, blkid is
> still run on the device. When multipath devices that are set to
> queue_if_no_path lose all their paths at close to the same time, udev


Hi


There is worth to be mentioned -   blkid has for a long time 'special' rule 
-built-in   for LVM (and some other devices i.e. crypt...)

So whenever lvm2  uses   LVM-uuid-suffix     with the '-suffix' in it's UUID - 
it's a private DM device which is never even opened by blkid. We would like to 
convert all remaining LV activations to use this logic for every 'private' 
device - since this logic is 'persistent' and survive even complete  removal 
od udevdb. However it takes some time and also the backward compatibility is 
complicating things a lot.

But all I say is - the relaing on any logic of  the udev flags is always kind 
of  problematic - and lvm2 does not depend on this fragile flagging too much.


Regards


Zdenk
Martin Wilck Nov. 4, 2024, 3:03 p.m. UTC | #2
On Mon, 2024-11-04 at 15:00 +0100, Zdenek Kabelac wrote:
> Dne 03. 11. 24 v 23:43 Martin Wilck napsal(a):
> > When multipath reloads a device or fails or restores a path, the
> > udev
> > rules disable LVM scanning, but since .DM_NOSCAN isn't set, blkid
> > is
> > still run on the device. When multipath devices that are set to
> > queue_if_no_path lose all their paths at close to the same time,
> > udev
> 
> 
> Hi
> 
> 
> There is worth to be mentioned -   blkid has for a long time
> 'special' rule 
> -built-in   for LVM (and some other devices i.e. crypt...)
> 
> So whenever lvm2  uses   LVM-uuid-suffix     with the '-suffix' in
> it's UUID - 
> it's a private DM device which is never even opened by blkid. We
> would like to 
> convert all remaining LV activations to use this logic for every
> 'private' 
> device - since this logic is 'persistent' and survive even complete 
> removal 
> od udevdb. However it takes some time and also the backward
> compatibility is 
> complicating things a lot.
> 
> But all I say is - the relaing on any logic of  the udev flags is
> always kind 
> of  problematic - and lvm2 does not depend on this fragile flagging
> too much.

Thanks for the hint. I think you're referring to libblkid's commit
20e1c3d ("libblkid: ignore private LVM devices"), which makes blkid
ignore devices with LVM UUIDs matching "LVM-.*-.*". I actually hadn't
been aware of this.

But note that multipath is overloading the .DM_NOSCAN flag here to tell
higher layers that blkid shouldn't be run in the first place if a
multipath map is in a certain state (we're reusing this flag, which was
originally created for LVM, in a similar but not identical case where
we want the blkid call to be skipped).

Unlike LVM, multipath has no way to set a UUID with special properties,
because multipath doesn't own any meta data on disk. In the case of
multipath, the .DM_NOSCAN property can, and will, change for a map
while its UUID remains constant. That's what this patch is about.

Thanks
Martin
Zdenek Kabelac Nov. 4, 2024, 4:55 p.m. UTC | #3
Dne 04. 11. 24 v 16:03 Martin Wilck napsal(a):
> On Mon, 2024-11-04 at 15:00 +0100, Zdenek Kabelac wrote:
>> Dne 03. 11. 24 v 23:43 Martin Wilck napsal(a):
>>> When multipath reloads a device or fails or restores a path, the
>>> udev
>>> rules disable LVM scanning, but since .DM_NOSCAN isn't set, blkid
>>> is
>>> still run on the device. When multipath devices that are set to
>>> queue_if_no_path lose all their paths at close to the same time,
>>> udev
> But note that multipath is overloading the .DM_NOSCAN flag here to tell
> higher layers that blkid shouldn't be run in the first place if a
> multipath map is in a certain state (we're reusing this flag, which was
> originally created for LVM, in a similar but not identical case where
> we want the blkid call to be skipped).
> 
> Unlike LVM, multipath has no way to set a UUID with special properties,
> because multipath doesn't own any meta data on disk. In the case of
> multipath, the .DM_NOSCAN property can, and will, change for a map
> while its UUID remains constant. That's what this patch is about.

Hi

It's been more about the idea - that you can try to communicate some
protocol logic for 'libblkid' in some way - that will automatically
make sure blkid will not try to open device in these unwanted cases.

It could be possibly something like:

if DM device is  'mpath' &&  file  /run/mpath/do_not_open_MPATH_UUID exists
the this will be makeing libblkid think it's a device it should not
be opened.

Then mpath tool is just manages present of this file - so this way you
do not rely on works of any udev flag.

Although the case here can be different if the device as such is supposed to 
be a udev visible volume - as the  blkid skipping might make the volume just 
'invisible' as whole.

Regards

Zdenek
Martin Wilck Nov. 4, 2024, 5:11 p.m. UTC | #4
On Mon, 2024-11-04 at 17:55 +0100, Zdenek Kabelac wrote:
> Dne 04. 11. 24 v 16:03 Martin Wilck napsal(a):
> > On Mon, 2024-11-04 at 15:00 +0100, Zdenek Kabelac wrote:
> > > Dne 03. 11. 24 v 23:43 Martin Wilck napsal(a):
> > > > When multipath reloads a device or fails or restores a path,
> > > > the
> > > > udev
> > > > rules disable LVM scanning, but since .DM_NOSCAN isn't set,
> > > > blkid
> > > > is
> > > > still run on the device. When multipath devices that are set to
> > > > queue_if_no_path lose all their paths at close to the same
> > > > time,
> > > > udev
> > But note that multipath is overloading the .DM_NOSCAN flag here to
> > tell
> > higher layers that blkid shouldn't be run in the first place if a
> > multipath map is in a certain state (we're reusing this flag, which
> > was
> > originally created for LVM, in a similar but not identical case
> > where
> > we want the blkid call to be skipped).
> > 
> > Unlike LVM, multipath has no way to set a UUID with special
> > properties,
> > because multipath doesn't own any meta data on disk. In the case of
> > multipath, the .DM_NOSCAN property can, and will, change for a map
> > while its UUID remains constant. That's what this patch is about.
> 
> Hi
> 
> It's been more about the idea - that you can try to communicate some
> protocol logic for 'libblkid' in some way - that will automatically
> make sure blkid will not try to open device in these unwanted cases.
> 
> It could be possibly something like:
> 
> if DM device is  'mpath' &&  file  /run/mpath/do_not_open_MPATH_UUID
> exists
> the this will be makeing libblkid think it's a device it should not
> be opened.
> 
> Then mpath tool is just manages present of this file - so this way
> you
> do not rely on works of any udev flag.

We'd still need to manipulate these "flag" files through udev rules,
and we'd have to make sure no stale such file remain in place. I don't
see the advantage of this approach yet (for multipath).

We are are already using this sort of "flag file" mechanism to
communicate between udev rules and multipathd, where environment
variables can't be used. Again, in the LVM case, certain devices are
always "private" to LVM and it makes no sense to run blkid on them, not
even manually. But that isn't the case for multipath.

Martin
diff mbox series

Patch

diff --git a/multipath/11-dm-mpath.rules.in b/multipath/11-dm-mpath.rules.in
index 79227be..a816edb 100644
--- a/multipath/11-dm-mpath.rules.in
+++ b/multipath/11-dm-mpath.rules.in
@@ -145,9 +145,11 @@  IMPORT{db}="ID_PART_GPT_AUTO_ROOT"
 
 LABEL="import_end"
 
-# If MPATH_UNCHANGED is set, adapt DM_ACTIVATION.
+# If MPATH_UNCHANGED is set, adapt DM_ACTIVATION and DM_NOSCAN.
+# .DM_NOSCAN controls whether blkid will be run in 13-dm-disk.rules;
+# we don't want to do that if MPATH_UNCHANGED is 1.
 ENV{MPATH_UNCHANGED}=="0", ENV{DM_ACTIVATION}="1"
-ENV{MPATH_UNCHANGED}=="1", ENV{DM_ACTIVATION}="0"
+ENV{MPATH_UNCHANGED}=="1", ENV{DM_ACTIVATION}="0", ENV{.DM_NOSCAN}="1"
 
 # Reset previous DM_COLDPLUG_SUSPENDED if activation happens now
 ENV{.DM_SUSPENDED}!="1", ENV{DM_ACTIVATION}=="1", ENV{DM_COLDPLUG_SUSPENDED}=""