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

device-mapper-multipath-0.8.7-21

Browse Source 
Add 0085-multipathd-make-pr-registration-consistent.patch
Add 0086-libmultipath-make-prflag-an-enum.patch
Add 0087-multipathd-handle-no-active-paths-in-update_map_pr.patch
Add 0088-libmpathpersist-fix-resource-leak-in-update_map_pr.patch
Resolves: bz #2164869
This commit is contained in:
Benjamin Marzinski 2023-03-14 23:37:04 -05:00
parent 598f5a73c4
commit 761d4c4a35
5 changed files with 557 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

159
0085-multipathd-make-pr-registration-consistent.patch Normal file
View File

@ -0,0 +1,159 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Tue, 20 Dec 2022 17:41:10 -0600
Subject: [PATCH] multipathd: make pr registration consistent
multipathd was inconsistent on what it did with persistent reservations
when a multipath device was created. If a multipath device with a
configured reservation key was created during configure(), multipathd
would try to read the registered keys using an active path. If it saw a
matching key, it would set the prflag, but not attempt to register the
key on any of the other paths. This means that if a new path had
appeared while multipathd was not running, it wouldn't register the key
on this path.
If the multipath device was created during ev_add_path(), multipathd
would used the added path to check if there was a matching key and if
there was, register the key only on the added path and then set the
prflag. This could be problematic if the device was created with
multiple paths, for instance because find_mutipaths was set to "yes" and
a second path just appeared. In this case, if the device happened to be
only registered on the second path, it would not get registered on the
first path.
If the multipath device was added to multipathd during a call to
ev_add_map(), multipathd wouldn't set the prflag or register the key on
any paths.
After a device was created with the prflag set, if a new path appeared
before the creation uevent, and multipathd was forced to delay adding
it, when it finally updated the multipath device, the key would be
registered on all paths, fixing any paths missed during creation.
However, if a new path appeared after the creation uevent, the key would
only be registered on that new path. Any paths that were missed on
creation would stay missed.
persistent key registration needs to be handled consistently. This
patch does so by making sure that however a multipath device is added to
multipathd, it will check to see if the configured key is registered. If
it is, multipathd will set the prflag and register the key on all the
currently active paths.
When a new path is added, multipathd will use it to check for active
keys, as before. But if it finds a matching key and prflag isn't
currently set, it will register the key on all paths.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
multipathd/main.c | 43 +++++++++++++++++++++++++++++--------------
1 file changed, 29 insertions(+), 14 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index a098f3c2..e7c17182 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -512,13 +512,26 @@ flush_map_nopaths(struct multipath *mpp, struct vectors *vecs) {
return false;
}
+static void
+pr_register_active_paths(struct multipath *mpp)
+{
+ unsigned int i, j;
+ struct path *pp;
+ struct pathgroup *pgp;
+
+ vector_foreach_slot (mpp->pg, pgp, i) {
+ vector_foreach_slot (pgp->paths, pp, j) {
+ if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST))
+ mpath_pr_event_handle(pp);
+ }
+ }
+}
+
static int
update_map (struct multipath *mpp, struct vectors *vecs, int new_map)
{
int retries = 3;
char *params __attribute__((cleanup(cleanup_charp))) = NULL;
- struct path *pp;
- int i;
retry:
condlog(4, "%s: updating new map", mpp->alias);
@@ -535,15 +548,6 @@ retry:
mpp->action = ACT_RELOAD;
- if (mpp->prflag) {
- vector_foreach_slot(mpp->paths, pp, i) {
- if ((pp->state == PATH_UP) || (pp->state == PATH_GHOST)) {
- /* persistent reseravtion check*/
- mpath_pr_event_handle(pp);
- }
- }
- }
-
if (setup_map(mpp, &params, vecs)) {
condlog(0, "%s: failed to setup new map in update", mpp->alias);
retries = -1;
@@ -569,6 +573,11 @@ fail:
sync_map_state(mpp);
+ if (!mpp->prflag)
+ update_map_pr(mpp);
+ if (mpp->prflag)
+ pr_register_active_paths(mpp);
+
if (retries < 0)
condlog(0, "%s: failed reload in new map update", mpp->alias);
return 0;
@@ -1073,6 +1082,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 = 0;
/*
* need path UID to go any further
@@ -1116,6 +1126,8 @@ rescan:
verify_paths(mpp);
mpp->action = ACT_RELOAD;
+ prflag = mpp->prflag;
+ mpath_pr_event_handle(pp);
} else {
if (!should_multipath(pp, vecs->pathvec, vecs->mpvec)) {
orphan_path(pp, "only one path");
@@ -1134,9 +1146,6 @@ rescan:
goto fail; /* leave path added to pathvec */
}
- /* persistent reservation check*/
- mpath_pr_event_handle(pp);
-
/* ro check - if new path is ro, force map to be ro as well */
ro = sysfs_get_ro(pp);
if (ro == 1)
@@ -1201,6 +1210,10 @@ rescan:
sync_map_state(mpp);
if (retries >= 0) {
+ if (start_waiter)
+ update_map_pr(mpp);
+ if (mpp->prflag && !prflag)
+ pr_register_active_paths(mpp);
condlog(2, "%s [%s]: path added to devmap %s",
pp->dev, pp->dev_t, mpp->alias);
return 0;
@@ -2745,6 +2758,8 @@ configure (struct vectors * vecs)
if (remember_wwid(mpp->wwid) == 1)
trigger_paths_udev_change(mpp, true);
update_map_pr(mpp);
+ if (mpp->prflag)
+ pr_register_active_paths(mpp);
}
/*

165
0086-libmultipath-make-prflag-an-enum.patch Normal file
View File

@ -0,0 +1,165 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Tue, 20 Dec 2022 17:41:11 -0600
Subject: [PATCH] libmultipath: make prflag an enum
In preparation for a future patch, make prflag an enum, and change the
reply of cli_getprstatus() to a string.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
libmpathpersist/mpath_persist.c | 2 +-
libmultipath/structs.h | 8 +++++++-
multipathd/cli_handlers.c | 16 +++++++++-------
multipathd/main.c | 14 +++++++-------
4 files changed, 24 insertions(+), 16 deletions(-)
diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 803a2a28..01e8ca25 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -924,7 +924,7 @@ int update_map_pr(struct multipath *mpp)
if (isFound)
{
- mpp->prflag = 1;
+ mpp->prflag = PRFLAG_SET;
condlog(2, "%s: prflag flag set.", mpp->alias );
}
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 4b308561..5ec591bc 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -368,6 +368,12 @@ struct path {
typedef int (pgpolicyfn) (struct multipath *, vector);
+
+enum prflag_value {
+ PRFLAG_UNSET,
+ PRFLAG_SET,
+};
+
struct multipath {
char wwid[WWID_SIZE];
char alias_old[WWID_SIZE];
@@ -441,7 +447,7 @@ struct multipath {
int prkey_source;
struct be64 reservation_key;
uint8_t sa_flags;
- unsigned char prflag;
+ int prflag;
int all_tg_pt;
struct gen_multipath generic_mp;
bool fpin_must_reload;
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 6d3a0ae2..90d2c388 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1304,6 +1304,10 @@ cli_shutdown (void * v, char ** reply, int * len, void * data)
int
cli_getprstatus (void * v, char ** reply, int * len, void * data)
{
+ static const char * const prflag_str[] = {
+ [PRFLAG_UNSET] = "unset\n",
+ [PRFLAG_SET] = "set\n",
+ };
struct multipath * mpp;
struct vectors * vecs = (struct vectors *)data;
char * param = get_keyparam(v, MAP);
@@ -1315,9 +1319,7 @@ cli_getprstatus (void * v, char ** reply, int * len, void * data)
if (!mpp)
return 1;
- condlog(3, "%s: prflag = %u", param, (unsigned int)mpp->prflag);
-
- *len = asprintf(reply, "%d", mpp->prflag);
+ *len = asprintf(reply, "%s", prflag_str[mpp->prflag]);
if (*len < 0)
return 1;
@@ -1340,8 +1342,8 @@ cli_setprstatus(void * v, char ** reply, int * len, void * data)
if (!mpp)
return 1;
- if (!mpp->prflag) {
- mpp->prflag = 1;
+ if (mpp->prflag != PRFLAG_SET) {
+ mpp->prflag = PRFLAG_SET;
condlog(2, "%s: prflag set", param);
}
@@ -1363,8 +1365,8 @@ cli_unsetprstatus(void * v, char ** reply, int * len, void * data)
if (!mpp)
return 1;
- if (mpp->prflag) {
- mpp->prflag = 0;
+ if (mpp->prflag != PRFLAG_UNSET) {
+ mpp->prflag = PRFLAG_UNSET;
condlog(2, "%s: prflag unset", param);
}
diff --git a/multipathd/main.c b/multipathd/main.c
index e7c17182..532ca15b 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -573,9 +573,9 @@ fail:
sync_map_state(mpp);
- if (!mpp->prflag)
+ if (mpp->prflag == PRFLAG_UNSET)
update_map_pr(mpp);
- if (mpp->prflag)
+ if (mpp->prflag == PRFLAG_SET)
pr_register_active_paths(mpp);
if (retries < 0)
@@ -1082,7 +1082,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 = 0;
+ unsigned char prflag = PRFLAG_UNSET;
/*
* need path UID to go any further
@@ -1212,7 +1212,7 @@ rescan:
if (retries >= 0) {
if (start_waiter)
update_map_pr(mpp);
- if (mpp->prflag && !prflag)
+ if (mpp->prflag == PRFLAG_SET && prflag == PRFLAG_UNSET)
pr_register_active_paths(mpp);
condlog(2, "%s [%s]: path added to devmap %s",
pp->dev, pp->dev_t, mpp->alias);
@@ -2414,7 +2414,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
}
if (newstate == PATH_UP || newstate == PATH_GHOST) {
- if (pp->mpp->prflag) {
+ if (pp->mpp->prflag == PRFLAG_SET) {
/*
* Check Persistent Reservation.
*/
@@ -2758,7 +2758,7 @@ configure (struct vectors * vecs)
if (remember_wwid(mpp->wwid) == 1)
trigger_paths_udev_change(mpp, true);
update_map_pr(mpp);
- if (mpp->prflag)
+ if (mpp->prflag == PRFLAG_SET)
pr_register_active_paths(mpp);
}
@@ -3668,7 +3668,7 @@ void * mpath_pr_event_handler_fn (void * pathp )
{
condlog(0,"%s: Reservation registration failed. Error: %d", pp->dev, ret);
}
- mpp->prflag = 1;
+ mpp->prflag = PRFLAG_SET;
free(param);
out:

