On Mon, Dec 08, 2025 at 03:46:56PM +0100, Martin Wilck wrote:
> On Mon, 2025-12-01 at 22:02 -0500, Benjamin Marzinski wrote:
> > When libmpathpersist notifies multipathd that a key has been
> > registered,
> > cli_setprstatus() calls pr_register_active_paths() with a flag to let
> > it
> > know that the paths are likely already registered, and it can skip
> > re-registering them, as long as the number of active paths matches
> > the
> > number of registered keys. This shortcut can fail, causing multipathd
> > to
> > not register needed paths, if either a path becomes usable and
> > another
> > becomes unusable while libmpathpersist is running or if there already
> > were registered keys for I_T Nexus's that don't correspond to path
> > devices.
> >
> > To make this shortcut work in cases like that, this commit adds a new
> > multipathd command "setprstatus map <map> pathlist <pathlist>", where
> > <pathlist> is a quoted, comma separated list of scsi path devices.
> > libmpathpersist will send out the list of paths it registered the key
> > on. pr_register_active_paths() will skip calling
> > mpath_pr_event_handle()
> > for paths on that list.
> >
> > In order to deal with the possiblity of a preempt occuring while
> > libmpathpersist was running, the code still needs to check that it
> > has
> > the expected number of keys.
> >
> > Fixes: f7d6cd17 ("multipathd: Fix race while registering PR key")
> > Signed-off-by: Benjamin Marzinski <[email protected]>
> > ---
> > libmpathpersist/mpath_persist_int.c | 6 +--
> > libmpathpersist/mpath_updatepr.c | 48 +++++++++++++++++------
> > libmpathpersist/mpathpr.h | 4 +-
> > multipathd/callbacks.c | 1 +
> > multipathd/cli.c | 1 +
> > multipathd/cli.h | 2 +
> > multipathd/cli_handlers.c | 39 ++++++++++++++++--
> > multipathd/main.c | 61 +++++++++++++++++++--------
> > --
> > multipathd/main.h | 3 +-
> > multipathd/multipathd.8.in | 6 +++
> > 10 files changed, 132 insertions(+), 39 deletions(-)
>
> I may be missing something, but this is quite a lot of additional
> complexity just for the shortcut. Have you considered just not taking
> the shortcut in the first place?
Yep. But that means that whenever you register a multipath device with
mpathpersist, multipathd will (almost always pointlessly) re-register
all the paths, just to make sure it doesn't hit these super rare corner
cases. So this added complexity keeps multipathd from running unnecesary
scsi PR commands, and should be quicker. All multipathd does is break
the new cli message up into pathnames, and then has
pr_register_active_paths() check the paths against the pathanames, and
skip them if they match and the number of keys is as big as expected
(and it needs to check the number of keys regardless of whether it uses
the shortcut).
I thought briefly about pulling all the path registration work into
multipathd, but it ends up making multipathd even more complex than this
shortcut, while not saving much complexity from libmpathpersist. If we
wanted to make multipathd handle all of libmpathpersists work, that
would be a different story, but I'm still not sold on the idea of doing
that.
That being said, we can drop a chunk of code if we just force
multipathd to automatically reregister the paths. So if you think that
the reduced work is not worth the added complexity, I'm o.k. with
pulling the shortcut out completely.
-Ben
> Martin
