[PATCH 0/4] multipathd: Miscellaneous fixes

[PATCH 0/4] multipathd: Miscellaneous fixes

[Benjamin Marzinski]

These are a just collection of unrelated multipath fixes. The first one
keeps new maps from adding non-discovered paths in configure(). The
second one makes sure that paths that were offline when multipathd was
started or reconfigured get the pathinfo checked and get the proper
initialization state, when the come online. The third improves the
"busy" state output in "multipathd show status". And the fourth one make
sure that multipathd prints the "path offline" message for paths that
are part of multipath devices but were offline when mutipathd was
started or reconfigured.

Benjamin Marzinski (4):
  multipathd: don't add removed/partial paths to new maps
  multipathd: finish initalization of paths added while offline
  multipathd: make "multipathd show status" busy checker better
  multipathd: print path offline message even without a checker

 libmultipath/configure.c   | 12 +++++++-
 libmultipath/discovery.c   |  2 ++
 libmultipath/structs_vec.c |  3 +-
 libmultipath/uevent.c      | 17 ++++++++----
 multipathd/main.c          | 57 ++++++++++++++++++++++++++++++++------
 5 files changed, 74 insertions(+), 17 deletions(-)

-- 
2.50.1

[PATCH 1/4] multipathd: don't add removed/partial paths to new maps

[Benjamin Marzinski]

During configure(), when the old multipath devices are added in
map_discovery(), it's possible that they have paths in the INIT_PARTIAL
or INIT_REMOVED state. These paths were not found during
path_discovery(). Don't add them to the new multipath maps.
coalesce_maps() will make sure that they point to the old maps, so they
will get cleaned up when those maps are removed.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/configure.c   | 12 +++++++++++-
 libmultipath/structs_vec.c |  3 ++-
 2 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 0bcec089..a713ddc4 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1113,7 +1113,17 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 			continue;
 		}
 
-		/* 4. path is out of scope */
+		/*
+		 * 4. The path wasn't found in path_discovery. It only exists
+		 *    in an old map.
+		 */
+		if (pp1->initialized == INIT_PARTIAL ||
+		    pp1->initialized == INIT_REMOVED) {
+			orphan_path(pp1, "path not found");
+			continue;
+		}
+
+		/* 5. path is out of scope */
 		if (refwwid && strncmp(pp1->wwid, refwwid, WWID_SIZE - 1))
 			continue;
 
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 9384de01..3e63e9c3 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -315,7 +315,8 @@ int adopt_paths(vector pathvec, struct multipath *mpp,
 					pp->dev, mpp->alias);
 				continue;
 			}
-			if (pp->initialized == INIT_REMOVED)
+			if (pp->initialized == INIT_REMOVED ||
+			    pp->initialized == INIT_PARTIAL)
 				continue;
 			if (mpp->queue_mode == QUEUE_MODE_RQ &&
 			    pp->bus == SYSFS_BUS_NVME &&
-- 
2.50.1

[PATCH 2/4] multipathd: finish initalization of paths added while offline

[Benjamin Marzinski]

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 <bmarzins@redhat.com>
---
 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;
+	}
+	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) &&
 	    (newstate == PATH_UP || newstate == PATH_GHOST) &&
 	    ((pp->state != PATH_UP && pp->state != PATH_GHOST) ||
-	     pp->dmstate == PSTATE_FAILED) &&
-	    check_path_wwid_change(pp)) {
-		condlog(0, "%s: path wwid change detected. Removing", pp->dev);
-		return handle_path_wwid_change(pp, vecs)? CHECK_PATH_REMOVED :
-							  CHECK_PATH_SKIPPED;
+	     pp->dmstate == PSTATE_FAILED)) {
+		bool wwid_changed;
+
+		if (pp->initialized == INIT_NEW)
+			wwid_changed = new_path_wwid_changed(pp, newstate);
+		else
+			wwid_changed = check_path_wwid_change(pp);
+		if (wwid_changed) {
+			condlog(0, "%s: path wwid change detected. Removing",
+				pp->dev);
+			return handle_path_wwid_change(pp, vecs)
+				       ? CHECK_PATH_REMOVED
+				       : CHECK_PATH_SKIPPED;
+		}
+		/*
+		 * Path was added to map while offline, mark it as
+		 * initialized.
+		 * DI_SYSFS was checked when the path was added
+		 * DI_IOCTL was checked when the checker was selected
+		 * DI_CHECKER just got checked
+		 * DI_WWID just got checked
+		 * DI_PRIO will get checked at the end of this checker loop
+		 */
+		if (pp->initialized == INIT_NEW)
+			pp->initialized = INIT_OK;
 	}
 	if ((newstate != PATH_UP && newstate != PATH_GHOST &&
 	     newstate != PATH_PENDING) && (pp->state == PATH_DELAYED)) {
-- 
2.50.1

[PATCH 3/4] multipathd: make "multipathd show status" busy checker better

[Benjamin Marzinski]

while uevent_listen() was grabbing new uevents, "multipathd show status"
would still show show busy as "False". Add a check there, to make catch
multipathd's uevent processing earlier. Also, access servicing_uev (as
well as the new variable, adding_uev) atomically, just to make sure that
the compiler doesn't do stupid things trying to optimize them.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/uevent.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index be199af0..7eae2859 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -51,7 +51,8 @@ static pthread_cond_t uev_cond = PTHREAD_COND_INITIALIZER;
 static pthread_cond_t *uev_condp = &uev_cond;
 static uev_trigger *my_uev_trigger;
 static void *my_trigger_data;
