Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multipathd does not trigger udev rules for a path that the user has added to the wwids file #103

Open
pcahyna opened this issue Nov 11, 2024 · 12 comments

Comments

@pcahyna
Copy link

pcahyna commented Nov 11, 2024

I would like to configure multipath on a device which has only one path (/dev/sdd). (This is for testing, but I believe there may be uses for this even in production, e.g. when only one path exists for a device, but one knows that more paths will appear.)

I tried to do this via adding the device to the wwids file (multipath -a /dev/sdd) and restarting multipathd (systemctl restart multipathd), which then picks up the device and creates the multipath map (/dev/mapper/mpatha). This mostly works, but I have encountered an irregularity where udevadm info on the path does not show that it is a path:

# udevadm info /dev/sdd
E: DM_MULTIPATH_DEVICE_PATH=0

Also, there are still partition device nodes on /dev/sdd (they should have been deleted by /usr/lib/udev/rules.d/68-del-part-nodes.rules):

# lsblk
NAME                          MAJ:MIN RM   SIZE RO TYPE  MOUNTPOINT
sdd                             8:48   0 558.9G  0 disk  
├─sdd1                          8:49   0 558.9G  0 part  
└─mpatha                      253:3    0 558.9G  0 mpath 
  └─mpatha1                   253:4    0 558.9G  0 part  

This gets corrected after reboot, udevadm info shows the expected results:

# udevadm info /dev/sdd
E: DM_DEL_PART_NODES=1
E: DM_MULTIPATH_DEVICE_PATH=1
E: ID_FS_TYPE=mpath_member

I found that multipathd configured by using mpathconf --find_multipaths greedy picks up the device without using multipath -a and in this case the udev information of the path is immediately correct (without a reboot). I would prefer to have a more fine-grained control over which devices are configured as multipath, though, and this behavior does not look correct.

When examining the difference between the two cases, I found that when using greedy multipathd emits a change event for /dev/sdd which then triggers the update of the udev variables and deletion of the partition device nodes, and this does not happen when using multipath -a and then restarting multipathd. (The rule that sets DM_MULTIPATH_DEVICE_PATH is in /usr/lib/udev/rules.d/62-multipath.rules .) I then found that using udevadm trigger -w -c change /dev/sdd after restarting multipathd corrects everything.

While it is nice to have a workaround, I believe that this is a bug and the event should have been emitted by multipathd itself, because a documented use of multipath and multipathd should not lead to a situation which is apparently inconsistent.

I looked into multipath-tools code and found two locations that are most likely related to the problem:

