On Wed, Jan 21, 2026 at 01:41:35PM +0100, Martin Wilck wrote:
> Hi Ben,
>
> On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> > If a path in a mulitpath device is offline while multipathd is
> > reconfigured, it will get added to the updated multipath device, just
> > like it was in the old multipath device. However the device will
> > still
> > be in the INIT_NEW state because it can't get initilized while
> > offline.
> > This is different than the INIT_PARTIAL state because the path was
> > discovered in path_discovery(). INIT_PARTIAL is for paths that
> > multipathd did not discover in path_discovery() or receive a uevent
> > for,
> > but are part of a multipath device that was added, and which should
> > receive a uevent shortly. There is no reason to expect a uevent for
> > these offline paths.
> >
> > When the path comes back online, multipathd will run the checker and
> > prioritizer on it. The only two pathinfo checks that won't happen
> > are the DI_WWID and DI_IOCTL ones. Modify pathinfo() to make sure
> > that if DI_IOCTL was skipped for offline paths, it gets called the
> > next time pathinfo() is called after the fd can be opened. Also,
> > make sure that when one of these offline paths becomes usable, its
> > WWID is rechecked. With those changes, all the DI_ALL checks will
> > be accounted for, and the path can be marked INIT_OK.
> >
> > Signed-off-by: Benjamin Marzinski <[email protected]>
>
> Great observation, thanks!
>
> > ---
> > libmultipath/discovery.c | 2 ++
> > multipathd/main.c | 52 +++++++++++++++++++++++++++++++++++---
> > --
> > 2 files changed, 48 insertions(+), 6 deletions(-)
> >
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index 0c5e5ca6..0efb8213 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -2585,6 +2585,8 @@ blank:
> > * Recoverable error, for example faulty or offline path
> > */
> > pp->chkrstate = pp->state = PATH_DOWN;
> > + if (mask & DI_IOCTL && pp->ioctl_info ==
> > IOCTL_INFO_NOT_REQUESTED)
> > + pp->ioctl_info = IOCTL_INFO_SKIPPED;
> > if (pp->initialized == INIT_NEW || pp->initialized ==
> > INIT_FAILED)
> > memset(pp->wwid, 0, WWID_SIZE);
> >
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index 61e0ea34..2140e432 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -2572,6 +2572,26 @@ static int sync_mpp(struct vectors *vecs,
> > struct multipath *mpp, unsigned int ti
> > return do_sync_mpp(vecs, mpp);
> > }
> >
> > +/*
> > + * pp->wwid should never be empty when this function is called, but
> > if it
> > + * is, this function can set it.
> > + */
> > +static bool new_path_wwid_changed(struct path *pp, int state)
> > +{
> > + char wwid[WWID_SIZE];
> > +
> > + strcpy(wwid, pp->wwid);
> > + if (get_uid(pp, state, pp->udev, 1) != 0) {
> > + strcpy(pp->wwid, wwid);
> > + return false;
> > + }
> > + if (strlen(wwid) && strcmp(wwid, pp->wwid) != 0) {
> > + strcpy(pp->wwid, wwid);
> > + return true;
> > + }
>
> It feels a bit odd to use strcpy() and strcmp() here. I am fine with
> doing that if the arguments are string constants, where it's
> immediately obvious while are obviously properly null-terminated, but
> this isn't the case here. I'd prefer using strlcpy() and strncmp(),
> even if we are both convinced that it's ok without the length check.
I'm fine with making that change. I assume that we can skip the
strnlen(), since we don't use that anywhere, and if we switch the
initial strcpy() to a strlcpy() it's pretty obvious that wwid must be
null terminated.
> > + return false;
> > +}
> > +
> > static int
> > update_path_state (struct vectors * vecs, struct path * pp)
> > {
> > @@ -2601,14 +2621,34 @@ update_path_state (struct vectors * vecs,
> > struct path * pp)
> > return CHECK_PATH_SKIPPED;
> > }
> >
> > - if (pp->recheck_wwid == RECHECK_WWID_ON &&
> > + if ((pp->recheck_wwid == RECHECK_WWID_ON || pp->initialized
> > == INIT_NEW) &&
>
> The readbility of this code would improve if you move the INIT_NEW vs.
> RECHECK_WWID_ON test into an inner conditional, like this:
Good point.
-Ben
>
> if ((newstate == PATH_UP || newstate == PATH_GHOST) &&
> ((pp->state != PATH_UP && pp->state != PATH_GHOST)) ||
> pp->dmstate == PSTATE_FAILED) {
> bool wwid_changed;
>
> if (pp->initialized == INIT_NEW) {
> ...
> } else ... ;
>
> if (wwid_changed) { ... };
> }
>
> Martin
