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@xxxxxxxx>
---
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", ¤t) != 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
