From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001

From: Benjamin Marzinski <bmarzins@redhat.com>

Date: Mon, 18 Jan 2021 22:46:04 -0600

Subject: [PATCH] multipathd: cleanup logging for marginal paths

io_err_stat logged at level 2 whenever it enqueued a path to check,

which could happen multiple times while a path was marginal.  On the

other hand if marginal_pathgroups wasn't set, multipathd didn't log when

paths were set to marginal. Now io_err_stat only logs at level 2 when

something unexpected happens, but multipathd will always log when a

path switches its marginal state.

This patch also fixes an issue where paths in the delayed state could

get set to the pending state if they could not be checked in time.

Aside from going against the idea the paths should not be set to pending

if they already have a valid state, this caused multipathd to log a

message whenever the path state switched to from delayed to pending and

then back.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Reviewed-by: Martin Wilck <mwilck@suse.com>

---

 libmultipath/io_err_stat.c |  7 +++----

 multipathd/main.c          | 20 +++++++++++++-------

 2 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c

index bf1d3910..ee711f7f 100644

--- a/libmultipath/io_err_stat.c

+++ b/libmultipath/io_err_stat.c

@@ -254,7 +254,7 @@ static int enqueue_io_err_stat_by_path(struct path *path)

 	vector_set_slot(io_err_pathvec, p);

 	pthread_mutex_unlock(&io_err_pathvec_lock);

-	io_err_stat_log(2, "%s: enqueue path %s to check",

+	io_err_stat_log(3, "%s: enqueue path %s to check",

 			path->mpp->alias, path->dev);

 	return 0;

@@ -353,7 +353,7 @@ int need_io_err_check(struct path *pp)

 	if (uatomic_read(&io_err_thread_running) == 0)

 		return 0;

 	if (count_active_paths(pp->mpp) <= 0) {

-		io_err_stat_log(2, "%s: recover path early", pp->dev);

+		io_err_stat_log(2, "%s: no paths. recovering early", pp->dev);

 		goto recover;

 	}

 	if (pp->io_err_pathfail_cnt != PATH_IO_ERR_WAITING_TO_CHECK)

@@ -371,8 +371,7 @@ int need_io_err_check(struct path *pp)

 		 * Or else,  return 1 to set path state to PATH_SHAKY

 		 */

 		if (r == 1) {

-			io_err_stat_log(3, "%s: enqueue fails, to recover",

-					pp->dev);

+			io_err_stat_log(2, "%s: enqueue failed. recovering early", pp->dev);

 			goto recover;

 		} else

 			pp->io_err_pathfail_cnt = PATH_IO_ERR_IN_CHECKING;

diff --git a/multipathd/main.c b/multipathd/main.c

index 1d0579e9..cc1aeea2 100644

--- a/multipathd/main.c

+++ b/multipathd/main.c

@@ -2041,8 +2041,8 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)

 		pathinfo(pp, conf, 0);

 		pthread_cleanup_pop(1);

 		return 1;

-	} else if ((newstate != PATH_UP && newstate != PATH_GHOST) &&

-			(pp->state == PATH_DELAYED)) {

+	} else if ((newstate != PATH_UP && newstate != PATH_GHOST &&

+		    newstate != PATH_PENDING) && (pp->state == PATH_DELAYED)) {

 		/* If path state become failed again cancel path delay state */

 		pp->state = newstate;

 		return 1;

@@ -2104,8 +2104,9 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)

 	if ((newstate == PATH_UP || newstate == PATH_GHOST) &&

 	    (san_path_check_enabled(pp->mpp) ||

 	     marginal_path_check_enabled(pp->mpp))) {

-		int was_marginal = pp->marginal;

 		if (should_skip_path(pp)) {

+			if (!pp->marginal && pp->state != PATH_DELAYED)

+				condlog(2, "%s: path is now marginal", pp->dev);

 			if (!marginal_pathgroups) {

 				if (marginal_path_check_enabled(pp->mpp))

 					/* to reschedule as soon as possible,

@@ -2115,13 +2116,18 @@ check_path (struct vectors * vecs, struct path * pp, unsigned int ticks)

 				pp->state = PATH_DELAYED;

 				return 1;

 			}

-			if (!was_marginal) {

+			if (!pp->marginal) {

 				pp->marginal = 1;

 				marginal_changed = 1;

 			}

-		} else if (marginal_pathgroups && was_marginal) {

-			pp->marginal = 0;

-			marginal_changed = 1;

+		} else {

+			if (pp->marginal || pp->state == PATH_DELAYED)

+				condlog(2, "%s: path is no longer marginal",

+					pp->dev);

+			if (marginal_pathgroups && pp->marginal) {

+				pp->marginal = 0;

+				marginal_changed = 1;

+			}

 		}

 	}

-- 

2.17.2