Re: [PATCH 1/2] libmutipath: handle blacklisted paths on map_discovery

[Benjamin Marzinski <bmarzins@redhat.com>]

On Tue, Apr 29, 2025 at 09:59:40PM +0200, Martin Wilck wrote:
> On Mon, 2025-04-28 at 17:45 -0400, Benjamin Marzinski wrote:
> > If the multipath configuration is changed to blacklist existing
> > devices,
> > and multipathd is reloaded but the blacklisted multipaths device
> > can't
> > be removed, multipathd was marking the paths as INIT_PARTIAL, causing
> > them to stay in the multipath device, at least until the
> > partial_retrigger_delay timeout elapsed. Instead, mark them as
> > INIT_REMOVED and set mpp->need_reload, so the device is reloaded and
> > the
> > paths are removed. To make sure the blacklisted paths are deleted
> > when
> > the multipath device is removed in coalesce_maps(), set their pp->mpp
> > to point to map before removing it.
> > 
> > Fixes d9c61332 ("multipathd: trigger uevents for blacklisted paths in
> > reconfigure")
> 
> The patch looks good to me, but I only vaguely understand why the bug
> is introduced in d9c61332. Are you positive that before d9c61332, the
> hang didn't occur?

Well, I'm pretty sure these blacklisted paths stopped getting deleted
during reconfigure by d9c61332. Before d9c61332, blacklisted paths were
immediately deleted in update_pathvec_from_dm(). After this change

@@ -193,7 +196,8 @@ static void update_pathvec_from_dm(vector pathvec, struct multipath *mpp,
 		rc = pathinfo(pp, conf,
 		              DI_SYSFS|DI_WWID|DI_BLACKLIST|DI_NOFALLBACK|pathinfo_flags);
 		pthread_cleanup_pop(1);
-		if (rc != PATHINFO_OK) {
+		if (rc == PATHINFO_FAILED ||
+		    (rc == PATHINFO_SKIPPED && !map_discovery)) {
 		        condlog(1, "%s: error %d in pathinfo, discarding path",
 		                pp->dev, rc);
 		        vector_del_slot(pgp->paths, j--);

they started hanging around, so that uevents could be generated for them
by trigger_paths_udev_change(). However, since coalesce_paths() will
usually clear pp->mpp, they won't get removed when orphan_paths() is
later called by remove_maps() to remove the old maps. I admit I didn't
test if the paths got removed before that commit, but the commit message
says that they did.

-Ben

> 
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Reviewed-by: Martin Wilck <mwilck@suse.com>