On Wed, 2021-10-20 at 14:15 -0500, Benjamin Marzinski wrote: > So, it turns out that commit 4ef67017 "libmultipath: add > update_pathvec_from_dm()" already does most of the hard work of making > multipath handle the uninitialized paths that exist during boot, after > the switch root, but before the all the coldplug uevents have been > processed. The only problem is that this code leaves the paths in a > partially initialized state, which doesn't completely get corrected > until > a reconfigure happens. > > This patchset makes multipath track these partially initailized paths, > and makes sure that they get fully initialized, or removed, as > necessary. > > The first patch is a tangentially related bug I found when trying > (unsuccessfully) to recreate multipathd's problem with dropping > uninitialized paths. Multipathd was not removing completely missing > paths from maps that it found while starting up. The solution I chose > was just to make sure that a full reload will happen on these maps, > even > if a weak reconfigure was requested. However, this means that multipath > still completely ignores these missing paths. A side effect of this is > that "multipath -l" does not show these paths, even though they exist > as > part of the multipath table. The full reloads are necessary, to take > care of issues that update_pathvec_from_dm() can run into, but we might > also want to think about marking these missing paths as INIT_REMOVED, > instead of not adding them at all. This would make "multipath -l" still > show them, until they actually get removed from the table. Yes, that might be a good thing. Completely missing paths (not existing in sysfs) in a map represent a dangerous condition; it would be good if multipath -l was able to tell the user about it. > > Patch 0005 makes sure to fully initialize the paths when their coldplug > event occurs, but if the path is already fully initialized in udev, but > multipathd finds out about it from update_pathvec_from_dm(), multipath > doesn't do anything to finish initializing the path and moving it to > INIT_OK, besides waiting for a uevent or a reconfigure. This is > probably > fine, since the only way I can see for a path to be in this state is > for > someone to manually remove the path from multipathd monitoring. But if > I'm missing some other way that paths could end up in this state, we > could try proactively finishing the initalization of INIT_PARTIAL paths > that have all their udev information. One option would be to try finishing the initialization in the path checker. What about the checker, in general? Should we take some care that partially initialized paths aren't mistakenly set as failed? I'm not sure if libudev is able to return a proper fd from udev_device_get_devnode() if the device isn't yet initialized in the udev db. Without a proper fd, the checker is doomed to fail. And other failure modes are possible too without proper udev initialization, I suppose? > > I'm also kind of on the fence about patch 0006. There is no reason > why > we have to remove INIT_PARTIAL paths if the multipath device goes > away. > But if a path is in INIT_PARTIAL for long enough that the multipath > device gets removed, then it's likely not a path we want to be > monitoring, and if a uevent occurs, we'll add it back, anyway. Also, > knowing that INIT_PARTIAL paths are always part of multipath devices > does make the code simpler. > > I've tested these patches both by rebooting with necessary and > unnecessary multipath devices in the initramfs and multipathd.service > set to make multipathd start up at various points after the switch > root, > and by manually sending remove uevents to unintialize some paths, and > then starting multipathd to test specific combinations of path > states. > But more testing is always welcome. I'll try to give this code a test shortly. Cheers, Martin > > Benjamin Marzinski (7): > multipathd: remove missing paths on startup > libmultipath: skip unneeded steps to get path name > libmultipath: don't use fallback wwid in update_pathvec_from_dm > libmultipath: always set INIT_REMOVED in set_path_removed > multipathd: fully initialize paths added by update_pathvec_from_dm > multipathd: remove INIT_PARTIAL paths that aren't in a multipath > device > multipathd: Remove dependency on systemd-udev-settle.service > > libmultipath/configure.c | 2 ++ > libmultipath/devmapper.c | 2 ++ > libmultipath/discovery.c | 7 ++--- > libmultipath/discovery.h | 2 ++ > libmultipath/structs.h | 7 +++++ > libmultipath/structs_vec.c | 40 ++++++++++++++++------------- > multipathd/cli_handlers.c | 4 +++ > multipathd/main.c | 48 ++++++++++++++++++++++++++++++++- > -- > multipathd/main.h | 1 + > multipathd/multipathd.service | 3 +-- > 10 files changed, 90 insertions(+), 26 deletions(-) > -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
