Blame SOURCES/0025-multipathd-fix-check_path-errors-with-removed-map.patch

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