packages/multipath-tools
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

!222 multipathd: fix check_path errors with removed map.

Browse Source 
From: @roygiteee 
Reviewed-by: @kouwq 
Signed-off-by: @kouwq
This commit is contained in:
openeuler-ci-bot 2025-04-18 06:36:03 +00:00 committed by Gitee
commit cddaf5a3be
No known key found for this signature in database
GPG Key ID: 173E9B9CA92EEF8F
3 changed files with 447 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

322
0054-libmultipath-make-dm_get_map-status-return-codes-sym.patch Normal file
View File

@ -0,0 +1,322 @@
From db7a0bc38d59e4fd67b1df2a01f111c64de5cdf3 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Thu, 2 Jul 2020 19:07:00 -0500
Subject: [PATCH] libmultipath: make dm_get_map/status return codes symbolic
dm_get_map() and dm_get_status() now use symbolic return codes. They
also differentiate between failing to get information from device-mapper
and not finding the requested device. These symboilc return codes are
also used by update_multipath_* functions.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/devmapper.c | 51 +++++++++++++++++++++++++-------------
libmultipath/devmapper.h | 6 +++++
libmultipath/structs_vec.c | 45 +++++++++++++++++++--------------
multipathd/main.c | 12 ++++-----
4 files changed, 72 insertions(+), 42 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index cb5a7b4..3d690e7 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -521,36 +521,43 @@ int dm_map_present(const char * str)
int dm_get_map(const char *name, unsigned long long *size, char *outparams)
{
- int r = 1;
+ int r = DMP_ERR;
struct dm_task *dmt;
uint64_t start, length;
char *target_type = NULL;
char *params = NULL;
if (!(dmt = libmp_dm_task_create(DM_DEVICE_TABLE)))
- return 1;
+ return r;
if (!dm_task_set_name(dmt, name))
goto out;
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt))
+ errno = 0;
+ if (!dm_task_run(dmt)) {
+ if (dm_task_get_errno(dmt) == ENXIO)
+ r = DMP_NOT_FOUND;
goto out;
+ }
+ r = DMP_NOT_FOUND;
/* Fetch 1st target */
- dm_get_next_target(dmt, NULL, &start, &length,
- &target_type, &params);
+ if (dm_get_next_target(dmt, NULL, &start, &length,
+ &target_type, &params) != NULL)
+ /* more than one target */
+ goto out;
if (size)
*size = length;
if (!outparams) {
- r = 0;
+ r = DMP_OK;
goto out;
}
if (snprintf(outparams, PARAMS_SIZE, "%s", params) <= PARAMS_SIZE)
- r = 0;
+ r = DMP_OK;
out:
dm_task_destroy(dmt);
return r;
@@ -624,35 +631,45 @@ is_mpath_part(const char *part_name, const char *map_name)
int dm_get_status(const char *name, char *outstatus)
{
- int r = 1;
+ int r = DMP_ERR;
struct dm_task *dmt;
uint64_t start, length;
char *target_type = NULL;
char *status = NULL;
if (!(dmt = libmp_dm_task_create(DM_DEVICE_STATUS)))
- return 1;
+ return r;
if (!dm_task_set_name(dmt, name))
goto out;
dm_task_no_open_count(dmt);
- if (!dm_task_run(dmt))
+ errno = 0;
+ if (!dm_task_run(dmt)) {
+ if (dm_task_get_errno(dmt) == ENXIO)
+ r = DMP_NOT_FOUND;
goto out;
+ }
+ r = DMP_NOT_FOUND;
/* Fetch 1st target */
- dm_get_next_target(dmt, NULL, &start, &length,
- &target_type, &status);
+ if (dm_get_next_target(dmt, NULL, &start, &length,
+ &target_type, &status) != NULL)
+ goto out;
+
+ if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
+ goto out;
+
if (!status) {
condlog(2, "get null status.");
goto out;
}
if (snprintf(outstatus, PARAMS_SIZE, "%s", status) <= PARAMS_SIZE)
- r = 0;
+ r = DMP_OK;
out:
- if (r)
+ if (r != DMP_OK)
condlog(0, "%s: error getting map status string", name);
dm_task_destroy(dmt);
@@ -907,7 +924,7 @@ int _dm_flush_map (const char * mapname, int need_sync, int deferred_remove,
return 1;
if (need_suspend &&
- !dm_get_map(mapname, &mapsize, params) &&
+ dm_get_map(mapname, &mapsize, params) == DMP_OK &&
strstr(params, "queue_if_no_path")) {
if (!dm_queue_if_no_path(mapname, 0))
queue_if_no_path = 1;
@@ -1116,7 +1133,7 @@ struct multipath *dm_get_multipath(const char *name)
if (!mpp->alias)
goto out;
- if (dm_get_map(name, &mpp->size, NULL))
+ if (dm_get_map(name, &mpp->size, NULL) != DMP_OK)
goto out;
dm_get_uuid(name, mpp->wwid, WWID_SIZE);
@@ -1300,7 +1317,7 @@ do_foreach_partmaps (const char * mapname,
/*
* and we can fetch the map table from the kernel
*/
- !dm_get_map(names->name, &size, &params[0]) &&
+ dm_get_map(names->name, &size, &params[0]) == DMP_OK &&
/*
* and the table maps over the multipath map
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 013e7e7..fb19763 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -27,6 +27,12 @@
#define UUID_PREFIX "mpath-"
#define UUID_PREFIX_LEN (sizeof(UUID_PREFIX) - 1)
+enum {
+ DMP_ERR,
+ DMP_OK,
+ DMP_NOT_FOUND,
+};
+
void dm_init(int verbosity);
void libmp_dm_init(void);
void libmp_udev_set_sync_support(int on);
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 4398274..6d5d7f7 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -203,43 +203,47 @@ extract_hwe_from_path(struct multipath * mpp)
int
update_multipath_table (struct multipath *mpp, vector pathvec, int is_daemon)
{
+ int r = DMP_ERR;
char params[PARAMS_SIZE] = {0};
if (!mpp)
- return 1;
+ return r;
- if (dm_get_map(mpp->alias, &mpp->size, params)) {
- condlog(3, "%s: cannot get map", mpp->alias);
- return 1;
+ r = dm_get_map(mpp->alias, &mpp->size, params);
+ if (r != DMP_OK) {
+ condlog(3, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting table" : "map not present");
+ return r;
}
if (disassemble_map(pathvec, params, mpp, is_daemon)) {
condlog(3, "%s: cannot disassemble map", mpp->alias);
- return 1;
+ return DMP_ERR;
}
- return 0;
+ return DMP_OK;
}
int
update_multipath_status (struct multipath *mpp)
{
+ int r = DMP_ERR;
char status[PARAMS_SIZE] = {0};
if (!mpp)
- return 1;
+ return r;
- if (dm_get_status(mpp->alias, status)) {
- condlog(3, "%s: cannot get status", mpp->alias);
- return 1;
+ r = dm_get_status(mpp->alias, status);
+ if (r != DMP_OK) {
+ condlog(3, "%s: %s", mpp->alias, (r == DMP_ERR)? "error getting status" : "map not present");
+ return r;
}
if (disassemble_status(status, mpp)) {
condlog(3, "%s: cannot disassemble status", mpp->alias);
- return 1;
+ return DMP_ERR;
}
- return 0;
+ return DMP_OK;
}
void sync_paths(struct multipath *mpp, vector pathvec)
@@ -285,10 +289,10 @@ int
update_multipath_strings(struct multipath *mpp, vector pathvec, int is_daemon)
{
struct pathgroup *pgp;
- int i;
+ int i, r = DMP_ERR;
if (!mpp)
- return 1;
+ return r;
update_mpp_paths(mpp, pathvec);
condlog(4, "%s: %s", mpp->alias, __FUNCTION__);
@@ -297,18 +301,21 @@ update_multipath_strings(struct multipath *mpp, vector pathvec, int is_daemon)
free_pgvec(mpp->pg, KEEP_PATHS);
mpp->pg = NULL;
- if (update_multipath_table(mpp, pathvec, is_daemon))
- return 1;
+ r = update_multipath_table(mpp, pathvec, is_daemon);
+ if (r != DMP_OK)
+ return r;
+
sync_paths(mpp, pathvec);
- if (update_multipath_status(mpp))
- return 1;
+ r = update_multipath_status(mpp);
+ if (r != DMP_OK)
+ return r;
vector_foreach_slot(mpp->pg, pgp, i)
if (pgp->paths)
path_group_prio_update(pgp);
- return 0;
+ return DMP_OK;
}
static void enter_recovery_mode(struct multipath *mpp)
diff --git a/multipathd/main.c b/multipathd/main.c
index 6a88b10..36f0e64 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -414,7 +414,7 @@ int __setup_multipath(struct vectors *vecs, struct multipath *mpp,
goto out;
}
- if (update_multipath_strings(mpp, vecs->pathvec, 1)) {
+ if (update_multipath_strings(mpp, vecs->pathvec, 1) != DMP_OK) {
condlog(0, "%s: failed to setup multipath", mpp->alias);
goto out;
}
@@ -553,9 +553,9 @@ add_map_without_path (struct vectors *vecs, const char *alias)
mpp->mpe = find_mpe(conf->mptable, mpp->wwid);
put_multipath_config(conf);
- if (update_multipath_table(mpp, vecs->pathvec, 1))
+ if (update_multipath_table(mpp, vecs->pathvec, 1) != DMP_OK)
goto out;
- if (update_multipath_status(mpp))
+ if (update_multipath_status(mpp) != DMP_OK)
goto out;
if (!vector_alloc_slot(vecs->mpvec))
@@ -1403,8 +1403,8 @@ map_discovery (struct vectors * vecs)
return 1;
vector_foreach_slot (vecs->mpvec, mpp, i)
- if (update_multipath_table(mpp, vecs->pathvec, 1) ||
- update_multipath_status(mpp)) {
+ if (update_multipath_table(mpp, vecs->pathvec, 1) != DMP_OK ||
+ update_multipath_status(mpp) != DMP_OK) {
remove_map(mpp, vecs, 1);
i--;
}
@@ -2093,7 +2093,7 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
/*
* Synchronize with kernel state
*/
- if (update_multipath_strings(pp->mpp, vecs->pathvec, 1)) {
+ if (update_multipath_strings(pp->mpp, vecs->pathvec, 1) != DMP_OK) {
condlog(1, "%s: Could not synchronize with kernel state",
pp->dev);
pp->dmstate = PSTATE_UNDEF;
--
2.33.0

116
0055-multipathd-fix-check_path-errors-with-removed-map.patch Normal file
View File

@ -0,0 +1,116 @@
From bf44dda958c4e8c5852626b3c6d5a449754fa730 Mon Sep 17 00:00:00 2001
From: Benjamin Marzinski <bmarzins@redhat.com>
Date: Thu, 2 Jul 2020 19:07:01 -0500
Subject: [PATCH] multipathd: fix check_path errors with removed map
If a multipath device is removed during, or immediately before the call
to check_path(), multipathd can behave incorrectly. A missing multpath
device will cause update_multipath_strings() to fail, setting
pp->dmstate to PSTATE_UNDEF. If the path is up, this state will cause
reinstate_path() to be called, which will also fail. This will trigger
a reload, restoring the recently removed device.
If update_multipath_strings() fails because there is no multipath
device, check_path should just quit, since the remove dmevent and uevent
are likely already queued up. Also, I don't see any reason to reload the
multipath device if reinstate fails. This code was added by
fac68d7a99ef17d496079538a5c6836acd7911ab, which clamined that reinstate
could fail if the path was disabled. Looking through the current kernel
code, I can't see any reason why a reinstate would fail, where a reload
would help. If the path was missing from the multipath device,
update_multipath_strings() would already catch that, and quit
check_path() early, which make more sense to me than reloading does.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
multipathd/main.c | 44 +++++++++++++++++++-------------------------
1 file changed, 19 insertions(+), 25 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index 36f0e64..48ac9e6 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1612,22 +1612,18 @@ fail_path (struct path * pp, int del_active)
/*
* caller must have locked the path list before calling that function
*/
-static int
+static void
reinstate_path (struct path * pp)
{
- int ret = 0;
-
if (!pp->mpp)
- return 0;
+ return;
- if (dm_reinstate_path(pp->mpp->alias, pp->dev_t)) {
+ if (dm_reinstate_path(pp->mpp->alias, pp->dev_t))
condlog(0, "%s: reinstate failed", pp->dev_t);
- ret = 1;
- } else {
+ else {
condlog(2, "%s: reinstated", pp->dev_t);
update_queue_mode_add_path(pp->mpp);
}
- return ret;
}
static void
@@ -2093,9 +2089,16 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
/*
* Synchronize with kernel state
*/
- if (update_multipath_strings(pp->mpp, vecs->pathvec, 1) != DMP_OK) {
- condlog(1, "%s: Could not synchronize with kernel state",
- pp->dev);
+ ret = update_multipath_strings(pp->mpp, vecs->pathvec, 1);
+ if (ret != DMP_OK) {
+ if (ret == DMP_NOT_FOUND) {
+ /* multipath device missing. Likely removed */
+ condlog(1, "%s: multipath device '%s' not found",
+ pp->dev, pp->mpp->alias);
+ return 0;
+ } else
+ condlog(1, "%s: Couldn't synchronize with kernel state",
+ pp->dev);
pp->dmstate = PSTATE_UNDEF;
}
/* if update_multipath_strings orphaned the path, quit early */
@@ -2185,12 +2188,8 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
/*
* reinstate this path
*/
- if (!disable_reinstate && reinstate_path(pp)) {
- condlog(3, "%s: reload map", pp->dev);
- ev_add_path(pp, vecs, 1);
- pp->tick = 1;
- return 0;
- }
+ if (!disable_reinstate)
+ reinstate_path(pp);
new_path_up = 1;
if (oldchkrstate != PATH_UP && oldchkrstate != PATH_GHOST)
@@ -2206,15 +2205,10 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)
else if (newstate == PATH_UP || newstate == PATH_GHOST) {
if ((pp->dmstate == PSTATE_FAILED ||
pp->dmstate == PSTATE_UNDEF) &&
- !disable_reinstate) {
+ !disable_reinstate)
/* Clear IO errors */
- if (reinstate_path(pp)) {
- condlog(3, "%s: reload map", pp->dev);
- ev_add_path(pp, vecs, 1);
- pp->tick = 1;
- return 0;
- }
- } else {
+ reinstate_path(pp);
+ else {
LOG_MSG(4, verbosity, pp);
if (pp->checkint != max_checkint) {
/*
--
2.33.0

10
multipath-tools.spec
View File

@ -2,7 +2,7 @@
Name: multipath-tools
Version: 0.8.4
Release: 31
Release: 32
Summary: Tools to manage multipath devices with the device-mapper
License: GPL-2.0-or-later and LGPL-2.0-only
URL: http://christophe.varoqui.free.fr/
@ -63,6 +63,8 @@ Patch50: 0050-multipath-display-the-correct-configuration-when-dum.patch
Patch51: 0051-multipath-return-failure-on-an-invalid-remove-comman.patch
Patch52: 0052-libmultipath-limit-paths-that-can-get-wwid-from-envi.patch
Patch53: 0053-multipathd-don-t-print-so-many-add-map-messages.patch
Patch54: 0054-libmultipath-make-dm_get_map-status-return-codes-sym.patch
Patch55: 0055-multipathd-fix-check_path-errors-with-removed-map.patch
BuildRequires: multipath-tools, libcmocka, libcmocka-devel
BuildRequires: gcc, libaio-devel, userspace-rcu-devel, device-mapper-devel >= 1.02.89
@ -211,6 +213,12 @@ fi
%changelog
* Thu Apr 17 2025 Yu Peng <yupeng@kylinos.cn> - 0.8.4-32
- multipathd: fix check_path errors with removed map.
sync community patches:
multipathd-fix-check_path-errors-with-removed-map.patch
libmultipath-make-dm_get_map-status-return-codes-sym.patch
* Fri Sep 27 2024 kouwenqi <kouwenqi@kylinos.cn> - 0.8.4-31
- multipathd: don't print so many add map messages