117 lines
3.9 KiB
Diff
117 lines
3.9 KiB
Diff
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
|
From: Benjamin Marzinski <bmarzins@redhat.com>
|
|
Date: Thu, 11 Jun 2020 15:41:18 -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 e3427d3d..1d9ce7f7 100644
|
|
--- a/multipathd/main.c
|
|
+++ b/multipathd/main.c
|
|
@@ -1611,22 +1611,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
|
|
@@ -2087,9 +2083,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 */
|
|
@@ -2179,12 +2182,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)
|
|
@@ -2200,15 +2199,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.17.2
|
|
|