(remember_wwid(mpp->wwid) == 1 ||

if (remember_wwid(mpp->wwid) == 1)

Not sure which of them is executed, but both seem to trigger udev on the paths only if the wwid gets newly added to the wwids file, which would explain the behavior (when using multipath -a the wwid is already there, with greedy it is not - multipathd adds it). If that's the case, what is the reason for this behavior?

This is on RHEL 8 and RHEL 9 btw (multipath tools version 0.8.4 and 0.8.7).

@mwilck
Copy link
Contributor

mwilck commented Nov 12, 2024

The classification of paths between multipath-and non-multipath depends on the find_multipaths setting.

Note that mpathconf is Fedora/RedHat specific and not part of upstream multipath tools. Please report to your distribution.

(But maybe we can short-cut your issue by having @bmarzins comment here).

@pcahyna
Copy link
Author

pcahyna commented Nov 12, 2024

@mwilck note that I have not mentioned any use of mpathconf in the problematic case (only in the working case). How is it relevant that mpathconf is not part of the upstream tools?

@mwilck
Copy link
Contributor

mwilck commented Nov 12, 2024

Usage of mpathconf is an indicator for someone not using standard procedures, at least from my personal point of view.

The expected behavior is that with find_multipaths greedy, your single-path device would be picked up by multipathd, like (almost) any other block device. "greedy" is best combined with some sort of blackisting. You could do something like this in multipath.conf:

blacklist {
    devnode .*
}
blacklist_exceptions {
    devnode sdd
}

(just a very crude example, we usually discourage blacklisting by devnode).

Without "greedy", you need to add it to the WWIDs file as you figured out yourself.

@pcahyna
Copy link
Author

pcahyna commented Nov 18, 2024

In some cases it is more practical to execute an one-shot command than to use greedy and change multipath.conf and this is easily achieved by adding the device to the WWIDs file using multipath -a. Given that this usage is documented and the comment in the file even says

# NOTE: this file is automatically maintained by the multipath program.
# You should not need to edit this file in normal circumstances.

I would say that this usage is considered supported and should not lead to inconsistencies. And if the udev rules are not triggered and DM_MULTIPATH_DEVICE_PATH=0, this looks as an inconsistency to me.

In the meantime I found another way to configure multipath on the device using a one-shot command:

multipathd -k"add map 35000c500cbbbe1df"

This works properly and triggers the udev rules (presumably because the device is added by multipathd itself?). But as it requires me to know the WWID of the device (multipathd -k"add map /dev/sdd" does not work), using multipath -a is still the most practical way of doing this.

@mwilck
Copy link
Contributor

mwilck commented Nov 19, 2024

0001-libmultipath-always-trigger-uevent-change-when-a-map.patch.txt

Can you try with this patch?

@bmarzins
Copy link
Contributor

Probably the easiest way to make this work is to just run:

# multipath /dev/sdd

The fact that:

# multipathd add map /dev/sdd

doesn't work is a bug. In "multipathd add map" we assume you are passing in an identifier of a multipath device. In the multipath command, we don't make those assumptions, so multipath figures out that this is a path and you want to make a multipath device out of it. That's an easy fix.

@bmarzins
Copy link
Contributor

@mwilck Instead of always setting needs_paths_uevent here, and then checking if it's set in domap() when a create succeeds, wouldn't it be easier to just always call trigger_paths_udev_change(mpp, true) in domap() when a create succeeds?

@mwilck
Copy link
Contributor

mwilck commented Nov 20, 2024

Possibly. I haven't thought it through though.

Anyway we should be aware that this is not a full solution to the problem of modifying the WWIDs file asynchronously. If we handle multipath -a followed by reconfigure in this way, we should also be able to handle multipath -r, and I couldn't think of a good solution for that so far.

@pcahyna
Copy link
Author

pcahyna commented Nov 22, 2024

0001-libmultipath-always-trigger-uevent-change-when-a-map.patch.txt

Can you try with this patch?

Thanks, this works in my test 👍

@pcahyna
Copy link
Author

pcahyna commented Nov 22, 2024

Probably the easiest way to make this work is to just run:

# multipath /dev/sdd

Thanks, this works even without the patch, I don't know why I thought that -a was necessary.

The fact that:

# multipathd add map /dev/sdd

doesn't work is a bug. In "multipathd add map" we assume you are passing in an identifier of a multipath device. In the multipath command, we don't make those assumptions, so multipath figures out that this is a path and you want to make a multipath device out of it. That's an easy fix.

Would be a nice extension, although the manual page describes the argument as

$map can either be a device-mapper device as listed in /sys/block
(e.g. dm-0) or it can be the alias for the multipath device
(e.g. mpath1) or the uid of the multipath device (e.g.
36005076303ffc56200000000000010aa).

so it currently behaves as described - I did not really expect multipathd add map /dev/sdd to work given the description.

@mwilck
Copy link
Contributor

mwilck commented Nov 22, 2024

Thanks, this works in my test 👍

Thank you for the feedback!

I'm not 100% convinced yet that this is the right thing to do. We'll need to do some more code review and testing to verify that we won't generate loads of additional uevents because of this change. I don't think we will, because multipathd only generates events if the state of the path in question does not match its expectations, but we must double-check, because uevent storms are a very bad thing.

mwilck added a commit to openSUSE/multipath-tools that referenced this issue Nov 27, 2024
If map creation succeeds, previously not multipathed devices are
now multipathed. udev may not have noticed this yet, thus trigger
path uevents to make it aware of the situation. Likewise, if
creating a map fails, the paths in question were likely considered
multipath members by udev, too. They will now be marked as failed,
so trigger an event in this situation as well.

Fixes: opensvc#103
Suggested-by: Benjamin Marzinski <[email protected]>
Signed-off-by: Martin Wilck <[email protected]>
@mwilck
Copy link
Contributor

mwilck commented Nov 27, 2024

I have posted a patch series to the dm-devel mailing list ("[PATCH v2 0/8] multipath-tools fixes") where patch 6 and 7 are an attempt to fix this issue. I have tested it successfully here, and I am no more worried about excessive uevent storms.

With this set, uevents are also triggered if WWIDs are removed from the WWIDs file with multipath -w and multipathd reconfigure is run.

The set is also on my tip branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants