From 695f9436f416b1943bae564004bed58cfecfd9bc Mon Sep 17 00:00:00 2001 From: Benjamin Marzinski Date: Wed, 1 Oct 2025 16:53:32 -0400 Subject: [PATCH] device-mapper-multipath-0.9.9-13 Add 0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch Add 0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch Add 0037-libmpathpersist-check-released-key-against-the-reser.patch Add 0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch Add 0039-libmpathpersist-remove-uneeded-wrapper-function.patch Add 0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch Add 0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch Add 0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch Add 0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch Add 0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch Add 0045-libmpathpersist-remove-useless-function-send_prout_a.patch Add 0046-libmpathpersist-redesign-failed-release-workaround.patch Add 0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch Add 0048-libmpathpersist-Handle-changing-key-corner-case.patch Add 0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch Add 0050-libmultipath-rename-prflag_value-enums.patch Add 0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch Add 0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch Add 0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch Add 0054-libmpathpersist-move-update_map_pr-to-multipathd.patch Add 0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch Add 0056-libmpathpersist-clean-up-duplicate-function-declarat.patch Add 0057-multipathd-wrap-setting-and-unsetting-prflag.patch Add 0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch Add 0059-libmpathpersist-Fix-up-reservation_key-checking.patch Add 0060-libmpathpersist-change-how-reservation-conflicts-are.patch Add 0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch Add 0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch Add 0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch Add 0064-libmpathpersist-update-reservation-key-before-checki.patch Add 0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch Add 0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch Add 0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch Add 0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch Add 0069-libmpathpersist-reregister-keys-when-self-preempting.patch Add 0070-libmpathpersist-handle-updating-key-race-condition.patch Add 0071-libmpathpersist-handle-preempting-all-registrants-re.patch Add 0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch Add 0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch Add 0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch Add 0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch Add 0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch Add 0077-multipathd-Fix-tracking-of-old-PR-key.patch * Fixes RHEL-118720 ("There are many bugs in multipath's persistent reservation handling [rhel-10]") Resolves: RHEL-118720 --- ...t-fix-memory-leak-in-mpath_prout_rel.patch | 72 +++ ...retry-commands-on-other-paths-in-mpa.patch | 164 +++++++ ...check-released-key-against-the-reser.patch | 39 ++ ...ve-thread-from-mpath_pr_event_handle.patch | 98 ++++ ...sist-remove-uneeded-wrapper-function.patch | 47 ++ ...reduce-log-level-for-persistent-rese.patch | 102 ++++ ...remove-pointless-update_map_pr-ret-v.patch | 28 ++ ...pdate_map_pr-in-mpath_pr_event_handl.patch | 217 +++++++++ ...limit-changing-prflag-in-update_map_.patch | 77 +++ ...n-t-call-update_map_pr-unnecessarily.patch | 77 +++ ...remove-useless-function-send_prout_a.patch | 74 +++ ...t-redesign-failed-release-workaround.patch | 293 ++++++++++++ ...fail-the-release-if-all-threads-fail.patch | 51 ++ ...sist-Handle-changing-key-corner-case.patch | 191 ++++++++ ...Handle-REGISTER-AND-IGNORE-changing-.patch | 182 +++++++ ...bmultipath-rename-prflag_value-enums.patch | 249 ++++++++++ ...use-a-switch-statement-for-prout-com.patch | 44 ++ ...Add-safety-check-for-preempting-on-k.patch | 444 ++++++++++++++++++ ...remove-update_map_pr-code-for-NULL-p.patch | 35 ++ ...ist-move-update_map_pr-to-multipathd.patch | 222 +++++++++ ...-up-update_map_pr-and-mpath_pr_event.patch | 139 ++++++ ...clean-up-duplicate-function-declarat.patch | 65 +++ ...hd-wrap-setting-and-unsetting-prflag.patch | 148 ++++++ ...ister-PR-key-when-path-is-restored-i.patch | 145 ++++++ ...sist-Fix-up-reservation_key-checking.patch | 112 +++++ ...change-how-reservation-conflicts-are.patch | 139 ++++++ ...Clear-prkey-in-multipathd-before-unr.patch | 51 ++ ...only-clear-the-key-if-we-are-using-t.patch | 28 ++ ...Restore-old-reservation-key-on-failu.patch | 51 ++ ...update-reservation-key-before-checki.patch | 151 ++++++ ...retry-on-conflicts-in-mpath_prout_co.patch | 79 ++++ ...Don-t-always-fail-registrations-for-.patch | 154 ++++++ ...Don-t-try-release-workaround-for-inv.patch | 46 ++ ...Don-t-fail-RESERVE-commands-unnecess.patch | 43 ++ ...reregister-keys-when-self-preempting.patch | 193 ++++++++ ...t-handle-updating-key-race-condition.patch | 108 +++++ ...handle-preempting-all-registrants-re.patch | 82 ++++ ...Fix-REGISTER-AND-IGNORE-while-holdin.patch | 73 +++ ...Handle-RESERVE-with-reservation-held.patch | 159 +++++++ ...use-check_holding_reservation-in-mpa.patch | 149 ++++++ ...Fix-unregistering-while-holding-the-.patch | 262 +++++++++++ ...Fix-race-between-restoring-a-path-an.patch | 214 +++++++++ ...ultipathd-Fix-tracking-of-old-PR-key.patch | 49 ++ device-mapper-multipath.spec | 93 +++- 44 files changed, 5438 insertions(+), 1 deletion(-) create mode 100644 0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch create mode 100644 0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch create mode 100644 0037-libmpathpersist-check-released-key-against-the-reser.patch create mode 100644 0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch create mode 100644 0039-libmpathpersist-remove-uneeded-wrapper-function.patch create mode 100644 0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch create mode 100644 0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch create mode 100644 0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch create mode 100644 0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch create mode 100644 0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch create mode 100644 0045-libmpathpersist-remove-useless-function-send_prout_a.patch create mode 100644 0046-libmpathpersist-redesign-failed-release-workaround.patch create mode 100644 0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch create mode 100644 0048-libmpathpersist-Handle-changing-key-corner-case.patch create mode 100644 0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch create mode 100644 0050-libmultipath-rename-prflag_value-enums.patch create mode 100644 0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch create mode 100644 0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch create mode 100644 0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch create mode 100644 0054-libmpathpersist-move-update_map_pr-to-multipathd.patch create mode 100644 0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch create mode 100644 0056-libmpathpersist-clean-up-duplicate-function-declarat.patch create mode 100644 0057-multipathd-wrap-setting-and-unsetting-prflag.patch create mode 100644 0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch create mode 100644 0059-libmpathpersist-Fix-up-reservation_key-checking.patch create mode 100644 0060-libmpathpersist-change-how-reservation-conflicts-are.patch create mode 100644 0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch create mode 100644 0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch create mode 100644 0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch create mode 100644 0064-libmpathpersist-update-reservation-key-before-checki.patch create mode 100644 0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch create mode 100644 0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch create mode 100644 0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch create mode 100644 0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch create mode 100644 0069-libmpathpersist-reregister-keys-when-self-preempting.patch create mode 100644 0070-libmpathpersist-handle-updating-key-race-condition.patch create mode 100644 0071-libmpathpersist-handle-preempting-all-registrants-re.patch create mode 100644 0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch create mode 100644 0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch create mode 100644 0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch create mode 100644 0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch create mode 100644 0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch create mode 100644 0077-multipathd-Fix-tracking-of-old-PR-key.patch diff --git a/0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch b/0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch new file mode 100644 index 0000000..33baaa8 --- /dev/null +++ b/0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch @@ -0,0 +1,72 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Martin Wilck +Date: Mon, 5 May 2025 12:47:47 +0200 +Subject: [PATCH] libmpathpersist: fix memory leak in mpath_prout_rel() + +Found by Fedora's static analysis [1]. + +[1] https://openscanhub.fedoraproject.org/task/51915/log/device-mapper-multipath-0.11.1-1.fc43/scan-results.html#def44 + +Signed-off-by: Martin Wilck +Reviewed-by: Benjamin Marzinski +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 12 ++++++------ + 1 file changed, 6 insertions(+), 6 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 178c2f54..7df74fb7 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -460,10 +460,10 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + int count = 0; + int status = MPATH_PR_SUCCESS; + struct prin_resp resp; +- struct prout_param_descriptor *pamp; ++ struct prout_param_descriptor *pamp = NULL; + struct prin_resp *pr_buff; + int length; +- struct transportid *pptr; ++ struct transportid *pptr = NULL; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -570,7 +570,7 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + pamp = (struct prout_param_descriptor *)malloc (length); + if (!pamp){ + condlog (0, "%s: failed to alloc pr out parameter.", mpp->wwid); +- goto out1; ++ goto out; + } + + memset(pamp, 0, length); +@@ -580,6 +580,7 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + condlog (0, "%s: failed to alloc pr out transportid.", mpp->wwid); + goto out1; + } ++ pptr = pamp->trnptid_list[0]; + + if (get_be64(mpp->reservation_key)){ + memcpy (pamp->key, &mpp->reservation_key, 8); +@@ -591,11 +592,10 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + + if (status) { + condlog(0, "%s: failed to send CLEAR_SA", mpp->wwid); +- goto out1; ++ goto out2; + } + + pamp->num_transportid = 1; +- pptr=pamp->trnptid_list[0]; + + for (i = 0; i < num; i++){ + if (get_be64(mpp->reservation_key) && +@@ -639,7 +639,7 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + status = mpath_prout_reg(mpp, MPATH_PROUT_REG_SA, rq_scope, rq_type, pamp, noisy); + } + +- ++out2: + free(pptr); + out1: + free (pamp); diff --git a/0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch b/0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch new file mode 100644 index 0000000..678452d --- /dev/null +++ b/0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch @@ -0,0 +1,164 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 3 Jun 2025 23:35:03 -0400 +Subject: [PATCH] libmpathpersist: retry commands on other paths in + mpath_prout_common + +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 +Reviewed-by: Martin Wilck +--- + .github/actions/spelling/expect.txt | 1 + + libmpathpersist/mpath_persist.h | 3 +++ + libmpathpersist/mpath_persist_int.c | 13 +++++++++---- + libmpathpersist/mpath_pr_ioctl.c | 25 ++++++++++++++----------- + 4 files changed, 27 insertions(+), 15 deletions(-) + +diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt +index def43a52..e2d4acf9 100644 +--- a/.github/actions/spelling/expect.txt ++++ b/.github/actions/spelling/expect.txt +@@ -169,6 +169,7 @@ reconfig + redhat + restorequeueing + retrigger ++RETRYABLE + rhabarber + rootprefix + rootprefixdir +diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h +index 0046f120..b83fc3bd 100644 +--- a/libmpathpersist/mpath_persist.h ++++ b/libmpathpersist/mpath_persist.h +@@ -63,6 +63,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 7df74fb7..db105c6b 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) +@@ -382,7 +382,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, +@@ -426,6 +426,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){ +@@ -436,12 +437,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; + } +@@ -645,7 +650,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..7e1d2896 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,17 @@ 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 +345,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 +491,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) + { diff --git a/0037-libmpathpersist-check-released-key-against-the-reser.patch b/0037-libmpathpersist-check-released-key-against-the-reser.patch new file mode 100644 index 0000000..01bbea2 --- /dev/null +++ b/0037-libmpathpersist-check-released-key-against-the-reser.patch @@ -0,0 +1,39 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 3 Jun 2025 23:35:04 -0400 +Subject: [PATCH] libmpathpersist: check released key against the reservation + key + +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 +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 6 ++++++ + 1 file changed, 6 insertions(+) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index db105c6b..128671e5 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -551,6 +551,12 @@ 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); diff --git a/0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch b/0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch new file mode 100644 index 0000000..d00ad12 --- /dev/null +++ b/0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch @@ -0,0 +1,98 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:46 -0400 +Subject: [PATCH] multipathd: remove thread from mpath_pr_event_handle + +mpath_pr_event_handle() creates a separate thread to do the persistent +reservation work, but it doesn't take any advantage of the work being +done in another thread. Merge mpath_pr_event_handle() and +mpath_pr_event_handler_fn() into a single function with no separate +thread. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + multipathd/main.c | 50 +++++++++++------------------------------------ + 1 file changed, 11 insertions(+), 39 deletions(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index a565ade5..04ca47d1 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -89,8 +89,7 @@ + #define CMDSIZE 160 + #define MSG_SIZE 32 + +-int mpath_pr_event_handle(struct path *pp); +-void * mpath_pr_event_handler_fn (void * ); ++static void mpath_pr_event_handle(struct path *pp); + + #define LOG_MSG(lvl, pp) \ + do { \ +@@ -3945,17 +3944,24 @@ main (int argc, char *argv[]) + return (child(NULL)); + } + +-void * mpath_pr_event_handler_fn (void * pathp ) ++static void mpath_pr_event_handle(struct path *pp) + { + struct multipath * mpp; + unsigned int i; + int ret, isFound; +- struct path * pp = (struct path *)pathp; + struct prout_param_descriptor *param; + struct prin_resp *resp; + +- rcu_register_thread(); + mpp = pp->mpp; ++ if (pp->bus != SYSFS_BUS_SCSI) { ++ mpp->prflag = PRFLAG_UNSET; ++ return; ++ } ++ ++ if (!get_be64(mpp->reservation_key)) { ++ mpp->prflag = PRFLAG_UNSET; ++ return; ++ } + + resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); + if (!resp){ +@@ -4022,38 +4028,4 @@ void * mpath_pr_event_handler_fn (void * pathp ) + out: + if (resp) + free(resp); +- rcu_unregister_thread(); +- return NULL; +-} +- +-int mpath_pr_event_handle(struct path *pp) +-{ +- pthread_t thread; +- int rc; +- pthread_attr_t attr; +- struct multipath * mpp; +- +- if (pp->bus != SYSFS_BUS_SCSI) +- goto no_pr; +- +- mpp = pp->mpp; +- +- if (!get_be64(mpp->reservation_key)) +- goto no_pr; +- +- pthread_attr_init(&attr); +- pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); +- +- rc = pthread_create(&thread, NULL , mpath_pr_event_handler_fn, pp); +- if (rc) { +- condlog(0, "%s: ERROR; return code from pthread_create() is %d", pp->dev, rc); +- return -1; +- } +- pthread_attr_destroy(&attr); +- rc = pthread_join(thread, NULL); +- return 0; +- +-no_pr: +- pp->mpp->prflag = PRFLAG_UNSET; +- return 0; + } diff --git a/0039-libmpathpersist-remove-uneeded-wrapper-function.patch b/0039-libmpathpersist-remove-uneeded-wrapper-function.patch new file mode 100644 index 0000000..175acaa --- /dev/null +++ b/0039-libmpathpersist-remove-uneeded-wrapper-function.patch @@ -0,0 +1,47 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:47 -0400 +Subject: [PATCH] libmpathpersist: remove uneeded wrapper function. + +mpath_send_prin_activepath() just calls prin_do_scsi_ioctl() with exactly +the same arguments that it is passed, and returns the same return. remove +it. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 14 +------------- + 1 file changed, 1 insertion(+), 13 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 128671e5..fb1d0081 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -49,17 +49,6 @@ struct threadinfo { + struct prout_param param; + }; + +-static int mpath_send_prin_activepath (char * dev, int rq_servact, +- struct prin_resp * resp, int noisy) +-{ +- +- int rc; +- +- rc = prin_do_scsi_ioctl(dev, rq_servact, resp, noisy); +- +- return (rc); +-} +- + static int mpath_prin_activepath (struct multipath *mpp, int rq_servact, + struct prin_resp * resp, int noisy) + { +@@ -80,8 +69,7 @@ static int mpath_prin_activepath (struct multipath *mpp, int rq_servact, + + condlog(3, "%s: sending pr in command to %s ", + mpp->wwid, pp->dev); +- ret = mpath_send_prin_activepath(pp->dev, rq_servact, +- resp, noisy); ++ ret = prin_do_scsi_ioctl(pp->dev, rq_servact, resp, noisy); + switch(ret) + { + case MPATH_PR_SUCCESS: diff --git a/0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch b/0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch new file mode 100644 index 0000000..2dd1d09 --- /dev/null +++ b/0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch @@ -0,0 +1,102 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:48 -0400 +Subject: [PATCH] libmpathpersist: reduce log level for persistent reservation + checking + +Move logging of minor expected behavior to INFO level. Modify the log +level of some messages by whether or not mpp->prflag changed values. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 34 +++++++++++++++++++---------- + 1 file changed, 22 insertions(+), 12 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index fb1d0081..6233e6b0 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -60,7 +60,7 @@ static int mpath_prin_activepath (struct multipath *mpp, int rq_servact, + vector_foreach_slot (pgp->paths, pp, i){ + if (!((pp->state == PATH_UP) || + (pp->state == PATH_GHOST))){ +- condlog(2, "%s: %s not available. Skip.", ++ condlog(3, "%s: %s not available. Skip.", + mpp->wwid, pp->dev); + condlog(3, "%s: status = %d.", + mpp->wwid, pp->state); +@@ -733,12 +733,14 @@ int update_map_pr(struct multipath *mpp) + struct prin_resp *resp; + unsigned int i; + int ret = MPATH_PR_OTHER, isFound; ++ bool was_set = (mpp->prflag == PRFLAG_SET); + + if (!get_be64(mpp->reservation_key)) + { + /* Nothing to do. Assuming pr mgmt feature is disabled*/ + mpp->prflag = PRFLAG_UNSET; +- condlog(4, "%s: reservation_key not set in multipath.conf", ++ condlog(was_set ? 2 : 4, ++ "%s: reservation_key not set in multipath.conf", + mpp->alias); + return MPATH_PR_SUCCESS; + } +@@ -751,8 +753,7 @@ int update_map_pr(struct multipath *mpp) + } + if (count_active_paths(mpp) == 0) + { +- condlog(0,"%s: No available paths to check pr status", +- mpp->alias); ++ condlog(2, "%s: No available paths to check pr status", mpp->alias); + goto out; + } + mpp->prflag = PRFLAG_UNSET; +@@ -768,22 +769,31 @@ int update_map_pr(struct multipath *mpp) + + if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) + { +- condlog(3,"%s: No key found. Device may not be registered. ", mpp->alias); ++ condlog(was_set ? 1 : 3, ++ "%s: No key found. Device may not be registered. ", ++ mpp->alias); + goto out; + } + +- condlog(2, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, ++ condlog(3, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, + get_be64(mpp->reservation_key)); + + isFound =0; + for (i = 0; i < resp->prin_descriptor.prin_readkeys.additional_length/8; i++ ) + { +- condlog(2, "%s: PR IN READKEYS[%d] reservation key:", mpp->alias, i); +- dumpHex((char *)&resp->prin_descriptor.prin_readkeys.key_list[i*8], 8 , 1); ++ if (libmp_verbosity >= 3) { ++ condlog(3, "%s: PR IN READKEYS[%d] reservation key:", ++ mpp->alias, i); ++ dumpHex((char *)&resp->prin_descriptor.prin_readkeys ++ .key_list[i * 8], ++ 8, 1); ++ } + +- if (!memcmp(&mpp->reservation_key, &resp->prin_descriptor.prin_readkeys.key_list[i*8], 8)) +- { +- condlog(2, "%s: reservation key found in pr in readkeys response", mpp->alias); ++ if (!memcmp(&mpp->reservation_key, ++ &resp->prin_descriptor.prin_readkeys.key_list[i * 8], ++ 8)) { ++ condlog(3, "%s: reservation key found in pr in readkeys response", ++ mpp->alias); + isFound =1; + } + } +@@ -791,7 +801,7 @@ int update_map_pr(struct multipath *mpp) + if (isFound) + { + mpp->prflag = PRFLAG_SET; +- condlog(2, "%s: prflag flag set.", mpp->alias ); ++ condlog(was_set ? 3 : 2, "%s: prflag flag set.", mpp->alias); + } + + out: diff --git a/0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch b/0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch new file mode 100644 index 0000000..7dce63e --- /dev/null +++ b/0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch @@ -0,0 +1,28 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:49 -0400 +Subject: [PATCH] libmpathpersist: remove pointless update_map_pr ret value + code + +Don't set ret to MPATH_PR_SUCCESS, after the code just made sure that +it was already set to MPATH_PR_SUCCESS. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 2 -- + 1 file changed, 2 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 6233e6b0..39bfc953 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -765,8 +765,6 @@ int update_map_pr(struct multipath *mpp) + goto out; + } + +- ret = MPATH_PR_SUCCESS; +- + if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) + { + condlog(was_set ? 1 : 3, diff --git a/0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch b/0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch new file mode 100644 index 0000000..e3d0b41 --- /dev/null +++ b/0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch @@ -0,0 +1,217 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:50 -0400 +Subject: [PATCH] multipathd: use update_map_pr in mpath_pr_event_handle + +Clean up the duplicate code in mpath_pr_event_handle() and +update_map_pr() by making update_map_pr() take an optional path device +to use for its check, instead of checking all path devices and make +mpath_pr_event_handle() call update_map_pr() to do its checking. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/libmpathpersist.version | 2 +- + libmpathpersist/mpath_persist_int.c | 18 ++++--- + libmpathpersist/mpath_persist_int.h | 2 +- + multipathd/main.c | 68 ++++--------------------- + 4 files changed, 22 insertions(+), 68 deletions(-) + +diff --git a/libmpathpersist/libmpathpersist.version b/libmpathpersist/libmpathpersist.version +index a8c6aae7..faa4257b 100644 +--- a/libmpathpersist/libmpathpersist.version ++++ b/libmpathpersist/libmpathpersist.version +@@ -27,7 +27,7 @@ global: + local: *; + }; + +-__LIBMPATHPERSIST_INT_1.0.0 { ++__LIBMPATHPERSIST_INT_2.0.0 { + /* Internal use by multipath-tools */ + dumpHex; + mpath_alloc_prin_response; +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 39bfc953..8b01492f 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -727,7 +727,7 @@ out1: + return ret; + } + +-int update_map_pr(struct multipath *mpp) ++int update_map_pr(struct multipath *mpp, struct path *pp) + { + int noisy=0; + struct prin_resp *resp; +@@ -742,7 +742,7 @@ int update_map_pr(struct multipath *mpp) + condlog(was_set ? 2 : 4, + "%s: reservation_key not set in multipath.conf", + mpp->alias); +- return MPATH_PR_SUCCESS; ++ return MPATH_PR_SKIP; + } + + resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); +@@ -751,13 +751,15 @@ int update_map_pr(struct multipath *mpp) + condlog(0,"%s : failed to alloc resp in update_map_pr", mpp->alias); + return MPATH_PR_OTHER; + } +- if (count_active_paths(mpp) == 0) +- { ++ if (!pp && count_active_paths(mpp) == 0) { + condlog(2, "%s: No available paths to check pr status", mpp->alias); + goto out; + } + mpp->prflag = PRFLAG_UNSET; +- ret = mpath_prin_activepath(mpp, MPATH_PRIN_RKEY_SA, resp, noisy); ++ if (pp) ++ ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); ++ else ++ ret = mpath_prin_activepath(mpp, MPATH_PRIN_RKEY_SA, resp, noisy); + + if (ret != MPATH_PR_SUCCESS ) + { +@@ -799,8 +801,10 @@ int update_map_pr(struct multipath *mpp) + if (isFound) + { + mpp->prflag = PRFLAG_SET; +- condlog(was_set ? 3 : 2, "%s: prflag flag set.", mpp->alias); +- } ++ condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); ++ } else ++ condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", ++ mpp->alias); + + out: + free(resp); +diff --git a/libmpathpersist/mpath_persist_int.h b/libmpathpersist/mpath_persist_int.h +index 31457535..73c95863 100644 +--- a/libmpathpersist/mpath_persist_int.h ++++ b/libmpathpersist/mpath_persist_int.h +@@ -20,6 +20,6 @@ int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * resp, int + int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope, + unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy); + void dumpHex(const char* , int len, int no_ascii); +-int update_map_pr(struct multipath *mpp); ++int update_map_pr(struct multipath *mpp, struct path *pp); + + #endif /* _MPATH_PERSIST_INT_H */ +diff --git a/multipathd/main.c b/multipathd/main.c +index 04ca47d1..390632a6 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -733,7 +733,7 @@ fail: + sync_map_state(mpp); + + if (mpp->prflag != PRFLAG_SET) +- update_map_pr(mpp); ++ update_map_pr(mpp, NULL); + if (mpp->prflag == PRFLAG_SET) + pr_register_active_paths(mpp); + +@@ -1383,7 +1383,7 @@ rescan: + + if (retries >= 0) { + if (start_waiter) +- update_map_pr(mpp); ++ update_map_pr(mpp, NULL); + if (mpp->prflag == PRFLAG_SET && prflag != PRFLAG_SET) + pr_register_active_paths(mpp); + condlog(2, "%s [%s]: path added to devmap %s", +@@ -3028,7 +3028,7 @@ configure (struct vectors * vecs, enum force_reload_types reload_type) + vector_foreach_slot(mpvec, mpp, i){ + if (remember_wwid(mpp->wwid) == 1) + trigger_paths_udev_change(mpp, true); +- update_map_pr(mpp); ++ update_map_pr(mpp, NULL); + if (mpp->prflag == PRFLAG_SET) + pr_register_active_paths(mpp); + } +@@ -3946,70 +3946,24 @@ main (int argc, char *argv[]) + + static void mpath_pr_event_handle(struct path *pp) + { +- struct multipath * mpp; +- unsigned int i; +- int ret, isFound; ++ struct multipath *mpp = pp->mpp; ++ int ret; + struct prout_param_descriptor *param; +- struct prin_resp *resp; + +- mpp = pp->mpp; + if (pp->bus != SYSFS_BUS_SCSI) { + mpp->prflag = PRFLAG_UNSET; + return; + } + +- if (!get_be64(mpp->reservation_key)) { +- mpp->prflag = PRFLAG_UNSET; ++ if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) + return; +- } +- +- resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); +- if (!resp){ +- condlog(0,"%s Alloc failed for prin response", pp->dev); +- goto out; +- } +- +- mpp->prflag = PRFLAG_UNSET; +- ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, 0); +- if (ret != MPATH_PR_SUCCESS ) +- { +- condlog(0,"%s : pr in read keys service action failed. Error=%d", pp->dev, ret); +- goto out; +- } +- +- condlog(3, " event pr=%d addlen=%d",resp->prin_descriptor.prin_readkeys.prgeneration, +- resp->prin_descriptor.prin_readkeys.additional_length ); +- +- if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) +- { +- condlog(1, "%s: No key found. Device may not be registered.", pp->dev); +- goto out; +- } +- condlog(2, "Multipath reservation_key: 0x%" PRIx64 " ", +- get_be64(mpp->reservation_key)); + +- isFound =0; +- for (i = 0; i < resp->prin_descriptor.prin_readkeys.additional_length/8; i++ ) +- { +- condlog(2, "PR IN READKEYS[%d] reservation key:",i); +- dumpHex((char *)&resp->prin_descriptor.prin_readkeys.key_list[i*8], 8 , -1); +- if (!memcmp(&mpp->reservation_key, &resp->prin_descriptor.prin_readkeys.key_list[i*8], 8)) +- { +- condlog(2, "%s: pr key found in prin readkeys response", mpp->alias); +- isFound =1; +- break; +- } +- } +- if (!isFound) +- { +- condlog(0, "%s: Either device not registered or ", pp->dev); +- condlog(0, "host is not authorised for registration. Skip path"); +- goto out; +- } ++ if (mpp->prflag != PRFLAG_SET) ++ return; + + param = (struct prout_param_descriptor *)calloc(1, sizeof(struct prout_param_descriptor)); + if (!param) +- goto out; ++ return; + + param->sa_flags = mpp->sa_flags; + memcpy(param->sa_key, &mpp->reservation_key, 8); +@@ -4022,10 +3976,6 @@ static void mpath_pr_event_handle(struct path *pp) + { + condlog(0,"%s: Reservation registration failed. Error: %d", pp->dev, ret); + } +- mpp->prflag = PRFLAG_SET; + + free(param); +-out: +- if (resp) +- free(resp); + } diff --git a/0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch b/0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch new file mode 100644 index 0000000..bb53693 --- /dev/null +++ b/0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch @@ -0,0 +1,77 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:51 -0400 +Subject: [PATCH] libmpathpersist: limit changing prflag in update_map_pr + +Do not unset mpp->prflag in update_map_pr() unless the device doesn't +support persistent reservations or it successfully reads the registered +keys and discovers that the device's key is not there. Also, nothing in +the libmpathpersist ever returns MPATH_PR_SENSE_INVALID_OP. It instead +returns MPATH_PR_ILLEGAL_REQ if the device doesn't support persistent +reservations, so check for that instead. + +Also, do not even check for the registered keys in update_map_pr() if +mpp->prflag is unset. Otherwise, multipathd will set mpp->prflag if the +key was unregistered (and thus prflag was unset) while a path is down +(and thus could not have its key unregistered) and then that path comes +back up. I should note that the above issue can only occur if multipath +is defining PR keys in /etc/multipath.conf, instead of the prkeys file. + +If a device has no registered keys, it must unset prflag, since that +means it's no longer registered. Possibly its registration was removed +by a CLEAR or PREEMPT action. But if a device has a registered key but +it is supposed to be unregistered, it should remain unregistered, since +that key is likely one that libmpathpersist could not unregister at the +time. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 10 ++++++++-- + 1 file changed, 8 insertions(+), 2 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 8b01492f..c34bc785 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -73,7 +73,7 @@ static int mpath_prin_activepath (struct multipath *mpp, int rq_servact, + switch(ret) + { + case MPATH_PR_SUCCESS: +- case MPATH_PR_SENSE_INVALID_OP: ++ case MPATH_PR_ILLEGAL_REQ: + return ret; + default: + continue; +@@ -735,6 +735,10 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + int ret = MPATH_PR_OTHER, isFound; + bool was_set = (mpp->prflag == PRFLAG_SET); + ++ /* If pr is explicitly unset, it must be manually set */ ++ if (mpp->prflag == PRFLAG_UNSET) ++ return MPATH_PR_SKIP; ++ + if (!get_be64(mpp->reservation_key)) + { + /* Nothing to do. Assuming pr mgmt feature is disabled*/ +@@ -755,7 +759,6 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + condlog(2, "%s: No available paths to check pr status", mpp->alias); + goto out; + } +- mpp->prflag = PRFLAG_UNSET; + if (pp) + ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); + else +@@ -763,9 +766,12 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + + if (ret != MPATH_PR_SUCCESS ) + { ++ if (ret == MPATH_PR_ILLEGAL_REQ) ++ mpp->prflag = PRFLAG_UNSET; + condlog(0,"%s : pr in read keys service action failed Error=%d", mpp->alias, ret); + goto out; + } ++ mpp->prflag = PRFLAG_UNSET; + + if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) + { diff --git a/0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch b/0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch new file mode 100644 index 0000000..7ca1477 --- /dev/null +++ b/0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch @@ -0,0 +1,77 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:52 -0400 +Subject: [PATCH] multipathd: Don't call update_map_pr unnecessarily + +None of the calls to update_map_pr() outside of mpath_pr_event_handle() +add any benefit. When update_map_pr() is called without a path, it tries +to read the pr keys list on each usable path until it succeeds, and then +checks the keys to see if they include the configured key. + +In all cases where update_map_pr() is called outside of +mpath_pr_event_handle(), after it is called, pr_register_active_paths() +is called if a matching key was found. pr_register_active_paths() calls +mpath_pr_event_handle() on each usable path, which calls update_map_pr() +with a path, so it only checks that path. If a matching key is found, it +registers a key on the current path. The result is that after +pr_register_active_paths() is called, update_map_pr() will be called for +each usable path, just like update_map_pr() did. So calling +update_map_pr() first doesn't change the results for multipathd, it just +adds duplicate work, so remove those calls. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + multipathd/main.c | 18 +++++++----------- + 1 file changed, 7 insertions(+), 11 deletions(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index 390632a6..4a1b38e9 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -644,6 +644,8 @@ pr_register_active_paths(struct multipath *mpp) + + vector_foreach_slot (mpp->pg, pgp, i) { + vector_foreach_slot (pgp->paths, pp, j) { ++ if (mpp->prflag == PRFLAG_UNSET) ++ return; + if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST)) + mpath_pr_event_handle(pp); + } +@@ -732,10 +734,7 @@ fail: + + sync_map_state(mpp); + +- if (mpp->prflag != PRFLAG_SET) +- update_map_pr(mpp, NULL); +- if (mpp->prflag == PRFLAG_SET) +- pr_register_active_paths(mpp); ++ pr_register_active_paths(mpp); + + if (VECTOR_SIZE(offline_paths) != 0) + handle_orphaned_offline_paths(offline_paths); +@@ -1382,10 +1381,9 @@ rescan: + sync_map_state(mpp); + + if (retries >= 0) { +- if (start_waiter) +- update_map_pr(mpp, NULL); +- if (mpp->prflag == PRFLAG_SET && prflag != PRFLAG_SET) +- pr_register_active_paths(mpp); ++ if ((mpp->prflag == PRFLAG_SET && prflag != PRFLAG_SET) || ++ start_waiter) ++ pr_register_active_paths(mpp); + condlog(2, "%s [%s]: path added to devmap %s", + pp->dev, pp->dev_t, mpp->alias); + return 0; +@@ -3028,9 +3026,7 @@ configure (struct vectors * vecs, enum force_reload_types reload_type) + vector_foreach_slot(mpvec, mpp, i){ + if (remember_wwid(mpp->wwid) == 1) + trigger_paths_udev_change(mpp, true); +- update_map_pr(mpp, NULL); +- if (mpp->prflag == PRFLAG_SET) +- pr_register_active_paths(mpp); ++ pr_register_active_paths(mpp); + } + + /* diff --git a/0045-libmpathpersist-remove-useless-function-send_prout_a.patch b/0045-libmpathpersist-remove-useless-function-send_prout_a.patch new file mode 100644 index 0000000..7b60e53 --- /dev/null +++ b/0045-libmpathpersist-remove-useless-function-send_prout_a.patch @@ -0,0 +1,74 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:53 -0400 +Subject: [PATCH] libmpathpersist: remove useless function + send_prout_activepath + +send_prout_activepath() creates a single separate thread that just calls +prout_do_scsi_ioctl() and it doesn't take any advantage of the work +being done in another thread. Remove the function and call +prout_do_scsi_ioctl() directly. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 39 ++--------------------------- + 1 file changed, 2 insertions(+), 37 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index c34bc785..d8f757b7 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -373,40 +373,6 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + +-static int send_prout_activepath(char *dev, int rq_servact, int rq_scope, +- unsigned int rq_type, +- struct prout_param_descriptor * paramp, int noisy) +-{ +- struct prout_param param; +- param.rq_servact = rq_servact; +- param.rq_scope = rq_scope; +- param.rq_type = rq_type; +- param.paramp = paramp; +- param.noisy = noisy; +- param.status = -1; +- +- pthread_t thread; +- pthread_attr_t attr; +- int rc; +- +- memset(&thread, 0, sizeof(thread)); +- strlcpy(param.dev, dev, FILE_NAME_SIZE); +- /* Initialize and set thread joinable attribute */ +- pthread_attr_init(&attr); +- pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE); +- +- rc = pthread_create(&thread, &attr, mpath_prout_pthread_fn, (void *)(¶m)); +- if (rc){ +- condlog (3, "%s: failed to create thread %d", dev, rc); +- return MPATH_PR_THREAD_ERROR; +- } +- /* Free attribute and wait for the other threads */ +- pthread_attr_destroy(&attr); +- rc = pthread_join(thread, NULL); +- +- return (param.status); +-} +- + static int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor* paramp, int noisy) +@@ -426,9 +392,8 @@ 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); ++ ret = prout_do_scsi_ioctl(pp->dev, rq_servact, rq_scope, ++ rq_type, paramp, noisy); + if (ret != MPATH_PR_RETRYABLE_ERROR) + return ret; + } diff --git a/0046-libmpathpersist-redesign-failed-release-workaround.patch b/0046-libmpathpersist-redesign-failed-release-workaround.patch new file mode 100644 index 0000000..a09f6cb --- /dev/null +++ b/0046-libmpathpersist-redesign-failed-release-workaround.patch @@ -0,0 +1,293 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:54 -0400 +Subject: [PATCH] libmpathpersist: redesign failed release workaround + +The workaround for releasing a reservation held by a failed path +(clearing all persistent reservation data and then reregistering all the +keys) has several problems. It requires devices that support the READ +FULL STATUS command and are capable of specifying TransportIDs with +REGISTRATION commands (SIP_C), neither of which are mandatory to support +SCSI Persistent Reservations. Non SIP_C devices will be left with no +registered keys. Also, not all cleared keys are registered, just the +ones going to the Target Port receiving the REGISTRATION command. To +reregister all the keys, the code would have to also use the "All Target +Ports" flag to register keys on different Target Ports, but this could +end up registering keys on paths they aren't supposed to be on (for +instance if one of the registered keys was only there because the path +was down and it couldn't be released). + +The redesign avoids these issues by only using mandatory Persistent +Reservation commands, without extra optional parameters or flags, and +only effects the keys of the multipath device it is being issued on. +The new workaround is: +1. Suspend the multipath device to prevent I/O +2. Preempt the key to move the reservation to an available path. This + also removes the registered keys from every path except the path + issuing the PREEMPT command. Since the device is suspended, not I/O + can go to these unregisted paths and fail. +3. Release the reservation on the path that now holds it. +4. Resume the device (since it no longer matters that most of the paths + no longer have a registered key) +5. Reregister the keys on all the paths. + +If steps 3 or 4 fail, the code will attempt to reregister the keys, and +then attempt (or possibly re-attempt) the resume. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 182 +++++++++++++--------------- + libmultipath/libmultipath.version | 5 + + 2 files changed, 87 insertions(+), 100 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index d8f757b7..36743d41 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -373,9 +373,10 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + +-static int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope, +- unsigned int rq_type, +- struct prout_param_descriptor* paramp, int noisy) ++static int ++mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, ++ unsigned int rq_type, struct prout_param_descriptor *paramp, ++ int noisy, struct path **pptr) + { + int i,j, ret; + struct pathgroup *pgp = NULL; +@@ -394,6 +395,8 @@ static int mpath_prout_common(struct multipath *mpp,int rq_servact, int rq_scope + found = true; + ret = prout_do_scsi_ioctl(pp->dev, rq_servact, rq_scope, + rq_type, paramp, noisy); ++ if (ret == MPATH_PR_SUCCESS && pptr) ++ *pptr = pp; + if (ret != MPATH_PR_RETRYABLE_ERROR) + return ret; + } +@@ -414,14 +417,12 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + struct path *pp = NULL; + int active_pathcount = 0; + pthread_attr_t attr; +- int rc, found = 0; ++ int rc; + int count = 0; + int status = MPATH_PR_SUCCESS; +- struct prin_resp resp; +- struct prout_param_descriptor *pamp = NULL; +- struct prin_resp *pr_buff; +- int length; +- struct transportid *pptr = NULL; ++ struct prin_resp resp = {{{.prgeneration = 0}}}; ++ uint16_t udev_flags = (mpp->skip_kpartx) ? MPATH_UDEV_NO_KPARTX_FLAG : 0; ++ bool did_resume = false; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -511,104 +512,78 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + } + + condlog (2, "%s: Path holding reservation is not available.", mpp->wwid); +- +- pr_buff = mpath_alloc_prin_response(MPATH_PRIN_RFSTAT_SA); +- if (!pr_buff){ +- condlog (0, "%s: failed to alloc pr in response buffer.", mpp->wwid); ++ /* ++ * Cannot free the reservation because the path that is holding it ++ * is not usable. Workaround this by: ++ * 1. Suspending the device ++ * 2. Preempting the reservation to move it to a usable path ++ * (this removes the registered keys on all paths except the ++ * preempting one. Since the device is suspended, no IO can ++ * go to these unregistered paths and fail). ++ * 3. Releasing the reservation on the path that now holds it. ++ * 4. Resuming the device (since it no longer matters that most of ++ * that paths no longer have a registered key) ++ * 5. Reregistering keys on all the paths ++ */ ++ ++ if (!dm_simplecmd_noflush(DM_DEVICE_SUSPEND, mpp->alias, 0)) { ++ condlog(0, "%s: release: failed to suspend dm device.", mpp->wwid); + return MPATH_PR_OTHER; + } + +- status = mpath_prin_activepath (mpp, MPATH_PRIN_RFSTAT_SA, pr_buff, noisy); +- +- if (status != MPATH_PR_SUCCESS){ +- condlog (0, "%s: pr in read full status command failed.", mpp->wwid); +- goto out; +- } +- +- num = pr_buff->prin_descriptor.prin_readfd.number_of_descriptor; +- if (0 == num){ +- goto out; +- } +- length = sizeof (struct prout_param_descriptor) + (sizeof (struct transportid *)); +- +- pamp = (struct prout_param_descriptor *)malloc (length); +- if (!pamp){ +- condlog (0, "%s: failed to alloc pr out parameter.", mpp->wwid); +- goto out; +- } +- +- memset(pamp, 0, length); +- +- pamp->trnptid_list[0] = (struct transportid *) malloc (sizeof (struct transportid)); +- if (!pamp->trnptid_list[0]){ +- condlog (0, "%s: failed to alloc pr out transportid.", mpp->wwid); +- goto out1; +- } +- pptr = pamp->trnptid_list[0]; +- +- if (get_be64(mpp->reservation_key)){ +- memcpy (pamp->key, &mpp->reservation_key, 8); +- condlog (3, "%s: reservation key set.", mpp->wwid); +- } +- +- status = mpath_prout_common (mpp, MPATH_PROUT_CLEAR_SA, +- rq_scope, rq_type, pamp, noisy); +- +- if (status) { +- condlog(0, "%s: failed to send CLEAR_SA", mpp->wwid); +- goto out2; ++ memset(paramp, 0, sizeof(*paramp)); ++ memcpy(paramp->key, &mpp->reservation_key, 8); ++ memcpy(paramp->sa_key, &mpp->reservation_key, 8); ++ status = mpath_prout_common(mpp, MPATH_PROUT_PREE_SA, rq_scope, ++ rq_type, paramp, noisy, &pp); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: release: pr preempt command failed.", mpp->wwid); ++ goto fail_resume; + } + +- pamp->num_transportid = 1; +- +- for (i = 0; i < num; i++){ +- if (get_be64(mpp->reservation_key) && +- memcmp(pr_buff->prin_descriptor.prin_readfd.descriptors[i]->key, +- &mpp->reservation_key, 8)){ +- /*register with transport id*/ +- memset(pamp, 0, length); +- pamp->trnptid_list[0] = pptr; +- memset (pamp->trnptid_list[0], 0, sizeof (struct transportid)); +- memcpy (pamp->sa_key, +- pr_buff->prin_descriptor.prin_readfd.descriptors[i]->key, 8); +- pamp->sa_flags = MPATH_F_SPEC_I_PT_MASK; +- pamp->num_transportid = 1; +- +- memcpy (pamp->trnptid_list[0], +- &pr_buff->prin_descriptor.prin_readfd.descriptors[i]->trnptid, +- sizeof (struct transportid)); +- status = mpath_prout_common (mpp, MPATH_PROUT_REG_SA, 0, rq_type, +- pamp, noisy); +- +- pamp->sa_flags = 0; +- memcpy (pamp->key, pr_buff->prin_descriptor.prin_readfd.descriptors[i]->key, 8); +- memset (pamp->sa_key, 0, 8); +- pamp->num_transportid = 0; +- status = mpath_prout_common (mpp, MPATH_PROUT_REG_SA, 0, rq_type, +- pamp, noisy); +- } +- else +- { +- if (get_be64(mpp->reservation_key)) +- found = 1; +- } +- +- ++ memset(paramp, 0, sizeof(*paramp)); ++ memcpy(paramp->key, &mpp->reservation_key, 8); ++ status = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REL_SA, rq_scope, ++ rq_type, paramp, noisy); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: release on alternate path failed.", mpp->wwid); ++ goto out_reregister; + } + +- if (found){ +- memset (pamp, 0, length); +- memcpy (pamp->sa_key, &mpp->reservation_key, 8); +- memset (pamp->key, 0, 8); +- status = mpath_prout_reg(mpp, MPATH_PROUT_REG_SA, rq_scope, rq_type, pamp, noisy); ++ if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { ++ condlog(0, "%s release: failed to resume dm device.", mpp->wwid); ++ /* ++ * leave status set to MPATH_PR_SUCCESS, we will have another ++ * chance to resume the device. ++ */ ++ goto out_reregister; ++ } ++ did_resume = true; ++ ++out_reregister: ++ memset(paramp, 0, sizeof(*paramp)); ++ memcpy(paramp->sa_key, &mpp->reservation_key, 8); ++ rc = mpath_prout_reg(mpp, MPATH_PROUT_REG_IGN_SA, rq_scope, rq_type, ++ paramp, noisy); ++ if (rc != MPATH_PR_SUCCESS) ++ condlog(0, "%s: release: failed to reregister paths.", mpp->wwid); ++ ++ /* ++ * If we failed releasing the reservation or resuming earlier ++ * try resuming now. Otherwise, return with the reregistering status ++ * This means we will report failure, even though the resevation ++ * has been released, since the keys were not reregistered. ++ */ ++ if (did_resume) ++ return rc; ++ else if (status == MPATH_PR_SUCCESS) ++ status = rc; ++fail_resume: ++ if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { ++ condlog(0, "%s: release: failed to resume dm device.", mpp->wwid); ++ if (status == MPATH_PR_SUCCESS) ++ status = MPATH_PR_OTHER; + } +- +-out2: +- free(pptr); +-out1: +- free (pamp); +-out: +- free (pr_buff); + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + +@@ -629,6 +604,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + conf = get_multipath_config(); + select_reservation_key(conf, mpp); + select_all_tg_pt(conf, mpp); ++ /* ++ * If a device preempts itself, it will need to suspend and resume. ++ * Set mpp->skip_kpartx to make sure we set the flags to skip kpartx ++ * if necessary, when doing this. ++ */ ++ select_skip_kpartx(conf, mpp); + put_multipath_config(conf); + + memcpy(&prkey, paramp->sa_key, 8); +@@ -665,7 +646,8 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + case MPATH_PROUT_PREE_SA : + case MPATH_PROUT_PREE_AB_SA : + case MPATH_PROUT_CLEAR_SA: +- ret = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); ++ ret = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, ++ paramp, noisy, NULL); + break; + case MPATH_PROUT_REL_SA: + ret = mpath_prout_rel(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); +diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version +index 0b7a1a2b..db125779 100644 +--- a/libmultipath/libmultipath.version ++++ b/libmultipath/libmultipath.version +@@ -246,3 +246,8 @@ LIBMULTIPATH_24.0.1 { + global: + can_recheck_wwid; + } LIBMULTIPATH_24.0.0; ++ ++LIBMULTIPATH_24.0.2 { ++global: ++ select_skip_kpartx; ++} LIBMULTIPATH_24.0.1; diff --git a/0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch b/0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch new file mode 100644 index 0000000..29af948 --- /dev/null +++ b/0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch @@ -0,0 +1,51 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:55 -0400 +Subject: [PATCH] libmpathpersist: fail the release if all threads fail + +If none of the threads succeeds in issuing the release, simply return +failure, instead of trying the workaround. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 14 +++++++++++--- + 1 file changed, 11 insertions(+), 3 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 36743d41..553fa509 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -423,6 +423,7 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + struct prin_resp resp = {{{.prgeneration = 0}}}; + uint16_t udev_flags = (mpp->skip_kpartx) ? MPATH_UDEV_NO_KPARTX_FLAG : 0; + bool did_resume = false; ++ bool all_threads_failed; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -484,15 +485,22 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + } + } + ++ all_threads_failed = true; + 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) ++ all_threads_failed = false; ++ 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 (all_threads_failed) { ++ 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){ diff --git a/0048-libmpathpersist-Handle-changing-key-corner-case.patch b/0048-libmpathpersist-Handle-changing-key-corner-case.patch new file mode 100644 index 0000000..c7cba21 --- /dev/null +++ b/0048-libmpathpersist-Handle-changing-key-corner-case.patch @@ -0,0 +1,191 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:56 -0400 +Subject: [PATCH] libmpathpersist: Handle changing key corner case + +When you change the reservation key of a registered multipath device, +some of paths might be down or even deleted since you originally +registered the key. libmpathpersist won't be able to change these +registrations. This can be a problem if the path that is down is holding +the reservation. Other nodes will assume that the reservation is now +under your new key, when it is still under the old one. If they try to +preempt the reservation, they will succeed. But they will just +unregister all the paths with the new key. They won't take over the +reservation, and if the path holding it comes back up, it will still be +able to write to the device. + +To avoid this, after libmpathpersist changes the key of a registered +device, it now checks to see if its old key is still holding the +reservation. If it is, limpathpersist preempts the reservation. +Currently this only works on REGISTER commands, not REGISTER AND IGNORE +ones. A future patch will add that capability. This patch also moves +mpath_prout_common() up, without changing it at all. + +I should note that this relies on the fact that libmpathpersist has +never worked correctly if two nodes use the same reservation key for the +same device. If another node used the same key, it could be the one +holding the reservation, and preempting the reservation would deregister +it. However, multipathd has always relied on the fact that two nodes +will not use the same registration key to know if it is safe to register +a key on a path that was just added or just came back up. If there is +already a registered key matching the configured key, multipathd assumes +that the device has not been preempted, and registers the key on the +path. If there are no keys matching the configured key, multipathd +assumes that the device has been preempted, and won't register a key. +Again, if another node shared the same key, multipathd could registered +paths on a node that had been preempted. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 127 ++++++++++++++++++++-------- + 1 file changed, 93 insertions(+), 34 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 553fa509..02b3e59a 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -230,6 +230,97 @@ static void *mpath_prout_pthread_fn(void *p) + pthread_exit(NULL); + } + ++static int ++mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, ++ unsigned int rq_type, struct prout_param_descriptor *paramp, ++ int noisy, struct path **pptr) ++{ ++ 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) { ++ if (!((pp->state == PATH_UP) || (pp->state == PATH_GHOST))) { ++ condlog(1, "%s: %s path not up. Skip", ++ mpp->wwid, pp->dev); ++ continue; ++ } ++ ++ condlog(3, "%s: sending pr out command to %s", ++ mpp->wwid, pp->dev); ++ found = true; ++ ret = prout_do_scsi_ioctl(pp->dev, rq_servact, rq_scope, ++ rq_type, paramp, noisy); ++ if (ret == MPATH_PR_SUCCESS && pptr) ++ *pptr = pp; ++ 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; ++} ++ ++/* ++ * If you are changing the key registered to a device, and that device is ++ * holding the reservation on a path that couldn't get its key updated, ++ * either because it is down or no longer part of the multipath device, ++ * you need to preempt the reservation to a usable path with the new key ++ */ ++void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, ++ int noisy) ++{ ++ uint8_t zero[8] = {0}; ++ struct prin_resp resp = {{{.prgeneration = 0}}}; ++ int rq_scope; ++ unsigned int rq_type; ++ struct prout_param_descriptor paramp = {.sa_flags = 0}; ++ int status; ++ ++ /* ++ * If you previously didn't have a key registered or you didn't ++ * switch to a different key, there's no need to preempt. Also, you ++ * can't preempt if you no longer have a registered key ++ */ ++ if (memcmp(key, zero, 8) == 0 || memcmp(sa_key, zero, 8) == 0 || ++ memcmp(key, sa_key, 8) == 0) ++ return; ++ ++ status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, noisy); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: register: pr in read reservation command failed.", ++ mpp->wwid); ++ return; ++ } ++ /* If there is no reservation, there's nothing to preempt */ ++ if (!resp.prin_descriptor.prin_readresv.additional_length) ++ return; ++ /* ++ * If there reservation is not held by the old key, you don't ++ * want to preempt it ++ */ ++ if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) != 0) ++ return; ++ /* Assume this key is being held by an inaccessable path on this ++ * node. libmpathpersist has never worked if multiple nodes share ++ * the same reservation key for a device ++ */ ++ rq_type = resp.prin_descriptor.prin_readresv.scope_type & MPATH_PR_TYPE_MASK; ++ rq_scope = (resp.prin_descriptor.prin_readresv.scope_type & ++ MPATH_PR_SCOPE_MASK) >> ++ 4; ++ memcpy(paramp.key, sa_key, 8); ++ memcpy(paramp.sa_key, key, 8); ++ status = mpath_prout_common(mpp, MPATH_PROUT_PREE_SA, rq_scope, ++ rq_type, ¶mp, noisy, NULL); ++ if (status != MPATH_PR_SUCCESS) ++ condlog(0, "%s: register: pr preempt command failed.", mpp->wwid); ++} ++ + static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) +@@ -370,43 +461,11 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + } + + pthread_attr_destroy(&attr); ++ if (status == MPATH_PR_SUCCESS) ++ preempt_missing_path(mpp, paramp->key, paramp->sa_key, noisy); + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + +-static int +-mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, +- unsigned int rq_type, struct prout_param_descriptor *paramp, +- int noisy, struct path **pptr) +-{ +- 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){ +- if (!((pp->state == PATH_UP) || (pp->state == PATH_GHOST))){ +- condlog (1, "%s: %s path not up. Skip", +- mpp->wwid, pp->dev); +- continue; +- } +- +- condlog (3, "%s: sending pr out command to %s", mpp->wwid, pp->dev); +- found = true; +- ret = prout_do_scsi_ioctl(pp->dev, rq_servact, rq_scope, +- rq_type, paramp, noisy); +- if (ret == MPATH_PR_SUCCESS && pptr) +- *pptr = pp; +- 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; +-} +- + static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) diff --git a/0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch b/0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch new file mode 100644 index 0000000..ac34fe4 --- /dev/null +++ b/0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch @@ -0,0 +1,182 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:57 -0400 +Subject: [PATCH] libmpathpersist: Handle REGISTER AND IGNORE changing key + corner case + +When you use REGISTER to change the reservation key of a registered +multipath device, where the path holding the reservation is down, +libmpathpersist will PREEMPT the failed path to move the reservation to +a usable path, so the reservation key will be updated. However, when you +use REGISTER AND IGNORE, you don't pass in the old key, so +libmpathpersist can't use it to check against the key holding the +reservation, which is necessary to know if it needs to PREEMPT. + +Since the SCSI spec says that devices must ignore any passed-in key on +REGISTER AND IGNORE, libmpathpersist can still use it to pass in the old +key value. But unlike with REGISTER, the command won't fail if device +isn't actually registered with the old key, so libmpathpersist needs to +check that itself. To do this, it calls update_map_pr() just like +multipathd does to check that the device is registered before +registering new paths. However, libmpathpersist doesn't track the +persistent reservation state like multipathd, so it needs to ask +multipathd if it thinks the device is registered, using the "get +prstatus map " command. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 20 ++++++++++ + libmpathpersist/mpath_updatepr.c | 59 +++++++++++++++++++++-------- + libmpathpersist/mpathpr.h | 1 + + 3 files changed, 65 insertions(+), 15 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 02b3e59a..19220d7a 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -654,6 +654,23 @@ fail_resume: + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + ++/* ++ * for MPATH_PROUT_REG_IGN_SA, we use the ignored paramp->key to store the ++ * currently registered key. ++ */ ++static void set_ignored_key(struct multipath *mpp, uint8_t *key) ++{ ++ memset(key, 0, 8); ++ if (!get_be64(mpp->reservation_key)) ++ return; ++ if (get_prflag(mpp->alias) == PRFLAG_UNSET) ++ return; ++ update_map_pr(mpp, NULL); ++ if (mpp->prflag != PRFLAG_SET) ++ return; ++ memcpy(key, &mpp->reservation_key, 8); ++} ++ + int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + int rq_servact, int rq_scope, unsigned int rq_type, + struct prout_param_descriptor *paramp, int noisy) +@@ -679,6 +696,9 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + select_skip_kpartx(conf, mpp); + put_multipath_config(conf); + ++ if (rq_servact == MPATH_PROUT_REG_IGN_SA) ++ set_ignored_key(mpp, paramp->key); ++ + memcpy(&prkey, paramp->sa_key, 8); + if (mpp->prkey_source == PRKEY_SOURCE_FILE && prkey && + (rq_servact == MPATH_PROUT_REG_IGN_SA || +diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c +index 36bd777e..7cf82f50 100644 +--- a/libmpathpersist/mpath_updatepr.c ++++ b/libmpathpersist/mpath_updatepr.c +@@ -19,14 +19,12 @@ + #include "config.h" + #include "uxsock.h" + #include "mpathpr.h" ++#include "structs.h" + +- +-static int do_update_pr(char *alias, char *cmd, char *key) ++static char *do_pr(char *alias, char *str) + { + int fd; +- char str[256]; + char *reply; +- int ret = 0; + int timeout; + struct config *conf; + +@@ -37,24 +35,35 @@ static int do_update_pr(char *alias, char *cmd, char *key) + fd = mpath_connect(); + if (fd == -1) { + condlog (0, "ux socket connect error"); +- return -1; ++ return NULL; + } + +- if (key) +- snprintf(str,sizeof(str),"%s map %s key %s", cmd, alias, key); +- else +- snprintf(str,sizeof(str),"%s map %s", cmd, alias); + condlog (2, "%s: pr message=%s", alias, str); + if (send_packet(fd, str) != 0) { + condlog(2, "%s: message=%s send error=%d", alias, str, errno); + mpath_disconnect(fd); +- return -1; ++ return NULL; + } +- ret = recv_packet(fd, &reply, timeout); +- if (ret < 0) { ++ if (recv_packet(fd, &reply, timeout) < 0) + condlog(2, "%s: message=%s recv error=%d", alias, str, errno); +- ret = -1; +- } else { ++ ++ mpath_disconnect(fd); ++ return reply; ++} ++ ++static int do_update_pr(char *alias, char *cmd, char *key) ++{ ++ char str[256]; ++ char *reply = NULL; ++ int ret = -1; ++ ++ if (key) ++ snprintf(str, sizeof(str), "%s map %s key %s", cmd, alias, key); ++ else ++ snprintf(str, sizeof(str), "%s map %s", cmd, alias); ++ ++ reply = do_pr(alias, str); ++ if (reply) { + condlog (2, "%s: message=%s reply=%s", alias, str, reply); + if (reply && strncmp(reply,"ok", 2) == 0) + ret = 0; +@@ -63,10 +72,30 @@ static int do_update_pr(char *alias, char *cmd, char *key) + } + + free(reply); +- mpath_disconnect(fd); + return ret; + } + ++int get_prflag(char *mapname) ++{ ++ char str[256]; ++ char *reply; ++ int prflag; ++ ++ snprintf(str, sizeof(str), "getprstatus map %s", mapname); ++ reply = do_pr(mapname, str); ++ if (!reply) ++ prflag = PRFLAG_UNKNOWN; ++ else if (strncmp(reply, "unset", 5) == 0) ++ prflag = PRFLAG_UNSET; ++ else if (strncmp(reply, "set", 3) == 0) ++ prflag = PRFLAG_SET; ++ else ++ prflag = PRFLAG_UNKNOWN; ++ ++ free(reply); ++ return prflag; ++} ++ + int update_prflag(char *mapname, int set) { + return do_update_pr(mapname, (set)? "setprstatus" : "unsetprstatus", + NULL); +diff --git a/libmpathpersist/mpathpr.h b/libmpathpersist/mpathpr.h +index 39a7d8ed..614be530 100644 +--- a/libmpathpersist/mpathpr.h ++++ b/libmpathpersist/mpathpr.h +@@ -8,6 +8,7 @@ + + int update_prflag(char *mapname, int set); + int update_prkey_flags(char *mapname, uint64_t prkey, uint8_t sa_flags); ++int get_prflag(char *mapname); + #define update_prkey(mapname, prkey) update_prkey_flags(mapname, prkey, 0) + + #endif diff --git a/0050-libmultipath-rename-prflag_value-enums.patch b/0050-libmultipath-rename-prflag_value-enums.patch new file mode 100644 index 0000000..c822229 --- /dev/null +++ b/0050-libmultipath-rename-prflag_value-enums.patch @@ -0,0 +1,249 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:58 -0400 +Subject: [PATCH] libmultipath: rename prflag_value enums + +These will also be used by another variable, so make their name more +generic. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 16 ++++++++-------- + libmpathpersist/mpath_updatepr.c | 8 ++++---- + libmultipath/structs.h | 9 ++++----- + multipathd/cli_handlers.c | 22 +++++++++++----------- + multipathd/main.c | 17 ++++++++--------- + 5 files changed, 35 insertions(+), 37 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 19220d7a..d596b5bd 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -663,10 +663,10 @@ static void set_ignored_key(struct multipath *mpp, uint8_t *key) + memset(key, 0, 8); + if (!get_be64(mpp->reservation_key)) + return; +- if (get_prflag(mpp->alias) == PRFLAG_UNSET) ++ if (get_prflag(mpp->alias) == PR_UNSET) + return; + update_map_pr(mpp, NULL); +- if (mpp->prflag != PRFLAG_SET) ++ if (mpp->prflag != PR_SET) + return; + memcpy(key, &mpp->reservation_key, 8); + } +@@ -767,16 +767,16 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + struct prin_resp *resp; + unsigned int i; + int ret = MPATH_PR_OTHER, isFound; +- bool was_set = (mpp->prflag == PRFLAG_SET); ++ bool was_set = (mpp->prflag == PR_SET); + + /* If pr is explicitly unset, it must be manually set */ +- if (mpp->prflag == PRFLAG_UNSET) ++ if (mpp->prflag == PR_UNSET) + return MPATH_PR_SKIP; + + if (!get_be64(mpp->reservation_key)) + { + /* Nothing to do. Assuming pr mgmt feature is disabled*/ +- mpp->prflag = PRFLAG_UNSET; ++ mpp->prflag = PR_UNSET; + condlog(was_set ? 2 : 4, + "%s: reservation_key not set in multipath.conf", + mpp->alias); +@@ -801,11 +801,11 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + if (ret != MPATH_PR_SUCCESS ) + { + if (ret == MPATH_PR_ILLEGAL_REQ) +- mpp->prflag = PRFLAG_UNSET; ++ mpp->prflag = PR_UNSET; + condlog(0,"%s : pr in read keys service action failed Error=%d", mpp->alias, ret); + goto out; + } +- mpp->prflag = PRFLAG_UNSET; ++ mpp->prflag = PR_UNSET; + + if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) + { +@@ -840,7 +840,7 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + + if (isFound) + { +- mpp->prflag = PRFLAG_SET; ++ mpp->prflag = PR_SET; + condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); + } else + condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", +diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c +index 7cf82f50..c8fbe4a2 100644 +--- a/libmpathpersist/mpath_updatepr.c ++++ b/libmpathpersist/mpath_updatepr.c +@@ -84,13 +84,13 @@ int get_prflag(char *mapname) + snprintf(str, sizeof(str), "getprstatus map %s", mapname); + reply = do_pr(mapname, str); + if (!reply) +- prflag = PRFLAG_UNKNOWN; ++ prflag = PR_UNKNOWN; + else if (strncmp(reply, "unset", 5) == 0) +- prflag = PRFLAG_UNSET; ++ prflag = PR_UNSET; + else if (strncmp(reply, "set", 3) == 0) +- prflag = PRFLAG_SET; ++ prflag = PR_SET; + else +- prflag = PRFLAG_UNKNOWN; ++ prflag = PR_UNKNOWN; + + free(reply); + return prflag; +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 5dc00fbc..24135aa6 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -406,11 +406,10 @@ struct path { + + typedef int (pgpolicyfn) (struct multipath *, vector); + +- +-enum prflag_value { +- PRFLAG_UNKNOWN, +- PRFLAG_UNSET, +- PRFLAG_SET, ++enum pr_value { ++ PR_UNKNOWN, ++ PR_UNSET, ++ PR_SET, + }; + + struct multipath { +diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c +index 117570e1..8e2e2cea 100644 +--- a/multipathd/cli_handlers.c ++++ b/multipathd/cli_handlers.c +@@ -1240,14 +1240,15 @@ cli_shutdown (void * v, struct strbuf *reply, void * data) + return 0; + } + ++static const char *const pr_str[] = { ++ [PR_UNKNOWN] = "unknown\n", ++ [PR_UNSET] = "unset\n", ++ [PR_SET] = "set\n", ++}; ++ + static int + cli_getprstatus (void * v, struct strbuf *reply, void * data) + { +- static const char * const prflag_str[] = { +- [PRFLAG_UNKNOWN] = "unknown\n", +- [PRFLAG_UNSET] = "unset\n", +- [PRFLAG_SET] = "set\n", +- }; + struct multipath * mpp; + struct vectors * vecs = (struct vectors *)data; + char * param = get_keyparam(v, KEY_MAP); +@@ -1258,7 +1259,7 @@ cli_getprstatus (void * v, struct strbuf *reply, void * data) + if (!mpp) + return -ENODEV; + +- append_strbuf_str(reply, prflag_str[mpp->prflag]); ++ append_strbuf_str(reply, pr_str[mpp->prflag]); + + condlog(3, "%s: reply = %s", param, get_strbuf_str(reply)); + +@@ -1278,12 +1279,11 @@ cli_setprstatus(void * v, struct strbuf *reply, void * data) + if (!mpp) + return -ENODEV; + +- if (mpp->prflag != PRFLAG_SET) { +- mpp->prflag = PRFLAG_SET; ++ if (mpp->prflag != PR_SET) { ++ mpp->prflag = PR_SET; + condlog(2, "%s: prflag set", param); + } + +- + return 0; + } + +@@ -1300,8 +1300,8 @@ cli_unsetprstatus(void * v, struct strbuf *reply, void * data) + if (!mpp) + return -ENODEV; + +- if (mpp->prflag != PRFLAG_UNSET) { +- mpp->prflag = PRFLAG_UNSET; ++ if (mpp->prflag != PR_UNSET) { ++ mpp->prflag = PR_UNSET; + condlog(2, "%s: prflag unset", param); + } + +diff --git a/multipathd/main.c b/multipathd/main.c +index 4a1b38e9..bd4342a3 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -644,7 +644,7 @@ pr_register_active_paths(struct multipath *mpp) + + vector_foreach_slot (mpp->pg, pgp, i) { + vector_foreach_slot (pgp->paths, pp, j) { +- if (mpp->prflag == PRFLAG_UNSET) ++ if (mpp->prflag == PR_UNSET) + return; + if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST)) + mpath_pr_event_handle(pp); +@@ -1253,7 +1253,7 @@ ev_add_path (struct path * pp, struct vectors * vecs, int need_do_map) + int start_waiter = 0; + int ret; + int ro; +- unsigned char prflag = PRFLAG_UNSET; ++ unsigned char prflag = PR_UNSET; + + /* + * need path UID to go any further +@@ -1381,8 +1381,7 @@ rescan: + sync_map_state(mpp); + + if (retries >= 0) { +- if ((mpp->prflag == PRFLAG_SET && prflag != PRFLAG_SET) || +- start_waiter) ++ if ((mpp->prflag == PR_SET && prflag != PR_SET) || start_waiter) + pr_register_active_paths(mpp); + condlog(2, "%s [%s]: path added to devmap %s", + pp->dev, pp->dev_t, mpp->alias); +@@ -2645,7 +2644,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks) + } + + if (newstate == PATH_UP || newstate == PATH_GHOST) { +- if (pp->mpp->prflag != PRFLAG_UNSET) { ++ if (pp->mpp->prflag != PR_UNSET) { + int prflag = pp->mpp->prflag; + /* + * Check Persistent Reservation. +@@ -2653,8 +2652,8 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks) + condlog(2, "%s: checking persistent " + "reservation registration", pp->dev); + mpath_pr_event_handle(pp); +- if (pp->mpp->prflag == PRFLAG_SET && +- prflag != PRFLAG_SET) ++ if (pp->mpp->prflag == PR_SET && ++ prflag != PR_SET) + pr_register_active_paths(pp->mpp); + } + } +@@ -3947,14 +3946,14 @@ static void mpath_pr_event_handle(struct path *pp) + struct prout_param_descriptor *param; + + if (pp->bus != SYSFS_BUS_SCSI) { +- mpp->prflag = PRFLAG_UNSET; ++ mpp->prflag = PR_UNSET; + return; + } + + if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) + return; + +- if (mpp->prflag != PRFLAG_SET) ++ if (mpp->prflag != PR_SET) + return; + + param = (struct prout_param_descriptor *)calloc(1, sizeof(struct prout_param_descriptor)); diff --git a/0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch b/0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch new file mode 100644 index 0000000..6c8d612 --- /dev/null +++ b/0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch @@ -0,0 +1,44 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:10:59 -0400 +Subject: [PATCH] libmpathpersist: use a switch statement for prout command + finalizing + +Change the code at the end of do_mpath_persistent_reserve_out() to +use a switch statement instead of multiple if statements. A future +patch will add more actions here, and a switch statement looks cleaner. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 12 ++++++++---- + 1 file changed, 8 insertions(+), 4 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index d596b5bd..679e82be 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -744,15 +744,19 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + goto out1; + } + +- if ((ret == MPATH_PR_SUCCESS) && ((rq_servact == MPATH_PROUT_REG_SA) || +- (rq_servact == MPATH_PROUT_REG_IGN_SA))) +- { ++ if (ret != MPATH_PR_SUCCESS) ++ goto out1; ++ ++ switch (rq_servact) { ++ case MPATH_PROUT_REG_SA: ++ case MPATH_PROUT_REG_IGN_SA: + if (prkey == 0) { + update_prflag(alias, 0); + update_prkey(alias, 0); + } else + update_prflag(alias, 1); +- } else if ((ret == MPATH_PR_SUCCESS) && (rq_servact == MPATH_PROUT_CLEAR_SA)) { ++ break; ++ case MPATH_PROUT_CLEAR_SA: + update_prflag(alias, 0); + update_prkey(alias, 0); + } diff --git a/0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch b/0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch new file mode 100644 index 0000000..18ae08c --- /dev/null +++ b/0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch @@ -0,0 +1,444 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 10 Jul 2025 14:11:00 -0400 +Subject: [PATCH] libmpathpersist: Add safety check for preempting on key + change + +When a reservation key is changed from one non-zero value to another, +libmpathpersist checks if the old key is still holding the reservation, +and preempts it if it is. This was only safe if two nodes never share +the same key. If a node uses the same key as another node that is +holding the reservation, and switches keys so that it no longer matches, +it will end up preempting the reservation. This is clearly unexpected +behavior, and it can come about by a simple accident of registering a +device with the wrong key, and then immediately fixing it. + +To handle this, add code to track if a device is the reservation holder +to multipathd. multipathd now has three new commands "getprhold", +"setprhold", and "unsetprhold". These commands work like the equivalent +*prstatus commands. libmpathpersist calls setprhold on RESERVE commands +and PREEMPT commands when the preempted key is holding the reservation +and unsetprhold on RELEASE commands. Also, calling unsetprflag causes +prhold to be unset as well, so CLEAR commands and REGISTER commands with +a 0x0 service action key will also unset prhold. libmpathpersist() will +also unset prhold if it notices that the device cannot be holding a +reservation in preempt_missing_path(). + +When a new multipath device is created, its initial prhold state is +PR_UNKNOWN until it checks the current reservation, just like with +prflag. If multipathd ever finds that a device's registration has been +preempted or cleared in update_map_pr(), it unsets prhold, just like +with prflag. + +Now, before libmpathpersist preempts a reservation when changing keys +it also checks if multipathd thinks that the device is holding +the reservation. If it does not, then libmpathpersist won't preempt +the key. It will assume that another node is holding the reservation +with the same key. + +I should note that this safety check only stops a node not holding the +reservation from preempting the node holding the reservation. If the +node holding the reservation changes its key, but it fails to change the +resevation key, because that path is down or gone, it will still issue +the preempt to move the reservation to a usable path, even if another +node is using the same key. This will remove the registrations for that +other node. It also will not work correctly if multipathd stops tracking +a device for some reason. It's only a best-effort safety check. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 85 ++++++++++++++++++++++++----- + libmpathpersist/mpath_updatepr.c | 19 ++++++- + libmpathpersist/mpathpr.h | 2 + + libmultipath/structs.h | 1 + + multipathd/callbacks.c | 3 + + multipathd/cli.c | 4 +- + multipathd/cli.h | 3 + + multipathd/cli_handlers.c | 46 ++++++++++++++++ + multipathd/main.c | 34 +++++++++++- + 9 files changed, 179 insertions(+), 18 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 679e82be..1948de8d 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -270,6 +270,10 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + * holding the reservation on a path that couldn't get its key updated, + * either because it is down or no longer part of the multipath device, + * you need to preempt the reservation to a usable path with the new key ++ * ++ * Also, it's possible that the reservation was preempted, and the device ++ * is being re-registered. If it appears that is the case, clear ++ * mpp->prhold in multipathd. + */ + void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, + int noisy) +@@ -282,12 +286,19 @@ void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, + int status; + + /* +- * If you previously didn't have a key registered or you didn't +- * switch to a different key, there's no need to preempt. Also, you +- * can't preempt if you no longer have a registered key ++ * If you previously didn't have a key registered, you can't ++ * be holding the reservation. Also, you can't preempt if you ++ * no longer have a registered key + */ +- if (memcmp(key, zero, 8) == 0 || memcmp(sa_key, zero, 8) == 0 || +- memcmp(key, sa_key, 8) == 0) ++ if (memcmp(key, zero, 8) == 0 || memcmp(sa_key, zero, 8) == 0) { ++ update_prhold(mpp->alias, false); ++ return; ++ } ++ /* ++ * If you didn't switch to a different key, there is no need to ++ * preempt. ++ */ ++ if (memcmp(key, sa_key, 8) == 0) + return; + + status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, noisy); +@@ -297,13 +308,29 @@ void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, + return; + } + /* If there is no reservation, there's nothing to preempt */ +- if (!resp.prin_descriptor.prin_readresv.additional_length) ++ if (!resp.prin_descriptor.prin_readresv.additional_length) { ++ update_prhold(mpp->alias, false); + return; ++ } + /* + * If there reservation is not held by the old key, you don't + * want to preempt it + */ +- if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) != 0) ++ if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) != 0) { ++ /* ++ * If reservation key doesn't match either the old or ++ * the new key, then clear prhold. ++ */ ++ if (memcmp(sa_key, resp.prin_descriptor.prin_readresv.key, 8) != 0) ++ update_prhold(mpp->alias, false); ++ return; ++ } ++ ++ /* ++ * If multipathd doesn't think it is holding the reservation, don't ++ * preempt it ++ */ ++ if (get_prhold(mpp->alias) != PR_SET) + return; + /* Assume this key is being held by an inaccessable path on this + * node. libmpathpersist has never worked if multiple nodes share +@@ -654,19 +681,36 @@ fail_resume: + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; + } + ++static int reservation_key_matches(struct multipath *mpp, uint8_t *key, int noisy) ++{ ++ struct prin_resp resp = {{{.prgeneration = 0}}}; ++ int status; ++ ++ status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, noisy); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: pr in read reservation command failed.", mpp->wwid); ++ return YNU_UNDEF; ++ } ++ if (!resp.prin_descriptor.prin_readresv.additional_length) ++ return YNU_NO; ++ if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) == 0) ++ return YNU_YES; ++ return YNU_NO; ++} ++ + /* + * for MPATH_PROUT_REG_IGN_SA, we use the ignored paramp->key to store the +- * currently registered key. ++ * currently registered key for use in preempt_missing_path(), but only if ++ * the key is holding the reservation. + */ + static void set_ignored_key(struct multipath *mpp, uint8_t *key) + { + memset(key, 0, 8); + if (!get_be64(mpp->reservation_key)) + return; +- if (get_prflag(mpp->alias) == PR_UNSET) ++ if (get_prhold(mpp->alias) == PR_UNSET) + return; +- update_map_pr(mpp, NULL); +- if (mpp->prflag != PR_SET) ++ if (reservation_key_matches(mpp, key, 0) == YNU_NO) + return; + memcpy(key, &mpp->reservation_key, 8); + } +@@ -680,6 +724,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + int ret; + uint64_t prkey; + struct config *conf; ++ bool preempting_reservation = false; + + ret = mpath_get_map(curmp, pathvec, fd, &alias, &mpp); + if (ret != MPATH_PR_SUCCESS) +@@ -729,9 +774,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + case MPATH_PROUT_REG_IGN_SA: + ret= mpath_prout_reg(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); + break; +- case MPATH_PROUT_RES_SA : +- case MPATH_PROUT_PREE_SA : +- case MPATH_PROUT_PREE_AB_SA : ++ case MPATH_PROUT_PREE_SA: ++ case MPATH_PROUT_PREE_AB_SA: ++ if (reservation_key_matches(mpp, paramp->sa_key, noisy) == YNU_YES) ++ preempting_reservation = true; ++ /* fallthrough */ ++ case MPATH_PROUT_RES_SA: + case MPATH_PROUT_CLEAR_SA: + ret = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, + paramp, noisy, NULL); +@@ -759,6 +807,15 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + case MPATH_PROUT_CLEAR_SA: + update_prflag(alias, 0); + update_prkey(alias, 0); ++ break; ++ case MPATH_PROUT_RES_SA: ++ case MPATH_PROUT_REL_SA: ++ update_prhold(alias, (rq_servact == MPATH_PROUT_RES_SA)); ++ break; ++ case MPATH_PROUT_PREE_SA: ++ case MPATH_PROUT_PREE_AB_SA: ++ if (preempting_reservation) ++ update_prhold(alias, true); + } + out1: + free(alias); +diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c +index c8fbe4a2..dd8dd48e 100644 +--- a/libmpathpersist/mpath_updatepr.c ++++ b/libmpathpersist/mpath_updatepr.c +@@ -75,13 +75,13 @@ static int do_update_pr(char *alias, char *cmd, char *key) + return ret; + } + +-int get_prflag(char *mapname) ++static int do_get_pr(char *mapname, const char *cmd) + { + char str[256]; + char *reply; + int prflag; + +- snprintf(str, sizeof(str), "getprstatus map %s", mapname); ++ snprintf(str, sizeof(str), "%s map %s", cmd, mapname); + reply = do_pr(mapname, str); + if (!reply) + prflag = PR_UNKNOWN; +@@ -96,11 +96,26 @@ int get_prflag(char *mapname) + return prflag; + } + ++int get_prflag(char *mapname) ++{ ++ return do_get_pr(mapname, "getprstatus"); ++} ++ ++int get_prhold(char *mapname) ++{ ++ return do_get_pr(mapname, "getprhold"); ++} ++ + int update_prflag(char *mapname, int set) { + return do_update_pr(mapname, (set)? "setprstatus" : "unsetprstatus", + NULL); + } + ++int update_prhold(char *mapname, bool set) ++{ ++ return do_update_pr(mapname, (set) ? "setprhold" : "unsetprhold", NULL); ++} ++ + int update_prkey_flags(char *mapname, uint64_t prkey, uint8_t sa_flags) { + char str[256]; + +diff --git a/libmpathpersist/mpathpr.h b/libmpathpersist/mpathpr.h +index 614be530..b668ef58 100644 +--- a/libmpathpersist/mpathpr.h ++++ b/libmpathpersist/mpathpr.h +@@ -9,6 +9,8 @@ + int update_prflag(char *mapname, int set); + int update_prkey_flags(char *mapname, uint64_t prkey, uint8_t sa_flags); + int get_prflag(char *mapname); ++int get_prhold(char *mapname); ++int update_prhold(char *mapname, bool set); + #define update_prkey(mapname, prkey) update_prkey_flags(mapname, prkey, 0) + + #endif +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 24135aa6..6d0cd867 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -489,6 +489,7 @@ struct multipath { + struct be64 reservation_key; + uint8_t sa_flags; + int prflag; ++ int prhold; + int all_tg_pt; + struct gen_multipath generic_mp; + bool fpin_must_reload; +diff --git a/multipathd/callbacks.c b/multipathd/callbacks.c +index fb87b280..b6b57f45 100644 +--- a/multipathd/callbacks.c ++++ b/multipathd/callbacks.c +@@ -69,4 +69,7 @@ void init_handler_callbacks(void) + set_handler_callback(VRB_UNSETMARGINAL | Q1_PATH, HANDLER(cli_unset_marginal)); + set_handler_callback(VRB_UNSETMARGINAL | Q1_MAP, + HANDLER(cli_unset_all_marginal)); ++ set_handler_callback(VRB_GETPRHOLD | Q1_MAP, HANDLER(cli_getprhold)); ++ set_handler_callback(VRB_SETPRHOLD | Q1_MAP, HANDLER(cli_setprhold)); ++ set_handler_callback(VRB_UNSETPRHOLD | Q1_MAP, HANDLER(cli_unsetprhold)); + } +diff --git a/multipathd/cli.c b/multipathd/cli.c +index 0c89b7cd..bccdda48 100644 +--- a/multipathd/cli.c ++++ b/multipathd/cli.c +@@ -224,7 +224,9 @@ load_keys (void) + r += add_key(keys, "setmarginal", VRB_SETMARGINAL, 0); + r += add_key(keys, "unsetmarginal", VRB_UNSETMARGINAL, 0); + r += add_key(keys, "all", KEY_ALL, 0); +- ++ r += add_key(keys, "getprhold", VRB_GETPRHOLD, 0); ++ r += add_key(keys, "setprhold", VRB_SETPRHOLD, 0); ++ r += add_key(keys, "unsetprhold", VRB_UNSETPRHOLD, 0); + + if (r) { + free_keys(keys); +diff --git a/multipathd/cli.h b/multipathd/cli.h +index c6b79c9d..925575ad 100644 +--- a/multipathd/cli.h ++++ b/multipathd/cli.h +@@ -38,6 +38,9 @@ enum { + VRB_UNSETMARGINAL = 23, + VRB_SHUTDOWN = 24, + VRB_QUIT = 25, ++ VRB_GETPRHOLD = 26, ++ VRB_SETPRHOLD = 27, ++ VRB_UNSETPRHOLD = 28, + + /* Qualifiers, values must be different from verbs */ + KEY_PATH = 65, +diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c +index 8e2e2cea..94d0b63f 100644 +--- a/multipathd/cli_handlers.c ++++ b/multipathd/cli_handlers.c +@@ -1304,6 +1304,10 @@ cli_unsetprstatus(void * v, struct strbuf *reply, void * data) + mpp->prflag = PR_UNSET; + condlog(2, "%s: prflag unset", param); + } ++ if (mpp->prhold != PR_UNSET) { ++ mpp->prhold = PR_UNSET; ++ condlog(2, "%s: prhold unset (by clearing prflag)", param); ++ } + + return 0; + } +@@ -1391,6 +1395,48 @@ cli_setprkey(void * v, struct strbuf *reply, void * data) + return ret; + } + ++static int do_prhold(struct vectors *vecs, char *param, int prhold) ++{ ++ struct multipath *mpp = find_mp_by_str(vecs->mpvec, param); ++ ++ if (!mpp) ++ return -ENODEV; ++ ++ if (mpp->prhold != prhold) { ++ mpp->prhold = prhold; ++ condlog(2, "%s: prhold %s", param, pr_str[prhold]); ++ } ++ ++ return 0; ++} ++ ++static int cli_setprhold(void *v, struct strbuf *reply, void *data) ++{ ++ return do_prhold((struct vectors *)data, get_keyparam(v, KEY_MAP), PR_SET); ++} ++ ++static int cli_unsetprhold(void *v, struct strbuf *reply, void *data) ++{ ++ return do_prhold((struct vectors *)data, get_keyparam(v, KEY_MAP), PR_UNSET); ++} ++ ++static int cli_getprhold(void *v, struct strbuf *reply, void *data) ++{ ++ struct multipath *mpp; ++ struct vectors *vecs = (struct vectors *)data; ++ char *param = get_keyparam(v, KEY_MAP); ++ ++ param = convert_dev(param, 0); ++ ++ mpp = find_mp_by_str(vecs->mpvec, param); ++ if (!mpp) ++ return -ENODEV; ++ ++ append_strbuf_str(reply, pr_str[mpp->prhold]); ++ condlog(3, "%s: reply = %s", param, get_strbuf_str(reply)); ++ return 0; ++} ++ + static int cli_set_marginal(void * v, struct strbuf *reply, void * data) + { + struct vectors * vecs = (struct vectors *)data; +diff --git a/multipathd/main.c b/multipathd/main.c +index bd4342a3..d1d209d3 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3939,6 +3939,33 @@ main (int argc, char *argv[]) + return (child(NULL)); + } + ++static void check_prhold(struct multipath *mpp, struct path *pp) ++{ ++ struct prin_resp resp = {{{.prgeneration = 0}}}; ++ int status; ++ ++ if (mpp->prflag == PR_UNSET) { ++ mpp->prhold = PR_UNSET; ++ return; ++ } ++ if (mpp->prflag != PR_SET || mpp->prhold != PR_UNKNOWN) ++ return; ++ ++ status = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RRES_SA, &resp, 0); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: pr in read reservation command failed: %d", ++ mpp->wwid, status); ++ return; ++ } ++ mpp->prhold = PR_UNSET; ++ if (!resp.prin_descriptor.prin_readresv.additional_length) ++ return; ++ ++ if (memcmp(&mpp->reservation_key, ++ resp.prin_descriptor.prin_readresv.key, 8) == 0) ++ mpp->prhold = PR_SET; ++} ++ + static void mpath_pr_event_handle(struct path *pp) + { + struct multipath *mpp = pp->mpp; +@@ -3950,8 +3977,13 @@ static void mpath_pr_event_handle(struct path *pp) + return; + } + +- if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) ++ if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) { ++ if (mpp->prflag == PR_UNSET) ++ mpp->prhold = PR_UNSET; + return; ++ } ++ ++ check_prhold(mpp, pp); + + if (mpp->prflag != PR_SET) + return; diff --git a/0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch b/0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch new file mode 100644 index 0000000..d337e1a --- /dev/null +++ b/0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch @@ -0,0 +1,35 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:42 -0400 +Subject: [PATCH] libmpathpersist: remove update_map_pr code for NULL pp + +Since update_map_pr is always called with pp set now, remove the code +to handle being called with NULL pp. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 9 +-------- + 1 file changed, 1 insertion(+), 8 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 1948de8d..04bbc455 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -850,15 +850,8 @@ int update_map_pr(struct multipath *mpp, struct path *pp) + condlog(0,"%s : failed to alloc resp in update_map_pr", mpp->alias); + return MPATH_PR_OTHER; + } +- if (!pp && count_active_paths(mpp) == 0) { +- condlog(2, "%s: No available paths to check pr status", mpp->alias); +- goto out; +- } +- if (pp) +- ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); +- else +- ret = mpath_prin_activepath(mpp, MPATH_PRIN_RKEY_SA, resp, noisy); + ++ ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); + if (ret != MPATH_PR_SUCCESS ) + { + if (ret == MPATH_PR_ILLEGAL_REQ) diff --git a/0054-libmpathpersist-move-update_map_pr-to-multipathd.patch b/0054-libmpathpersist-move-update_map_pr-to-multipathd.patch new file mode 100644 index 0000000..8437c81 --- /dev/null +++ b/0054-libmpathpersist-move-update_map_pr-to-multipathd.patch @@ -0,0 +1,222 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:43 -0400 +Subject: [PATCH] libmpathpersist: move update_map_pr to multipathd + +multipathd is now the only program that calls update_map_pr(), so move +it there, and make it static. There are no other code changes. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/libmpathpersist.version | 1 - + libmpathpersist/mpath_persist_int.c | 83 ------------------------- + libmpathpersist/mpath_persist_int.h | 3 +- + multipathd/main.c | 80 ++++++++++++++++++++++++ + 4 files changed, 81 insertions(+), 86 deletions(-) + +diff --git a/libmpathpersist/libmpathpersist.version b/libmpathpersist/libmpathpersist.version +index faa4257b..8068920e 100644 +--- a/libmpathpersist/libmpathpersist.version ++++ b/libmpathpersist/libmpathpersist.version +@@ -33,5 +33,4 @@ __LIBMPATHPERSIST_INT_2.0.0 { + mpath_alloc_prin_response; + prin_do_scsi_ioctl; + prout_do_scsi_ioctl; +- update_map_pr; + }; +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 04bbc455..76bdbc63 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -821,86 +821,3 @@ out1: + free(alias); + return ret; + } +- +-int update_map_pr(struct multipath *mpp, struct path *pp) +-{ +- int noisy=0; +- struct prin_resp *resp; +- unsigned int i; +- int ret = MPATH_PR_OTHER, isFound; +- bool was_set = (mpp->prflag == PR_SET); +- +- /* If pr is explicitly unset, it must be manually set */ +- if (mpp->prflag == PR_UNSET) +- return MPATH_PR_SKIP; +- +- if (!get_be64(mpp->reservation_key)) +- { +- /* Nothing to do. Assuming pr mgmt feature is disabled*/ +- mpp->prflag = PR_UNSET; +- condlog(was_set ? 2 : 4, +- "%s: reservation_key not set in multipath.conf", +- mpp->alias); +- return MPATH_PR_SKIP; +- } +- +- resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); +- if (!resp) +- { +- condlog(0,"%s : failed to alloc resp in update_map_pr", mpp->alias); +- return MPATH_PR_OTHER; +- } +- +- ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); +- if (ret != MPATH_PR_SUCCESS ) +- { +- if (ret == MPATH_PR_ILLEGAL_REQ) +- mpp->prflag = PR_UNSET; +- condlog(0,"%s : pr in read keys service action failed Error=%d", mpp->alias, ret); +- goto out; +- } +- mpp->prflag = PR_UNSET; +- +- if (resp->prin_descriptor.prin_readkeys.additional_length == 0 ) +- { +- condlog(was_set ? 1 : 3, +- "%s: No key found. Device may not be registered. ", +- mpp->alias); +- goto out; +- } +- +- condlog(3, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, +- get_be64(mpp->reservation_key)); +- +- isFound =0; +- for (i = 0; i < resp->prin_descriptor.prin_readkeys.additional_length/8; i++ ) +- { +- if (libmp_verbosity >= 3) { +- condlog(3, "%s: PR IN READKEYS[%d] reservation key:", +- mpp->alias, i); +- dumpHex((char *)&resp->prin_descriptor.prin_readkeys +- .key_list[i * 8], +- 8, 1); +- } +- +- if (!memcmp(&mpp->reservation_key, +- &resp->prin_descriptor.prin_readkeys.key_list[i * 8], +- 8)) { +- condlog(3, "%s: reservation key found in pr in readkeys response", +- mpp->alias); +- isFound =1; +- } +- } +- +- if (isFound) +- { +- mpp->prflag = PR_SET; +- condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); +- } else +- condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", +- mpp->alias); +- +-out: +- free(resp); +- return ret; +-} +diff --git a/libmpathpersist/mpath_persist_int.h b/libmpathpersist/mpath_persist_int.h +index 73c95863..d9fc7448 100644 +--- a/libmpathpersist/mpath_persist_int.h ++++ b/libmpathpersist/mpath_persist_int.h +@@ -19,7 +19,6 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp * resp, int noisy); + int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope, + unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy); +-void dumpHex(const char* , int len, int no_ascii); +-int update_map_pr(struct multipath *mpp, struct path *pp); ++void dumpHex(const char *, int len, int no_ascii); + + #endif /* _MPATH_PERSIST_INT_H */ +diff --git a/multipathd/main.c b/multipathd/main.c +index d1d209d3..0af9cb1c 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3966,6 +3966,86 @@ static void check_prhold(struct multipath *mpp, struct path *pp) + mpp->prhold = PR_SET; + } + ++static int update_map_pr(struct multipath *mpp, struct path *pp) ++{ ++ int noisy = 0; ++ struct prin_resp *resp; ++ unsigned int i; ++ int ret = MPATH_PR_OTHER, isFound; ++ bool was_set = (mpp->prflag == PR_SET); ++ ++ /* If pr is explicitly unset, it must be manually set */ ++ if (mpp->prflag == PR_UNSET) ++ return MPATH_PR_SKIP; ++ ++ if (!get_be64(mpp->reservation_key)) { ++ /* Nothing to do. Assuming pr mgmt feature is disabled*/ ++ mpp->prflag = PR_UNSET; ++ condlog(was_set ? 2 : 4, ++ "%s: reservation_key not set in multipath.conf", ++ mpp->alias); ++ return MPATH_PR_SKIP; ++ } ++ ++ resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); ++ if (!resp) { ++ condlog(0, "%s : failed to alloc resp in update_map_pr", ++ mpp->alias); ++ return MPATH_PR_OTHER; ++ } ++ ++ ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); ++ if (ret != MPATH_PR_SUCCESS) { ++ if (ret == MPATH_PR_ILLEGAL_REQ) ++ mpp->prflag = PR_UNSET; ++ condlog(0, "%s : pr in read keys service action failed Error=%d", ++ mpp->alias, ret); ++ goto out; ++ } ++ mpp->prflag = PR_UNSET; ++ ++ if (resp->prin_descriptor.prin_readkeys.additional_length == 0) { ++ condlog(was_set ? 1 : 3, ++ "%s: No key found. Device may not be registered. ", ++ mpp->alias); ++ goto out; ++ } ++ ++ condlog(3, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, ++ get_be64(mpp->reservation_key)); ++ ++ isFound = 0; ++ for (i = 0; ++ i < resp->prin_descriptor.prin_readkeys.additional_length / 8; i++) { ++ if (libmp_verbosity >= 3) { ++ condlog(3, "%s: PR IN READKEYS[%d] reservation key:", ++ mpp->alias, i); ++ dumpHex((char *)&resp->prin_descriptor.prin_readkeys ++ .key_list[i * 8], ++ 8, 1); ++ } ++ ++ if (!memcmp(&mpp->reservation_key, ++ &resp->prin_descriptor.prin_readkeys.key_list[i * 8], ++ 8)) { ++ condlog(3, "%s: reservation key found in pr in readkeys response", ++ mpp->alias); ++ isFound = 1; ++ } ++ } ++ ++ if (isFound) { ++ mpp->prflag = PR_SET; ++ condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); ++ } else ++ condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", ++ mpp->alias); ++ ++out: ++ free(resp); ++ return ret; ++} ++ + static void mpath_pr_event_handle(struct path *pp) + { + struct multipath *mpp = pp->mpp; diff --git a/0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch b/0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch new file mode 100644 index 0000000..130de5c --- /dev/null +++ b/0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch @@ -0,0 +1,139 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:44 -0400 +Subject: [PATCH] multipathd: clean up update_map_pr and mpath_pr_event_handle + +Store the READ KEYS response and the prout_param_descriptor on the stack +to avoid having to fail these functions for allocation reasons. Don't +explicitly check for additional_length == 0, since the for-loop already +handles that. Also cleanup formatting issues,remove redundant messages, +and reduce the log level of others. + +Signed-off-by: Benjamin Marzinski +--- + multipathd/main.c | 62 +++++++++++++++-------------------------------- + 1 file changed, 19 insertions(+), 43 deletions(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index 0af9cb1c..bc42f2fa 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3968,8 +3968,7 @@ static void check_prhold(struct multipath *mpp, struct path *pp) + + static int update_map_pr(struct multipath *mpp, struct path *pp) + { +- int noisy = 0; +- struct prin_resp *resp; ++ struct prin_resp resp; + unsigned int i; + int ret = MPATH_PR_OTHER, isFound; + bool was_set = (mpp->prflag == PR_SET); +@@ -3987,51 +3986,34 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + return MPATH_PR_SKIP; + } + +- resp = mpath_alloc_prin_response(MPATH_PRIN_RKEY_SA); +- if (!resp) { +- condlog(0, "%s : failed to alloc resp in update_map_pr", +- mpp->alias); +- return MPATH_PR_OTHER; +- } ++ memset(&resp, 0, sizeof(resp)); + +- ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, noisy); ++ ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, &resp, 0); + if (ret != MPATH_PR_SUCCESS) { + if (ret == MPATH_PR_ILLEGAL_REQ) + mpp->prflag = PR_UNSET; + condlog(0, "%s : pr in read keys service action failed Error=%d", + mpp->alias, ret); +- goto out; ++ return ret; + } + mpp->prflag = PR_UNSET; + +- if (resp->prin_descriptor.prin_readkeys.additional_length == 0) { +- condlog(was_set ? 1 : 3, +- "%s: No key found. Device may not be registered. ", +- mpp->alias); +- goto out; +- } +- +- condlog(3, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, ++ condlog(4, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, + get_be64(mpp->reservation_key)); + + isFound = 0; + for (i = 0; +- i < resp->prin_descriptor.prin_readkeys.additional_length / 8; i++) { +- if (libmp_verbosity >= 3) { +- condlog(3, "%s: PR IN READKEYS[%d] reservation key:", ++ i < resp.prin_descriptor.prin_readkeys.additional_length / 8; i++) { ++ uint8_t *keyp = &resp.prin_descriptor.prin_readkeys.key_list[i * 8]; ++ ++ if (libmp_verbosity >= 4) { ++ condlog(4, "%s: PR IN READKEYS[%d] reservation key:", + mpp->alias, i); +- dumpHex((char *)&resp->prin_descriptor.prin_readkeys +- .key_list[i * 8], +- 8, 1); ++ dumpHex((char *)keyp, 8, 1); + } + +- if (!memcmp(&mpp->reservation_key, +- &resp->prin_descriptor.prin_readkeys.key_list[i * 8], +- 8)) { +- condlog(3, "%s: reservation key found in pr in readkeys response", +- mpp->alias); ++ if (!memcmp(&mpp->reservation_key, keyp, 8)) + isFound = 1; +- } + } + + if (isFound) { +@@ -4041,16 +4023,14 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", + mpp->alias); + +-out: +- free(resp); +- return ret; ++ return MPATH_PR_SUCCESS; + } + + static void mpath_pr_event_handle(struct path *pp) + { + struct multipath *mpp = pp->mpp; + int ret; +- struct prout_param_descriptor *param; ++ struct prout_param_descriptor param; + + if (pp->bus != SYSFS_BUS_SCSI) { + mpp->prflag = PR_UNSET; +@@ -4068,21 +4048,17 @@ static void mpath_pr_event_handle(struct path *pp) + if (mpp->prflag != PR_SET) + return; + +- param = (struct prout_param_descriptor *)calloc(1, sizeof(struct prout_param_descriptor)); +- if (!param) +- return; ++ memset(¶m, 0, sizeof(param)); + +- param->sa_flags = mpp->sa_flags; +- memcpy(param->sa_key, &mpp->reservation_key, 8); +- param->num_transportid = 0; ++ param.sa_flags = mpp->sa_flags; ++ memcpy(param.sa_key, &mpp->reservation_key, 8); ++ param.num_transportid = 0; + + condlog(3, "device %s:%s", pp->dev, pp->mpp->wwid); + +- ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, param, 0); ++ ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, ¶m, 0); + if (ret != MPATH_PR_SUCCESS ) + { + condlog(0,"%s: Reservation registration failed. Error: %d", pp->dev, ret); + } +- +- free(param); + } diff --git a/0056-libmpathpersist-clean-up-duplicate-function-declarat.patch b/0056-libmpathpersist-clean-up-duplicate-function-declarat.patch new file mode 100644 index 0000000..3d9ad49 --- /dev/null +++ b/0056-libmpathpersist-clean-up-duplicate-function-declarat.patch @@ -0,0 +1,65 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:45 -0400 +Subject: [PATCH] libmpathpersist: clean up duplicate function declarations + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.h | 1 - + libmpathpersist/mpath_pr_ioctl.c | 10 +++------- + mpathpersist/main.c | 2 -- + 3 files changed, 3 insertions(+), 10 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.h b/libmpathpersist/mpath_persist_int.h +index d9fc7448..aefc17e4 100644 +--- a/libmpathpersist/mpath_persist_int.h ++++ b/libmpathpersist/mpath_persist_int.h +@@ -6,7 +6,6 @@ + * but aren't part of the public libmpathpersist API. + */ + +-void * mpath_alloc_prin_response(int prin_sa); + int do_mpath_persistent_reserve_in(vector curmp, vector pathvec, + int fd, int rq_servact, + struct prin_resp *resp, int noisy); +diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c +index 7e1d2896..dfdbbb65 100644 +--- a/libmpathpersist/mpath_pr_ioctl.c ++++ b/libmpathpersist/mpath_pr_ioctl.c +@@ -14,19 +14,15 @@ + #include "mpath_pr_ioctl.h" + #include "mpath_persist.h" + #include "unaligned.h" +- + #include "debug.h" + #include "structs.h" /* FILE_NAME_SIZE */ ++#include "mpath_persist_int.h" + + #define TIMEOUT 2000 + #define MAXRETRY 5 + +-int prin_do_scsi_ioctl(char * dev, int rq_servact, struct prin_resp *resp, int noisy); +-int mpath_translate_response (char * dev, struct sg_io_hdr io_hdr, +- SenseData_t *Sensedata); +-void dumpHex(const char* str, int len, int no_ascii); +-int prout_do_scsi_ioctl( char * dev, int rq_servact, int rq_scope, +- unsigned int rq_type, struct prout_param_descriptor *paramp, int noisy); ++int mpath_translate_response(char *dev, struct sg_io_hdr io_hdr, ++ SenseData_t *Sensedata); + uint32_t format_transportids(struct prout_param_descriptor *paramp); + void convert_be32_to_cpu(uint32_t *num); + void convert_be16_to_cpu(uint16_t *num); +diff --git a/mpathpersist/main.c b/mpathpersist/main.c +index b6617902..2cacafb7 100644 +--- a/mpathpersist/main.c ++++ b/mpathpersist/main.c +@@ -38,8 +38,6 @@ void mpath_print_buf_readcap(struct prin_resp *pr_buff); + void mpath_print_buf_readfullstat(struct prin_resp *pr_buff); + void mpath_print_buf_readresv(struct prin_resp *pr_buff); + void mpath_print_buf_readkeys(struct prin_resp *pr_buff); +-void dumpHex(const char* str, int len, int no_ascii); +-void * mpath_alloc_prin_response(int prin_sa); + void mpath_print_transport_id(struct prin_fulldescr *fdesc); + int construct_transportid(const char * inp, struct transportid transid[], int num_transportids); + diff --git a/0057-multipathd-wrap-setting-and-unsetting-prflag.patch b/0057-multipathd-wrap-setting-and-unsetting-prflag.patch new file mode 100644 index 0000000..7e6248d --- /dev/null +++ b/0057-multipathd-wrap-setting-and-unsetting-prflag.patch @@ -0,0 +1,148 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:46 -0400 +Subject: [PATCH] multipathd: wrap setting and unsetting prflag + +When prflag is unset, prhold and sa_flags should also be unset. A future +patch will add another variable to be set when prflag is set. Wrap all +these actions in set_pr() and unset_pr(). + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + multipathd/cli_handlers.c | 10 ++++------ + multipathd/main.c | 34 ++++++++++++++++++++-------------- + multipathd/main.h | 2 ++ + 3 files changed, 26 insertions(+), 20 deletions(-) + +diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c +index 94d0b63f..ee2764b9 100644 +--- a/multipathd/cli_handlers.c ++++ b/multipathd/cli_handlers.c +@@ -1280,7 +1280,7 @@ cli_setprstatus(void * v, struct strbuf *reply, void * data) + return -ENODEV; + + if (mpp->prflag != PR_SET) { +- mpp->prflag = PR_SET; ++ set_pr(mpp); + condlog(2, "%s: prflag set", param); + } + +@@ -1301,12 +1301,10 @@ cli_unsetprstatus(void * v, struct strbuf *reply, void * data) + return -ENODEV; + + if (mpp->prflag != PR_UNSET) { +- mpp->prflag = PR_UNSET; + condlog(2, "%s: prflag unset", param); +- } +- if (mpp->prhold != PR_UNSET) { +- mpp->prhold = PR_UNSET; +- condlog(2, "%s: prhold unset (by clearing prflag)", param); ++ if (mpp->prhold != PR_UNSET) ++ condlog(2, "%s: prhold unset (by clearing prflag)", param); ++ unset_pr(mpp); + } + + return 0; +diff --git a/multipathd/main.c b/multipathd/main.c +index bc42f2fa..e29ab2b8 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3944,10 +3944,6 @@ static void check_prhold(struct multipath *mpp, struct path *pp) + struct prin_resp resp = {{{.prgeneration = 0}}}; + int status; + +- if (mpp->prflag == PR_UNSET) { +- mpp->prhold = PR_UNSET; +- return; +- } + if (mpp->prflag != PR_SET || mpp->prhold != PR_UNKNOWN) + return; + +@@ -3966,6 +3962,18 @@ static void check_prhold(struct multipath *mpp, struct path *pp) + mpp->prhold = PR_SET; + } + ++void set_pr(struct multipath *mpp) ++{ ++ mpp->prflag = PR_SET; ++} ++ ++void unset_pr(struct multipath *mpp) ++{ ++ mpp->prflag = PR_UNSET; ++ mpp->prhold = PR_UNSET; ++ mpp->sa_flags = 0; ++} ++ + static int update_map_pr(struct multipath *mpp, struct path *pp) + { + struct prin_resp resp; +@@ -3979,7 +3987,7 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + + if (!get_be64(mpp->reservation_key)) { + /* Nothing to do. Assuming pr mgmt feature is disabled*/ +- mpp->prflag = PR_UNSET; ++ unset_pr(mpp); + condlog(was_set ? 2 : 4, + "%s: reservation_key not set in multipath.conf", + mpp->alias); +@@ -3991,12 +3999,11 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, &resp, 0); + if (ret != MPATH_PR_SUCCESS) { + if (ret == MPATH_PR_ILLEGAL_REQ) +- mpp->prflag = PR_UNSET; ++ unset_pr(mpp); + condlog(0, "%s : pr in read keys service action failed Error=%d", + mpp->alias, ret); + return ret; + } +- mpp->prflag = PR_UNSET; + + condlog(4, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, + get_be64(mpp->reservation_key)); +@@ -4017,11 +4024,13 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + } + + if (isFound) { +- mpp->prflag = PR_SET; ++ set_pr(mpp); + condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); +- } else ++ } else { ++ unset_pr(mpp); + condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", + mpp->alias); ++ } + + return MPATH_PR_SUCCESS; + } +@@ -4033,15 +4042,12 @@ static void mpath_pr_event_handle(struct path *pp) + struct prout_param_descriptor param; + + if (pp->bus != SYSFS_BUS_SCSI) { +- mpp->prflag = PR_UNSET; ++ unset_pr(mpp); + return; + } + +- if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) { +- if (mpp->prflag == PR_UNSET) +- mpp->prhold = PR_UNSET; ++ if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) + return; +- } + + check_prhold(mpp, pp); + +diff --git a/multipathd/main.h b/multipathd/main.h +index 4fcd6402..c9b3e0fd 100644 +--- a/multipathd/main.h ++++ b/multipathd/main.h +@@ -52,4 +52,6 @@ bool check_path_wwid_change(struct path *pp); + int finish_path_init(struct path *pp, struct vectors * vecs); + int resize_map(struct multipath *mpp, unsigned long long size, + struct vectors *vecs); ++void set_pr(struct multipath *mpp); ++void unset_pr(struct multipath *mpp); + #endif /* MAIN_H */ diff --git a/0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch b/0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch new file mode 100644 index 0000000..5fb328a --- /dev/null +++ b/0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch @@ -0,0 +1,145 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:47 -0400 +Subject: [PATCH] multipathd: unregister PR key when path is restored if + necessary + +It is possible that a path was unavailable and either the registered PR +key was removed or the registered PR key was changed and then that new +key was preempted. In both of these situations, this path will still +have a registered key (just not one that matches mpp->reservation_key) +but it should not have one. If the path becomes usable again in this +state, it may allow the multipath device to access storage that it +shouldn't be allowed to access. + +To deal with this, track if a multipath device ever had a registered PR +key. If so, and the device no longer has a registered key, explicitly +clear the key when paths get restored. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmultipath/structs.h | 2 ++ + multipathd/main.c | 46 ++++++++++++++++++++++++++++++++---------- + 2 files changed, 37 insertions(+), 11 deletions(-) + +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 6d0cd867..90127641 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -491,6 +491,8 @@ struct multipath { + int prflag; + int prhold; + int all_tg_pt; ++ bool ever_registered_pr; ++ + struct gen_multipath generic_mp; + bool fpin_must_reload; + }; +diff --git a/multipathd/main.c b/multipathd/main.c +index e29ab2b8..484c8ac1 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -2644,7 +2644,8 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks) + } + + if (newstate == PATH_UP || newstate == PATH_GHOST) { +- if (pp->mpp->prflag != PR_UNSET) { ++ if (pp->mpp->prflag != PR_UNSET || ++ pp->mpp->ever_registered_pr) { + int prflag = pp->mpp->prflag; + /* + * Check Persistent Reservation. +@@ -3964,6 +3965,7 @@ static void check_prhold(struct multipath *mpp, struct path *pp) + + void set_pr(struct multipath *mpp) + { ++ mpp->ever_registered_pr = true; + mpp->prflag = PR_SET; + } + +@@ -3974,16 +3976,21 @@ void unset_pr(struct multipath *mpp) + mpp->sa_flags = 0; + } + ++/* ++ * Returns MPATH_PR_SUCCESS unless if fails to read the PR keys. If ++ * MPATH_PR_SUCCESS is returned, mpp->prflag will be either PR_SET or ++ * PR_UNSET. ++ */ + static int update_map_pr(struct multipath *mpp, struct path *pp) + { + struct prin_resp resp; + unsigned int i; +- int ret = MPATH_PR_OTHER, isFound; ++ int ret, isFound; + bool was_set = (mpp->prflag == PR_SET); + + /* If pr is explicitly unset, it must be manually set */ + if (mpp->prflag == PR_UNSET) +- return MPATH_PR_SKIP; ++ return MPATH_PR_SUCCESS; + + if (!get_be64(mpp->reservation_key)) { + /* Nothing to do. Assuming pr mgmt feature is disabled*/ +@@ -3991,7 +3998,7 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + condlog(was_set ? 2 : 4, + "%s: reservation_key not set in multipath.conf", + mpp->alias); +- return MPATH_PR_SKIP; ++ return MPATH_PR_SUCCESS; + } + + memset(&resp, 0, sizeof(resp)); +@@ -4040,6 +4047,7 @@ static void mpath_pr_event_handle(struct path *pp) + struct multipath *mpp = pp->mpp; + int ret; + struct prout_param_descriptor param; ++ bool clear_reg = false; + + if (pp->bus != SYSFS_BUS_SCSI) { + unset_pr(mpp); +@@ -4051,20 +4059,36 @@ static void mpath_pr_event_handle(struct path *pp) + + check_prhold(mpp, pp); + +- if (mpp->prflag != PR_SET) +- return; ++ if (mpp->prflag != PR_SET) { ++ if (!mpp->ever_registered_pr) ++ return; ++ /* ++ * This path may have been unusable and either the ++ * registration was cleared or the registered ++ * key was switched and then that new key was preempted. ++ * In either case, this path should not have a registration ++ * but it might still have one, just with a different ++ * key than mpp->reservation_key is currently set to. ++ * clear it to be sure. ++ */ ++ clear_reg = true; ++ } + + memset(¶m, 0, sizeof(param)); + +- param.sa_flags = mpp->sa_flags; +- memcpy(param.sa_key, &mpp->reservation_key, 8); +- param.num_transportid = 0; ++ if (!clear_reg) { ++ param.sa_flags = mpp->sa_flags; ++ memcpy(param.sa_key, &mpp->reservation_key, 8); ++ param.num_transportid = 0; ++ } + +- condlog(3, "device %s:%s", pp->dev, pp->mpp->wwid); ++ condlog(3, "%s registration for device %s:%s", ++ clear_reg ? "Clearing" : "Setting", pp->dev, pp->mpp->wwid); + + ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, ¶m, 0); + if (ret != MPATH_PR_SUCCESS ) + { +- condlog(0,"%s: Reservation registration failed. Error: %d", pp->dev, ret); ++ condlog(0, "%s: %s reservation registration failed. Error: %d", ++ clear_reg ? "Clearing" : "Setting", pp->dev, ret); + } + } diff --git a/0059-libmpathpersist-Fix-up-reservation_key-checking.patch b/0059-libmpathpersist-Fix-up-reservation_key-checking.patch new file mode 100644 index 0000000..7f97c90 --- /dev/null +++ b/0059-libmpathpersist-Fix-up-reservation_key-checking.patch @@ -0,0 +1,112 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:48 -0400 +Subject: [PATCH] libmpathpersist: Fix-up reservation_key checking + +The reservation key checking in do_mpath_persistent_reserve_out() was +slightly wrong. It allowed invalid keys for preempting. It now correctly +checks the reservation key for the preempt commands. + +It also was a little overly strict in some places. Formerly, it only +allowed registering from any key to the configured key or registering +from the configured key to any key (as long as you use the prkeys file). +Now it also allows unregistering from any key and registering an +unregistered device to any key (as long as you use the prkeys file). + +Also, clarify the code by replacing prkey with a bool tracking if you +are registering or unregistering. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 53 +++++++++++++++++++++++------ + 1 file changed, 42 insertions(+), 11 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 76bdbc63..ded1af38 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -722,9 +722,9 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + struct multipath *mpp; + char *alias; + int ret; +- uint64_t prkey; ++ uint64_t zerokey = 0; + struct config *conf; +- bool preempting_reservation = false; ++ bool unregistering, preempting_reservation = false; + + ret = mpath_get_map(curmp, pathvec, fd, &alias, &mpp); + if (ret != MPATH_PR_SUCCESS) +@@ -744,11 +744,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + if (rq_servact == MPATH_PROUT_REG_IGN_SA) + set_ignored_key(mpp, paramp->key); + +- memcpy(&prkey, paramp->sa_key, 8); +- if (mpp->prkey_source == PRKEY_SOURCE_FILE && prkey && ++ unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); ++ if (mpp->prkey_source == PRKEY_SOURCE_FILE && !unregistering && + (rq_servact == MPATH_PROUT_REG_IGN_SA || + (rq_servact == MPATH_PROUT_REG_SA && + (!get_be64(mpp->reservation_key) || ++ memcmp(paramp->key, &zerokey, 8) == 0 || + memcmp(paramp->key, &mpp->reservation_key, 8) == 0)))) { + memcpy(&mpp->reservation_key, paramp->sa_key, 8); + if (update_prkey_flags(alias, get_be64(mpp->reservation_key), +@@ -760,12 +761,42 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + } + } + +- if (memcmp(paramp->key, &mpp->reservation_key, 8) && +- memcmp(paramp->sa_key, &mpp->reservation_key, 8) && +- (prkey || rq_servact != MPATH_PROUT_REG_IGN_SA)) { +- condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, alias, get_be64(mpp->reservation_key)); +- ret = MPATH_PR_SYNTAX_ERROR; +- goto out1; ++ /* ++ * If you are registering a non-zero key, mpp->reservation_key ++ * must be set and must equal paramp->sa_key. ++ * If you're not registering a key, mpp->reservation_key must be ++ * set, and must equal paramp->key ++ * If you updated the reservation key above, then you cannot fail ++ * these checks, since mpp->reservation_key has already been set ++ * to match paramp->sa_key, and if you are registering a non-zero ++ * key, then it must be set to a non-zero value. ++ */ ++ if ((rq_servact == MPATH_PROUT_REG_IGN_SA || ++ rq_servact == MPATH_PROUT_REG_SA)) { ++ if (!unregistering && !get_be64(mpp->reservation_key)) { ++ condlog(0, "%s: no configured reservation key", alias); ++ ret = MPATH_PR_SYNTAX_ERROR; ++ goto out1; ++ } ++ if (!unregistering && ++ memcmp(paramp->sa_key, &mpp->reservation_key, 8)) { ++ condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, ++ alias, get_be64(mpp->reservation_key)); ++ ret = MPATH_PR_SYNTAX_ERROR; ++ goto out1; ++ } ++ } else { ++ if (!get_be64(mpp->reservation_key)) { ++ condlog(0, "%s: no configured reservation key", alias); ++ ret = MPATH_PR_SYNTAX_ERROR; ++ goto out1; ++ } ++ if (memcmp(paramp->key, &mpp->reservation_key, 8)) { ++ condlog(0, "%s: configured reservation key doesn't match: 0x%" PRIx64, ++ alias, get_be64(mpp->reservation_key)); ++ ret = MPATH_PR_SYNTAX_ERROR; ++ goto out1; ++ } + } + + switch(rq_servact) +@@ -798,7 +829,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + switch (rq_servact) { + case MPATH_PROUT_REG_SA: + case MPATH_PROUT_REG_IGN_SA: +- if (prkey == 0) { ++ if (unregistering) { + update_prflag(alias, 0); + update_prkey(alias, 0); + } else diff --git a/0060-libmpathpersist-change-how-reservation-conflicts-are.patch b/0060-libmpathpersist-change-how-reservation-conflicts-are.patch new file mode 100644 index 0000000..ccbd971 --- /dev/null +++ b/0060-libmpathpersist-change-how-reservation-conflicts-are.patch @@ -0,0 +1,139 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:49 -0400 +Subject: [PATCH] libmpathpersist: change how reservation conflicts are handled + +If registering a key on a path fails with a reservation conflict in +mpath_prout_reg(), libmpathpersist currently tries to roll back the +registration. This code doesn't always make much sense. First, it +updates the configured key, but doesn't fix it if it does a rollback. +Second, it always rolls the key back to 0x0, unregistering paths that +may have been previously registered. These rollback only happen on the +paths where the registration succeeded, meaning that they were in the +expected state when the command was run. The paths where the command +failed, that were in an unexpected state, remain in that state. + +The code no longer attempts to rollback registrations that failed +with a reservation conflict. Instead, it checks that at least one +path was in the expected state and was successfully registered. If +so, then it assumes that the registration command was a resonable one +and retries it on the paths that failed with a reservation conflict. +But instead of using MPATH_PROUT_REG_SA, it uses MPATH_PROUT_REG_IGN_SA +so that it will ignore the current key. This will keep it from +failing with a reservation conflict because the path doesn't have the +expected key registered on it. If path reservations failed for reasons +other than a reservation conflict, the command still returns failure. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 71 ++++++++++++++++++----------- + 1 file changed, 45 insertions(+), 26 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index ded1af38..e2dc5773 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -356,13 +356,13 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + int i, j, k; + struct pathgroup *pgp = NULL; + struct path *pp = NULL; +- int rollback = 0; ++ bool can_retry = false; ++ bool need_retry = false; + int active_pathcount=0; + int rc; + int count=0; + int status = MPATH_PR_SUCCESS; + int all_tg_pt; +- uint64_t sa_key = 0; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -451,43 +451,62 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + condlog (0, "%s: Thread[%d] failed to join thread %d", mpp->wwid, i, rc); + } + } +- if (!rollback && (thread[i].param.status == MPATH_PR_RESERV_CONFLICT)){ +- rollback = 1; +- sa_key = get_unaligned_be64(¶mp->sa_key[0]); +- status = MPATH_PR_RESERV_CONFLICT ; +- } +- if (!rollback && (status == MPATH_PR_SUCCESS)){ ++ /* ++ * We only retry if there is at least one registration that ++ * returned a reservation conflict (which we need to retry) ++ * and at least one registration the return success, so we ++ * know that the command worked on some of the paths. If ++ * the registation fails on all paths, then it wasn't a ++ * valid request, so there's no need to retry. ++ */ ++ if (thread[i].param.status == MPATH_PR_RESERV_CONFLICT) ++ need_retry = true; ++ else if (thread[i].param.status == MPATH_PR_SUCCESS) ++ can_retry = true; ++ else if (status == MPATH_PR_SUCCESS) + status = thread[i].param.status; +- } + } +- if (rollback && ((rq_servact == MPATH_PROUT_REG_SA) && sa_key != 0 )){ +- condlog (3, "%s: ERROR: initiating pr out rollback", mpp->wwid); +- memcpy(¶mp->key, ¶mp->sa_key, 8); +- memset(¶mp->sa_key, 0, 8); +- for( i=0 ; i < count ; i++){ +- if(thread[i].param.status == MPATH_PR_SUCCESS) { +- rc = pthread_create(&thread[i].id, &attr, mpath_prout_pthread_fn, +- (void *)(&thread[i].param)); +- if (rc){ +- condlog (0, "%s: failed to create thread for rollback. %d", mpp->wwid, rc); +- thread[i].param.status = MPATH_PR_THREAD_ERROR; +- } +- } else ++ if (need_retry && can_retry && rq_servact == MPATH_PROUT_REG_SA && ++ status == MPATH_PR_SUCCESS) { ++ condlog(3, "%s: ERROR: initiating pr out retry", mpp->wwid); ++ for (i = 0; i < count; i++) { ++ if (thread[i].param.status != MPATH_PR_RESERV_CONFLICT) { + thread[i].param.status = MPATH_PR_SKIP; ++ continue; ++ } ++ /* ++ * retry using MPATH_PROUT_REG_IGN_SA to avoid ++ * conflicts. We already know that some paths ++ * succeeded using MPATH_PROUT_REG_SA. ++ */ ++ thread[i].param.rq_servact = MPATH_PROUT_REG_IGN_SA; ++ rc = pthread_create(&thread[i].id, &attr, ++ mpath_prout_pthread_fn, ++ (void *)(&thread[i].param)); ++ if (rc) { ++ condlog(0, "%s: failed to create thread for retry. %d", ++ mpp->wwid, rc); ++ thread[i].param.status = MPATH_PR_THREAD_ERROR; ++ } + } +- for(i=0; i < count ; i++){ ++ for (i = 0; i < count; i++) { + 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", +- mpp->wwid, i); ++ if (rc) { ++ condlog(3, "%s: failed to join thread while retrying %d", ++ mpp->wwid, i); + } ++ if (status == MPATH_PR_SUCCESS) ++ status = thread[i].param.status; + } + } ++ need_retry = false; + } + + pthread_attr_destroy(&attr); ++ if (need_retry) ++ status = MPATH_PR_RESERV_CONFLICT; + if (status == MPATH_PR_SUCCESS) + preempt_missing_path(mpp, paramp->key, paramp->sa_key, noisy); + return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; diff --git a/0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch b/0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch new file mode 100644 index 0000000..d291888 --- /dev/null +++ b/0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch @@ -0,0 +1,51 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:50 -0400 +Subject: [PATCH] libmpathpersist: Clear prkey in multipathd before + unregistering + +When you register or switch keys in libmpathpersist, it updates +mpp->reservation_key in multipathd before doing the registration. This +means that any paths that come online while you are doing the +registration get the new key registered. libmpathpersist didn't do +this when unregistering a key. This could cause the same problem. A +path that got restored while unregistering the device could end up +getting the old key registered on it. Fix this by unsetting the key +before doing the unregister, instead of afterwards. + +There is still a race condition associated with updating +mpp->reservation_key before doing the registration (but not on +unregistration). This will be dealt with by a future patch. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 7 +++---- + 1 file changed, 3 insertions(+), 4 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index e2dc5773..dd056135 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -764,7 +764,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + set_ignored_key(mpp, paramp->key); + + unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); +- if (mpp->prkey_source == PRKEY_SOURCE_FILE && !unregistering && ++ if (mpp->prkey_source == PRKEY_SOURCE_FILE && + (rq_servact == MPATH_PROUT_REG_IGN_SA || + (rq_servact == MPATH_PROUT_REG_SA && + (!get_be64(mpp->reservation_key) || +@@ -848,10 +848,9 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + switch (rq_servact) { + case MPATH_PROUT_REG_SA: + case MPATH_PROUT_REG_IGN_SA: +- if (unregistering) { ++ if (unregistering) + update_prflag(alias, 0); +- update_prkey(alias, 0); +- } else ++ else + update_prflag(alias, 1); + break; + case MPATH_PROUT_CLEAR_SA: diff --git a/0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch b/0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch new file mode 100644 index 0000000..6aff8fb --- /dev/null +++ b/0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch @@ -0,0 +1,28 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:51 -0400 +Subject: [PATCH] libmpathpersist: only clear the key if we are using the + prkeys file + +Otherwise this request will create a useless prkeys file. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index dd056135..a6940d12 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -855,7 +855,8 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + break; + case MPATH_PROUT_CLEAR_SA: + update_prflag(alias, 0); +- update_prkey(alias, 0); ++ if (mpp->prkey_source == PRKEY_SOURCE_FILE) ++ update_prkey(alias, 0); + break; + case MPATH_PROUT_RES_SA: + case MPATH_PROUT_REL_SA: diff --git a/0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch b/0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch new file mode 100644 index 0000000..b0b678c --- /dev/null +++ b/0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch @@ -0,0 +1,51 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:52 -0400 +Subject: [PATCH] libmpathpersist: Restore old reservation key on failure + +If we updated the key and then failed, restore the old key. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index a6940d12..302bebc2 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -742,8 +742,10 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + char *alias; + int ret; + uint64_t zerokey = 0; ++ struct be64 oldkey = {0}; + struct config *conf; + bool unregistering, preempting_reservation = false; ++ bool updated_prkey = false; + + ret = mpath_get_map(curmp, pathvec, fd, &alias, &mpp); + if (ret != MPATH_PR_SUCCESS) +@@ -770,6 +772,8 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + (!get_be64(mpp->reservation_key) || + memcmp(paramp->key, &zerokey, 8) == 0 || + memcmp(paramp->key, &mpp->reservation_key, 8) == 0)))) { ++ updated_prkey = true; ++ memcpy(&oldkey, &mpp->reservation_key, 8); + memcpy(&mpp->reservation_key, paramp->sa_key, 8); + if (update_prkey_flags(alias, get_be64(mpp->reservation_key), + paramp->sa_flags)) { +@@ -842,8 +846,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + goto out1; + } + +- if (ret != MPATH_PR_SUCCESS) ++ if (ret != MPATH_PR_SUCCESS) { ++ if (updated_prkey) ++ update_prkey_flags(mpp->alias, get_be64(oldkey), ++ mpp->sa_flags); + goto out1; ++ } + + switch (rq_servact) { + case MPATH_PROUT_REG_SA: diff --git a/0064-libmpathpersist-update-reservation-key-before-checki.patch b/0064-libmpathpersist-update-reservation-key-before-checki.patch new file mode 100644 index 0000000..21af4e4 --- /dev/null +++ b/0064-libmpathpersist-update-reservation-key-before-checki.patch @@ -0,0 +1,151 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:53 -0400 +Subject: [PATCH] libmpathpersist: update reservation key before checking paths + +There is a race condition when changing reservation keys where a failed +path could come back online after libmpathpersist checks the paths, but +before it updates the reservation key. In this case, the path would come +up and get reregistered with the old key by multipathd, and +libmpathpersist would not update its key, because the path was down +when it checked. + +To fix this, check the paths after updating the key, so any path that +comes up after getting checked will use the updated key. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 80 ++++++++++++----------------- + 1 file changed, 34 insertions(+), 46 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 302bebc2..d498e69e 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -110,39 +110,18 @@ void *mpath_alloc_prin_response(int prin_sa) + return ptr; + } + +-static int get_mpvec(vector curmp, vector pathvec, char *refwwid) ++static int get_path_info(struct multipath *mpp, vector pathvec) + { +- int i; +- struct multipath *mpp; +- +- vector_foreach_slot (curmp, mpp, i){ +- /* +- * discard out of scope maps +- */ +- if (!mpp->alias) { +- condlog(0, "%s: map with empty alias!", __func__); +- continue; +- } +- +- if (mpp->pg != NULL) +- /* Already seen this one */ +- continue; +- +- if (refwwid && strncmp (mpp->alias, refwwid, WWID_SIZE - 1)) +- continue; +- +- if (update_multipath_table(mpp, pathvec, DI_CHECKER) != DMP_OK || +- update_mpp_paths(mpp, pathvec)) { +- condlog(1, "error parsing map %s", mpp->wwid); +- remove_map(mpp, pathvec, curmp); +- i--; +- } else +- extract_hwe_from_path(mpp); ++ if (update_multipath_table(mpp, pathvec, DI_CHECKER) != DMP_OK || ++ update_mpp_paths(mpp, pathvec)) { ++ condlog(0, "error parsing map %s", mpp->wwid); ++ return MPATH_PR_DMMP_ERROR; + } ++ extract_hwe_from_path(mpp); + return MPATH_PR_SUCCESS ; + } + +-static int mpath_get_map(vector curmp, vector pathvec, int fd, char **palias, ++static int mpath_get_map(vector curmp, int fd, char **palias, + struct multipath **pmpp) + { + int ret = MPATH_PR_DMMP_ERROR; +@@ -178,12 +157,6 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, char **palias, + goto out; + } + +- /* get info of all paths from the dm device */ +- if (get_mpvec(curmp, pathvec, alias)){ +- condlog(0, "%s: failed to get device info.", alias); +- goto out; +- } +- + mpp = find_mp_by_alias(curmp, alias); + + if (!mpp) { +@@ -210,7 +183,11 @@ int do_mpath_persistent_reserve_in(vector curmp, vector pathvec, + struct multipath *mpp; + int ret; + +- ret = mpath_get_map(curmp, pathvec, fd, NULL, &mpp); ++ ret = mpath_get_map(curmp, fd, NULL, &mpp); ++ if (ret != MPATH_PR_SUCCESS) ++ return ret; ++ ++ ret = get_path_info(mpp, pathvec); + if (ret != MPATH_PR_SUCCESS) + return ret; + +@@ -747,24 +724,14 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + bool unregistering, preempting_reservation = false; + bool updated_prkey = false; + +- ret = mpath_get_map(curmp, pathvec, fd, &alias, &mpp); ++ ret = mpath_get_map(curmp, fd, &alias, &mpp); + if (ret != MPATH_PR_SUCCESS) + return ret; + + conf = get_multipath_config(); + select_reservation_key(conf, mpp); +- select_all_tg_pt(conf, mpp); +- /* +- * If a device preempts itself, it will need to suspend and resume. +- * Set mpp->skip_kpartx to make sure we set the flags to skip kpartx +- * if necessary, when doing this. +- */ +- select_skip_kpartx(conf, mpp); + put_multipath_config(conf); + +- if (rq_servact == MPATH_PROUT_REG_IGN_SA) +- set_ignored_key(mpp, paramp->key); +- + unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); + if (mpp->prkey_source == PRKEY_SOURCE_FILE && + (rq_servact == MPATH_PROUT_REG_IGN_SA || +@@ -822,6 +789,27 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + } + } + ++ ret = get_path_info(mpp, pathvec); ++ if (ret != MPATH_PR_SUCCESS) { ++ if (updated_prkey) ++ update_prkey_flags(alias, get_be64(oldkey), ++ mpp->sa_flags); ++ goto out1; ++ } ++ ++ conf = get_multipath_config(); ++ select_all_tg_pt(conf, mpp); ++ /* ++ * If a device preempts itself, it will need to suspend and resume. ++ * Set mpp->skip_kpartx to make sure we set the flags to skip kpartx ++ * if necessary, when doing this. ++ */ ++ select_skip_kpartx(conf, mpp); ++ put_multipath_config(conf); ++ ++ if (rq_servact == MPATH_PROUT_REG_IGN_SA) ++ set_ignored_key(mpp, paramp->key); ++ + switch(rq_servact) + { + case MPATH_PROUT_REG_SA: diff --git a/0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch b/0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch new file mode 100644 index 0000000..c5daee4 --- /dev/null +++ b/0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch @@ -0,0 +1,79 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:54 -0400 +Subject: [PATCH] libmpathpersist: retry on conflicts in mpath_prout_common + +mpath_prout_common() just needs to execute a prout command down one +path. If it uses a path that was down when the key was changed and has +since been restored, but multipathd hasn't noticed yet, that path will +still be using the old key. This was causing mpath_prout_common() to +fail with MPATH_PR_RESERV_CONFLICT, even if there were other paths that +would work. + +Now, if prout command fails with MPATH_PR_RESERV_CONFLICT, +mpath_prout_common() checks if pp->dmstate is PSTATE_FAILED. If it is, +mpath_prout_common() assumes that multipathd has not yet noticed that +the path is back online and it might still have and old key, so it +doesn't immediately return. If it can't successfully send the command +down another path, it will still return MPATH_PR_RESERV_CONFLICT. + +Also, make sure prout_do_scsi_ioctl() always returns a MPATH_PR_* +type error. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 14 +++++++++++++- + libmpathpersist/mpath_pr_ioctl.c | 2 +- + 2 files changed, 14 insertions(+), 2 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index d498e69e..762958e8 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -216,6 +216,7 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + struct pathgroup *pgp = NULL; + struct path *pp = NULL; + bool found = false; ++ bool conflict = false; + + vector_foreach_slot (mpp->pg, pgp, j) { + vector_foreach_slot (pgp->paths, pp, i) { +@@ -232,12 +233,23 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + rq_type, paramp, noisy); + if (ret == MPATH_PR_SUCCESS && pptr) + *pptr = pp; ++ /* ++ * If this path is considered down by the kernel, ++ * it may have just come back up, and multipathd ++ * may not have had time to update the key. Allow ++ * reservation conflicts. ++ */ ++ if (ret == MPATH_PR_RESERV_CONFLICT && ++ pp->dmstate == PSTATE_FAILED) { ++ conflict = true; ++ continue; ++ } + if (ret != MPATH_PR_RETRYABLE_ERROR) + return ret; + } + } + if (found) +- return MPATH_PR_OTHER; ++ return conflict ? MPATH_PR_RESERV_CONFLICT : MPATH_PR_OTHER; + condlog(0, "%s: no path available", mpp->wwid); + return MPATH_PR_DMMP_ERROR; + } +diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c +index dfdbbb65..6eaec7cd 100644 +--- a/libmpathpersist/mpath_pr_ioctl.c ++++ b/libmpathpersist/mpath_pr_ioctl.c +@@ -103,7 +103,7 @@ retry : + { + condlog(0, "%s: ioctl failed %d", dev, ret); + close(fd); +- return ret; ++ return MPATH_PR_OTHER; + } + + condlog(4, "%s: Duration=%u (ms)", dev, io_hdr.duration); diff --git a/0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch b/0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch new file mode 100644 index 0000000..2098521 --- /dev/null +++ b/0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch @@ -0,0 +1,154 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 25 Jul 2025 23:58:55 -0400 +Subject: [PATCH] libmpathpersist: Don't always fail registrations for + retryable errors + +When libmpathpersist registers a key, it's possible that a path fails +between when it checks the path's status, and when it tries to do the +registrations on the path. In this case, the registration will fail with +a retryable error. If the registration was allowed to succeed, +multipathd would update the now failed path's key when it came back +online, and everything would work correctly. However it is possible for +a registration to fail with a retryable error on a path that is still +usable. + +Libmpathpersist needs to avoid the case where it does not update the +key of a usable path. Otherwise the path might be able to write to +storage it shouldn't be allowed to. Or it could fail writing to storage +that it should be allowed to write to. So if a registration would +succeed except for retryable errors, libmpathpersist now rechecks all +those paths to see if they are still usable. If they are, then it fails +the registration as before. If they are not, then the registration +succeeds. + +Also, rename can_retry to had_success, since it is used for checking +more than retries now. + +Signed-off-by: Benjamin Marzinski +Reviewed-by: Martin Wilck +--- + libmpathpersist/mpath_persist_int.c | 72 ++++++++++++++++++++++++++--- + 1 file changed, 65 insertions(+), 7 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 762958e8..de757c09 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -337,6 +337,48 @@ void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, + condlog(0, "%s: register: pr preempt command failed.", mpp->wwid); + } + ++/* ++ * If libmpathpersist fails at updating the key on a path with a retryable ++ * error, it has probably failed. But there is a chance that the path is ++ * still usable. To make sure a path isn't active without a key, when it ++ * should have one, or with a key, when it shouldn't have one, check if ++ * the path is still usable. If it is, we must fail the registration. ++ */ ++static int ++check_failed_paths(struct multipath *mpp, struct threadinfo *thread, int count) ++{ ++ int i, j, k; ++ int ret; ++ struct pathgroup *pgp; ++ struct path *pp; ++ struct config *conf; ++ ++ for (i = 0; i < count; i++) { ++ if (thread[i].param.status != MPATH_PR_RETRYABLE_ERROR) ++ continue; ++ vector_foreach_slot (mpp->pg, pgp, j) { ++ vector_foreach_slot (pgp->paths, pp, k) { ++ if (strncmp(pp->dev, thread[i].param.dev, ++ FILE_NAME_SIZE) == 0) ++ goto match; ++ } ++ } ++ /* no match. This shouldn't ever happen. */ ++ condlog(0, "%s: Error: can't find path %s", mpp->wwid, ++ thread[i].param.dev); ++ continue; ++ match: ++ conf = get_multipath_config(); ++ ret = pathinfo(pp, conf, DI_CHECKER); ++ put_multipath_config(conf); ++ /* If pathinfo fails, or if the path is active, return error */ ++ if (ret != PATHINFO_OK || pp->state == PATH_UP || ++ pp->state == PATH_GHOST) ++ return MPATH_PR_OTHER; ++ } ++ return MPATH_PR_SUCCESS; ++} ++ + static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) +@@ -345,8 +387,9 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + int i, j, k; + struct pathgroup *pgp = NULL; + struct path *pp = NULL; +- bool can_retry = false; ++ bool had_success = false; + bool need_retry = false; ++ bool retryable_error = false; + int active_pathcount=0; + int rc; + int count=0; +@@ -450,16 +493,21 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + */ + if (thread[i].param.status == MPATH_PR_RESERV_CONFLICT) + need_retry = true; ++ else if (thread[i].param.status == MPATH_PR_RETRYABLE_ERROR) ++ retryable_error = true; + else if (thread[i].param.status == MPATH_PR_SUCCESS) +- can_retry = true; ++ had_success = true; + else if (status == MPATH_PR_SUCCESS) + status = thread[i].param.status; + } +- if (need_retry && can_retry && rq_servact == MPATH_PROUT_REG_SA && ++ if (need_retry && had_success && rq_servact == MPATH_PROUT_REG_SA && + status == MPATH_PR_SUCCESS) { + condlog(3, "%s: ERROR: initiating pr out retry", mpp->wwid); ++ retryable_error = false; + for (i = 0; i < count; i++) { +- if (thread[i].param.status != MPATH_PR_RESERV_CONFLICT) { ++ /* retry retryable errors and conflicts */ ++ if (thread[i].param.status != MPATH_PR_RESERV_CONFLICT && ++ thread[i].param.status != MPATH_PR_RETRYABLE_ERROR) { + thread[i].param.status = MPATH_PR_SKIP; + continue; + } +@@ -486,7 +534,10 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + condlog(3, "%s: failed to join thread while retrying %d", + mpp->wwid, i); + } +- if (status == MPATH_PR_SUCCESS) ++ if (thread[i].param.status == ++ MPATH_PR_RETRYABLE_ERROR) ++ retryable_error = true; ++ else if (status == MPATH_PR_SUCCESS) + status = thread[i].param.status; + } + } +@@ -495,10 +546,17 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + + pthread_attr_destroy(&attr); + if (need_retry) +- status = MPATH_PR_RESERV_CONFLICT; ++ return MPATH_PR_RESERV_CONFLICT; ++ if (status != MPATH_PR_SUCCESS) ++ return status; ++ /* If you had retryable errors on all paths, fail the registration */ ++ if (!had_success) ++ return MPATH_PR_OTHER; ++ if (retryable_error) ++ status = check_failed_paths(mpp, thread, count); + if (status == MPATH_PR_SUCCESS) + preempt_missing_path(mpp, paramp->key, paramp->sa_key, noisy); +- return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; ++ return status; + } + + static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, diff --git a/0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch b/0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch new file mode 100644 index 0000000..a3bb6b4 --- /dev/null +++ b/0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch @@ -0,0 +1,46 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Mon, 25 Aug 2025 14:09:29 -0400 +Subject: [PATCH] libmpathpersist: Don't try release workaround for invalid + type + +When trying to release a reservation, if the user specified the wrong +reservation type, libmpathpersist would try to preempt the reservation, +because the reservation key matched the device key, but it was not +removed. In this case, the preemption would also fail because it also +requires a matching type. + +Check if the reservation type matches, to avoid attempting the +workaround in this case. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 8 ++++++++ + 1 file changed, 8 insertions(+) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index de757c09..e5ae0836 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -576,6 +576,7 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + uint16_t udev_flags = (mpp->skip_kpartx) ? MPATH_UDEV_NO_KPARTX_FLAG : 0; + bool did_resume = false; + bool all_threads_failed; ++ unsigned int scope_type; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -671,6 +672,13 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + return MPATH_PR_SUCCESS; + } + ++ scope_type = resp.prin_descriptor.prin_readresv.scope_type; ++ if ((scope_type & MPATH_PR_TYPE_MASK) != rq_type) { ++ condlog(2, "%s: --prout_type %u doesn't match reservation %u", ++ mpp->wwid, rq_type, scope_type & MPATH_PR_TYPE_MASK); ++ return MPATH_PR_RESERV_CONFLICT; ++ } ++ + condlog (2, "%s: Path holding reservation is not available.", mpp->wwid); + /* + * Cannot free the reservation because the path that is holding it diff --git a/0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch b/0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch new file mode 100644 index 0000000..a1af1b6 --- /dev/null +++ b/0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch @@ -0,0 +1,43 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 4 Sep 2025 16:33:27 -0400 +Subject: [PATCH] libmpathpersist: Don't fail RESERVE commands unnecessarily + +If you issue a RESERVE to a regular SCSI device that already holds the +reservation, it succeeds (and does nothing). If you issue a RESERVE to a +multipath device that already holds the reservation, it can fail with +a reservation conflict error if you issue the RESERVE to a path that isn't +holding the reservation. Instead, it should try all paths and +succeed if the reservation command succeeds on any of them. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 13 ++++++++++++- + 1 file changed, 12 insertions(+), 1 deletion(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index e5ae0836..45ba68c0 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -238,9 +238,20 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + * it may have just come back up, and multipathd + * may not have had time to update the key. Allow + * reservation conflicts. ++ * ++ * If you issue a RESERVE to a regular scsi device ++ * that already holds the reservation, it succeeds ++ * (and does nothing). A multipath device that ++ * holds the reservation should not return a ++ * reservation conflict on a RESERVE command, just ++ * because it issued the RESERVE to a path that ++ * isn't holding the reservation. It should instead ++ * keep trying to see if it succeeds on another ++ * path. + */ + if (ret == MPATH_PR_RESERV_CONFLICT && +- pp->dmstate == PSTATE_FAILED) { ++ (pp->dmstate == PSTATE_FAILED || ++ rq_servact == MPATH_PROUT_RES_SA)) { + conflict = true; + continue; + } diff --git a/0069-libmpathpersist-reregister-keys-when-self-preempting.patch b/0069-libmpathpersist-reregister-keys-when-self-preempting.patch new file mode 100644 index 0000000..5679469 --- /dev/null +++ b/0069-libmpathpersist-reregister-keys-when-self-preempting.patch @@ -0,0 +1,193 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 5 Sep 2025 14:04:58 -0400 +Subject: [PATCH] libmpathpersist: reregister keys when self preempting + +When a SCSI device preempts its own reservation key, it will remove the +registered keys from all other devices with that reservation key, but +retain its registered key (and possibly acquire the reservation). If a +multipath device preempts its own reservation key, it will also remove +the registered keys from all its paths except the one issuing the +reservation. This means that IO to the device can fail if it goes to one +of these unregistered paths. + +To avoid this, whenever a multipath device preempts itself, it must +first suspend, then do the preemption and reregister the removed keys, +and finally resume the device. This is already what libmpathpersist does +if a release fails because the path holding the reservation is currently +unavailable, with the addition of releasing the reservation after +preempting it. This commit refactors that code into a separate function, +makes the release optional, and calls the new function, preempt_self(), +whenever a device preempts itself. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 133 ++++++++++++++-------------- + 1 file changed, 68 insertions(+), 65 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 45ba68c0..da8a0d04 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -570,6 +570,65 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + return status; + } + ++/* ++ * Called to make a multipath device preempt its own reservation (and ++ * optionally release the reservation). Doing this causes the reservation ++ * keys to be removed from all the device paths except that path used to issue ++ * the preempt, so they need to be restored. To avoid the chance that IO ++ * goes to these paths when they don't have a registered key, the device ++ * is suspended before issuing the preemption, and the keys are reregistered ++ * before resuming it. ++ */ ++static int preempt_self(struct multipath *mpp, int rq_servact, int rq_scope, ++ unsigned int rq_type, int noisy, bool do_release) ++{ ++ int status, rel_status = MPATH_PR_SUCCESS; ++ struct path *pp = NULL; ++ struct prout_param_descriptor paramp = {.sa_flags = 0}; ++ uint16_t udev_flags = (mpp->skip_kpartx) ? MPATH_UDEV_NO_KPARTX_FLAG : 0; ++ ++ if (!dm_simplecmd_noflush(DM_DEVICE_SUSPEND, mpp->alias, 0)) { ++ condlog(0, "%s: self preempt failed to suspend device.", mpp->wwid); ++ return MPATH_PR_OTHER; ++ } ++ ++ memcpy(paramp.key, &mpp->reservation_key, 8); ++ memcpy(paramp.sa_key, &mpp->reservation_key, 8); ++ status = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, ++ ¶mp, noisy, &pp); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: self preempt command failed.", mpp->wwid); ++ goto fail_resume; ++ } ++ ++ if (do_release) { ++ memset(¶mp, 0, sizeof(paramp)); ++ memcpy(paramp.key, &mpp->reservation_key, 8); ++ rel_status = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REL_SA, ++ rq_scope, rq_type, ¶mp, noisy); ++ if (rel_status != MPATH_PR_SUCCESS) ++ condlog(0, "%s: release on alternate path failed.", ++ mpp->wwid); ++ } ++ ++ memset(¶mp, 0, sizeof(paramp)); ++ memcpy(paramp.sa_key, &mpp->reservation_key, 8); ++ status = mpath_prout_reg(mpp, MPATH_PROUT_REG_IGN_SA, rq_scope, ++ rq_type, ¶mp, noisy); ++ if (status != MPATH_PR_SUCCESS) ++ condlog(0, "%s: self preempt failed to reregister paths.", ++ mpp->wwid); ++ ++fail_resume: ++ if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { ++ condlog(0, "%s: self preempt failed to resume device.", mpp->wwid); ++ if (status == MPATH_PR_SUCCESS) ++ status = MPATH_PR_OTHER; ++ } ++ /* return the first error we encountered */ ++ return (rel_status != MPATH_PR_SUCCESS) ? rel_status : status; ++} ++ + static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) +@@ -584,8 +643,6 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + int count = 0; + int status = MPATH_PR_SUCCESS; + struct prin_resp resp = {{{.prgeneration = 0}}}; +- uint16_t udev_flags = (mpp->skip_kpartx) ? MPATH_UDEV_NO_KPARTX_FLAG : 0; +- bool did_resume = false; + bool all_threads_failed; + unsigned int scope_type; + +@@ -700,70 +757,10 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + * preempting one. Since the device is suspended, no IO can + * go to these unregistered paths and fail). + * 3. Releasing the reservation on the path that now holds it. +- * 4. Resuming the device (since it no longer matters that most of +- * that paths no longer have a registered key) +- * 5. Reregistering keys on all the paths ++ * 4. Reregistering keys on all the paths ++ * 5. Resuming the device + */ +- +- if (!dm_simplecmd_noflush(DM_DEVICE_SUSPEND, mpp->alias, 0)) { +- condlog(0, "%s: release: failed to suspend dm device.", mpp->wwid); +- return MPATH_PR_OTHER; +- } +- +- memset(paramp, 0, sizeof(*paramp)); +- memcpy(paramp->key, &mpp->reservation_key, 8); +- memcpy(paramp->sa_key, &mpp->reservation_key, 8); +- status = mpath_prout_common(mpp, MPATH_PROUT_PREE_SA, rq_scope, +- rq_type, paramp, noisy, &pp); +- if (status != MPATH_PR_SUCCESS) { +- condlog(0, "%s: release: pr preempt command failed.", mpp->wwid); +- goto fail_resume; +- } +- +- memset(paramp, 0, sizeof(*paramp)); +- memcpy(paramp->key, &mpp->reservation_key, 8); +- status = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REL_SA, rq_scope, +- rq_type, paramp, noisy); +- if (status != MPATH_PR_SUCCESS) { +- condlog(0, "%s: release on alternate path failed.", mpp->wwid); +- goto out_reregister; +- } +- +- if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { +- condlog(0, "%s release: failed to resume dm device.", mpp->wwid); +- /* +- * leave status set to MPATH_PR_SUCCESS, we will have another +- * chance to resume the device. +- */ +- goto out_reregister; +- } +- did_resume = true; +- +-out_reregister: +- memset(paramp, 0, sizeof(*paramp)); +- memcpy(paramp->sa_key, &mpp->reservation_key, 8); +- rc = mpath_prout_reg(mpp, MPATH_PROUT_REG_IGN_SA, rq_scope, rq_type, +- paramp, noisy); +- if (rc != MPATH_PR_SUCCESS) +- condlog(0, "%s: release: failed to reregister paths.", mpp->wwid); +- +- /* +- * If we failed releasing the reservation or resuming earlier +- * try resuming now. Otherwise, return with the reregistering status +- * This means we will report failure, even though the resevation +- * has been released, since the keys were not reregistered. +- */ +- if (did_resume) +- return rc; +- else if (status == MPATH_PR_SUCCESS) +- status = rc; +-fail_resume: +- if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { +- condlog(0, "%s: release: failed to resume dm device.", mpp->wwid); +- if (status == MPATH_PR_SUCCESS) +- status = MPATH_PR_OTHER; +- } +- return (status == MPATH_PR_RETRYABLE_ERROR) ? MPATH_PR_OTHER : status; ++ return preempt_self(mpp, MPATH_PROUT_PREE_SA, rq_scope, rq_type, noisy, true); + } + + static int reservation_key_matches(struct multipath *mpp, uint8_t *key, int noisy) +@@ -909,6 +906,12 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + case MPATH_PROUT_PREE_AB_SA: + if (reservation_key_matches(mpp, paramp->sa_key, noisy) == YNU_YES) + preempting_reservation = true; ++ /* if we are preempting ourself */ ++ if (memcmp(paramp->sa_key, paramp->key, 8) == 0) { ++ ret = preempt_self(mpp, rq_servact, rq_scope, rq_type, ++ noisy, false); ++ break; ++ } + /* fallthrough */ + case MPATH_PROUT_RES_SA: + case MPATH_PROUT_CLEAR_SA: diff --git a/0070-libmpathpersist-handle-updating-key-race-condition.patch b/0070-libmpathpersist-handle-updating-key-race-condition.patch new file mode 100644 index 0000000..7d02b5e --- /dev/null +++ b/0070-libmpathpersist-handle-updating-key-race-condition.patch @@ -0,0 +1,108 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 9 Sep 2025 18:55:49 -0400 +Subject: [PATCH] libmpathpersist: handle updating key race condition + +If a multipath device's registered key is changed, mpathpersist needs +to update the key in multipathd before it registers the new key on the +paths. This means that there is a time when multipathd thinks the paths +should be using the new key, but none of them are. If a path is +restored after mpathpersist checks which paths are usable to set the +key on, but before it sets the key on any path, multipathd will see +that the new key is not registered on any paths, and think that the +key has been preempted or cleared. This will leave the path without +a key, and possibly make multipathd think the device does not hold +the reservation, even if it does. + +To avoid this, multipathd will now remember the old key when registering +a new one. Once the registration is finished, and (un)setprstatus is +called, multipathd will forget the old key. Until then, multipathd will +check for either key when it looks to see if there is an existing key. +If the registration fails and the key get reverted, multipathd will +also forget the old key. + +Signed-off-by: Benjamin Marzinski +--- + libmultipath/prkey.c | 16 ++++++++++++++-- + libmultipath/structs.h | 1 + + multipathd/main.c | 12 +++++++++++- + 3 files changed, 26 insertions(+), 3 deletions(-) + +diff --git a/libmultipath/prkey.c b/libmultipath/prkey.c +index c66d293b..4fbde5ad 100644 +--- a/libmultipath/prkey.c ++++ b/libmultipath/prkey.c +@@ -222,10 +222,22 @@ int set_prkey(struct config *conf, struct multipath *mpp, uint64_t prkey, + } + else + ret = do_prkey(fd, mpp->wwid, NULL, PRKEY_WRITE); +- if (ret == 0) ++ if (ret == 0) { ++ /* ++ * If you are reverting back to the old key, because you ++ * did not successfully set a new key, don't remember the ++ * key you never successfully set. ++ */ ++ if (get_be64(mpp->old_pr_key) == prkey) ++ memset(&mpp->old_pr_key, 0, 8); ++ else ++ memcpy(&mpp->old_pr_key, &mpp->reservation_key, 8); + select_reservation_key(conf, mpp); +- if (get_be64(mpp->reservation_key) != prkey) ++ } ++ if (get_be64(mpp->reservation_key) != prkey) { ++ memset(&mpp->old_pr_key, 0, 8); + ret = 1; ++ } + out_file: + close(fd); + out: +diff --git a/libmultipath/structs.h b/libmultipath/structs.h +index 90127641..2ff195f3 100644 +--- a/libmultipath/structs.h ++++ b/libmultipath/structs.h +@@ -487,6 +487,7 @@ struct multipath { + /* persistent management data*/ + int prkey_source; + struct be64 reservation_key; ++ struct be64 old_pr_key; + uint8_t sa_flags; + int prflag; + int prhold; +diff --git a/multipathd/main.c b/multipathd/main.c +index 484c8ac1..3b53d140 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3967,6 +3967,7 @@ void set_pr(struct multipath *mpp) + { + mpp->ever_registered_pr = true; + mpp->prflag = PR_SET; ++ memset(&mpp->old_pr_key, 0, 8); + } + + void unset_pr(struct multipath *mpp) +@@ -3974,6 +3975,7 @@ void unset_pr(struct multipath *mpp) + mpp->prflag = PR_UNSET; + mpp->prhold = PR_UNSET; + mpp->sa_flags = 0; ++ memset(&mpp->old_pr_key, 0, 8); + } + + /* +@@ -4026,7 +4028,15 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + dumpHex((char *)keyp, 8, 1); + } + +- if (!memcmp(&mpp->reservation_key, keyp, 8)) ++ /* ++ * If you are in the middle of updating a key (old_pr_key ++ * is set) check for either the new key or the old key, ++ * since you might be checking before any paths have ++ * updated their keys. ++ */ ++ if (!memcmp(&mpp->reservation_key, keyp, 8) || ++ (get_be64(mpp->old_pr_key) && ++ !memcmp(&mpp->old_pr_key, keyp, 8))) + isFound = 1; + } + diff --git a/0071-libmpathpersist-handle-preempting-all-registrants-re.patch b/0071-libmpathpersist-handle-preempting-all-registrants-re.patch new file mode 100644 index 0000000..26edd55 --- /dev/null +++ b/0071-libmpathpersist-handle-preempting-all-registrants-re.patch @@ -0,0 +1,82 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Wed, 10 Sep 2025 17:52:12 -0400 +Subject: [PATCH] libmpathpersist: handle preempting all registrants + reservations + +All Registrants reservations (types 7 and 8) are held by key 0x0. When +preempting one, all registrations are cleared, except for the one on the +path that issued the PREEMPT. libmpathpersist needs to handle this just +like the other cases of self-preemption, so that all the paths of the +preempting multipath device have their registrations restored while the +device is suspended. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 31 +++++++++++++++++++++++++---- + 1 file changed, 27 insertions(+), 4 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index da8a0d04..87d7cb6b 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -579,8 +579,9 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + * is suspended before issuing the preemption, and the keys are reregistered + * before resuming it. + */ +-static int preempt_self(struct multipath *mpp, int rq_servact, int rq_scope, +- unsigned int rq_type, int noisy, bool do_release) ++static int do_preempt_self(struct multipath *mpp, struct be64 sa_key, ++ int rq_servact, int rq_scope, unsigned int rq_type, ++ int noisy, bool do_release) + { + int status, rel_status = MPATH_PR_SUCCESS; + struct path *pp = NULL; +@@ -593,7 +594,7 @@ static int preempt_self(struct multipath *mpp, int rq_servact, int rq_scope, + } + + memcpy(paramp.key, &mpp->reservation_key, 8); +- memcpy(paramp.sa_key, &mpp->reservation_key, 8); ++ memcpy(paramp.sa_key, &sa_key, 8); + status = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, + ¶mp, noisy, &pp); + if (status != MPATH_PR_SUCCESS) { +@@ -629,6 +630,21 @@ fail_resume: + return (rel_status != MPATH_PR_SUCCESS) ? rel_status : status; + } + ++static int preempt_self(struct multipath *mpp, int rq_servact, int rq_scope, ++ unsigned int rq_type, int noisy, bool do_release) ++{ ++ return do_preempt_self(mpp, mpp->reservation_key, rq_servact, rq_scope, ++ rq_type, noisy, do_release); ++} ++ ++static int preempt_all(struct multipath *mpp, int rq_servact, int rq_scope, ++ unsigned int rq_type, int noisy) ++{ ++ struct be64 zerokey = {0}; ++ return do_preempt_self(mpp, zerokey, rq_servact, rq_scope, rq_type, ++ noisy, false); ++} ++ + static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) +@@ -904,8 +920,15 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + break; + case MPATH_PROUT_PREE_SA: + case MPATH_PROUT_PREE_AB_SA: +- if (reservation_key_matches(mpp, paramp->sa_key, noisy) == YNU_YES) ++ if (reservation_key_matches(mpp, paramp->sa_key, noisy) == YNU_YES) { + preempting_reservation = true; ++ if (memcmp(paramp->sa_key, &zerokey, 8) == 0) { ++ /* all registrants case */ ++ ret = preempt_all(mpp, rq_servact, rq_scope, ++ rq_type, noisy); ++ break; ++ } ++ } + /* if we are preempting ourself */ + if (memcmp(paramp->sa_key, paramp->key, 8) == 0) { + ret = preempt_self(mpp, rq_servact, rq_scope, rq_type, diff --git a/0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch b/0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch new file mode 100644 index 0000000..da1b8f3 --- /dev/null +++ b/0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch @@ -0,0 +1,73 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 18 Sep 2025 18:12:07 -0400 +Subject: [PATCH] libmpathpersist: Fix REGISTER AND IGNORE while holding a + reservation + +If a device that is holding a reservation changes its registered key, +but the path holding the reservation is unavailable, libmpathpersist +must preempt the old key to update the reservation. If the key is +changed using REGISTER AND IGNORE, set_ignored_key() determines the old +key to preempt. Unfortunately, commit 165427dda broke it, by comparing +the wrong key against the actual reservation key. Then commit cf0eea85 +broke it more, by using mpp->reservation_key after it had been updated, +so it was no longer the old key. Fix this by correctly comparing the old +key against the actual reservation key. + +Fixes: 165427dda ("libmpathpersist: Add safety check for preempting on key change") +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 13 +++++++------ + 1 file changed, 7 insertions(+), 6 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 87d7cb6b..e3514137 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -801,16 +801,17 @@ static int reservation_key_matches(struct multipath *mpp, uint8_t *key, int nois + * currently registered key for use in preempt_missing_path(), but only if + * the key is holding the reservation. + */ +-static void set_ignored_key(struct multipath *mpp, uint8_t *key) ++static void set_ignored_key(struct multipath *mpp, uint8_t *curr_key, ++ uint8_t *key) + { + memset(key, 0, 8); +- if (!get_be64(mpp->reservation_key)) ++ if (memcmp(curr_key, key, 8) == 0) + return; + if (get_prhold(mpp->alias) == PR_UNSET) + return; +- if (reservation_key_matches(mpp, key, 0) == YNU_NO) ++ if (reservation_key_matches(mpp, curr_key, 0) == YNU_NO) + return; +- memcpy(key, &mpp->reservation_key, 8); ++ memcpy(key, curr_key, 8); + } + + int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, +@@ -834,6 +835,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + select_reservation_key(conf, mpp); + put_multipath_config(conf); + ++ memcpy(&oldkey, &mpp->reservation_key, 8); + unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); + if (mpp->prkey_source == PRKEY_SOURCE_FILE && + (rq_servact == MPATH_PROUT_REG_IGN_SA || +@@ -842,7 +844,6 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + memcmp(paramp->key, &zerokey, 8) == 0 || + memcmp(paramp->key, &mpp->reservation_key, 8) == 0)))) { + updated_prkey = true; +- memcpy(&oldkey, &mpp->reservation_key, 8); + memcpy(&mpp->reservation_key, paramp->sa_key, 8); + if (update_prkey_flags(alias, get_be64(mpp->reservation_key), + paramp->sa_flags)) { +@@ -910,7 +911,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + put_multipath_config(conf); + + if (rq_servact == MPATH_PROUT_REG_IGN_SA) +- set_ignored_key(mpp, paramp->key); ++ set_ignored_key(mpp, (uint8_t *)&oldkey, paramp->key); + + switch(rq_servact) + { diff --git a/0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch b/0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch new file mode 100644 index 0000000..d8731de --- /dev/null +++ b/0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch @@ -0,0 +1,159 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 18 Sep 2025 19:29:08 -0400 +Subject: [PATCH] libmpathpersist: Handle RESERVE with reservation held by + failed path + +Issuing a RESERVE on a device that already holds the reservation should +succeed, as long as the type is the same. But if the path that holds the +reservation is unavailable, mpathpersist fails, since it gets a +reservation conflict on all available paths. To deal with this, if the +multipath device has failed paths, and the key holding the reservation +matches the multipath device's key, and multipathd says that it is +holding the reservation, assume the reservation is held by a failed path +and claim the RESERVE succeeded, even though none of the actual scsi +commands did + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 47 +++++++++++++++++++++++------ + 1 file changed, 37 insertions(+), 10 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index e3514137..0cb08f88 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -210,7 +210,7 @@ static void *mpath_prout_pthread_fn(void *p) + static int + mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + unsigned int rq_type, struct prout_param_descriptor *paramp, +- int noisy, struct path **pptr) ++ int noisy, struct path **pptr, bool *failed_paths) + { + int i, j, ret; + struct pathgroup *pgp = NULL; +@@ -223,6 +223,8 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + if (!((pp->state == PATH_UP) || (pp->state == PATH_GHOST))) { + condlog(1, "%s: %s path not up. Skip", + mpp->wwid, pp->dev); ++ if (failed_paths) ++ *failed_paths = true; + continue; + } + +@@ -257,6 +259,8 @@ mpath_prout_common(struct multipath *mpp, int rq_servact, int rq_scope, + } + if (ret != MPATH_PR_RETRYABLE_ERROR) + return ret; ++ if (failed_paths) ++ *failed_paths = true; + } + } + if (found) +@@ -343,7 +347,7 @@ void preempt_missing_path(struct multipath *mpp, uint8_t *key, uint8_t *sa_key, + memcpy(paramp.key, sa_key, 8); + memcpy(paramp.sa_key, key, 8); + status = mpath_prout_common(mpp, MPATH_PROUT_PREE_SA, rq_scope, +- rq_type, ¶mp, noisy, NULL); ++ rq_type, ¶mp, noisy, NULL, NULL); + if (status != MPATH_PR_SUCCESS) + condlog(0, "%s: register: pr preempt command failed.", mpp->wwid); + } +@@ -596,7 +600,7 @@ static int do_preempt_self(struct multipath *mpp, struct be64 sa_key, + memcpy(paramp.key, &mpp->reservation_key, 8); + memcpy(paramp.sa_key, &sa_key, 8); + status = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, +- ¶mp, noisy, &pp); ++ ¶mp, noisy, &pp, NULL); + if (status != MPATH_PR_SUCCESS) { + condlog(0, "%s: self preempt command failed.", mpp->wwid); + goto fail_resume; +@@ -779,20 +783,25 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + return preempt_self(mpp, MPATH_PROUT_PREE_SA, rq_scope, rq_type, noisy, true); + } + +-static int reservation_key_matches(struct multipath *mpp, uint8_t *key, int noisy) ++static int reservation_key_matches(struct multipath *mpp, uint8_t *key, ++ unsigned int *type) + { + struct prin_resp resp = {{{.prgeneration = 0}}}; + int status; + +- status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, noisy); ++ status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, 0); + if (status != MPATH_PR_SUCCESS) { + condlog(0, "%s: pr in read reservation command failed.", mpp->wwid); + return YNU_UNDEF; + } + if (!resp.prin_descriptor.prin_readresv.additional_length) + return YNU_NO; +- if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) == 0) ++ if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) == 0) { ++ if (type) ++ *type = resp.prin_descriptor.prin_readresv.scope_type & ++ MPATH_PR_TYPE_MASK; + return YNU_YES; ++ } + return YNU_NO; + } + +@@ -809,11 +818,20 @@ static void set_ignored_key(struct multipath *mpp, uint8_t *curr_key, + return; + if (get_prhold(mpp->alias) == PR_UNSET) + return; +- if (reservation_key_matches(mpp, curr_key, 0) == YNU_NO) ++ if (reservation_key_matches(mpp, curr_key, NULL) == YNU_NO) + return; + memcpy(key, curr_key, 8); + } + ++static bool check_holding_reservation(struct multipath *mpp, unsigned int *type) ++{ ++ if (get_be64(mpp->reservation_key) && ++ get_prhold(mpp->alias) == PR_SET && ++ reservation_key_matches(mpp, (uint8_t *)&mpp->reservation_key, type) == YNU_YES) ++ return true; ++ return false; ++} ++ + int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + int rq_servact, int rq_scope, unsigned int rq_type, + struct prout_param_descriptor *paramp, int noisy) +@@ -826,6 +844,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + struct config *conf; + bool unregistering, preempting_reservation = false; + bool updated_prkey = false; ++ bool failed_paths = false; + + ret = mpath_get_map(curmp, fd, &alias, &mpp); + if (ret != MPATH_PR_SUCCESS) +@@ -921,7 +940,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + break; + case MPATH_PROUT_PREE_SA: + case MPATH_PROUT_PREE_AB_SA: +- if (reservation_key_matches(mpp, paramp->sa_key, noisy) == YNU_YES) { ++ if (reservation_key_matches(mpp, paramp->sa_key, NULL) == YNU_YES) { + preempting_reservation = true; + if (memcmp(paramp->sa_key, &zerokey, 8) == 0) { + /* all registrants case */ +@@ -938,10 +957,18 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + } + /* fallthrough */ + case MPATH_PROUT_RES_SA: +- case MPATH_PROUT_CLEAR_SA: ++ case MPATH_PROUT_CLEAR_SA: { ++ unsigned int res_type; + ret = mpath_prout_common(mpp, rq_servact, rq_scope, rq_type, +- paramp, noisy, NULL); ++ paramp, noisy, NULL, &failed_paths); ++ if (rq_servact == MPATH_PROUT_RES_SA && ++ ret != MPATH_PR_SUCCESS && failed_paths && ++ check_holding_reservation(mpp, &res_type) && ++ res_type == rq_type) ++ /* The reserve failed, but multipathd says we hold it */ ++ ret = MPATH_PR_SUCCESS; + break; ++ } + case MPATH_PROUT_REL_SA: + ret = mpath_prout_rel(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); + break; diff --git a/0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch b/0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch new file mode 100644 index 0000000..09750c1 --- /dev/null +++ b/0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch @@ -0,0 +1,149 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Thu, 25 Sep 2025 16:48:49 -0400 +Subject: [PATCH] libmpathpersist: use check_holding_reservation in + mpath_prout_rel + +Instead of open-coding mostly the same work, just make mpath_prout_rel() +call check_holding_reservation(). + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist_int.c | 86 ++++++++++++----------------- + 1 file changed, 35 insertions(+), 51 deletions(-) + +diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c +index 0cb08f88..0eb7041e 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -649,12 +649,42 @@ static int preempt_all(struct multipath *mpp, int rq_servact, int rq_scope, + noisy, false); + } + ++static int reservation_key_matches(struct multipath *mpp, uint8_t *key, ++ unsigned int *type) ++{ ++ struct prin_resp resp = {{{.prgeneration = 0}}}; ++ int status; ++ ++ status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, 0); ++ if (status != MPATH_PR_SUCCESS) { ++ condlog(0, "%s: pr in read reservation command failed.", mpp->wwid); ++ return YNU_UNDEF; ++ } ++ if (!resp.prin_descriptor.prin_readresv.additional_length) ++ return YNU_NO; ++ if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) == 0) { ++ if (type) ++ *type = resp.prin_descriptor.prin_readresv.scope_type & ++ MPATH_PR_TYPE_MASK; ++ return YNU_YES; ++ } ++ return YNU_NO; ++} ++ ++static bool check_holding_reservation(struct multipath *mpp, unsigned int *type) ++{ ++ if (get_be64(mpp->reservation_key) && ++ get_prhold(mpp->alias) == PR_SET && ++ reservation_key_matches(mpp, (uint8_t *)&mpp->reservation_key, type) == YNU_YES) ++ return true; ++ return false; ++} ++ + static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + unsigned int rq_type, + struct prout_param_descriptor * paramp, int noisy) + { + int i, j; +- int num = 0; + struct pathgroup *pgp = NULL; + struct path *pp = NULL; + int active_pathcount = 0; +@@ -662,9 +692,8 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + int rc; + int count = 0; + int status = MPATH_PR_SUCCESS; +- struct prin_resp resp = {{{.prgeneration = 0}}}; + bool all_threads_failed; +- unsigned int scope_type; ++ unsigned int res_type; + + if (!mpp) + return MPATH_PR_DMMP_ERROR; +@@ -743,27 +772,13 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + return status; + } + +- status = mpath_prin_activepath (mpp, MPATH_PRIN_RRES_SA, &resp, noisy); +- if (status != MPATH_PR_SUCCESS){ +- condlog (0, "%s: pr in read reservation command failed.", mpp->wwid); +- return MPATH_PR_OTHER; +- } +- +- num = resp.prin_descriptor.prin_readresv.additional_length / 8; +- if (num == 0){ +- 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)) { ++ if (!check_holding_reservation(mpp, &res_type)) { + condlog(2, "%s: Releasing key not holding reservation.", mpp->wwid); + return MPATH_PR_SUCCESS; + } +- +- scope_type = resp.prin_descriptor.prin_readresv.scope_type; +- if ((scope_type & MPATH_PR_TYPE_MASK) != rq_type) { ++ if (res_type != rq_type) { + condlog(2, "%s: --prout_type %u doesn't match reservation %u", +- mpp->wwid, rq_type, scope_type & MPATH_PR_TYPE_MASK); ++ mpp->wwid, rq_type, res_type); + return MPATH_PR_RESERV_CONFLICT; + } + +@@ -783,28 +798,6 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + return preempt_self(mpp, MPATH_PROUT_PREE_SA, rq_scope, rq_type, noisy, true); + } + +-static int reservation_key_matches(struct multipath *mpp, uint8_t *key, +- unsigned int *type) +-{ +- struct prin_resp resp = {{{.prgeneration = 0}}}; +- int status; +- +- status = mpath_prin_activepath(mpp, MPATH_PRIN_RRES_SA, &resp, 0); +- if (status != MPATH_PR_SUCCESS) { +- condlog(0, "%s: pr in read reservation command failed.", mpp->wwid); +- return YNU_UNDEF; +- } +- if (!resp.prin_descriptor.prin_readresv.additional_length) +- return YNU_NO; +- if (memcmp(key, resp.prin_descriptor.prin_readresv.key, 8) == 0) { +- if (type) +- *type = resp.prin_descriptor.prin_readresv.scope_type & +- MPATH_PR_TYPE_MASK; +- return YNU_YES; +- } +- return YNU_NO; +-} +- + /* + * for MPATH_PROUT_REG_IGN_SA, we use the ignored paramp->key to store the + * currently registered key for use in preempt_missing_path(), but only if +@@ -823,15 +816,6 @@ static void set_ignored_key(struct multipath *mpp, uint8_t *curr_key, + memcpy(key, curr_key, 8); + } + +-static bool check_holding_reservation(struct multipath *mpp, unsigned int *type) +-{ +- if (get_be64(mpp->reservation_key) && +- get_prhold(mpp->alias) == PR_SET && +- reservation_key_matches(mpp, (uint8_t *)&mpp->reservation_key, type) == YNU_YES) +- return true; +- return false; +-} +- + int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + int rq_servact, int rq_scope, unsigned int rq_type, + struct prout_param_descriptor *paramp, int noisy) diff --git a/0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch b/0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch new file mode 100644 index 0000000..1c2f288 --- /dev/null +++ b/0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch @@ -0,0 +1,262 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Fri, 19 Sep 2025 18:17:20 -0400 +Subject: [PATCH] libmpathpersist: Fix unregistering while holding the + reservation + +There were two problems with how libmpathpersist handled unregistering +a key while holding the reseravation (which should also release the +reservation). +1. If the path holding the reservation is not unregistered first, there + will be unregistered paths, while a reservation is still held, which + would cause IO to those paths to fail, when it shouldn't. +2. If the path that holds the reservation is down, libmpathpersist was + not clearing the reservation, since the there were no registered keys + it could use for the PREEMPT command workaround + +To fix these, libmpathpersist now releases the reservation first when +trying to unregister a key that is holding the reservation. +mpath_prout_rel() has a new option so that if it needs to self preempt +to clear the reservation, it won't re-register the paths when called +as part of unregistering a key. Also, instead of checking if the device +is currently holding a reservation using mpp->reservation_key in +check_holding_reservation() (which will already be set to 0 when called +as part of unregistering a key), pass in the key to check. + +Signed-off-by: Benjamin Marzinski +--- + libmpathpersist/mpath_persist.h | 3 + + libmpathpersist/mpath_persist_int.c | 108 ++++++++++++++++++++-------- + 2 files changed, 80 insertions(+), 31 deletions(-) + +diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h +index b83fc3bd..22c1d4ac 100644 +--- a/libmpathpersist/mpath_persist.h ++++ b/libmpathpersist/mpath_persist.h +@@ -66,6 +66,9 @@ extern "C" { + #define MPATH_PR_RETRYABLE_ERROR 16 /* error that might be succeed + down another path. Internal + only. */ ++#define MPATH_PR_SUCCESS_UNREGISTER 17 /* Success, and additionally, all ++ paths were unregistered. ++ 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 0eb7041e..00334a1a 100644 +--- a/libmpathpersist/mpath_persist_int.c ++++ b/libmpathpersist/mpath_persist_int.c +@@ -574,18 +574,23 @@ static int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope, + return status; + } + ++enum preempt_work { ++ PREE_WORK_NONE, ++ PREE_WORK_REL, ++ PREE_WORK_REL_UNREG, ++}; + /* + * Called to make a multipath device preempt its own reservation (and +- * optionally release the reservation). Doing this causes the reservation +- * keys to be removed from all the device paths except that path used to issue +- * the preempt, so they need to be restored. To avoid the chance that IO +- * goes to these paths when they don't have a registered key, the device +- * is suspended before issuing the preemption, and the keys are reregistered +- * before resuming it. ++ * optional extra work). Doing this causes the reservation keys to be removed ++ * from all the device paths except that path used to issue the preempt, so ++ * they may need to be restored. To avoid the chance that IO goes to these ++ * paths when they don't have a registered key and a reservation exists, the ++ * device is suspended before issuing the preemption, and the keys are ++ * reregistered (or the reservation is released) before resuming it. + */ + static int do_preempt_self(struct multipath *mpp, struct be64 sa_key, + int rq_servact, int rq_scope, unsigned int rq_type, +- int noisy, bool do_release) ++ int noisy, enum preempt_work work) + { + int status, rel_status = MPATH_PR_SUCCESS; + struct path *pp = NULL; +@@ -606,7 +611,7 @@ static int do_preempt_self(struct multipath *mpp, struct be64 sa_key, + goto fail_resume; + } + +- if (do_release) { ++ if (work != PREE_WORK_NONE) { + memset(¶mp, 0, sizeof(paramp)); + memcpy(paramp.key, &mpp->reservation_key, 8); + rel_status = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REL_SA, +@@ -614,15 +619,26 @@ static int do_preempt_self(struct multipath *mpp, struct be64 sa_key, + if (rel_status != MPATH_PR_SUCCESS) + condlog(0, "%s: release on alternate path failed.", + mpp->wwid); ++ else if (work == PREE_WORK_REL_UNREG) { ++ /* unregister the last path */ ++ rel_status = prout_do_scsi_ioctl(pp->dev, ++ MPATH_PROUT_REG_IGN_SA, ++ rq_scope, rq_type, ++ ¶mp, noisy); ++ if (rel_status != MPATH_PR_SUCCESS) ++ condlog(0, "%s: final self preempt unregister failed,", ++ mpp->wwid); ++ } ++ } ++ if (work != PREE_WORK_REL_UNREG) { ++ memset(¶mp, 0, sizeof(paramp)); ++ memcpy(paramp.sa_key, &mpp->reservation_key, 8); ++ status = mpath_prout_reg(mpp, MPATH_PROUT_REG_IGN_SA, rq_scope, ++ rq_type, ¶mp, noisy); ++ if (status != MPATH_PR_SUCCESS) ++ condlog(0, "%s: self preempt failed to reregister paths.", ++ mpp->wwid); + } +- +- memset(¶mp, 0, sizeof(paramp)); +- memcpy(paramp.sa_key, &mpp->reservation_key, 8); +- status = mpath_prout_reg(mpp, MPATH_PROUT_REG_IGN_SA, rq_scope, +- rq_type, ¶mp, noisy); +- if (status != MPATH_PR_SUCCESS) +- condlog(0, "%s: self preempt failed to reregister paths.", +- mpp->wwid); + + fail_resume: + if (!dm_simplecmd_noflush(DM_DEVICE_RESUME, mpp->alias, udev_flags)) { +@@ -635,10 +651,10 @@ fail_resume: + } + + static int preempt_self(struct multipath *mpp, int rq_servact, int rq_scope, +- unsigned int rq_type, int noisy, bool do_release) ++ unsigned int rq_type, int noisy, enum preempt_work work) + { + return do_preempt_self(mpp, mpp->reservation_key, rq_servact, rq_scope, +- rq_type, noisy, do_release); ++ rq_type, noisy, work); + } + + static int preempt_all(struct multipath *mpp, int rq_servact, int rq_scope, +@@ -646,7 +662,7 @@ static int preempt_all(struct multipath *mpp, int rq_servact, int rq_scope, + { + struct be64 zerokey = {0}; + return do_preempt_self(mpp, zerokey, rq_servact, rq_scope, rq_type, +- noisy, false); ++ noisy, PREE_WORK_NONE); + } + + static int reservation_key_matches(struct multipath *mpp, uint8_t *key, +@@ -671,18 +687,21 @@ static int reservation_key_matches(struct multipath *mpp, uint8_t *key, + return YNU_NO; + } + +-static bool check_holding_reservation(struct multipath *mpp, unsigned int *type) ++static bool check_holding_reservation(struct multipath *mpp, uint8_t *curr_key, ++ unsigned int *type) + { +- if (get_be64(mpp->reservation_key) && ++ uint64_t zerokey = 0; ++ if (memcmp(curr_key, &zerokey, 8) != 0 && + get_prhold(mpp->alias) == PR_SET && +- reservation_key_matches(mpp, (uint8_t *)&mpp->reservation_key, type) == YNU_YES) ++ reservation_key_matches(mpp, curr_key, type) == YNU_YES) + return true; + return false; + } + +-static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, ++static int mpath_prout_rel(struct multipath *mpp, int rq_servact, int rq_scope, + unsigned int rq_type, +- struct prout_param_descriptor * paramp, int noisy) ++ struct prout_param_descriptor *paramp, int noisy, ++ bool unregister) + { + int i, j; + struct pathgroup *pgp = NULL; +@@ -772,7 +791,8 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + return status; + } + +- if (!check_holding_reservation(mpp, &res_type)) { ++ if (!check_holding_reservation(mpp, (uint8_t *)&mpp->reservation_key, ++ &res_type)) { + condlog(2, "%s: Releasing key not holding reservation.", mpp->wwid); + return MPATH_PR_SUCCESS; + } +@@ -795,7 +815,13 @@ static int mpath_prout_rel(struct multipath *mpp,int rq_servact, int rq_scope, + * 4. Reregistering keys on all the paths + * 5. Resuming the device + */ +- return preempt_self(mpp, MPATH_PROUT_PREE_SA, rq_scope, rq_type, noisy, true); ++ status = preempt_self(mpp, MPATH_PROUT_PREE_SA, rq_scope, rq_type, ++ noisy, ++ unregister ? PREE_WORK_REL_UNREG : PREE_WORK_REL); ++ if (status == MPATH_PR_SUCCESS && unregister) ++ return MPATH_PR_SUCCESS_UNREGISTER; ++ return status; ++ + } + + /* +@@ -838,7 +864,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + select_reservation_key(conf, mpp); + put_multipath_config(conf); + +- memcpy(&oldkey, &mpp->reservation_key, 8); ++ oldkey = mpp->reservation_key; + unregistering = (memcmp(&zerokey, paramp->sa_key, 8) == 0); + if (mpp->prkey_source == PRKEY_SOURCE_FILE && + (rq_servact == MPATH_PROUT_REG_IGN_SA || +@@ -920,7 +946,27 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + { + case MPATH_PROUT_REG_SA: + case MPATH_PROUT_REG_IGN_SA: +- ret= mpath_prout_reg(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); ++ if (unregistering && check_holding_reservation(mpp, (uint8_t *)&oldkey, &rq_type)) { ++ struct be64 newkey = mpp->reservation_key; ++ /* temporarily restore reservation key */ ++ mpp->reservation_key = oldkey; ++ ret = mpath_prout_rel(mpp, MPATH_PROUT_REL_SA, rq_scope, ++ rq_type, paramp, noisy, true); ++ mpp->reservation_key = newkey; ++ if (ret == MPATH_PR_SUCCESS) ++ /* ++ * Since unregistering it true, paramp->sa_key ++ * must be zero here. So this command will ++ * unregister the key. ++ */ ++ ret = mpath_prout_reg(mpp, rq_servact, rq_scope, ++ rq_type, paramp, noisy); ++ else if (ret == MPATH_PR_SUCCESS_UNREGISTER) ++ /* We already unregistered the key */ ++ ret = MPATH_PR_SUCCESS; ++ } else ++ ret = mpath_prout_reg(mpp, rq_servact, rq_scope, ++ rq_type, paramp, noisy); + break; + case MPATH_PROUT_PREE_SA: + case MPATH_PROUT_PREE_AB_SA: +@@ -936,7 +982,7 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + /* if we are preempting ourself */ + if (memcmp(paramp->sa_key, paramp->key, 8) == 0) { + ret = preempt_self(mpp, rq_servact, rq_scope, rq_type, +- noisy, false); ++ noisy, PREE_WORK_NONE); + break; + } + /* fallthrough */ +@@ -947,14 +993,14 @@ int do_mpath_persistent_reserve_out(vector curmp, vector pathvec, int fd, + paramp, noisy, NULL, &failed_paths); + if (rq_servact == MPATH_PROUT_RES_SA && + ret != MPATH_PR_SUCCESS && failed_paths && +- check_holding_reservation(mpp, &res_type) && ++ check_holding_reservation(mpp, paramp->key, &res_type) && + res_type == rq_type) + /* The reserve failed, but multipathd says we hold it */ + ret = MPATH_PR_SUCCESS; + break; + } + case MPATH_PROUT_REL_SA: +- ret = mpath_prout_rel(mpp, rq_servact, rq_scope, rq_type, paramp, noisy); ++ ret = mpath_prout_rel(mpp, rq_servact, rq_scope, rq_type, paramp, noisy, false); + break; + default: + ret = MPATH_PR_OTHER; diff --git a/0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch b/0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch new file mode 100644 index 0000000..d48a8f1 --- /dev/null +++ b/0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch @@ -0,0 +1,214 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 30 Sep 2025 19:36:36 -0400 +Subject: [PATCH] libmpathpersist: Fix race between restoring a path and + preemption + +If a multipath device gets preempted while a path is being restored, the +path could end up getting the old key registered on it anyways. This +happens by: +1. multipathd calling update_map_pr() in mpath_pr_event_handle() and + seeing that there are matching keys. +2. Those matching keys get preempted. +3. multipathd registering the preempted key on the path. + +Since there is no way to guarantee that the key won't get preempted +after multipathd checks for it, mpath_pr_event_handle() now calls +update_map_pr() to check the registered keys again after registering the +key. There must be at least as many keys registered after registering +the key on the restored path (which may already have a key registered on +it, so it's possible that the number of registered keys doesn't change). + +pr_register_active_paths() calls mpath_pr_event_handle() for all working +paths on a device. In order to avoid calling update_map_pr() twice for +every path, mpath_pr_event_handle() now takes the number of keys to +expect, and skips the initial call to update_map_pr() if it already +knows how many keys are registered before trying to register the new +path. + +Signed-off-by: Benjamin Marzinski +--- + multipathd/main.c | 77 ++++++++++++++++++++++++++++++++--------------- + 1 file changed, 52 insertions(+), 25 deletions(-) + +diff --git a/multipathd/main.c b/multipathd/main.c +index 3b53d140..06ff6858 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -89,7 +89,8 @@ + #define CMDSIZE 160 + #define MSG_SIZE 32 + +-static void mpath_pr_event_handle(struct path *pp); ++static unsigned int ++mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed); + + #define LOG_MSG(lvl, pp) \ + do { \ +@@ -638,7 +639,7 @@ flush_map_nopaths(struct multipath *mpp, struct vectors *vecs) { + static void + pr_register_active_paths(struct multipath *mpp) + { +- unsigned int i, j; ++ unsigned int i, j, nr_keys = 0; + struct path *pp; + struct pathgroup *pgp; + +@@ -647,7 +648,7 @@ pr_register_active_paths(struct multipath *mpp) + if (mpp->prflag == PR_UNSET) + return; + if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST)) +- mpath_pr_event_handle(pp); ++ nr_keys = mpath_pr_event_handle(pp, nr_keys); + } + } + } +@@ -1298,7 +1299,7 @@ rescan: + verify_paths(mpp); + mpp->action = ACT_RELOAD; + prflag = mpp->prflag; +- mpath_pr_event_handle(pp); ++ mpath_pr_event_handle(pp, 0); + } else { + if (!should_multipath(pp, vecs->pathvec, vecs->mpvec)) { + orphan_path(pp, "only one path"); +@@ -2652,7 +2653,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks) + */ + condlog(2, "%s: checking persistent " + "reservation registration", pp->dev); +- mpath_pr_event_handle(pp); ++ mpath_pr_event_handle(pp, 0); + if (pp->mpp->prflag == PR_SET && + prflag != PR_SET) + pr_register_active_paths(pp->mpp); +@@ -3982,12 +3983,16 @@ void unset_pr(struct multipath *mpp) + * Returns MPATH_PR_SUCCESS unless if fails to read the PR keys. If + * MPATH_PR_SUCCESS is returned, mpp->prflag will be either PR_SET or + * PR_UNSET. ++ * ++ * The number of found keys must be at least as large as *nr_keys, ++ * and if MPATH_PR_SUCCESS is returned and mpp->prflag is PR_SET after ++ * the call, *nr_keys will be set to the number of found keys. + */ +-static int update_map_pr(struct multipath *mpp, struct path *pp) ++static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *nr_keys) + { + struct prin_resp resp; +- unsigned int i; +- int ret, isFound; ++ unsigned int i, nr_found = 0; ++ int ret; + bool was_set = (mpp->prflag == PR_SET); + + /* If pr is explicitly unset, it must be manually set */ +@@ -4017,7 +4022,6 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + condlog(4, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias, + get_be64(mpp->reservation_key)); + +- isFound = 0; + for (i = 0; + i < resp.prin_descriptor.prin_readkeys.additional_length / 8; i++) { + uint8_t *keyp = &resp.prin_descriptor.prin_readkeys.key_list[i * 8]; +@@ -4037,41 +4041,57 @@ static int update_map_pr(struct multipath *mpp, struct path *pp) + if (!memcmp(&mpp->reservation_key, keyp, 8) || + (get_be64(mpp->old_pr_key) && + !memcmp(&mpp->old_pr_key, keyp, 8))) +- isFound = 1; ++ nr_found++; + } + +- if (isFound) { ++ if (nr_found >= *nr_keys) { + set_pr(mpp); +- condlog(was_set ? 3 : 2, "%s: key found. prflag set.", mpp->alias); ++ condlog(was_set ? 3 : 2, "%s: %u keys found. prflag set.", ++ mpp->alias, nr_found); ++ *nr_keys = nr_found; + } else { + unset_pr(mpp); +- condlog(was_set ? 1 : 3, "%s: key not found. prflag unset.", +- mpp->alias); ++ condlog(was_set ? 1 : 3, ++ "%s: %u keys found. needed %u. prflag unset.", ++ mpp->alias, nr_found, *nr_keys); + } + + return MPATH_PR_SUCCESS; + } + +-static void mpath_pr_event_handle(struct path *pp) ++/* ++ * This function is called with the number of registered keys that should be ++ * seen for this device to know that the key has not been preempted while the ++ * path was getting registered. If 0 is passed in, update_mpath_pr is called ++ * before registering the key to figure out the number, assuming that at ++ * least one key must exist. ++ * ++ * The function returns the number of keys that are registered or 0 if ++ * it's unknown. ++ */ ++static unsigned int mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed) + { + struct multipath *mpp = pp->mpp; + int ret; +- struct prout_param_descriptor param; ++ struct prout_param_descriptor param = {.sa_flags = 0}; + bool clear_reg = false; + + if (pp->bus != SYSFS_BUS_SCSI) { + unset_pr(mpp); +- return; ++ return 0; + } + +- if (update_map_pr(mpp, pp) != MPATH_PR_SUCCESS) +- return; ++ if (nr_keys_needed == 0) { ++ nr_keys_needed = 1; ++ if (update_map_pr(mpp, pp, &nr_keys_needed) != MPATH_PR_SUCCESS) ++ return 0; ++ } + + check_prhold(mpp, pp); + + if (mpp->prflag != PR_SET) { + if (!mpp->ever_registered_pr) +- return; ++ return 0; + /* + * This path may have been unusable and either the + * registration was cleared or the registered +@@ -4084,21 +4104,28 @@ static void mpath_pr_event_handle(struct path *pp) + clear_reg = true; + } + +- memset(¶m, 0, sizeof(param)); +- + if (!clear_reg) { + param.sa_flags = mpp->sa_flags; + memcpy(param.sa_key, &mpp->reservation_key, 8); + param.num_transportid = 0; + } +- ++retry: + condlog(3, "%s registration for device %s:%s", + clear_reg ? "Clearing" : "Setting", pp->dev, pp->mpp->wwid); + + ret = prout_do_scsi_ioctl(pp->dev, MPATH_PROUT_REG_IGN_SA, 0, 0, ¶m, 0); +- if (ret != MPATH_PR_SUCCESS ) +- { ++ if (ret != MPATH_PR_SUCCESS) { + condlog(0, "%s: %s reservation registration failed. Error: %d", + clear_reg ? "Clearing" : "Setting", pp->dev, ret); ++ } else if (!clear_reg) { ++ if (update_map_pr(mpp, pp, &nr_keys_needed) != MPATH_PR_SUCCESS) ++ return 0; ++ if (mpp->prflag != PR_SET) { ++ memset(¶m, 0, sizeof(param)); ++ clear_reg = true; ++ goto retry; ++ } ++ return nr_keys_needed; + } ++ return 0; + } diff --git a/0077-multipathd-Fix-tracking-of-old-PR-key.patch b/0077-multipathd-Fix-tracking-of-old-PR-key.patch new file mode 100644 index 0000000..9d23d93 --- /dev/null +++ b/0077-multipathd-Fix-tracking-of-old-PR-key.patch @@ -0,0 +1,49 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Benjamin Marzinski +Date: Tue, 30 Sep 2025 23:32:11 -0400 +Subject: [PATCH] multipathd: Fix tracking of old PR key + +When libmpathpersist is in the process of changing a registered key, +multipathd needs to remember both the old and new values of the key to +deal with a race between limpathpersist updating the key and multipathd +restoring a failed path. It was supposed to stop remembering the old +value when libmpathpersist was done changing the key and issued a +"setprstatus" command. However, clearing the old key was done in +set_pr(), which only gets called by cli_setprstatus() when registering a +new device, not when changing the key on a registered device. Also, +set_pr() is called by update_map_pr(). This means that multipathd was +forgetting the key when it shouldn't and not when it should. Fix this +to only forget the key when cli_setprstatus() is called, and always +then. + +Fixes: 1aeffa9e ("libmpathpersist: handle updating key race condition") +Signed-off-by: Benjamin Marzinski +--- + multipathd/cli_handlers.c | 1 + + multipathd/main.c | 1 - + 2 files changed, 1 insertion(+), 1 deletion(-) + +diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c +index ee2764b9..1696dbd4 100644 +--- a/multipathd/cli_handlers.c ++++ b/multipathd/cli_handlers.c +@@ -1283,6 +1283,7 @@ cli_setprstatus(void * v, struct strbuf *reply, void * data) + set_pr(mpp); + condlog(2, "%s: prflag set", param); + } ++ memset(&mpp->old_pr_key, 0, 8); + + return 0; + } +diff --git a/multipathd/main.c b/multipathd/main.c +index 06ff6858..e17a3355 100644 +--- a/multipathd/main.c ++++ b/multipathd/main.c +@@ -3968,7 +3968,6 @@ void set_pr(struct multipath *mpp) + { + mpp->ever_registered_pr = true; + mpp->prflag = PR_SET; +- memset(&mpp->old_pr_key, 0, 8); + } + + void unset_pr(struct multipath *mpp) diff --git a/device-mapper-multipath.spec b/device-mapper-multipath.spec index fa8846d..bb0423d 100644 --- a/device-mapper-multipath.spec +++ b/device-mapper-multipath.spec @@ -1,6 +1,6 @@ Name: device-mapper-multipath Version: 0.9.9 -Release: 12%{?dist} +Release: 13%{?dist} Summary: Tools to manage multipath devices using device-mapper License: GPLv2 URL: http://christophe.varoqui.free.fr/ @@ -44,6 +44,49 @@ Patch0031: 0031-libmultipath-add-helper-function-check_path_wwid_cha.patch Patch0032: 0032-multipathd-re-add-paths-skipped-because-they-were-of.patch Patch0033: 0033-multipath-tools-add-HPE-MSA-Gen7-2070-2072-to-hwtabl.patch Patch0034: 0034-libmultipath-fix-crash-in-print_foreign_topology.patch +Patch0035: 0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch +Patch0036: 0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch +Patch0037: 0037-libmpathpersist-check-released-key-against-the-reser.patch +Patch0038: 0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch +Patch0039: 0039-libmpathpersist-remove-uneeded-wrapper-function.patch +Patch0040: 0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch +Patch0041: 0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch +Patch0042: 0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch +Patch0043: 0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch +Patch0044: 0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch +Patch0045: 0045-libmpathpersist-remove-useless-function-send_prout_a.patch +Patch0046: 0046-libmpathpersist-redesign-failed-release-workaround.patch +Patch0047: 0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch +Patch0048: 0048-libmpathpersist-Handle-changing-key-corner-case.patch +Patch0049: 0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch +Patch0050: 0050-libmultipath-rename-prflag_value-enums.patch +Patch0051: 0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch +Patch0052: 0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch +Patch0053: 0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch +Patch0054: 0054-libmpathpersist-move-update_map_pr-to-multipathd.patch +Patch0055: 0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch +Patch0056: 0056-libmpathpersist-clean-up-duplicate-function-declarat.patch +Patch0057: 0057-multipathd-wrap-setting-and-unsetting-prflag.patch +Patch0058: 0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch +Patch0059: 0059-libmpathpersist-Fix-up-reservation_key-checking.patch +Patch0060: 0060-libmpathpersist-change-how-reservation-conflicts-are.patch +Patch0061: 0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch +Patch0062: 0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch +Patch0063: 0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch +Patch0064: 0064-libmpathpersist-update-reservation-key-before-checki.patch +Patch0065: 0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch +Patch0066: 0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch +Patch0067: 0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch +Patch0068: 0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch +Patch0069: 0069-libmpathpersist-reregister-keys-when-self-preempting.patch +Patch0070: 0070-libmpathpersist-handle-updating-key-race-condition.patch +Patch0071: 0071-libmpathpersist-handle-preempting-all-registrants-re.patch +Patch0072: 0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch +Patch0073: 0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch +Patch0074: 0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch +Patch0075: 0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch +Patch0076: 0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch +Patch0077: 0077-multipathd-Fix-tracking-of-old-PR-key.patch # runtime Requires: %{name}-libs = %{version}-%{release} @@ -253,6 +296,54 @@ fi %{_pkgconfdir}/libdmmp.pc %changelog +* Wed Oct 1 2025 Benjamin Marzinski - 0.9.9-13 +- Add 0035-libmpathpersist-fix-memory-leak-in-mpath_prout_rel.patch +- Add 0036-libmpathpersist-retry-commands-on-other-paths-in-mpa.patch +- Add 0037-libmpathpersist-check-released-key-against-the-reser.patch +- Add 0038-multipathd-remove-thread-from-mpath_pr_event_handle.patch +- Add 0039-libmpathpersist-remove-uneeded-wrapper-function.patch +- Add 0040-libmpathpersist-reduce-log-level-for-persistent-rese.patch +- Add 0041-libmpathpersist-remove-pointless-update_map_pr-ret-v.patch +- Add 0042-multipathd-use-update_map_pr-in-mpath_pr_event_handl.patch +- Add 0043-libmpathpersist-limit-changing-prflag-in-update_map_.patch +- Add 0044-multipathd-Don-t-call-update_map_pr-unnecessarily.patch +- Add 0045-libmpathpersist-remove-useless-function-send_prout_a.patch +- Add 0046-libmpathpersist-redesign-failed-release-workaround.patch +- Add 0047-libmpathpersist-fail-the-release-if-all-threads-fail.patch +- Add 0048-libmpathpersist-Handle-changing-key-corner-case.patch +- Add 0049-libmpathpersist-Handle-REGISTER-AND-IGNORE-changing-.patch +- Add 0050-libmultipath-rename-prflag_value-enums.patch +- Add 0051-libmpathpersist-use-a-switch-statement-for-prout-com.patch +- Add 0052-libmpathpersist-Add-safety-check-for-preempting-on-k.patch +- Add 0053-libmpathpersist-remove-update_map_pr-code-for-NULL-p.patch +- Add 0054-libmpathpersist-move-update_map_pr-to-multipathd.patch +- Add 0055-multipathd-clean-up-update_map_pr-and-mpath_pr_event.patch +- Add 0056-libmpathpersist-clean-up-duplicate-function-declarat.patch +- Add 0057-multipathd-wrap-setting-and-unsetting-prflag.patch +- Add 0058-multipathd-unregister-PR-key-when-path-is-restored-i.patch +- Add 0059-libmpathpersist-Fix-up-reservation_key-checking.patch +- Add 0060-libmpathpersist-change-how-reservation-conflicts-are.patch +- Add 0061-libmpathpersist-Clear-prkey-in-multipathd-before-unr.patch +- Add 0062-libmpathpersist-only-clear-the-key-if-we-are-using-t.patch +- Add 0063-libmpathpersist-Restore-old-reservation-key-on-failu.patch +- Add 0064-libmpathpersist-update-reservation-key-before-checki.patch +- Add 0065-libmpathpersist-retry-on-conflicts-in-mpath_prout_co.patch +- Add 0066-libmpathpersist-Don-t-always-fail-registrations-for-.patch +- Add 0067-libmpathpersist-Don-t-try-release-workaround-for-inv.patch +- Add 0068-libmpathpersist-Don-t-fail-RESERVE-commands-unnecess.patch +- Add 0069-libmpathpersist-reregister-keys-when-self-preempting.patch +- Add 0070-libmpathpersist-handle-updating-key-race-condition.patch +- Add 0071-libmpathpersist-handle-preempting-all-registrants-re.patch +- Add 0072-libmpathpersist-Fix-REGISTER-AND-IGNORE-while-holdin.patch +- Add 0073-libmpathpersist-Handle-RESERVE-with-reservation-held.patch +- Add 0074-libmpathpersist-use-check_holding_reservation-in-mpa.patch +- Add 0075-libmpathpersist-Fix-unregistering-while-holding-the-.patch +- Add 0076-libmpathpersist-Fix-race-between-restoring-a-path-an.patch +- Add 0077-multipathd-Fix-tracking-of-old-PR-key.patch + * Fixes RHEL-118720 ("There are many bugs in multipath's persistent + reservation handling [rhel-10]") +- Resolves: RHEL-118720 + * Sat Sep 13 2025 Lin Li - 0.9.9-12 Add 0034-libmultipath-fix-crash-in-print_foreign_topology.patch * Fixes RHEL-107436 ("Running multipath -ll on system with "enable_foreign