Blame SOURCES/0126-libmultipath-copy-mpp-hwe-from-pp-hwe.patch

c80bfc
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
c80bfc
From: Martin Wilck <mwilck@suse.com>
c80bfc
Date: Wed, 16 Sep 2020 22:22:36 +0200
c80bfc
Subject: [PATCH] libmultipath: copy mpp->hwe from pp->hwe
c80bfc
c80bfc
Since f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe"),
c80bfc
we've been trying to fix issues caused by paths getting freed and mpp->hwe
c80bfc
dangling. This approach couldn't work because we need mpp->hwe to persist,
c80bfc
even if all paths are removed from the map. Before f0462f0, a simple
c80bfc
assignment worked, because the lifetime of the hwe wasn't bound to the
c80bfc
path. But now, we need to copy the vector. It turns out that we need to set
c80bfc
mpp->hwe only in two places, add_map_with_path() and setup_map(), and
c80bfc
that the code is simplified overall.
c80bfc
c80bfc
Even now, it can happen that a map is added with add_map_without_paths(),
c80bfc
and has no paths. In that case, calling do_set_from_hwe() with a NULL
c80bfc
pointer is not a bug, so remove the message.
c80bfc
c80bfc
Fixes: f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe")
c80bfc
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
c80bfc
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
c80bfc
---
c80bfc
 libmultipath/configure.c   |  8 ++++++++
c80bfc
 libmultipath/propsel.c     |  2 +-
c80bfc
 libmultipath/structs.c     | 15 ++++++++++++++
c80bfc
 libmultipath/structs.h     |  1 +
c80bfc
 libmultipath/structs_vec.c | 41 +++++++++++++++++++-------------------
c80bfc
 multipathd/main.c          | 10 ----------
c80bfc
 6 files changed, 45 insertions(+), 32 deletions(-)
c80bfc
c80bfc
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
108c2a
index c341793c..6e06fea2 100644
c80bfc
--- a/libmultipath/configure.c
c80bfc
+++ b/libmultipath/configure.c
108c2a
@@ -324,6 +324,14 @@ int setup_map(struct multipath *mpp, char *params, int params_size,
c80bfc
 	if (mpp->disable_queueing && VECTOR_SIZE(mpp->paths) != 0)
c80bfc
 		mpp->disable_queueing = 0;
c80bfc
 
c80bfc
+	/*
c80bfc
+	 * If this map was created with add_map_without_path(),
c80bfc
+	 * mpp->hwe might not be set yet.
c80bfc
+	 */
c80bfc
+	if (!mpp->hwe)
c80bfc
+		extract_hwe_from_path(mpp);
c80bfc
+
c80bfc
+
c80bfc
 	/*
c80bfc
 	 * properties selectors
c80bfc
 	 *
c80bfc
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
108c2a
index 3f119dd9..bc5c27ab 100644
c80bfc
--- a/libmultipath/propsel.c
c80bfc
+++ b/libmultipath/propsel.c
108c2a
@@ -66,7 +66,7 @@ do {									\
c80bfc
 	__do_set_from_vec(struct hwentry, var, (src)->hwe, dest)
c80bfc
 
c80bfc
 #define do_set_from_hwe(var, src, dest, msg)				\
c80bfc
-	if (__do_set_from_hwe(var, src, dest)) {			\
c80bfc
+	if (src->hwe && __do_set_from_hwe(var, src, dest)) {		\
c80bfc
 		origin = msg;						\
c80bfc
 		goto out;						\
c80bfc
 	}
c80bfc
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
108c2a
index 7bdf9152..5bb6caf8 100644
c80bfc
--- a/libmultipath/structs.c
c80bfc
+++ b/libmultipath/structs.c
108c2a
@@ -242,6 +242,17 @@ alloc_multipath (void)
c80bfc
 	return mpp;
c80bfc
 }
c80bfc
 
c80bfc
+void *set_mpp_hwe(struct multipath *mpp, const struct path *pp)
c80bfc
+{
c80bfc
+	if (!mpp || !pp || !pp->hwe)
c80bfc
+		return NULL;
c80bfc
+	if (mpp->hwe)
c80bfc
+		return mpp->hwe;
c80bfc
+	mpp->hwe = vector_convert(NULL, pp->hwe,
c80bfc
+				  struct hwentry, identity);
c80bfc
+	return mpp->hwe;
c80bfc
+}
c80bfc
+
c80bfc
 void free_multipath_attributes(struct multipath *mpp)
c80bfc
 {
c80bfc
 	if (!mpp)
108c2a
@@ -283,6 +294,10 @@ free_multipath (struct multipath * mpp, enum free_path_mode free_paths)
c80bfc
 
c80bfc
 	free_pathvec(mpp->paths, free_paths);
c80bfc
 	free_pgvec(mpp->pg, free_paths);
c80bfc
+	if (mpp->hwe) {
c80bfc
+		vector_free(mpp->hwe);
c80bfc
+		mpp->hwe = NULL;
c80bfc
+	}
c80bfc
 	FREE_PTR(mpp->mpcontext);
c80bfc
 	FREE(mpp);
c80bfc
 }
c80bfc
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
108c2a
index 9130efb5..44980b4e 100644
c80bfc
--- a/libmultipath/structs.h
c80bfc
+++ b/libmultipath/structs.h
108c2a
@@ -499,6 +499,7 @@ struct host_group {
c80bfc
 struct path * alloc_path (void);
c80bfc
 struct pathgroup * alloc_pathgroup (void);
c80bfc
 struct multipath * alloc_multipath (void);
c80bfc
+void *set_mpp_hwe(struct multipath *mpp, const struct path *pp);
c80bfc
 void free_path (struct path *);
c80bfc
 void free_pathvec (vector vec, enum free_path_mode free_paths);
c80bfc
 void free_pathgroup (struct pathgroup * pgp, enum free_path_mode free_paths);
c80bfc
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
108c2a
index 24ac022e..9613252f 100644
c80bfc
--- a/libmultipath/structs_vec.c
c80bfc
+++ b/libmultipath/structs_vec.c
108c2a
@@ -180,24 +180,24 @@ extract_hwe_from_path(struct multipath * mpp)
c80bfc
 	if (mpp->hwe || !mpp->paths)
c80bfc
 		return;
c80bfc
 
c80bfc
-	condlog(3, "%s: searching paths for valid hwe", mpp->alias);
c80bfc
+	condlog(4, "%s: searching paths for valid hwe", mpp->alias);
c80bfc
 	/* doing this in two passes seems like paranoia to me */
c80bfc
 	vector_foreach_slot(mpp->paths, pp, i) {
c80bfc
-		if (pp->state != PATH_UP)
c80bfc
-			continue;
c80bfc
-		if (pp->hwe) {
c80bfc
-			mpp->hwe = pp->hwe;
c80bfc
-			return;
c80bfc
-		}
c80bfc
+		if (pp->state == PATH_UP && pp->hwe)
c80bfc
+			goto done;
c80bfc
 	}
c80bfc
 	vector_foreach_slot(mpp->paths, pp, i) {
c80bfc
-		if (pp->state == PATH_UP)
c80bfc
-			continue;
c80bfc
-		if (pp->hwe) {
c80bfc
-			mpp->hwe = pp->hwe;
c80bfc
-			return;
c80bfc
-		}
c80bfc
+		if (pp->state != PATH_UP && pp->hwe)
c80bfc
+			goto done;
c80bfc
 	}
c80bfc
+done:
c80bfc
+	if (i < VECTOR_SIZE(mpp->paths))
c80bfc
+		(void)set_mpp_hwe(mpp, pp);
c80bfc
+
c80bfc
+	if (mpp->hwe)
c80bfc
+		condlog(3, "%s: got hwe from path %s", mpp->alias, pp->dev);
c80bfc
+	else
c80bfc
+		condlog(2, "%s: no hwe found", mpp->alias);
c80bfc
 }
