[PATCH 6/8] libmultipath: set max_sectors_kb in adopt_paths()

[Martin Wilck <martin.wilck@suse.com>]

As explained in the previous commit, setting max_sectors_kb for
paths that are members of a live map is dangerous. But as long as
a path is not a member of a map, setting max_sectors_kb poses no risk.
Set the parameter in adopt_paths(), for paths that aren't members
of the map yet. In order to determine whether or not a path is currently
member of the map, adopt_paths() needs to passed the current member list. If
(and only if) it's called from coalesce_paths(), the passed struct multipath
does not represent the current state of the map; adopt_paths() must therefore
get a new argument current_mpp that represents the kernel state.

We must still call sysfs_set_max_sectors_kb() from domap() in the ACT_CREATE
case, because when adopt_paths is called from coalesce_paths() for a map that
doesn't exist yet, mpp->max_sectors_kb is not set.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c   |  2 +-
 libmultipath/structs_vec.c | 62 ++++++++++++++++++++++++++++++++++----
 libmultipath/structs_vec.h |  6 ++--
 multipathd/main.c          |  6 ++--
 tests/Makefile             |  2 +-
 tests/test-lib.c           |  9 +++++-
 6 files changed, 73 insertions(+), 14 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 8cbb2a8..b5c701f 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1105,7 +1105,7 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 		/*
 		 * at this point, we know we really got a new mp
 		 */