152
0087-multipathd-handle-no-active-paths-in-update_map_pr.patch Normal file
View File

@ -0,0 +1,152 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Tue, 20 Dec 2022 17:41:12 -0600
Subject: [PATCH] multipathd: handle no active paths in update_map_pr
When a multipath device is first created, if it has a reservation key
configured, update_map_pr() will check for a matching key on the active
paths. If there were no active paths to check with, multipathd was
leaving mpp->prflag in PRFLAG_UNSET, as if there were no matching keys.
It's possible that when update_map_pr() is called, all the paths will be
in the PATH_PENDING state because the checkers haven't completed yet. In
this case, multipathd was treating the device as having no registered
keys without ever checking.
To solve this, multipath devices now start with prflag = PRFLAG_UNKNOWN.
It will remain in this state until multipathd actually tries to get the
registered keys down a path. If the map is in this state, it will check
newly active paths, and if it finds a matching key, it will register
the key down all active paths.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
---
libmpathpersist/mpath_persist.c | 8 ++++++++
libmultipath/structs.h | 1 +
multipathd/cli_handlers.c | 1 +
multipathd/main.c | 19 ++++++++++++++-----
4 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 01e8ca25..3fc60da2 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -879,6 +879,7 @@ int update_map_pr(struct multipath *mpp)
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",
mpp->alias);
return MPATH_PR_SUCCESS;
@@ -890,6 +891,13 @@ 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)
+ {
+ condlog(0,"%s: No available paths to check pr status",
+ mpp->alias);
+ return MPATH_PR_OTHER;
+ }
+ mpp->prflag = PRFLAG_UNSET;
ret = mpath_prin_activepath(mpp, MPATH_PRIN_RKEY_SA, resp, noisy);
if (ret != MPATH_PR_SUCCESS )
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 5ec591bc..c1e93e6e 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -370,6 +370,7 @@ typedef int (pgpolicyfn) (struct multipath *, vector);
enum prflag_value {
+ PRFLAG_UNKNOWN,
PRFLAG_UNSET,
PRFLAG_SET,
};
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 90d2c388..f322f10f 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1305,6 +1305,7 @@ int
cli_getprstatus (void * v, char ** reply, int * len, void * data)
{
static const char * const prflag_str[] = {
+ [PRFLAG_UNKNOWN] = "unknown\n",
[PRFLAG_UNSET] = "unset\n",
[PRFLAG_SET] = "set\n",
};
diff --git a/multipathd/main.c b/multipathd/main.c
index 532ca15b..075e7b13 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -573,7 +573,7 @@ fail:
sync_map_state(mpp);
- if (mpp->prflag == PRFLAG_UNSET)
+ if (mpp->prflag != PRFLAG_SET)
update_map_pr(mpp);
if (mpp->prflag == PRFLAG_SET)
pr_register_active_paths(mpp);
@@ -1212,7 +1212,7 @@ rescan:
if (retries >= 0) {
if (start_waiter)
update_map_pr(mpp);
- if (mpp->prflag == PRFLAG_SET && prflag == PRFLAG_UNSET)
+ if (mpp->prflag == PRFLAG_SET && prflag != PRFLAG_SET)
pr_register_active_paths(mpp);
condlog(2, "%s [%s]: path added to devmap %s",
pp->dev, pp->dev_t, mpp->alias);
@@ -2414,13 +2414,17 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
}
if (newstate == PATH_UP || newstate == PATH_GHOST) {
- if (pp->mpp->prflag == PRFLAG_SET) {
+ if (pp->mpp->prflag != PRFLAG_UNSET) {
+ int prflag = pp->mpp->prflag;
/*
* Check Persistent Reservation.
*/
condlog(2, "%s: checking persistent "
"reservation registration", pp->dev);
mpath_pr_event_handle(pp);
+ if (pp->mpp->prflag == PRFLAG_SET &&
+ prflag != PRFLAG_SET)
+ pr_register_active_paths(pp->mpp);
}
}
@@ -3616,6 +3620,7 @@ void * mpath_pr_event_handler_fn (void * pathp )
goto out;
}
+ mpp->prflag = PRFLAG_UNSET;
ret = prin_do_scsi_ioctl(pp->dev, MPATH_PRIN_RKEY_SA, resp, 0);
if (ret != MPATH_PR_SUCCESS )
{
@@ -3686,12 +3691,12 @@ int mpath_pr_event_handle(struct path *pp)
struct multipath * mpp;
if (pp->bus != SYSFS_BUS_SCSI)
- return 0;
+ goto no_pr;
mpp = pp->mpp;
if (!get_be64(mpp->reservation_key))
- return -1;
+ goto no_pr;
pthread_attr_init(&attr);
pthread_attr_setdetachstate(&attr, PTHREAD_CREATE_JOINABLE);
@@ -3704,4 +3709,8 @@ int mpath_pr_event_handle(struct path *pp)
pthread_attr_destroy(&attr);
rc = pthread_join(thread, NULL);
return 0;
+
+no_pr:
+ pp->mpp->prflag = PRFLAG_UNSET;
+ return 0;
}

68
0088-libmpathpersist-fix-resource-leak-in-update_map_pr.patch Normal file
View File

@ -0,0 +1,68 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Martin Wilck <mwilck@suse.com>
Date: Thu, 2 Feb 2023 09:20:35 +0100
Subject: [PATCH] libmpathpersist: fix resource leak in update_map_pr()
The "no available paths" case would leak the memory resp points to.
Found by coverity.
Fixes: 50e2c16 ("multipathd: handle no active paths in update_map_pr")
Signed-off-by: Martin Wilck <mwilck@suse.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmpathpersist/mpath_persist.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 3fc60da2..29b64937 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -874,7 +874,7 @@ int update_map_pr(struct multipath *mpp)
int noisy=0;
struct prin_resp *resp;
unsigned int i;
- int ret, isFound;
+ int ret = MPATH_PR_OTHER, isFound;
if (!get_be64(mpp->reservation_key))
{
@@ -895,7 +895,7 @@ int update_map_pr(struct multipath *mpp)
{
condlog(0,"%s: No available paths to check pr status",
mpp->alias);
- return MPATH_PR_OTHER;
+ goto out;
}
mpp->prflag = PRFLAG_UNSET;
ret = mpath_prin_activepath(mpp, MPATH_PRIN_RKEY_SA, resp, noisy);
@@ -903,15 +903,15 @@ int update_map_pr(struct multipath *mpp)
if (ret != MPATH_PR_SUCCESS )
{
condlog(0,"%s : pr in read keys service action failed Error=%d", mpp->alias, ret);
- free(resp);
- return ret;
+ goto out;
}
+ ret = MPATH_PR_SUCCESS;
+
if (resp->prin_descriptor.prin_readkeys.additional_length == 0 )
{
condlog(3,"%s: No key found. Device may not be registered. ", mpp->alias);
- free(resp);
- return MPATH_PR_SUCCESS;
+ goto out;
}
condlog(2, "%s: Multipath reservation_key: 0x%" PRIx64 " ", mpp->alias,
@@ -936,6 +936,7 @@ int update_map_pr(struct multipath *mpp)
condlog(2, "%s: prflag flag set.", mpp->alias );
}
+out:
free(resp);
- return MPATH_PR_SUCCESS;
+ return ret;
}

14
device-mapper-multipath.spec
View File

@ -1,6 +1,6 @@
Name: device-mapper-multipath
Version: 0.8.7
Release: 20%{?dist}
Release: 21%{?dist}
Summary: Tools to manage multipath devices using device-mapper
License: GPLv2
URL: http://christophe.varoqui.free.fr/
@ -94,6 +94,11 @@ Patch0081: 0081-libmultipath-cleanup-ACT_CREATE-code-in-select_actio.patch
Patch0082: 0082-libmultipath-keep-renames-from-stopping-other-multip.patch
Patch0083: 0083-multipath.rules-fix-smart-bug-with-failed-valid-path.patch
Patch0084: 0084-libmultipath-limit-paths-that-can-get-wwid-from-envi.patch
Patch0085: 0085-multipathd-make-pr-registration-consistent.patch
Patch0086: 0086-libmultipath-make-prflag-an-enum.patch
Patch0087: 0087-multipathd-handle-no-active-paths-in-update_map_pr.patch
Patch0088: 0088-libmpathpersist-fix-resource-leak-in-update_map_pr.patch
# runtime
@ -295,6 +300,13 @@ fi
%{_pkgconfdir}/libdmmp.pc
%changelog
* Tue Mar 14 2023 Benjamin Marzinski <bmarzins@redhat.com> - 0.8.7-21
- Add 0085-multipathd-make-pr-registration-consistent.patch
- Add 0086-libmultipath-make-prflag-an-enum.patch
- Add 0087-multipathd-handle-no-active-paths-in-update_map_pr.patch
- Add 0088-libmpathpersist-fix-resource-leak-in-update_map_pr.patch
- Resolves: bz #2164869
* Thu Feb 9 2023 Benjamin Marzinski <bmarzins@redhat.com> - 0.8.7-20
- Add 0083-multipath.rules-fix-smart-bug-with-failed-valid-path.patch
- Add 0084-libmultipath-limit-paths-that-can-get-wwid-from-envi.patch