c80bfc
 
c80bfc
 int
108c2a
@@ -445,9 +445,15 @@ struct multipath *add_map_with_path(struct vectors *vecs, struct path *pp,
c80bfc
 
c80bfc
 	conf = get_multipath_config();
c80bfc
 	mpp->mpe = find_mpe(conf->mptable, pp->wwid);
c80bfc
-	mpp->hwe = pp->hwe;
c80bfc
 	put_multipath_config(conf);
c80bfc
 
c80bfc
+	/*
c80bfc
+	 * We need to call this before select_alias(),
c80bfc
+	 * because that accesses hwe properties.
c80bfc
+	 */
c80bfc
+	if (pp->hwe && !set_mpp_hwe(mpp, pp))
c80bfc
+		goto out;
c80bfc
+
c80bfc
 	strcpy(mpp->wwid, pp->wwid);
c80bfc
 	find_existing_alias(mpp, vecs);
c80bfc
 	if (select_alias(conf, mpp))
108c2a
@@ -497,12 +503,6 @@ int verify_paths(struct multipath *mpp, struct vectors *vecs)
c80bfc
 			vector_del_slot(mpp->paths, i);
c80bfc
 			i--;
c80bfc
 
c80bfc
-			/* Make sure mpp->hwe doesn't point to freed memory.
c80bfc
-			 * We call extract_hwe_from_path() below to restore
c80bfc
-			 * mpp->hwe
c80bfc
-			 */
c80bfc
-			if (mpp->hwe == pp->hwe)
c80bfc
-				mpp->hwe = NULL;
c80bfc
 			if ((j = find_slot(vecs->pathvec,
c80bfc
 					   (void *)pp)) != -1)
c80bfc
 				vector_del_slot(vecs->pathvec, j);
108c2a
@@ -512,7 +512,6 @@ int verify_paths(struct multipath *mpp, struct vectors *vecs)
c80bfc
 				mpp->alias, pp->dev, pp->dev_t);
c80bfc
 		}
c80bfc
 	}
c80bfc
-	extract_hwe_from_path(mpp);
c80bfc
 	return count;
c80bfc
 }
c80bfc
 
c80bfc
diff --git a/multipathd/main.c b/multipathd/main.c
108c2a
index cd68a9d2..769dcaee 100644
c80bfc
--- a/multipathd/main.c
c80bfc
+++ b/multipathd/main.c
108c2a
@@ -1198,13 +1198,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
c80bfc
 			goto fail;
c80bfc
 		}
c80bfc
 
c80bfc
-		/*
c80bfc
-		 * Make sure mpp->hwe doesn't point to freed memory
c80bfc
-		 * We call extract_hwe_from_path() below to restore mpp->hwe
c80bfc
-		 */
c80bfc
-		if (mpp->hwe == pp->hwe)
c80bfc
-			mpp->hwe = NULL;
c80bfc
-
c80bfc
 		if ((i = find_slot(mpp->paths, (void *)pp)) != -1)
c80bfc
 			vector_del_slot(mpp->paths, i);
c80bfc
 
108c2a
@@ -1216,9 +1209,6 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
108c2a
 		    flush_map_nopaths(mpp, vecs))
108c2a
 			goto out;
c80bfc
 
c80bfc
-		if (mpp->hwe == NULL)
c80bfc
-			extract_hwe_from_path(mpp);
c80bfc
-
c80bfc
 		if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {
c80bfc
 			condlog(0, "%s: failed to setup map for"
c80bfc
 				" removal of path %s", mpp->alias, pp->dev);