Re: [PATCH v4] mulitpath-tools Add purge capability to multipath-tools

[Benjamin Marzinski <bmarzins@redhat.com>]

On Thu, Dec 11, 2025 at 08:49:54AM -0800, Brian Bunker wrote:
>
> >> @@ -2474,6 +2572,20 @@ get_new_state(struct path *pp)
> >> if (newstate == PATH_REMOVED)
> >> newstate = PATH_DOWN;
> >> 
> >> + /*
> >> +  * For PATH_DISOCONNECTED mark the path as OK to purge if that is
> >> +  * enabled. Whether or not purge is enabled mark the path as down.
> >> +  */
> >> + if (newstate == PATH_DISCONNECTED) {
> >> + if (pp->mpp->purge_disconnected == PURGE_DISCONNECTED_ON &&
> >> +     !pp->purge_path) {
> >> + condlog(2, "%s: mark (%s) path for purge",
> >> + pp->dev, checker_state_name(newstate));
> >> + pp->purge_path = true;
> >> + }
> >> + newstate = PATH_DOWN;
> >> + }
> >> +
> > 
> > There are a number of cases where we do not immediately reinstate paths
> > that are usable. But we should cancel any pending purges as soon as we
> > notice that the path is not longer disconnected, instead of in
> > reinstate_path().
> > 
> > Here's another thought. If a path ever goes into PATH_DISCONNECTED, we
> > probably want to force a wwid recheck if it ever comes back, even if we
> > don't purge it.  Possibly we could rename pp->purge_path to
> > pp->disconnected, and make it an enum with values like
> > 
> > NOT_DICONNECTED
> > DISCONNECTED_NO_PURGE
> > DISCONNECTED_NEEDS_PURGE
> 
> I like this idea. I have added the enum. It makes the decisions about
> what to do when things fail easy to distinguish between the original
> state and some step in the purge process.
> > 
> > That way, even if mpp->purge_disconnected is not set, we could still
> > flag the path as disconnected. In the code to check
> > if we should call check_path_wwid_change() in update_path_state(),
> > we could then have the first part of that check be
> > 
> > if ((pp->recheck_wwid == RECHECK_WWID_ON ||
> >     pp->disconnected != NOT_DICONNECTED) && ...
> > 
> > so that we enforce a wwid_recheck of a path that was disconnected.  For
> > this to work, the above code would set paths in PATH_DISCONNECTED
> > to DISCONNECTED_NEEDS_PURGE or DISCONNECTED_NO_PURGE depending on
> > pp->mpp->purge_disconnected. If the path wasn't in PATH_DISCONNECTED,
> > but pp->disconnected was set to DISCONNECTED_NEEDS_PURGE, it would
> > change pp->disconnected to DISCONNECTED_NO_PURGE, so that that path
> > wouldn't get purged.
> > 
> > Paths in DISCONNECTED_NO_PURGE would get changed back to NOT_DICONNECTED
> > once check_path_wwid_change() was called on them (that call would need
> > to be broken out of the large if statement in update_path_state)
> > 
> > Thoughts? That can also go in as a later patch.
> 
> I am not sure that recheck_wwid comes into play with what I am doing.
> Not that it doesn’t still happen. It still does. With the disconnected state
> not affecting the path state, I don’t think any extra logic is required. The
> disconnected state puts the path state to PATH_DOWN. There aren’t
> any extra situations are there where recheck_wwid is needed except
> after coming out of PATH_DOWN right? It happens with or without 
> disconnected in play, doesn’t it? Am I missing something?

recheck_wwid is disabled by default. I'm not sure how many people turn
it on. Likewise, I'm not sure how many people will turn
purge_disconnected on. My idea is to modify the check for calling
check_path_wwid_change() so it will be called when a path is restored if
EITHER recheck_wwid is enabled OR if the path was previously
disconnected (regardless of the purge_disconnected setting). That way,
even if users don't enable recheck_wwid or purge_disconnected (the
common case, I assume) we will enforce wwid_rechecks if a path got
disconnected, since it's WAY more likely to be pointing at something
different now, in that case.

> 
> I will add the recheck_wwid in the reinstate_path case though if that
> is a place PATH_DOWN could return to PATH_UP.

Just to be clear, I'm not asking you to add another place to recheck the
wwid. I'm saying that we should modify the "if" statement that calls
check_path_wwid_change() in update_path_state() so that, instead of
starting with

if (pp->recheck_wwid == RECHECK_WWID_ON &&
...

it starts with

if ((pp->recheck_wwid == RECHECK_WWID_ON ||
     (pp->disconnected != NOT_DICONNECTED && can_recheck_wwid(pp))) &&
...

(I forget the necessity of can_recheck_wwid() in my previous comment)

-Ben