[dm-devel] [PATCH] mpathpersist: fix one more crash possiblity

Benjamin Marzinski bmarzins at redhat.com
Sat May 13 05:44:58 UTC 2017


when mpathpersist goes to rollback reservations, it doesn't do a
rollback (and thus doesn't create a pthread) on paths that didn't
successfully create a reservation in the first place. It also doesn't
do any rollbacks at all if the last path in the multipath device was down,
which is completely broken. However it still tries to join these
non-existant pthreads, causing crashes.  To fix this, set the status to
-1 (now MPATH_PR_SKIP) on threadinfos that don't get rollbacks, and
remove the broken path state checking.

Signed-off-by: Benjamin Marzinski <bmarzins at redhat.com>
---
 libmpathpersist/mpath_persist.c | 13 +++++++------
 libmpathpersist/mpath_persist.h |  1 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index bf06efe..7fd4cb8 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -481,7 +481,7 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 		thread[i].param.rq_type = rq_type;
 		thread[i].param.paramp = paramp;
 		thread[i].param.noisy = noisy;
-		thread[i].param.status = -1;
+		thread[i].param.status = MPATH_PR_SKIP;
 
 		condlog (3, "THRED ID [%d] INFO]", i);
 		condlog (3, "rq_servact=%d ", thread[i].param.rq_servact);
@@ -548,8 +548,7 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 	if (rollback && ((rq_servact == MPATH_PROUT_REG_SA) && sa_key != 0 )){
 		condlog (3, "%s: ERROR: initiating pr out rollback", mpp->wwid);
 		for( i=0 ; i < active_pathcount ; i++){
-			if((thread[i].param.status == MPATH_PR_SUCCESS) &&
-					((pp->state == PATH_UP) || (pp->state == PATH_GHOST))){
+			if(thread[i].param.status == MPATH_PR_SUCCESS) {
 				memcpy(&thread[i].param.paramp->key, &thread[i].param.paramp->sa_key, 8);
 				memset(&thread[i].param.paramp->sa_key, 0, 8);
 				thread[i].param.status = MPATH_PR_SUCCESS;
@@ -559,10 +558,12 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 					condlog (0, "%s: failed to create thread for rollback. %d",  mpp->wwid, rc);
 					thread[i].param.status = MPATH_PR_THREAD_ERROR;
 				}
-			}
+			} else
+				thread[i].param.status = MPATH_PR_SKIP;
 		}
 		for(i=0; i < active_pathcount ; i++){
-			if (thread[i].param.status != MPATH_PR_THREAD_ERROR) {
+			if (thread[i].param.status != MPATH_PR_SKIP &&
+			    thread[i].param.status != MPATH_PR_THREAD_ERROR) {
 				rc = pthread_join(thread[i].id, NULL);
 				if (rc){
 					condlog (3, "%s: failed to join thread while rolling back %d",
@@ -676,7 +677,7 @@ int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope,
 		thread[i].param.rq_type = rq_type;
 		thread[i].param.paramp = paramp;
 		thread[i].param.noisy = noisy;
-		thread[i].param.status = -1;
+		thread[i].param.status = MPATH_PR_SKIP;
 
 		condlog (3, " path count = %d", i);
 		condlog (3, "rq_servact=%d ", thread[i].param.rq_servact);
diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h
index 0f95943..1a34ce7 100644
--- a/libmpathpersist/mpath_persist.h
+++ b/libmpathpersist/mpath_persist.h
@@ -43,6 +43,7 @@ extern "C" {
 
 
 /* PR RETURN_STATUS */
+#define MPATH_PR_SKIP			-1  /* skipping this path */
 #define MPATH_PR_SUCCESS		0
 #define MPATH_PR_SYNTAX_ERROR		1   /*  syntax error or invalid parameter */
 					    /* status for check condition */
-- 
1.8.3.1




More information about the dm-devel mailing list