rpms/device-mapper-multipath
6
0
Fork 0
Code Issues Pull Requests Releases Activity

device-mapper-multipath-0.9.9-14

Browse Source 
Add 0078-multipathd-Fix-race-while-registering-PR-key.patch
Add 0079-mpathpersist-Fix-REPORT-CAPABILITIES-output.patch
  * Fixes RHEL-118720 ("There are many bugs in multipath's persistent
    reservation handling [rhel-10]")
Resolves: RHEL-118720
This commit is contained in:
Benjamin Marzinski 2025-11-11 19:20:49 -05:00
parent 695f9436f4
commit 7f8db5e1b7
3 changed files with 357 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

239
0078-multipathd-Fix-race-while-registering-PR-key.patch Normal file
View File

@ -0,0 +1,239 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Thu, 6 Nov 2025 20:08:14 -0500
Subject: [PATCH] multipathd: Fix race while registering PR key
When libmpathpersist registers a key, It first checks which paths are
active, then registers the key on those paths, and then tells multipathd
that the key has been registered, and it should start tracking it. If
a path comes up after libmpathpersist checks for active paths, but before
it tells multipathd that the key has been registered, multipathd will not
register a key no that path (since it hasn't been told to that the device
has a registered key yet). This can leave the device with a path that is
missing a key.
To solve this, when multipathd is told that a key has been registered,
it checks if there are the same number of registered keys as active
paths. If there aren't, it registers keys on all the paths (at least
until the number of registered keys and active paths are equal). To
avoid doing a bunch of unnecessary PR work, pr_register_active_paths()
has a new option to track the number of active paths, and
mpath_pr_event_handle() now takes the number of keys to expect. The
first time it's called, when there is an unknown number of keys, if the
number of keys it finds matches the number of active_paths, it and
pr_register_active_paths() exit early, since there is no registering
work to do.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
multipathd/cli_handlers.c | 6 ++++-
multipathd/main.c | 57 ++++++++++++++++++++++++++++-----------
multipathd/main.h | 1 +
3 files changed, 48 insertions(+), 16 deletions(-)
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 1696dbd4..1677dd1e 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1281,7 +1281,11 @@ cli_setprstatus(void * v, struct strbuf *reply, void * data)
if (mpp->prflag != PR_SET) {
set_pr(mpp);
- condlog(2, "%s: prflag set", param);
+ pr_register_active_paths(mpp, true);
+ if (mpp->prflag == PR_SET)
+ condlog(2, "%s: prflag set", param);
+ else
+ condlog(0, "%s: Failed to set prflag", param);
}
memset(&mpp->old_pr_key, 0, 8);
diff --git a/multipathd/main.c b/multipathd/main.c
index e17a3355..0882ef2a 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -90,7 +90,8 @@
#define MSG_SIZE 32
static unsigned int
-mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed);
+mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed,
+ unsigned int nr_keys_wanted);
#define LOG_MSG(lvl, pp) \
do { \
@@ -636,19 +637,28 @@ flush_map_nopaths(struct multipath *mpp, struct vectors *vecs) {
return true;
}
-static void
-pr_register_active_paths(struct multipath *mpp)
+void pr_register_active_paths(struct multipath *mpp, bool check_nr_active)
{
unsigned int i, j, nr_keys = 0;
+ unsigned int nr_active = 0;
struct path *pp;
struct pathgroup *pgp;
+ if (check_nr_active) {
+ nr_active = count_active_paths(mpp);
+ if (!nr_active)
+ return;
+ }
+
vector_foreach_slot (mpp->pg, pgp, i) {
vector_foreach_slot (pgp->paths, pp, j) {
if (mpp->prflag == PR_UNSET)
return;
- if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST))
- nr_keys = mpath_pr_event_handle(pp, nr_keys);
+ if (pp->state == PATH_UP || pp->state == PATH_GHOST) {
+ nr_keys = mpath_pr_event_handle(pp, nr_keys, nr_active);
+ if (check_nr_active && nr_keys == nr_active)
+ return;
+ }
}
}
}
@@ -735,7 +745,7 @@ fail:
sync_map_state(mpp);
- pr_register_active_paths(mpp);
+ pr_register_active_paths(mpp, false);
if (VECTOR_SIZE(offline_paths) != 0)
handle_orphaned_offline_paths(offline_paths);
@@ -1299,7 +1309,7 @@ rescan:
verify_paths(mpp);
mpp->action = ACT_RELOAD;
prflag = mpp->prflag;
- mpath_pr_event_handle(pp, 0);
+ mpath_pr_event_handle(pp, 0, 0);
} else {
if (!should_multipath(pp, vecs->pathvec, vecs->mpvec)) {
orphan_path(pp, "only one path");
@@ -1383,7 +1393,7 @@ rescan:
if (retries >= 0) {
if ((mpp->prflag == PR_SET && prflag != PR_SET) || start_waiter)
- pr_register_active_paths(mpp);
+ pr_register_active_paths(mpp, false);
condlog(2, "%s [%s]: path added to devmap %s",
pp->dev, pp->dev_t, mpp->alias);
return 0;
@@ -2653,10 +2663,10 @@ 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, 0);
+ mpath_pr_event_handle(pp, 0, 0);
if (pp->mpp->prflag == PR_SET &&
prflag != PR_SET)
- pr_register_active_paths(pp->mpp);
+ pr_register_active_paths(pp->mpp, false);
}
}
@@ -3027,7 +3037,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);
- pr_register_active_paths(mpp);
+ pr_register_active_paths(mpp, false);
}
/*
@@ -3985,7 +3995,8 @@ void unset_pr(struct multipath *mpp)
*
* 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.
+ * the call, *nr_keys will be set to the number of found keys. Otherwise
+ * it will be set to 0.
*/
static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *nr_keys)
{
@@ -3995,8 +4006,10 @@ static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *n
bool was_set = (mpp->prflag == PR_SET);
/* If pr is explicitly unset, it must be manually set */
- if (mpp->prflag == PR_UNSET)
+ if (mpp->prflag == PR_UNSET) {
+ *nr_keys = 0;
return MPATH_PR_SUCCESS;
+ }
if (!get_be64(mpp->reservation_key)) {
/* Nothing to do. Assuming pr mgmt feature is disabled*/
@@ -4004,6 +4017,7 @@ static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *n
condlog(was_set ? 2 : 4,
"%s: reservation_key not set in multipath.conf",
mpp->alias);
+ *nr_keys = 0;
return MPATH_PR_SUCCESS;
}
@@ -4015,6 +4029,7 @@ static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *n
unset_pr(mpp);
condlog(0, "%s : pr in read keys service action failed Error=%d",
mpp->alias, ret);
+ *nr_keys = 0;
return ret;
}
@@ -4053,22 +4068,32 @@ static int update_map_pr(struct multipath *mpp, struct path *pp, unsigned int *n
condlog(was_set ? 1 : 3,
"%s: %u keys found. needed %u. prflag unset.",
mpp->alias, nr_found, *nr_keys);
+ *nr_keys = 0;
}
return MPATH_PR_SUCCESS;
}
/*
- * This function is called with the number of registered keys that should be
+ * This function is called with two numbers
+ *
+ * nr_keys_needed: 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.
*
+ * nr_keys_wanted: Only used if nr_keys_needed is 0, so we don't know how
+ * many keys we currently have. If nr_keys_wanted in non-zero and the
+ * number of keys found by the initial call to update_map_pr() matches it,
+ * exit early, since we have all the keys we are expecting.
+ *
* 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)
+static unsigned int
+mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_needed,
+ unsigned int nr_keys_wanted)
{
struct multipath *mpp = pp->mpp;
int ret;
@@ -4084,6 +4109,8 @@ static unsigned int mpath_pr_event_handle(struct path *pp, unsigned int nr_keys_
nr_keys_needed = 1;
if (update_map_pr(mpp, pp, &nr_keys_needed) != MPATH_PR_SUCCESS)
return 0;
+ if (nr_keys_wanted && nr_keys_wanted == nr_keys_needed)
+ return nr_keys_needed;
}
check_prhold(mpp, pp);
diff --git a/multipathd/main.h b/multipathd/main.h
index c9b3e0fd..25ddbaa7 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -54,4 +54,5 @@ 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);
+void pr_register_active_paths(struct multipath *mpp, bool check_active_nr);
#endif /* MAIN_H */