-static int servicing_uev;
+static int servicing_uev; /* uatomic access only */
+static int adding_uev;	  /* uatomic access only */
 
 struct uevent_filter_state {
 	struct list_head uevq;
@@ -70,13 +71,14 @@ static void reset_filter_state(struct uevent_filter_state *st)
 
 int is_uevent_busy(void)
 {
-	int empty, servicing;
+	int empty, servicing, adding;
 
 	pthread_mutex_lock(uevq_lockp);
 	empty = list_empty(&uevq);
-	servicing = servicing_uev;
+	servicing = uatomic_read(&servicing_uev);
+	adding = uatomic_read(&adding_uev);
 	pthread_mutex_unlock(uevq_lockp);
-	return (!empty || servicing);
+	return (!empty || servicing || adding);
 }
 
 struct uevent * alloc_uevent (void)
@@ -536,7 +538,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),
 		pthread_cleanup_push(cleanup_mutex, uevq_lockp);
 		pthread_mutex_lock(uevq_lockp);
 
-		servicing_uev = !list_empty(&filter_state.uevq);
+		uatomic_set(&servicing_uev, !list_empty(&filter_state.uevq));
 
 		while (list_empty(&filter_state.uevq) && list_empty(&uevq)) {
 			condlog(4, "%s: waiting for events", __func__);
@@ -544,7 +546,7 @@ int uevent_dispatch(int (*uev_trigger)(struct uevent *, void * trigger_data),
 			condlog(4, "%s: waking up", __func__);
 		}
 
-		servicing_uev = 1;
+		uatomic_set(&servicing_uev, 1);
 		/*
 		 * "old_tail" is the list element towards which merge_uevq()
 		 * will iterate: the last element of uevq before
@@ -730,6 +732,7 @@ int uevent_listen(struct udev *udev)
 		int fdcount, events;
 		struct pollfd ev_poll = { .fd = fd, .events = POLLIN, };
 
+		uatomic_set(&adding_uev, 0);
 		fdcount = poll(&ev_poll, 1, -1);
 		if (fdcount < 0) {
 			if (errno == EINTR)
@@ -739,6 +742,8 @@ int uevent_listen(struct udev *udev)
 			err = -errno;
 			break;
 		}
+		uatomic_set(&adding_uev, 1);
+
 		events = uevent_receive_events(fd, &uevlisten_tmp, monitor);
 		if (events <= 0)
 			continue;
-- 
2.50.1

[PATCH 4/4] multipathd: print path offline message even without a checker

[Benjamin Marzinski]

If a path has a checker selected and is offline, multipathd will print a
"path offline" message. However if the checker isn't selected, for
instance because multipathd was started or reconfigured while the path
was offline, multipathd was not printing the "path offline" message.
Fix that.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/main.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 2140e432..69e4808f 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -96,12 +96,11 @@ mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed,
 
 #define LOG_MSG(lvl, pp)					\
 do {								\
-	if (pp->mpp && checker_selected(&pp->checker) &&	\
-	    lvl <= libmp_verbosity) {					\
+	if (pp->mpp && lvl <= libmp_verbosity) {		\
 		if (pp->sysfs_state == PATH_DOWN)		\
 			condlog(lvl, "%s: %s - path offline",	\
 				pp->mpp->alias, pp->dev);	\
-		else  {						\
+		else if (checker_selected(&pp->checker)) {	\
 			const char *__m =			\
 				checker_message(&pp->checker);	\
 								\
-- 
2.50.1

Re: [PATCH 4/4] multipathd: print path offline message even without a checker

[Martin Wilck]

On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> If a path has a checker selected and is offline, multipathd will
> print a
> "path offline" message. However if the checker isn't selected, for
> instance because multipathd was started or reconfigured while the
> path
> was offline, multipathd was not printing the "path offline" message.
> Fix that.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  multipathd/main.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 2140e432..69e4808f 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -96,12 +96,11 @@ mpath_pr_event_handle(struct path *pp, unsigned
> int nr_keys_needed,
>  
>  #define LOG_MSG(lvl, pp)					\
>  do {								\
> -	if (pp->mpp && checker_selected(&pp->checker) &&	\
> -	    lvl <= libmp_verbosity)
> {					\
> +	if (pp->mpp && lvl <= libmp_verbosity) {		\
>  		if (pp->sysfs_state == PATH_DOWN)		\

While at it, we should probably change this to "p->sysfs_state !=
PATH_UP", because start_path_check will only call the checker for the
PATH_UP case, and in other cases, the checker state will be stale.

Regards,
Martin

Re: [PATCH 3/4] multipathd: make "multipathd show status" busy checker better

[Martin Wilck]

On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> while uevent_listen() was grabbing new uevents, "multipathd show
> status"
> would still show show busy as "False". Add a check there, to make
> catch
> multipathd's uevent processing earlier. Also, access servicing_uev
> (as
> well as the new variable, adding_uev) atomically, just to make sure
> that
> the compiler doesn't do stupid things trying to optimize them.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

access to servicing_uev is protected by uevq_lockp. The
atomic annotation isn't necessary for it, AFAICT.

I have to admit that I wasn't fully aware of the semantics of "busy".
Naïvely, one would assume that it means "some thread of multipathd is
doing something". But your intention in 69cb2d8 ("multipath: add "count
paths" multipathd command") was that it means "states of paths and/or
maps are imminent because of current uevent processing". With that in
mind, taking "adding_uev" into account makes sense. I'm not so sure
about the practical relevance, though. How likely is it to hit a
situation where the uevent listener is working but the uevent
dispatcher hasn't been woken up yet?

Martin

Re: [PATCH 1/4] multipathd: don't add removed/partial paths to new maps

[Martin Wilck]

On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> During configure(), when the old multipath devices are added in
> map_discovery(), it's possible that they have paths in the
> INIT_PARTIAL
> or INIT_REMOVED state. These paths were not found during
> path_discovery(). Don't add them to the new multipath maps.
> coalesce_maps() will make sure that they point to the old maps, so
> they
> will get cleaned up when those maps are removed.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

Re: [PATCH 2/4] multipathd: finish initalization of paths added while offline

[Martin Wilck]

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 <bmarzins@redhat.com>

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.

> +	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:


	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

Re: [PATCH 2/4] multipathd: finish initalization of paths added while offline

[Benjamin Marzinski]

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 <bmarzins@redhat.com>
> 
> 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

Re: [PATCH 3/4] multipathd: make "multipathd show status" busy checker better

[Benjamin Marzinski]

On Wed, Jan 21, 2026 at 10:27:51AM +0100, Martin Wilck wrote:
> On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> > while uevent_listen() was grabbing new uevents, "multipathd show
> > status"
> > would still show show busy as "False". Add a check there, to make
> > catch
> > multipathd's uevent processing earlier. Also, access servicing_uev
> > (as
> > well as the new variable, adding_uev) atomically, just to make sure
> > that
> > the compiler doesn't do stupid things trying to optimize them.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> access to servicing_uev is protected by uevq_lockp. The
> atomic annotation isn't necessary for it, AFAICT.

True. I can remove that.
 
> I have to admit that I wasn't fully aware of the semantics of "busy".
> Naïvely, one would assume that it means "some thread of multipathd is
> doing something". But your intention in 69cb2d8 ("multipath: add "count
> paths" multipathd command") was that it means "states of paths and/or
> maps are imminent because of current uevent processing". With that in
> mind, taking "adding_uev" into account makes sense. I'm not so sure
> about the practical relevance, though. How likely is it to hit a
> situation where the uevent listener is working but the uevent
> dispatcher hasn't been woken up yet?

I saw it happen at the start of a uevent surge. To be honest, I don't
know why uevent_receive_events() took so long to return, but it seemed
like extending the check is a pretty painless way to catch that.

-Ben

> Martin

Re: [PATCH 4/4] multipathd: print path offline message even without a checker

[Benjamin Marzinski]

On Wed, Jan 21, 2026 at 10:00:21AM +0100, Martin Wilck wrote:
> On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> > If a path has a checker selected and is offline, multipathd will
> > print a
> > "path offline" message. However if the checker isn't selected, for
> > instance because multipathd was started or reconfigured while the
> > path
> > was offline, multipathd was not printing the "path offline" message.
> > Fix that.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  multipathd/main.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/multipathd/main.c b/multipathd/main.c
> > index 2140e432..69e4808f 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -96,12 +96,11 @@ mpath_pr_event_handle(struct path *pp, unsigned
> > int nr_keys_needed,
> >  
> >  #define LOG_MSG(lvl, pp)					\
> >  do {								\
> > -	if (pp->mpp && checker_selected(&pp->checker) &&	\
> > -	    lvl <= libmp_verbosity)
> > {					\
> > +	if (pp->mpp && lvl <= libmp_verbosity) {		\
> >  		if (pp->sysfs_state == PATH_DOWN)		\
> 
> While at it, we should probably change this to "p->sysfs_state !=
> PATH_UP", because start_path_check will only call the checker for the
> PATH_UP case, and in other cases, the checker state will be stale.

Sure, although I'm pretty sure that will just mean that removed paths
might see a final log message, since PATH_PENDING paths won't see any,
and the only other sysfs_state options are PATH_UP and PATH_DOWN.
Actually, if we print an explicit "path removed" message, it could be
useful on the off chance that a removed path didn't transistion to
INIT_REMOVED like it should.

-Ben
 
> Regards,
> Martin

Re: [PATCH 4/4] multipathd: print path offline message even without a checker

[Martin Wilck]

On Wed, 2026-01-21 at 11:02 -0500, Benjamin Marzinski wrote:
> On Wed, Jan 21, 2026 at 10:00:21AM +0100, Martin Wilck wrote:
> > On Tue, 2026-01-20 at 23:23 -0500, Benjamin Marzinski wrote:
> > > If a path has a checker selected and is offline, multipathd will
> > > print a
> > > "path offline" message. However if the checker isn't selected,
> > > for
> > > instance because multipathd was started or reconfigured while the
> > > path
> > > was offline, multipathd was not printing the "path offline"
> > > message.
> > > Fix that.
> > > 
> > > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > > ---
> > >  multipathd/main.c | 5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/multipathd/main.c b/multipathd/main.c
> > > index 2140e432..69e4808f 100644
> > > --- a/multipathd/main.c
> > > +++ b/multipathd/main.c
> > > @@ -96,12 +96,11 @@ mpath_pr_event_handle(struct path *pp,
> > > unsigned
> > > int nr_keys_needed,
> > >  
> > >  #define LOG_MSG(lvl,
> > > pp)					\
> > >  do
> > > {								\
> > > -	if (pp->mpp && checker_selected(&pp->checker)
> > > &&	\
> > > -	    lvl <= libmp_verbosity)
> > > {					\
> > > +	if (pp->mpp && lvl <= libmp_verbosity)
> > > {		\
> > >  		if (pp->sysfs_state ==
> > > PATH_DOWN)		\
> > 
> > While at it, we should probably change this to "p->sysfs_state !=
> > PATH_UP", because start_path_check will only call the checker for
> > the
> > PATH_UP case, and in other cases, the checker state will be stale.
> 
> Sure, although I'm pretty sure that will just mean that removed paths
> might see a final log message, since PATH_PENDING paths won't see
> any,
> and the only other sysfs_state options are PATH_UP and PATH_DOWN.
> Actually, if we print an explicit "path removed" message, it could be
> useful on the off chance that a removed path didn't transistion to
> INIT_REMOVED like it should.

I'm fine with always printing "offline" here, which at that point would
just mean that no checker state could be retrieved.

Martin