[dm-devel] [PATCH 14/35] multipathd: add "force_reconfigure" option

[mwilck@suse.com]

From: Martin Wilck <mwilck@suse.com>

Since e3270f7 ("multipathd: use weaker "force_reload" at startup"),
(multipath-tools 0.7.0), we only reload those maps that must be
reloaded during startup. "multipath reconfigure", OTOH, reloads
every map, which may take a long time on systems with lots of
storage devices, as every DM_DEVICE_RELOAD ioctl involves a
suspend/resume cycle.

The logic we use during startup is actually very robust and catches
all cases in which a reload is necessary. "reconfigure" operations
are often done because of configuration changes, and usually don't
require a full reload of every map.

This patch changes the default behavior of "multipath reconfigure"
to "weak" reload, like we do on startup since e3270f7. The behavior
can be changed by setting the configuration option
"force_reconfigure yes" before starting the reconfigure operation.
"multipath -r" is also affected, but "multipath -D -r" is not.

It would have been nice to have introduced a new cli command
"reconfigure force" instead, but the way "reconfigure" is
implemented, that would have required a major rewrite of the code.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/config.c      |  1 +
 libmultipath/config.h      |  1 +
 libmultipath/configure.c   | 19 ++++++++++++++++++-
 libmultipath/defaults.h    |  1 +
 libmultipath/dict.c        |  4 ++++
 multipath/multipath.8      |  6 ++++--
 multipath/multipath.conf.5 | 17 +++++++++++++++++
 multipathd/main.c          | 18 +++++-------------
 multipathd/multipathd.8    |  6 ++++--
 9 files changed, 55 insertions(+), 18 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 30046a1..a1ef4c3 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -869,6 +869,7 @@ int _init_config (const char *file, struct config *conf)
 	conf->ghost_delay = DEFAULT_GHOST_DELAY;
 	conf->all_tg_pt = DEFAULT_ALL_TG_PT;
 	conf->recheck_wwid = DEFAULT_RECHECK_WWID;
+	conf->force_reconfigure = DEFAULT_FORCE_RECONFIGURE;
 	/*
 	 * preload default hwtable
 	 */
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 933fe0d..4617177 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -189,6 +189,7 @@ struct config {
 	int skip_delegate;
 	unsigned int sequence_nr;
 	int recheck_wwid;
+	int force_reconfigure;
 
 	char * multipath_dir;
 	char * selector;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 7edb355..262657e 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1093,12 +1093,27 @@ out:
 	return ret;
 }
 
+static const char* reconfigure_str(int force)
+{
+	switch(force) {
+	case FORCE_RELOAD_NONE:
+		return "new";
+	case FORCE_RELOAD_WEAK:
+		return "changed";
+	case FORCE_RELOAD_YES:
+		return "all";
+	default:
+		return "<undefined>";
+	}
+}
+
 /*
  * The force_reload parameter determines how coalesce_paths treats existing maps.
  * FORCE_RELOAD_NONE: existing maps aren't touched at all
  * FORCE_RELOAD_YES: all maps are rebuilt from scratch and (re)loaded in DM
  * FORCE_RELOAD_WEAK: existing maps are compared to the current conf and only
- * reloaded in DM if there's a difference. This is useful during startup.
+ * reloaded in DM if there's a difference. This is normally sufficient.
+ * This is controlled by the "force_reconfigure" config option.
  */
 int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 		    int force_reload, enum mpath_cmds cmd)