108
0079-mpathpersist-Fix-REPORT-CAPABILITIES-output.patch Normal file
View File

@ -0,0 +1,108 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Mon, 10 Nov 2025 15:25:51 -0500
Subject: [PATCH] mpathpersist: Fix REPORT CAPABILITIES output
mpathpersist was incorrectly parsing the REPORT CAPABILITES service
action output. In reality, the type mask is two bytes where the type
information is stored in bits 7, 6, 5, 3, & 1 (0xea) of the first byte
and bit 0 (0x01) of the second byte. libmpathpersist was treating these
two bytes as a big endian 16 bit number, but mpathpersist was looking
for bits in that number as if it was little endian number.
Ideally, libmpathpersist would treat prin_capdescr.pr_type_mask as
two bytes, like it does for the flags. But we already expose this
as a 16 bit number, where we treated the input bytes as a big endian
number. There's no great reason to mess with the libmpathpersist API,
when we can just make mpathpersist treat this data like libmpathpersist
provides it. So, fix mpathpersist to print the data out correctly.
Additionally, instead of printing a 1 or a 0 to indicate if a type was
supported or not, it was printing the value of the type flag. Also,
Persist Through Power Loss Capable (PTPL_C) was being reported if any
bit in flags[0] was set. Fix these as well. Reformat all of the
capability printing lines, since it is less confusing than only
reformatting some of them.
Fixes: ae4e8a6 ("mpathpersist: Add new utility for managing persistent reservation on dm multipath device")
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathpersist/mpath_persist.h | 4 ++-
mpathpersist/main.c | 43 +++++++++++++++++++++++----------
2 files changed, 33 insertions(+), 14 deletions(-)
diff --git a/libmpathpersist/mpath_persist.h b/libmpathpersist/mpath_persist.h
index 22c1d4ac..4ac40bed 100644
--- a/libmpathpersist/mpath_persist.h
+++ b/libmpathpersist/mpath_persist.h
@@ -118,7 +118,9 @@ struct prin_capdescr
{
uint16_t length;
uint8_t flags[2];
- uint16_t pr_type_mask;
+ uint16_t pr_type_mask; /* The two bytes of the type mask are treated
+ as a single big-endian number. So the valid
+ type bits are 0xea01 */
uint16_t _reserved;
};
diff --git a/mpathpersist/main.c b/mpathpersist/main.c
index 2cacafb7..33ab69cb 100644
--- a/mpathpersist/main.c
+++ b/mpathpersist/main.c
@@ -748,25 +748,42 @@ void mpath_print_buf_readcap( struct prin_resp *pr_buff)
printf("Report capabilities response:\n");
- printf(" Compatible Reservation Handling(CRH): %d\n", !!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x10));
- printf(" Specify Initiator Ports Capable(SIP_C): %d\n",!!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x8));
- printf(" All Target Ports Capable(ATP_C): %d\n",!!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x4 ));
- printf(" Persist Through Power Loss Capable(PTPL_C): %d\n",!!(pr_buff->prin_descriptor.prin_readcap.flags[0]));
- printf(" Type Mask Valid(TMV): %d\n", !!(pr_buff->prin_descriptor.prin_readcap.flags[1] & 0x80));
- printf(" Allow Commands: %d\n", !!(( pr_buff->prin_descriptor.prin_readcap.flags[1] >> 4) & 0x7));
+ printf(" Compatible Reservation Handling(CRH): %d\n",
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x10));
+ printf(" Specify Initiator Ports Capable(SIP_C): %d\n",
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x8));
+ printf(" All Target Ports Capable(ATP_C): %d\n",
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x4));
+ printf(" Persist Through Power Loss Capable(PTPL_C): %d\n",
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[0] & 0x1));
+ printf(" Type Mask Valid(TMV): %d\n",
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[1] & 0x80));
+ printf(" Allow Commands: %d\n",
+ !!((pr_buff->prin_descriptor.prin_readcap.flags[1] >> 4) & 0x7));
printf(" Persist Through Power Loss Active(PTPL_A): %d\n",
- !!(pr_buff->prin_descriptor.prin_readcap.flags[1] & 0x1));
+ !!(pr_buff->prin_descriptor.prin_readcap.flags[1] & 0x1));
if(pr_buff->prin_descriptor.prin_readcap.flags[1] & 0x80)
{
printf(" Support indicated in Type mask:\n");
- printf(" %s: %d\n", pr_type_strs[7], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x80);
- printf(" %s: %d\n", pr_type_strs[6], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x40);
- printf(" %s: %d\n", pr_type_strs[5], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x20);
- printf(" %s: %d\n", pr_type_strs[3], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x8);
- printf(" %s: %d\n", pr_type_strs[1], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x2);
- printf(" %s: %d\n", pr_type_strs[8], pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x100);
+ printf(" %s: %d\n", pr_type_strs[7],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask &
+ 0x8000));
+ printf(" %s: %d\n", pr_type_strs[6],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask &
+ 0x4000));
+ printf(" %s: %d\n", pr_type_strs[5],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask &
+ 0x2000));
+ printf(" %s: %d\n", pr_type_strs[3],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask &
+ 0x800));
+ printf(" %s: %d\n", pr_type_strs[1],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask &
+ 0x200));
+ printf(" %s: %d\n", pr_type_strs[8],
+ !!(pr_buff->prin_descriptor.prin_readcap.pr_type_mask & 0x1));
}
}

11
device-mapper-multipath.spec
View File

@ -1,6 +1,6 @@
Name: device-mapper-multipath
Version: 0.9.9
Release: 13%{?dist}
Release: 14%{?dist}
Summary: Tools to manage multipath devices using device-mapper
License: GPLv2
URL: http://christophe.varoqui.free.fr/
@ -87,6 +87,8 @@ 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
Patch0078: 0078-multipathd-Fix-race-while-registering-PR-key.patch
Patch0079: 0079-mpathpersist-Fix-REPORT-CAPABILITIES-output.patch
# runtime
Requires: %{name}-libs = %{version}-%{release}
@ -296,6 +298,13 @@ fi
%{_pkgconfdir}/libdmmp.pc
%changelog
* Tue Nov 11 2025 Benjamin Marzinski <bmarzins@redhat.com> - 0.9.9-14
- Add 0078-multipathd-Fix-race-while-registering-PR-key.patch
- Add 0079-mpathpersist-Fix-REPORT-CAPABILITIES-output.patch
* Fixes RHEL-118720 ("There are many bugs in multipath's persistent
reservation handling [rhel-10]")
- Resolves: RHEL-118720
* Wed Oct 1 2025 Benjamin Marzinski <bmarzins@redhat.com> - 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