[dm-devel] [PATCH v2 0/9] multipathd: remove udev settle dependency

[Martin Wilck]

On Wed, 2021-11-17 at 15:21 -0600, 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.
> 
> 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. Patch 0006
> handles this by triggering a uevent if the path has been in partial
> for more than 3 minutes.
> 
> 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.
> 
> Notes on v2:
> 
> After playing around with the initialization states a bit, I decided
> that cleaning them up is a bigger task than I want to do in this
> patchset. This set just concentrates on cleaning up our handling of
> paths that get added in update_pathvec_from_dm() to allow us to
> drop the systemd-udev-settle dependency.
> 
> Changes from v1, base on suggestions by Martin Wilck.
> 
> 0005: Made cli_add_path() verify the wwid can be gotten, and hasn't
> changed, before attempting to finish initalizing the path. Also don't
> intialize a path that relies on udev if the udev device isn't
> initalized.
> 
> 0006: New patch. If a patch is in INIT_PARTIAL for over 3 minutes,
> trigger a uevent on it.  This is kind of a long waiting period, but I
> want to avoid firing off another uevent in cases where the problem is
> that udev is in the middle of a uevent storm, and the expected event
> is delayed.
> 
> 0009: Repost of Martin's new init state wildcard patch
> 
> Benjamin Marzinski (8):
>   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: retrigger uevent for partial paths
>   multipathd: remove INIT_PARTIAL paths that aren't in a multipath
>     device
>   multipathd: Remove dependency on systemd-udev-settle.service
> 
> Martin Wilck (1):
>   libmultipath: add path wildcard "%I" for init state
> 
>  libmultipath/configure.c          |  2 +
>  libmultipath/devmapper.c          |  2 +
>  libmultipath/discovery.c          |  7 +--
>  libmultipath/discovery.h          |  2 +
>  libmultipath/libmultipath.version |  2 +-
>  libmultipath/print.c              | 21 +++++++++
>  libmultipath/structs.h            |  9 ++++
>  libmultipath/structs_vec.c        | 41 ++++++++++--------
>  multipathd/cli_handlers.c         | 35 ++++++++++++++-
>  multipathd/main.c                 | 71
> +++++++++++++++++++++++++++++--
>  multipathd/main.h                 |  1 +
>  multipathd/multipathd.service     |  3 +-
>  12 files changed, 167 insertions(+), 29 deletions(-)
> 

For the series:

Reviewed-by: Martin Wilck <mwilck at suse.com>
(You may have noticed by the tags in my re-sent series yesterday).

I still have to give it some more testing, but it's looking good so
far.

As these patches are based on top of my series, I'll apply this (and
the "reconfigure all" series) to the "queue" branch when that series is
finalized.

Regards
Martin