-		mpp = add_map_with_path(vecs, pp1, 0);
+		mpp = add_map_with_path(vecs, pp1, 0, cmpp);
 		if (!mpp) {
 			orphan_path(pp1, "failed to create multipath device");
 			continue;
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 0fc608c..c0c5cc9 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -242,7 +242,38 @@ static bool update_pathvec_from_dm(vector pathvec, struct multipath *mpp,
 	return must_reload;
 }
 
-int adopt_paths(vector pathvec, struct multipath *mpp)
+static bool set_path_max_sectors_kb(const struct path *pp, int max_sectors_kb)
+{
+	char buf[11];
+	ssize_t len;
+	int ret, current;
+	bool rc = false;
+
+	if (max_sectors_kb == MAX_SECTORS_KB_UNDEF)
+		return rc;
+
+	if (sysfs_attr_get_value(pp->udev, "queue/max_sectors_kb", buf, sizeof(buf)) <= 0
+	    || sscanf(buf, "%d\n", &current) != 1)
+		current = MAX_SECTORS_KB_UNDEF;
+	if (current == max_sectors_kb)
+		return rc;
+
+	len = snprintf(buf, sizeof(buf), "%d", max_sectors_kb);
+	ret = sysfs_attr_set_value(pp->udev, "queue/max_sectors_kb", buf, len);
+	if (ret != len)
+		log_sysfs_attr_set_value(3, ret,
+					 "failed setting max_sectors_kb on %s",
+					 pp->dev);
+	else {
+		condlog(3, "%s: set max_sectors_kb to %d for %s", __func__,
+			max_sectors_kb, pp->dev);
+		rc = true;
+	}
+	return rc;
+}
+
+int adopt_paths(vector pathvec, struct multipath *mpp,
+		const struct multipath *current_mpp)
 {
 	int i, ret;
 	struct path * pp;
@@ -285,9 +316,28 @@ int adopt_paths(vector pathvec, struct multipath *mpp)
 				continue;
 			}
 
-			if (!find_path_by_devt(mpp->paths, pp->dev_t) &&
-			    store_path(mpp->paths, pp))
-				goto err;
+			if (!find_path_by_devt(mpp->paths, pp->dev_t)) {
+
+				if (store_path(mpp->paths, pp))
+					goto err;
+				/*
+				 * Setting max_sectors_kb on live paths is dangerous.
+				 * But we can do it here on a path that isn't yet part
+				 * of the map. If this value is lower than the current
+				 * max_sectors_kb and the map is reloaded, the map's
+				 * max_sectors_kb will be safely adjusted by the kernel.
+				 *
+				 * We must make sure that the path is not part of the
+				 * map yet. Normally we can check this in mpp->paths.
+				 * But if adopt_paths is called from coalesce_paths,
+				 * we need to check the separate struct multipath that
+				 * has been obtained from map_discovery().
+				 */
+				if (!current_mpp ||
+				    !mp_find_path_by_devt(current_mpp, pp->dev_t))
+					mpp->need_reload = mpp->need_reload ||
+						set_path_max_sectors_kb(pp, mpp->max_sectors_kb);
+			}
 
 			pp->mpp = mpp;
 			condlog(3, "%s: ownership set to %s",
@@ -693,7 +743,7 @@ find_existing_alias (struct multipath * mpp,
 }
 
 struct multipath *add_map_with_path(struct vectors *vecs, struct path *pp,
-				    int add_vec)
+				    int add_vec, const struct multipath *current_mpp)
 {
 	struct multipath * mpp;
 	struct config *conf = NULL;
@@ -721,7 +771,7 @@ struct multipath *add_map_with_path(struct vectors *vecs, struct path *pp,
 		goto out;
 	mpp->size = pp->size;
 
-	if (adopt_paths(vecs->pathvec, mpp) || pp->mpp != mpp ||
+	if (adopt_paths(vecs->pathvec, mpp, current_mpp) || pp->mpp != mpp ||
 	    find_slot(mpp->paths, pp) == -1)
 		goto out;
 
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index 3253f1b..dbc4305 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -13,7 +13,8 @@ struct vectors {
 
 void set_no_path_retry(struct multipath *mpp);
 
-int adopt_paths (vector pathvec, struct multipath * mpp);
+int adopt_paths (vector pathvec, struct multipath *mpp,
+		 const struct multipath *current_mpp);
 void orphan_path (struct path * pp, const char *reason);
 void set_path_removed(struct path *pp);
 
@@ -28,7 +29,8 @@ void remove_maps (struct vectors * vecs);
 
 void sync_map_state (struct multipath *);
 struct multipath * add_map_with_path (struct vectors * vecs,
-				struct path * pp, int add_vec);
+				      struct path * pp, int add_vec,
+				      const struct multipath *current_mpp);
 void update_queue_mode_del_path(struct multipath *mpp);
 void update_queue_mode_add_path(struct multipath *mpp);
 int update_multipath_table (struct multipath *mpp, vector pathvec, int flags);
diff --git a/multipathd/main.c b/multipathd/main.c
index dd17d5c..d8518a9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -636,7 +636,7 @@ update_map (struct multipath *mpp, struct vectors *vecs, int new_map)
 
 retry:
 	condlog(4, "%s: updating new map", mpp->alias);
-	if (adopt_paths(vecs->pathvec, mpp)) {
+	if (adopt_paths(vecs->pathvec, mpp, NULL)) {
 		condlog(0, "%s: failed to adopt paths for new map update",
 			mpp->alias);
 		retries = -1;
@@ -1231,7 +1231,7 @@ rescan:
 	if (mpp) {
 		condlog(4,"%s: adopting all paths for path %s",
 			mpp->alias, pp->dev);
-		if (adopt_paths(vecs->pathvec, mpp) || pp->mpp != mpp ||
+		if (adopt_paths(vecs->pathvec, mpp, NULL) || pp->mpp != mpp ||
 		    find_slot(mpp->paths, pp) == -1)
 			goto fail; /* leave path added to pathvec */
 
@@ -1245,7 +1245,7 @@ rescan:
 			return 0;
 		}
 		condlog(4,"%s: creating new map", pp->dev);
-		if ((mpp = add_map_with_path(vecs, pp, 1))) {
+		if ((mpp = add_map_with_path(vecs, pp, 1, NULL))) {
 			mpp->action = ACT_CREATE;
 			/*
 			 * We don't depend on ACT_CREATE, as domap will
diff --git a/tests/Makefile b/tests/Makefile
index d1d52dd..4005204 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -45,7 +45,7 @@ dmevents-test_OBJDEPS = $(multipathdir)/devmapper.o
 dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
 hwtable-test_TESTDEPS := test-lib.o
 hwtable-test_OBJDEPS := $(multipathdir)/discovery.o $(multipathdir)/blacklist.o \
-	$(multipathdir)/structs.o $(multipathdir)/propsel.o
+	$(multipathdir)/structs_vec.o $(multipathdir)/structs.o $(multipathdir)/propsel.o
 hwtable-test_LIBDEPS := -ludev -lpthread -ldl
 blacklist-test_TESTDEPS := test-log.o
 blacklist-test_LIBDEPS := -ludev
diff --git a/tests/test-lib.c b/tests/test-lib.c
index cdb2780..88f35e9 100644
--- a/tests/test-lib.c
+++ b/tests/test-lib.c
@@ -152,6 +152,13 @@ int __wrap_sysfs_get_size(struct path *pp, unsigned long long *sz)
 	return 0;
 }
 
+int __wrap_sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
+			   const char * value, size_t value_len)
+{
+	condlog(5, "%s: %s", __func__, value);
+	return value_len;
+}
+
 void *__wrap_udev_device_get_parent_with_subsystem_devtype(
 	struct udev_device *ud, const char *subsys, char *type)
 {
@@ -400,7 +407,7 @@ struct multipath *__mock_multipath(struct vectors *vecs, struct path *pp)
 	/* pathinfo() call in adopt_paths */
 	mock_pathinfo(DI_CHECKER|DI_PRIO, &mop);
 
-	mp = add_map_with_path(vecs, pp, 1);
+	mp = add_map_with_path(vecs, pp, 1, NULL);
 	assert_ptr_not_equal(mp, NULL);
 
 	/* TBD: mock setup_map() ... */
-- 
2.44.0