@@ -1117,6 +1132,8 @@ int coalesce_paths (struct vectors *vecs, vector mpvec, char *refwwid,
 	int allow_queueing;
 	struct bitfield *size_mismatch_seen;
 
+	condlog(2, "%s: reloading %s maps", __func__,
+		reconfigure_str(force_reload));
 	/* ignore refwwid if it's empty */
 	if (refwwid && !strlen(refwwid))
 		refwwid = NULL;
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index c27946c..eab08a0 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -56,6 +56,7 @@
 #define DEFAULT_RECHECK_WWID RECHECK_WWID_OFF
 /* Enable no foreign libraries by default */
 #define DEFAULT_ENABLE_FOREIGN "NONE"
+#define DEFAULT_FORCE_RECONFIGURE YN_NO
 
 #define CHECKINT_UNDEF		UINT_MAX
 #define DEFAULT_CHECKINT	5
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 7a72738..fee08cf 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -299,6 +299,9 @@ declare_def_snprint(verbosity, print_int)
 declare_def_handler(reassign_maps, set_yes_no)
 declare_def_snprint(reassign_maps, print_yes_no)
 
+declare_def_handler(force_reconfigure, set_yes_no)
+declare_def_snprint(force_reconfigure, print_yes_no)
+
 declare_def_handler(multipath_dir, set_str)
 declare_def_snprint(multipath_dir, print_str)
 
@@ -1713,6 +1716,7 @@ init_keywords(vector keywords)
 	install_keyword("polling_interval", &checkint_handler, &snprint_def_checkint);
 	install_keyword("max_polling_interval", &def_max_checkint_handler, &snprint_def_max_checkint);
 	install_keyword("reassign_maps", &def_reassign_maps_handler, &snprint_def_reassign_maps);
+	install_keyword("force_reconfigure", &def_force_reconfigure_handler, &snprint_def_force_reconfigure);
 	install_keyword("multipath_dir", &def_multipath_dir_handler, &snprint_def_multipath_dir);
 	install_keyword("path_selector", &def_selector_handler, &snprint_def_selector);
 	install_keyword("path_grouping_policy", &def_pgpolicy_handler, &snprint_def_pgpolicy);
diff --git a/multipath/multipath.8 b/multipath/multipath.8
index 17df59f..b3980c0 100644
--- a/multipath/multipath.8
+++ b/multipath/multipath.8
@@ -264,9 +264,11 @@ Force new maps to use the specified policy, overriding the configuration in
 .
 .TP
 .B \-r
-Force a reload of all existing multipath maps. This command is delegated to
+Reload existing multipath maps. This command is delegated to
 the multipathd daemon if it's running. In this case, other command line
-switches of the \fImultipath\fR command have no effect.
+switches of the \fImultipath\fR command have no effect, and multipathd
+executes a \fIreconfigure\fR command. See the \fIforce_reconfigure\fR option
+in \fBmultipath.conf(5)\fR.
 .
 .TP
 .BI \-R " retries"
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 42a15ff..814de66 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -34,6 +34,7 @@ or \fBmultipathd show config\fR command.
 .SH SYNTAX
 .\" ----------------------------------------------------------------------------
 .
+.
 The configuration file contains entries of the form:
 .RS
 .nf
@@ -165,6 +166,22 @@ The default is: \fB4 * polling_interval\fR
 .
 .
 .TP
+.B force_reconfigure
+This controls what happens when \fBmultipathd reconfigure\fR or
+\fBmultipath -r\fR is executed. If set to \fIyes\fR, all multipath
+maps will be reloaded, regardless if this is necessary or not, which
+may take a lot of time on large systems. This used to be the default
+on previous versions of multipath-tools. If set to \fIno\fR,
+only those maps will be reloaded for which some parameters
+have changed that are relevant for the device-mapper configuration of the map.
+This is the behavior during \fImultipathd\fR startup.
+.RS
+.TP
+The default is: \fBno\fR
+.RE
+.
+.
+.TP
 .B reassign_maps
 Enable reassigning of device-mapper maps. With this option multipathd
 will remap existing device-mapper maps to always point to multipath
diff --git a/multipathd/main.c b/multipathd/main.c
index bda51c9..6d7c8c9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2594,14 +2594,13 @@ checkerloop (void *ap)
 }
 
 int
-configure (struct vectors * vecs)
+configure (struct vectors *vecs, bool force)
 {
 	struct multipath * mpp;
 	struct path * pp;
 	vector mpvec;
 	int i, ret;
 	struct config *conf;
-	static int force_reload = FORCE_RELOAD_WEAK;
 
 	if (!vecs->pathvec && !(vecs->pathvec = vector_alloc())) {
 		condlog(0, "couldn't allocate path vec in configure");
@@ -2649,15 +2648,9 @@ configure (struct vectors * vecs)
 	if (should_exit())
 		goto fail;
 
-	/*
-	 * create new set of maps & push changed ones into dm
-	 * In the first call, use FORCE_RELOAD_WEAK to avoid making
-	 * superfluous ACT_RELOAD ioctls. Later calls are done
-	 * with FORCE_RELOAD_YES.
-	 */
-	ret = coalesce_paths(vecs, mpvec, NULL, force_reload, CMD_NONE);
-	if (force_reload == FORCE_RELOAD_WEAK)
-		force_reload = FORCE_RELOAD_YES;
+	ret = coalesce_paths(vecs, mpvec, NULL,
+			     force ? FORCE_RELOAD_YES : FORCE_RELOAD_WEAK,
+			     CMD_NONE);
 	if (ret != CP_OK) {
 		condlog(0, "configure failed while coalescing paths");
 		goto fail;
@@ -2769,8 +2762,7 @@ reconfigure (struct vectors * vecs)
 	rcu_assign_pointer(multipath_conf, conf);
 	call_rcu(&old->rcu, rcu_free_config);
 
-	configure(vecs);
-
+	configure(vecs, conf->force_reconfigure == YN_YES);
 
 	return 0;
 }
diff --git a/multipathd/multipathd.8 b/multipathd/multipathd.8
index 048a838..b52a617 100644
--- a/multipathd/multipathd.8
+++ b/multipathd/multipathd.8
@@ -195,8 +195,10 @@ group index, starting with 1.
 .
 .TP
 .B reconfigure
-Reconfigures the multipaths. This should be triggered automatically after anyi
-hotplug event.
+Reconfigure the multipaths. This is only necessary after applying configuration
+changes, as multipathd sets up new devices automatically. See the
+\fIforce_reconfigure\fR option in \fBmultipath.conf(5)\fR. Note that multipathd
+can't process most commands while reconfiguring.
 .
 .TP
 .B suspend map|multipath $map
-- 
2.33.0


--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel