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 <bmarzins@xxxxxxxxxx>
> ---
> 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?
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){
