On Sun, Aug 24, 2025 at 05:33:07PM +0200, Martin Wilck wrote:
> On Thu, 2025-07-10 at 14:10 -0400, Benjamin Marzinski wrote:
> > If none of the threads succeeds in issuing the release, simply return
> > failure, instead of trying the workaround.
> >
> > Signed-off-by: Benjamin Marzinski <bmarz...@redhat.com>
> > ---
> > libmpathpersist/mpath_persist_int.c | 12 +++++++++---
> > 1 file changed, 9 insertions(+), 3 deletions(-)
> >
> > diff --git a/libmpathpersist/mpath_persist_int.c
> > b/libmpathpersist/mpath_persist_int.c
> > index 7fb08b2e..ad5a4ee7 100644
> > --- a/libmpathpersist/mpath_persist_int.c
> > +++ b/libmpathpersist/mpath_persist_int.c
> > @@ -475,15 +475,21 @@ static int mpath_prout_rel(struct multipath
> > *mpp,int rq_servact, int rq_scope,
> > }
> > }
> >
> > + rc = MPATH_PR_DMMP_ERROR;
>
> I find the relationship between "status" and "rc" a bit confusing in
> this patch. Can we use something like "bool all_failed" instead?
Sure.
-Ben
>
> Martin
>
>
> > for (i = 0; i < count; i++){
> > /* check thread status here and return the status
> > */
> >
> > - if (thread[i].param.status ==
> > MPATH_PR_RESERV_CONFLICT)
> > + if (thread[i].param.status == MPATH_PR_SUCCESS)
> > + rc = MPATH_PR_SUCCESS;
> > + else if (thread[i].param.status ==
> > MPATH_PR_RESERV_CONFLICT)
> > status = MPATH_PR_RESERV_CONFLICT;
> > - else if (status == MPATH_PR_SUCCESS
> > - && thread[i].param.status !=
> > MPATH_PR_RESERV_CONFLICT)
> > + else if (status == MPATH_PR_SUCCESS)
> > status = thread[i].param.status;
> > }
> > + if (rc != MPATH_PR_SUCCESS) {
> > + condlog(0, "%s: all threads failed to release
> > reservation.", mpp->wwid);
> > + return status;
> > + }
> >
> > status = mpath_prin_activepath (mpp, MPATH_PRIN_RRES_SA,
> > &resp, noisy);
> > if (status != MPATH_PR_SUCCESS){
