* [PATCH 1/2] libmpathpersist: retry commands on other paths in mpath_prout_common
2025-06-04 3:35 [PATCH 0/2] libmpathpersist: reserve and release fixes Benjamin Marzinski
@ 2025-06-04 3:35 ` Benjamin Marzinski
2025-06-04 3:35 ` [PATCH 2/2] libmpathpersist: check released key against the reservation key Benjamin Marzinski
2025-06-11 20:12 ` [PATCH 0/2] libmpathpersist: reserve and release fixes Martin Wilck
2 siblings, 0 replies; 4+ messages in thread
From: Benjamin Marzinski @ 2025-06-04 3:35 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
mpath_prout_common() will only try sending the prout command to one
path. If that fails, it will give up. There are a number of cases where
it is reasonable to assume that sending the command down another path
could succeed. Keep trying other available paths in these cases.
Do do this, this patch adds a new error code, MPATH_PR_RETRYABLE_ERROR.
libmpathpersist will not return this error to users. It will change it
to MPATH_PR_OTHER if it fails trying on all the paths.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathpersist/mpath_persist.h | 3 +++
libmpathpersist/mpath_persist_int.c | 13 +++++++++----
libmpathpersist/mpath_pr_ioctl.c | 24 +++++++++++++-----------
3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h
index d94205f0..32aa7ffe 100644
--- a/libmpathpersist/mpath_persist.h
+++ b/libmpathpersist/mpath_persist.h
@@ -62,6 +62,9 @@ extern "C" {
#define MPATH_PR_THREAD_ERROR 14 /* pthreads error (e.g. unable to create new thread) */
#define MPATH_PR_OTHER 15 /*other error/warning has occurred(transport
or driver error) */
+#define MPATH_PR_RETRYABLE_ERROR 16 /* error that might be succeed
+ down another path. Internal
+ only. */
/* PR MASK */
#define MPATH_F_APTPL_MASK 0x01 /* APTPL MASK*/
diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c
index 7a6992c2..b838a014 100644
--- a/libmpathpersist/mpath_persist_int.c
+++ b/libmpathpersist/mpath_persist_int.c
@@ -92,7 +92,7 @@ static int mpath_prin_activepath (struct multipath *mpp, int rq_servact,
}
}
}
- return ret;
+ return (ret == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : ret;
}
void *mpath_alloc_prin_response(int prin_sa)
@@ -372,7 +372,7 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
}
pthread_attr_destroy(&attr);
- return (status);
+ return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status;
}
static int send_prout_activepath(char *dev, int rq_servact, int rq_scope,
@@ -416,6 +416,7 @@ static int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope
int i,j, ret;
struct pathgroup *pgp = NULL;
struct path *pp = NULL;
+ bool found = false;
vector_foreach_slot (mpp->pg, pgp, j){
vector_foreach_slot (pgp->paths, pp, i){
@@ -426,12 +427,16 @@ static int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope
}
condlog (3, "%s: sending pr out command to %s", mpp->wwid, pp->dev);
+ found = true;
ret = send_prout_activepath(pp->dev, rq_servact,
rq_scope, rq_type,
paramp, noisy);
- return ret ;
+ if (ret != MPATH_PR_RETRYABLE_ERROR)
+ return ret ;
}
}
+ if (found)
+ return MPATH_PR_OTHER;
condlog (0, "%s: no path available", mpp->wwid);
return MPATH_PR_DMMP_ERROR;
}
@@ -635,7 +640,7 @@ out1:
free (pamp);
out:
free (pr_buff);
- return (status);
+ return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status;
}
int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd,
diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 093ec71b..bf3a817d 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -52,7 +52,7 @@ int prout_do_scsi_ioctl(char * dev, int rq_servact, int rq_scope,
fd = open(devname, O_RDONLY);
if(fd < 0){
condlog (1, "%s: unable to open device.", dev);
- return MPATH_PR_FILE_ERROR;
+ return MPATH_PR_RETRYABLE_ERROR;
}
unsigned char cdb[MPATH_PROUT_CMDLEN] =
@@ -123,14 +123,16 @@ retry :
goto retry;
}
- if (((status == MPATH_PR_SENSE_NOT_READY )&& (Sensedata.ASC == 0x04)&&
- (Sensedata.ASCQ == 0x07))&& (retry > 0))
- {
- usleep(1000);
- --retry;
- condlog(3, "%s: retrying for sense 02/04/07."
- " Remaining retries = %d", dev, retry);
- goto retry;
+ if (status == MPATH_PR_SENSE_NOT_READY) {
+ if (Sensedata.ASC == 0x04 && Sensedata.ASCQ == 0x07 &&
+ retry > 0) {
+ usleep(1000);
+ --retry;
+ condlog(3, "%s: retrying for sense 02/04/07."
+ " Remaining retries = %d", dev, retry);
+ goto retry;
+ } else
+ status = MPATH_PR_RETRYABLE_ERROR;
}
close(fd);
@@ -342,7 +344,7 @@ int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * resp, int
fd = open(devname, O_RDONLY);
if(fd < 0){
condlog(0, "%s: Unable to open device ", dev);
- return MPATH_PR_FILE_ERROR;
+ return MPATH_PR_RETRYABLE_ERROR;
}
if (mpath_mx_alloc_len)
@@ -488,7 +490,7 @@ int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr,
case DID_OK :
break;
default :
- return MPATH_PR_OTHER;
+ return MPATH_PR_RETRYABLE_ERROR;
}
switch(io_hdr.driver_status)
{
--
2.48.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* [PATCH 2/2] libmpathpersist: check released key against the reservation key
2025-06-04 3:35 [PATCH 0/2] libmpathpersist: reserve and release fixes Benjamin Marzinski
2025-06-04 3:35 ` [PATCH 1/2] libmpathpersist: retry commands on other paths in mpath_prout_common Benjamin Marzinski
@ 2025-06-04 3:35 ` Benjamin Marzinski
2025-06-11 20:12 ` [PATCH 0/2] libmpathpersist: reserve and release fixes Martin Wilck
2 siblings, 0 replies; 4+ messages in thread
From: Benjamin Marzinski @ 2025-06-04 3:35 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
According to the SCSI Spec, if a persistent reservation RELEASE is
issued using the correct key for the I_T_L Nexus that it is issued on
but the reservation is held by a different key, the command should
return Success and do nothing. When libmpathpersist tried to release a
reservation that was held by a different key, it ended up using the
failback code designed to release a reservation held by a failed path to
release it anyways. This means that any node could release a scsi
persistent reservation held by another node. Fix this to follow the SCSI
Spec.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathpersist/mpath_persist_int.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c
index b838a014..05739db8 100644
--- a/libmpathpersist/mpath_persist_int.c
+++ b/libmpathpersist/mpath_persist_int.c
@@ -541,6 +541,13 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope,
condlog (2, "%s: Path holding reservation is released.", mpp->wwid);
return MPATH_PR_SUCCESS;
}
+ if (!get_be64(mpp->reservation_key) ||
+ memcmp(&mpp->reservation_key, resp.prin_descriptor.prin_readresv.key, 8)) {
+ condlog(2, "%s: Releasing key not holding reservation.",
+ mpp->wwid);
+ return MPATH_PR_SUCCESS;
+ }
+
condlog (2, "%s: Path holding reservation is not available.", mpp->wwid);
pr_buff = mpath_alloc_prin_response(MPATH_PRIN_RFSTAT_SA);
--
2.48.1
^ permalink raw reply related [flat|nested] 4+ messages in thread