import CS device-mapper-multipath-0.8.4-39.el8

This commit is contained in:
eabdullin 2023-09-27 12:49:33 +00:00
parent 9e9d41a1be
commit 8aa95a4ef9
5 changed files with 539 additions and 2 deletions

View File

@ -0,0 +1,136 @@
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 | 32 +++++++++++++++++++++++++++++---
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 769dcaee..d84027dc 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -511,6 +511,21 @@ 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)
{
@@ -556,6 +571,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;
@@ -1014,6 +1034,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
@@ -1056,6 +1077,8 @@ rescan:
verify_paths(mpp, vecs);
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");
@@ -1074,9 +1097,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)
@@ -1140,6 +1160,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;
@@ -2608,6 +2632,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);
}
/*

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 d0744773..e361435a 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -949,7 +949,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 44980b4e..8acea3cb 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -377,6 +377,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];
@@ -450,7 +456,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 8b4bd187..6dbbb7e2 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1407,6 +1407,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);
@@ -1418,9 +1422,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;
@@ -1443,8 +1445,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);
}
@@ -1466,8 +1468,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 d84027dc..81bb0deb 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -571,9 +571,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)
@@ -1034,7 +1034,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
@@ -1162,7 +1162,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);
@@ -2306,7 +2306,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.
*/
@@ -2632,7 +2632,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);
}
@@ -3438,7 +3438,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:

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 e361435a..440a329f 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -904,6 +904,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;
@@ -915,6 +916,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 8acea3cb..7f2fb944 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -379,6 +379,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 6dbbb7e2..260b7a17 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -1408,6 +1408,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 81bb0deb..e7dad6b9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -571,7 +571,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);
@@ -1162,7 +1162,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);
@@ -2306,13 +2306,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);
}
}
@@ -3386,6 +3390,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 )
{
@@ -3456,12 +3461,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);
@@ -3474,4 +3479,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;
}

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 440a329f..a4c1461f 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -899,7 +899,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))
{
@@ -920,7 +920,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);
@@ -928,15 +928,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,
@@ -961,6 +961,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;
}

View File

@ -1,7 +1,7 @@
Summary: Tools to manage multipath devices using device-mapper Summary: Tools to manage multipath devices using device-mapper
Name: device-mapper-multipath Name: device-mapper-multipath
Version: 0.8.4 Version: 0.8.4
Release: 37%{?dist} Release: 39%{?dist}
License: GPLv2 License: GPLv2
Group: System Environment/Base Group: System Environment/Base
URL: http://christophe.varoqui.free.fr/ URL: http://christophe.varoqui.free.fr/
@ -142,6 +142,10 @@ Patch00128: 0128-libmutipath-validate-the-argument-count-of-config-st.patch
Patch00129: 0129-libmultipath-select-resize-action-even-if-reload-is-.patch Patch00129: 0129-libmultipath-select-resize-action-even-if-reload-is-.patch
Patch00130: 0130-libmultipath-cleanup-ACT_CREATE-code-in-select_actio.patch Patch00130: 0130-libmultipath-cleanup-ACT_CREATE-code-in-select_actio.patch
Patch00131: 0131-libmultipath-keep-renames-from-stopping-other-multip.patch Patch00131: 0131-libmultipath-keep-renames-from-stopping-other-multip.patch
Patch00132: 0132-multipathd-make-pr-registration-consistent.patch
Patch00133: 0133-libmultipath-make-prflag-an-enum.patch
Patch00134: 0134-multipathd-handle-no-active-paths-in-update_map_pr.patch
Patch00135: 0135-libmpathpersist-fix-resource-leak-in-update_map_pr.patch
# runtime # runtime
Requires: %{name}-libs = %{version}-%{release} Requires: %{name}-libs = %{version}-%{release}
@ -320,7 +324,7 @@ fi
%{!?_licensedir:%global license %%doc} %{!?_licensedir:%global license %%doc}
%license LICENSES/GPL-2.0 %license LICENSES/GPL-2.0
%{_sbindir}/kpartx %{_sbindir}/kpartx
/usr/lib/udev/kpartx_id %{_udevrulesdir}/../kpartx_id
%{_mandir}/man8/kpartx.8.gz %{_mandir}/man8/kpartx.8.gz
%config %{_udevrulesdir}/11-dm-parts.rules %config %{_udevrulesdir}/11-dm-parts.rules
%config %{_udevrulesdir}/66-kpartx.rules %config %{_udevrulesdir}/66-kpartx.rules
@ -346,6 +350,18 @@ fi
%{_pkgconfdir}/libdmmp.pc %{_pkgconfdir}/libdmmp.pc
%changelog %changelog
* Thu Mar 16 2023 Benjamin Marzinski <bmarzins@redhat.com> 0.8.4-39
- Add OSCI tests directory
- Make kpartx_id installation location relative to %{_udevrulesdir}
- Resolves: bz #2164871
* Wed Mar 15 2023 Benjamin Marzinski <bmarzins@redhat.com> 0.8.4-38
- Add 0132-multipathd-make-pr-registration-consistent.patch
- Add 0133-libmultipath-make-prflag-an-enum.patch
- Add 0134-multipathd-handle-no-active-paths-in-update_map_pr.patch
- Add 0135-libmpathpersist-fix-resource-leak-in-update_map_pr.patch
- Resolves: bz #2164871
* Fri Feb 3 2023 Benjamin Marzinski <bmarzins@redhat.com> 0.8.4-37 * Fri Feb 3 2023 Benjamin Marzinski <bmarzins@redhat.com> 0.8.4-37
- Fix bugzilla linked to the changes (was previously linked to - Fix bugzilla linked to the changes (was previously linked to
the wrong bug, 2162537) the wrong bug, 2162537)