All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/17] Multipath patch sync
@ 2016-03-29  3:12 Benjamin Marzinski
  2016-03-29  3:12 ` [PATCH 01/17] multipathd: use /run instead of /var/run Benjamin Marzinski
                   ` (16 more replies)
  0 siblings, 17 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:12 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Here's a bunch of miscellaneous multipath patches. The first couple are
resends, with the following changes:

multipathd: use /run instead of /var/run
-----------------------------------------
Added Makefile code to verify that /var/run is actually a symlink 

retrigger uevents to try and get the uid through udev
-----------------------------------------------------
This is the same as before. There is no good udev hint to use here. If
the whole udev worker thread is killed, udev sends an empty uevent, which
we could check for, but if a killable callout takes too long, udev with
just not run any later callouts, and not flag this in any way. So if we're
just missing the id information, there's no way to know why.

Fix issues with user_friendly_names initramfs bindings
------------------------------------------------------
This no longer has the initramfs checking code. Instead it just fixes
the -B option to make it actually work with multipathd.

Add libmpathcmd library and use it internally
---------------------------------------------
This is the same as before. As far as I know, there were no objections
to it last time.

libmultipath: add ignore_new_boot_devs option
---------------------------------------------
This also no longer uses the initramfs checking code. It is simply a
commandline option.

libmultipath: Cut down on alua prioritizer ioctls
-------------------------------------------------
This now uses the sysfs files, as Hannes suggested.


The rest of the patches are new. Please apply.
Thanks.

Benjamin Marzinski (17):
  multipathd: use /run instead of /var/run
  retrigger uevents to try and get the uid through udev
  Fix issues with user_friendly_names initramfs bindings
  Add libmpathcmd library and use it internally
  libmultipath: add ignore_new_boot_devs option
  libmultipath: fix PAD and PRINT macros
  libmultipath: Cut down on alua prioritizer ioctls
  multipathd: fail if pidfile can't be created
  libmultipath: check correct function for define
  multipathd: delay reloads during creation
  multipath: Fix minor text issues
  kpartx: verify partition devices
  multipath: add exclusive_pref_bit for alua prio
  multipathd: print "fail" when remove fails
  multipath: check partitions unused before removing
  multipathd.service: remove blk-availability Requires
  multipathd: use 64-bit int for command key

 Makefile                              |   1 +
 Makefile.inc                          |  12 ++-
 kpartx/devmapper.c                    |  17 ++-
 kpartx/devmapper.h                    |   2 +-
 kpartx/kpartx.c                       |  70 ++++++++++++-
 libmpathcmd/Makefile                  |  30 ++++++
 libmpathcmd/mpath_cmd.c               | 178 +++++++++++++++++++++++++++++++
 libmpathcmd/mpath_cmd.h               | 125 ++++++++++++++++++++++
 libmpathpersist/Makefile              |   9 +-
 libmpathpersist/mpath_persist.c       |   2 +-
 libmpathpersist/mpath_updatepr.c      |  14 +--
 libmultipath/Makefile                 |   5 +-
 libmultipath/config.c                 |   7 +-
 libmultipath/config.h                 |   5 +
 libmultipath/configure.c              |  20 ++--
 libmultipath/defaults.h               |   6 +-
 libmultipath/devmapper.c              |  46 ++++++--
 libmultipath/devmapper.h              |   2 +-
 libmultipath/dict.c                   |  20 +++-
 libmultipath/discovery.c              |  28 ++---
 libmultipath/discovery.h              |   2 +
 libmultipath/print.c                  |  17 ++-
 libmultipath/prioritizers/alua.c      |  44 +++++---
 libmultipath/prioritizers/alua_rtpg.c |  69 ++++++++----
 libmultipath/prioritizers/alua_rtpg.h |   2 +-
 libmultipath/propsel.c                |   2 +-
 libmultipath/structs.h                |  10 ++
 libmultipath/uxsock.c                 |  73 +++----------
 libmultipath/uxsock.h                 |   5 +-
 libmultipath/wwids.c                  |  19 ++--
 mpathpersist/Makefile                 |   2 +-
 multipath.conf.defaults               |   1 +
 multipath/Makefile                    |   5 +-
 multipath/main.c                      |   6 +-
 multipath/multipath.conf.5            |  29 ++++-
 multipathd/Makefile                   |   5 +-
 multipathd/cli.c                      |  22 ++--
 multipathd/cli.h                      |  20 ++--
 multipathd/cli_handlers.c             |  73 ++++++++++---
 multipathd/main.c                     | 192 ++++++++++++++++++++++++++++------
 multipathd/main.h                     |   1 +
 multipathd/multipathd.8               |   4 +
 multipathd/multipathd.init.suse       |   2 +-
 multipathd/multipathd.service         |   3 +-
 multipathd/uxclnt.c                   |  13 ++-
 multipathd/uxlsnr.c                   |  11 +-
 46 files changed, 966 insertions(+), 265 deletions(-)
 create mode 100644 libmpathcmd/Makefile
 create mode 100644 libmpathcmd/mpath_cmd.c
 create mode 100644 libmpathcmd/mpath_cmd.h

-- 
1.8.3.1

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 01/17] multipathd: use /run instead of /var/run
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
@ 2016-03-29  3:12 ` Benjamin Marzinski
  2016-03-29 13:57   ` John Stoffel
  2016-03-29  3:12 ` [PATCH 02/17] retrigger uevents to try and get the uid through udev Benjamin Marzinski
                   ` (15 subsequent siblings)
  16 siblings, 1 reply; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:12 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

/var/run is now usually a symlink to /run.  If /var is on a separate
filesytem, when multipathd starts up, it might end up writing to
/var/run before the /var filesytem is mounted and thus not have its
pidfile accessible at /var/run afterwards.  On most distrubutions /run
is now a tmpfs and should always be available before multipathd is
started, so multipath should just write there directly, instead of
through the symlink.

If /var/run is not a symlink, continue using it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile.inc                    | 10 +++++++++-
 libmultipath/defaults.h         |  2 +-
 multipathd/multipathd.init.suse |  2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/Makefile.inc b/Makefile.inc
index c3ed73f..357dd33 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -21,6 +21,14 @@ ifndef LIB
 	endif
 endif
 
+ifndef RUN
+	ifeq ($(shell test -L /var/run -o ! -d /var/run && echo 1),1)
+		RUN=run
+	else
+		RUN=var/run
+	endif
+endif
+
 ifndef SYSTEMD
 	ifeq ($(shell systemctl --version > /dev/null 2>&1 && echo 1), 1)
 		SYSTEMD = $(shell systemctl --version 2> /dev/null |  sed -n 's/systemd \([0-9]*\)/\1/p')
@@ -54,7 +62,7 @@ ifndef RPM_OPT_FLAGS
 endif
 
 OPTFLAGS     = $(RPM_OPT_FLAGS) -Wunused -Wstrict-prototypes
-CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\"
+CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
 SHARED_FLAGS = -shared
 
 %.o:	%.c
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 8902f40..cf1d5be 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -26,7 +26,7 @@
 #define MAX_CHECKINT(a)		(a << 2)
 
 #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
-#define DEFAULT_PIDFILE		"/var/run/multipathd.pid"
+#define DEFAULT_PIDFILE		"/" RUN_DIR "/multipathd.pid"
 #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
 #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
diff --git a/multipathd/multipathd.init.suse b/multipathd/multipathd.init.suse
index d1319b1..ed699fa 100644
--- a/multipathd/multipathd.init.suse
+++ b/multipathd/multipathd.init.suse
@@ -17,7 +17,7 @@
 
 PATH=/bin:/usr/bin:/sbin:/usr/sbin
 DAEMON=/sbin/multipathd
-PIDFILE=/var/run/multipathd.pid
+PIDFILE=/run/multipathd.pid
 MPATH_INIT_TIMEOUT=10
 ARGS=""
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 02/17] retrigger uevents to try and get the uid through udev
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
  2016-03-29  3:12 ` [PATCH 01/17] multipathd: use /run instead of /var/run Benjamin Marzinski
@ 2016-03-29  3:12 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 03/17] Fix issues with user_friendly_names initramfs bindings Benjamin Marzinski
                   ` (14 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:12 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Ideally, udev will be able to grab the wwid when a path device is
discovered, but sometimes this isn't possible. In these cases, the
best thing that could happen would be for udev to actually get the
information, and add it to its database. This patch makes multipath
retrigger uevents a limited number of times before giving up and
trying to get the information itself.

There are two configurables that control how it does this,
"retrigger_tries" and "retrigger_delay". The first sets the number of
times it will try to retrigger a uevent to get the wwid, the second
sets the amount of time to wait between retriggers.

This patch currently only tries reinitializing the path on change events
after multipathd has triggered a change event, and it only tries once
per triggered change event.  Now, its possible that other change events
could occur on the device without multipathd tirggering them.  As the
patch stands now, it won't try to initialize the device on those.  It will,
however still try in the checkerloop, but only after it has finished
retriggering the uevents. We could be much more aggressive here, and assume
that devices that simply won't have a WWID should already be taken care of
by the blacklists, so it would be always a good idea to recheck devices on
change events. What would be ideal is if udev would let us know when it had
problems or timed out when processing a uevent, so we would know if
retiggering the uevent would be useful.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c    |  2 ++
 libmultipath/config.h    |  2 ++
 libmultipath/defaults.h  |  2 ++
 libmultipath/dict.c      |  8 ++++++++
 libmultipath/discovery.c | 28 ++++++++++++++++------------
 libmultipath/structs.h   |  8 ++++++++
 multipathd/main.c        | 40 +++++++++++++++++++++++++---------------
 7 files changed, 63 insertions(+), 27 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index c788cf6..cfcc685 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -619,6 +619,8 @@ load_config (char * file, struct udev *udev)
 	conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
 	conf->uxsock_timeout = DEFAULT_UXSOCK_TIMEOUT;
 	conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
+	conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
+	conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
 
 	/*
 	 * preload default hwtable
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 0183969..372eace 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -135,6 +135,8 @@ struct config {
 	int delay_watch_checks;
 	int delay_wait_checks;
 	int uxsock_timeout;
+	int retrigger_tries;
+	int retrigger_delay;
 	unsigned int version[3];
 
 	char * dev;
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index cf1d5be..7ef5adf 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -21,6 +21,8 @@
 #define DEFAULT_DELAY_CHECKS DELAY_CHECKS_OFF
 #define DEFAULT_UEVENT_STACKSIZE 256
 #define DEFAULT_UXSOCK_TIMEOUT  1000
+#define DEFAULT_RETRIGGER_DELAY 10
+#define DEFAULT_RETRIGGER_TRIES 3
 
 #define DEFAULT_CHECKINT	5
 #define MAX_CHECKINT(a)		(a << 2)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 5c2da43..e3bc67e 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -389,6 +389,12 @@ declare_hw_snprint(deferred_remove, print_yes_no_undef)
 declare_mp_handler(deferred_remove, set_yes_no_undef)
 declare_mp_snprint(deferred_remove, print_yes_no_undef)
 
+declare_def_handler(retrigger_tries, set_int)
+declare_def_snprint(retrigger_tries, print_int)
+
+declare_def_handler(retrigger_delay, set_int)
+declare_def_snprint(retrigger_delay, print_int)
+
 static int
 def_config_dir_handler(vector strvec)
 {
@@ -1365,6 +1371,8 @@ init_keywords(void)
 	install_keyword("delay_wait_checks", &def_delay_wait_checks_handler, &snprint_def_delay_wait_checks);
 	install_keyword("find_multipaths", &def_find_multipaths_handler, &snprint_def_find_multipaths);
 	install_keyword("uxsock_timeout", &def_uxsock_timeout_handler, &snprint_def_uxsock_timeout);
+	install_keyword("retrigger_tries", &def_retrigger_tries_handler, &snprint_def_retrigger_tries);
+	install_keyword("retrigger_delay", &def_retrigger_delay_handler, &snprint_def_retrigger_delay);
 	__deprecated install_keyword("default_selector", &def_selector_handler, NULL);
 	__deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
 	__deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 4582a20..bd65354 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1547,7 +1547,7 @@ get_uid (struct path * pp)
 					pp->dev, strerror(-len));
 
 		}
-		if (len <= 0 &&
+		if (len <= 0 && pp->retriggers >= conf->retrigger_tries &&
 		    !strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
 			len = get_vpd_uid(pp);
 			origin = "sysfs";
@@ -1651,11 +1651,19 @@ pathinfo (struct path *pp, vector hwtable, int mask)
 		}
 	}
 
-	if ((mask & DI_WWID) && !strlen(pp->wwid))
+	if ((mask & DI_WWID) && !strlen(pp->wwid)) {
 		get_uid(pp);
+		if (!strlen(pp->wwid)) {
+			pp->initialized = INIT_MISSING_UDEV;
+			pp->tick = conf->retrigger_delay;
+			return PATHINFO_OK;
+		}
+		else
+			pp->tick = 1;
+	}
+
 	if (mask & DI_BLACKLIST && mask & DI_WWID) {
-		if (!strlen(pp->wwid) ||
-		    filter_wwid(conf->blist_wwid, conf->elist_wwid,
+		if (filter_wwid(conf->blist_wwid, conf->elist_wwid,
 				pp->wwid, pp->dev) > 0) {
 			return PATHINFO_SKIPPED;
 		}
@@ -1665,17 +1673,13 @@ pathinfo (struct path *pp, vector hwtable, int mask)
 	  * Retrieve path priority, even for PATH_DOWN paths if it has never
 	  * been successfully obtained before.
 	  */
-	if ((mask & DI_PRIO) && path_state == PATH_UP) {
+	if ((mask & DI_PRIO) && path_state == PATH_UP && strlen(pp->wwid)) {
 		if (pp->state != PATH_DOWN || pp->priority == PRIO_UNDEF) {
-			if (!strlen(pp->wwid))
-				get_uid(pp);
-			if (!strlen(pp->wwid))
-				return PATHINFO_SKIPPED;
 			get_prio(pp);
 		}
 	}
 
-	pp->initialized = 1;
+	pp->initialized = INIT_OK;
 	return PATHINFO_OK;
 
 blank:
@@ -1684,7 +1688,7 @@ blank:
 	 */
 	memset(pp->wwid, 0, WWID_SIZE);
 	pp->chkrstate = pp->state = PATH_DOWN;
-	pp->initialized = 0;
+	pp->initialized = INIT_FAILED;
 
-	return 0;
+	return PATHINFO_OK;
 }
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index ef5fb7e..c56221b 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -145,6 +145,13 @@ enum delay_checks_states {
 	DELAY_CHECKS_UNDEF = 0,
 };
 
+enum initialized_states {
+	INIT_FAILED,
+	INIT_MISSING_UDEV,
+	INIT_REQUESTED_UDEV,
+	INIT_OK,
+};
+
 struct sg_id {
 	int host_no;
 	int channel;
@@ -202,6 +209,7 @@ struct path {
 	struct multipath * mpp;
 	int fd;
 	int initialized;
+	int retriggers;
 
 	/* configlet pointers */
 	struct hwentry * hwe;
diff --git a/multipathd/main.c b/multipathd/main.c
index 60c6365..38a2b42 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -458,11 +458,6 @@ uev_add_path (struct uevent *uev, struct vectors * vecs)
 		condlog(3, "%s: failed to get path info", uev->kernel);
 		return 1;
 	}
-	if (!strlen(pp->wwid)) {
-		condlog(3, "%s: Failed to get path wwid", uev->kernel);
-		free_path(pp);
-		return 1;
-	}
 	ret = store_path(vecs->pathvec, pp);
 	if (!ret) {
 		pp->checkint = conf->checkint;
@@ -722,20 +717,23 @@ static int
 uev_update_path (struct uevent *uev, struct vectors * vecs)
 {
 	int ro, retval = 0;
+	struct path * pp;
+
+	pp = find_path_by_dev(vecs->pathvec, uev->kernel);
+	if (!pp) {
+		condlog(0, "%s: spurious uevent, path not found",
+			uev->kernel);
+		return 1;
+	}
+
+	if (pp->initialized == INIT_REQUESTED_UDEV)
+		return uev_add_path(uev, vecs);
 
 	ro = uevent_get_disk_ro(uev);
 
 	if (ro >= 0) {
-		struct path * pp;
-
 		condlog(2, "%s: update path write_protect to '%d' (uevent)",
 			uev->kernel, ro);
-		pp = find_path_by_dev(vecs->pathvec, uev->kernel);
-		if (!pp) {
-			condlog(0, "%s: spurious uevent, path not found",
-				uev->kernel);
-			return 1;
-		}
 		if (pp->mpp) {
 			retval = reload_map(vecs, pp->mpp, 0);
 
@@ -1165,12 +1163,24 @@ check_path (struct vectors * vecs, struct path * pp)
 	int disable_reinstate = 0;
 	int oldchkrstate = pp->chkrstate;
 
-	if (pp->initialized && !pp->mpp)
+	if ((pp->initialized == INIT_OK ||
+	     pp->initialized == INIT_REQUESTED_UDEV) && !pp->mpp)
 		return 0;
 
 	if (pp->tick && --pp->tick)
 		return 0; /* don't check this path yet */
 
+	if (!pp->mpp && pp->initialized == INIT_MISSING_UDEV &&
+	    pp->retriggers < conf->retrigger_tries) {
+		condlog(2, "%s: triggering change event to reinitialize",
+			pp->dev);
+		pp->initialized = INIT_REQUESTED_UDEV;
+		pp->retriggers++;
+		sysfs_attr_set_value(pp->udev, "uevent", "change",
+				     strlen("change"));
+		return 0;
+	} 
+
 	/*
 	 * provision a next check soonest,
 	 * in case we exit abnormaly from here
@@ -1197,7 +1207,7 @@ check_path (struct vectors * vecs, struct path * pp)
 		return 1;
 	}
 	if (!pp->mpp) {
-		if (!strlen(pp->wwid) &&
+		if (!strlen(pp->wwid) && pp->initialized != INIT_MISSING_UDEV &&
 		    (newstate == PATH_UP || newstate == PATH_GHOST)) {
 			condlog(2, "%s: add missing path", pp->dev);
 			if (pathinfo(pp, conf->hwtable, DI_ALL) == 0) {
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 03/17] Fix issues with user_friendly_names initramfs bindings
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
  2016-03-29  3:12 ` [PATCH 01/17] multipathd: use /run instead of /var/run Benjamin Marzinski
  2016-03-29  3:12 ` [PATCH 02/17] retrigger uevents to try and get the uid through udev Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 04/17] Add libmpathcmd library and use it internally Benjamin Marzinski
                   ` (13 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Multipath has an issue with user_friendly_names set in the initramfs.
If the bindings are in the initramfs bindings file, it will create them,
and it may use bindings that are different than the ones in the regular
file system.  Once multipathd starts up in the regular file system, it
will try to register the existing bindings, but that may (and in many
cases, is likely to) fail. If it can't reanme it, will pick a new
binding. Since when multipathd starts discovering the existing devices,
it obviously doesn't know all of the existing devices yet, it may very
well pick a binding that's already in use by a device that it hasn't
discovered yet. In this case multipath will be unable to rename the
device to the new binding. Unfortunately, if it fails the rename, it
never resets the alias of the device stored in the mpvec to current
alias of the actual dm device. So multipath will have devices in the
mpvec where the alias and the wwid don't match the actual dm devices
that exist.  This can cause all sorts of problems.

This patch does two things to deal with this.  First, it makes sure that
if the rename fails, the device will reset its alias to the one that it
currently has. Second, it fixes the -B option to actually work
correctly, so that it can be started that way in the initramfs.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c    | 1 -
 libmultipath/configure.c | 3 +++
 multipathd/main.c        | 1 +
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index cfcc685..005252b 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -601,7 +601,6 @@ load_config (char * file, struct udev *udev)
 	get_sys_max_fds(&conf->max_fds);
 	conf->bindings_file = set_default(DEFAULT_BINDINGS_FILE);
 	conf->wwids_file = set_default(DEFAULT_WWIDS_FILE);
-	conf->bindings_read_only = 0;
 	conf->multipath_dir = set_default(DEFAULT_MULTIPATHDIR);
 	conf->features = set_default(DEFAULT_FEATURES);
 	conf->flush_on_last_del = 0;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 24ad948..3559c01 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -421,6 +421,9 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 		condlog(2, "%s: unable to rename %s to %s (%s is used by %s)",
 			mpp->wwid, cmpp->alias, mpp->alias,
 			mpp->alias, cmpp_by_name->wwid);
+		/* reset alias to existing alias */
+		FREE(mpp->alias);
+		mpp->alias = STRDUP(cmpp->alias);
 		mpp->action = ACT_NOTHING;
 		return;
 	}
diff --git a/multipathd/main.c b/multipathd/main.c
index 38a2b42..687c697 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1570,6 +1570,7 @@ reconfigure (struct vectors * vecs)
 	if (!load_config(DEFAULT_CONFIGFILE, udev)) {
 		dm_drv_version(conf->version, TGT_MPATH);
 		conf->verbosity = old->verbosity;
+		conf->bindings_read_only = old->bindings_read_only;
 		conf->daemon = 1;
 		configure(vecs, 1);
 		free_config(old);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 04/17] Add libmpathcmd library and use it internally
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (2 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 03/17] Fix issues with user_friendly_names initramfs bindings Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 05/17] libmultipath: add ignore_new_boot_devs option Benjamin Marzinski
                   ` (12 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Other programs would like to communicate with multipathd to issue
command or check status.  Instead of having them exec multipathd,
I've pulled the code that sends commands and receives replies from
multipathd into its own library.  I've made the multipath tools use
this library internally as well.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile                         |   1 +
 Makefile.inc                     |   2 +
 libmpathcmd/Makefile             |  30 +++++++
 libmpathcmd/mpath_cmd.c          | 178 +++++++++++++++++++++++++++++++++++++++
 libmpathcmd/mpath_cmd.h          | 125 +++++++++++++++++++++++++++
 libmpathpersist/Makefile         |   9 +-
 libmpathpersist/mpath_updatepr.c |  14 +--
 libmultipath/Makefile            |   3 +-
 libmultipath/config.c            |   3 +-
 libmultipath/configure.c         |  10 +--
 libmultipath/defaults.h          |   1 -
 libmultipath/dict.c              |   8 +-
 libmultipath/uxsock.c            |  73 ++++------------
 libmultipath/uxsock.h            |   5 +-
 mpathpersist/Makefile            |   2 +-
 multipath/Makefile               |   5 +-
 multipath/main.c                 |   5 +-
 multipathd/Makefile              |   5 +-
 multipathd/main.c                |   1 +
 multipathd/uxclnt.c              |  13 ++-
 multipathd/uxlsnr.c              |  11 +--
 21 files changed, 398 insertions(+), 106 deletions(-)
 create mode 100644 libmpathcmd/Makefile
 create mode 100644 libmpathcmd/mpath_cmd.c
 create mode 100644 libmpathcmd/mpath_cmd.h

diff --git a/Makefile b/Makefile
index baf7753..06f50c8 100644
--- a/Makefile
+++ b/Makefile
@@ -20,6 +20,7 @@ export KRNLSRC
 export KRNLOBJ
 
 BUILDDIRS = \
+	libmpathcmd \
 	libmultipath \
 	libmultipath/prioritizers \
 	libmultipath/checkers \
diff --git a/Makefile.inc b/Makefile.inc
index 357dd33..ecc20ef 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -50,9 +50,11 @@ man5dir     = $(prefix)/usr/share/man/man5
 man3dir      = $(prefix)/usr/share/man/man3
 rcdir	    = $(prefix)/etc/init.d
 syslibdir   = $(prefix)/$(LIB)
+incdir      = $(prefix)/usr/include
 libdir	    = $(prefix)/$(LIB)/multipath
 unitdir     = $(prefix)/$(SYSTEMDPATH)/systemd/system
 mpathpersistdir = $(TOPDIR)/libmpathpersist
+mpathcmddir = $(TOPDIR)/libmpathcmd
 
 GZIP        = gzip -9 -c
 INSTALL_PROGRAM = install
diff --git a/libmpathcmd/Makefile b/libmpathcmd/Makefile
new file mode 100644
index 0000000..0304ecc
--- /dev/null
+++ b/libmpathcmd/Makefile
@@ -0,0 +1,30 @@
+# Makefile
+#
+include ../Makefile.inc
+
+SONAME=0
+DEVLIB = libmpathcmd.so
+LIBS = $(DEVLIB).$(SONAME)
+
+OBJS = mpath_cmd.o
+
+all: $(LIBS)
+
+$(LIBS): $(OBJS)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ $(CFLAGS) -o $@ $(OBJS) $(LIBDEPS)
+	ln -sf $@ $(DEVLIB)
+
+install: $(LIBS)
+	$(INSTALL_PROGRAM) -d $(DESTDIR)$(syslibdir)
+	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
+	ln -sf $(LIBS) $(DESTDIR)$(syslibdir)/$(DEVLIB)
+	$(INSTALL_PROGRAM) -d $(DESTDIR)$(incdir)
+	$(INSTALL_PROGRAM) -m 755 mpath_cmd.h $(DESTDIR)$(incdir)
+
+uninstall:
+	rm -f $(DESTDIR)$(syslibdir)/$(LIBS)
+	rm -f $(DESTDIR)$(syslibdir)/$(DEVLIB)
+	rm -f $(DESTDIR)$(incdir)/mpath_cmd.h
+
+clean:
+	rm -f core *.a *.o *.gz *.so *.so.*
diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
new file mode 100644
index 0000000..1aaf5ea
--- /dev/null
+++ b/libmpathcmd/mpath_cmd.c
@@ -0,0 +1,178 @@
+#include <stdlib.h>
+#include <unistd.h>
+#include <stdio.h>
+#include <sys/types.h>
+#include <sys/socket.h>
+#include <sys/un.h>
+#include <poll.h>
+#include <string.h>
+#include <errno.h>
+
+#include "mpath_cmd.h"
+
+/*
+ * keep reading until its all read
+ */
+static ssize_t read_all(int fd, void *buf, size_t len, unsigned int timeout)
+{
+	size_t total = 0;
+	ssize_t n;
+	int ret;
+	struct pollfd pfd;
+
+	while (len) {
+		pfd.fd = fd;
+		pfd.events = POLLIN;
+		ret = poll(&pfd, 1, timeout);
+		if (!ret) {
+			errno = ETIMEDOUT;
+			return -1;
+		} else if (ret < 0) {
+			if (errno == EINTR)
+				continue;
+			return -1;
+		} else if (!pfd.revents & POLLIN)
+			continue;
+		n = read(fd, buf, len);
+		if (n < 0) {
+			if ((errno == EINTR) || (errno == EAGAIN))
+				continue;
+			return -1;
+		}
+		if (!n)
+			return total;
+		buf = n + (char *)buf;
+		len -= n;
+		total += n;
+	}
+	return total;
+}
+
+/*
+ * keep writing until it's all sent
+ */
+static size_t write_all(int fd, const void *buf, size_t len)
+{
+	size_t total = 0;
+
+	while (len) {
+		ssize_t n = write(fd, buf, len);
+		if (n < 0) {
+			if ((errno == EINTR) || (errno == EAGAIN))
+				continue;
+			return total;
+		}
+		if (!n)
+			return total;
+		buf = n + (char *)buf;
+		len -= n;
+		total += n;
+	}
+	return total;
+}
+
+/*
+ * connect to a unix domain socket
+ */
+int mpath_connect(void)
+{
+	int fd, len;
+	struct sockaddr_un addr;
+
+	memset(&addr, 0, sizeof(addr));
+	addr.sun_family = AF_LOCAL;
+	addr.sun_path[0] = '\0';
+	len = strlen(DEFAULT_SOCKET) + 1 + sizeof(sa_family_t);
+	strncpy(&addr.sun_path[1], DEFAULT_SOCKET, len);
+
+	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
+	if (fd == -1)
+		return -1;
+
+	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
+		close(fd);
+		return -1;
+	}
+
+	return fd;
+}
+
+int mpath_disconnect(int fd)
+{
+	return close(fd);
+}
+
+ssize_t mpath_recv_reply_len(int fd, unsigned int timeout)
+{
+	size_t len;
+	ssize_t ret;
+
+	ret = read_all(fd, &len, sizeof(len), timeout);
+	if (ret < 0)
+		return ret;
+	if (ret != sizeof(len)) {
+		errno = EIO;
+		return ret;
+	}
+	return len;
+}
+
+int mpath_recv_reply_data(int fd, char *reply, size_t len,
+			  unsigned int timeout)
+{
+	ssize_t ret;
+
+	ret = read_all(fd, reply, len, timeout);
+	if (ret < 0)
+		return ret;
+	if (ret != len) {
+		errno = EIO;
+		return -1;
+	}
+	reply[len - 1] = '\0';
+	return 0;
+}
+
+int mpath_recv_reply(int fd, char **reply, unsigned int timeout)
+{
+	int err;
+	ssize_t len;
+
+	*reply = NULL;
+	len = mpath_recv_reply_len(fd, timeout);
+	if (len <= 0)
+		return len;
+	*reply = malloc(len);
+	if (!*reply)
+		return -1;
+	err = mpath_recv_reply_data(fd, *reply, len, timeout);
+	if (err) {
+		free(*reply);
+		*reply = NULL;
+		return err;
+	}
+	return 0;
+}
+
+int mpath_send_cmd(int fd, const char *cmd)
+{
+	size_t len;
+
+	if (cmd != NULL)
+		len = strlen(cmd) + 1;
+	else
+		len = 0;
+	if (write_all(fd, &len, sizeof(len)) != sizeof(len))
+		return -1;
+	if (len && write_all(fd, cmd, len) != len)
+		return -1;
+	return 0;
+}
+
+int mpath_process_cmd(int fd, const char *cmd, char **reply,
+		      unsigned int timeout)
+{
+	if (mpath_send_cmd(fd, cmd) != 0)
+		return -1;
+	return mpath_recv_reply(fd, reply, timeout);
+}
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
new file mode 100644
index 0000000..6d80a2f
--- /dev/null
+++ b/libmpathcmd/mpath_cmd.h
@@ -0,0 +1,125 @@
+/*
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This file is part of the device-mapper multipath userspace tools.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA  02111-1307,
+ * USA.
+ */
+
+#ifndef LIB_MPATH_CMD_H
+#define LIB_MPATH_CMD_H
+
+#ifdef __cpluscplus
+extern "C" {
+#endif
+
+#define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
+#define DEFAULT_REPLY_TIMEOUT	1000
+
+
+/*
+ * DESCRIPTION:
+ * 	Connect to the running multipathd daemon. On systems with the
+ * 	multipathd.socket systemd unit file installed, this command will
+ * 	start multipathd if it is not already running. This function
+ * 	must be run before any of the others in this library
+ *
+ * RETURNS:
+ * 	A file descriptor on success. -1 on failure (with errno set).
+ */
+int mpath_connect(void);
+
+
+/*
+ * DESCRIPTION:
+ * 	Disconnect from the multipathd daemon. This function must be
+ * 	run after after processing all the multipath commands.
+ *
+ * RETURNS:
+ * 	0 on success. -1 on failure (with errno set).
+ */
+int mpath_disconnect(int fd);
+
+
+/*
+ * DESCRIPTION
+ * 	Send multipathd a command and return the reply. This function
+ * 	does the same as calling mpath_send_cmd() and then
+ *	mpath_recv_reply()
+ *
+ * RETURNS:
+ * 	0 on successs, and reply will either be NULL (if there was no
+ * 	reply data), or point to the reply string, which must be freed by
+ * 	the caller. -1 on failure (with errno set).
+ */
+int mpath_process_cmd(int fd, const char *cmd, char **reply,
+		      unsigned int timeout);
+
+
+/*
+ * DESCRIPTION:
+ * 	Send a command to multipathd
+ *
+ * RETURNS:
+ * 	0 on success. -1 on failure (with errno set)
+ */
+int mpath_send_cmd(int fd, const char *cmd);
+
+
+/*
+ * DESCRIPTION:
+ * 	Return a reply from multipathd for a previously sent command.
+ * 	This is equivalent to calling mpath_recv_reply_len(), allocating
+ * 	a buffer of the appropriate size, and then calling
+ *	mpath_recv_reply_data() with that buffer.
+ *
+ * RETURNS:
+ * 	0 on success, and reply will either be NULL (if there was no
+ * 	reply data), or point to the reply string, which must be freed by
+ * 	the caller, -1 on failure (with errno set).
+ */
+int mpath_recv_reply(int fd, char **reply, unsigned int timeout);
+
+
+/*
+ * DESCRIPTION:
+ * 	Return the size of the upcoming reply data from the sent multipath
+ * 	command. This must be called before calling mpath_recv_reply_data().
+ *
+ * RETURNS:
+ * 	The required size of the reply data buffer on success. -1 on
+ * 	failure (with errno set).
+ */
+ssize_t mpath_recv_reply_len(int fd, unsigned int timeout);
+
+
+/*
+ * DESCRIPTION:
+ * 	Return the reply data from the sent multipath command.
+ * 	mpath_recv_reply_len must be called first. reply must point to a
+ * 	buffer of len size.
+ *
+ * RETURNS:
+ * 	0 on success, and reply will contain the reply data string. -1
+ * 	on failure (with errno set).
+ */
+int mpath_recv_reply_data(int fd, char *reply, size_t len,
+			  unsigned int timeout);
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* LIB_MPATH_CMD_H */
diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
index 7f726a7..e49cdb9 100644
--- a/libmpathpersist/Makefile
+++ b/libmpathpersist/Makefile
@@ -10,8 +10,9 @@ DEVLIB = libmpathpersist.so
 LIBS = $(DEVLIB).$(SONAME)
 
 
-CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) 
-LIBDEPS +=  -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath
+CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir) 
+LIBDEPS +=  -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath \
+	-L$(mpathcmddir) -lmpathcmd
 
 OBJS = mpath_persist.o mpath_updatepr.o mpath_pr_ioctl.o 
 
@@ -30,12 +31,12 @@ install: $(LIBS)
 	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
 	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(syslibdir)
 	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(man3dir)
-	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)/usr/include/
+	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(incdir)
 	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)/usr/share/doc/mpathpersist/
 	ln -sf $(DESTDIR)$(syslibdir)/$(LIBS) $(DESTDIR)$(syslibdir)/$(DEVLIB)
 	install -m 644 mpath_persistent_reserve_in.3.gz $(DESTDIR)$(man3dir)	
 	install -m 644 mpath_persistent_reserve_out.3.gz $(DESTDIR)$(man3dir)	
-	install -m 644 mpath_persist.h $(DESTDIR)/usr/include/
+	install -m 644 mpath_persist.h $(DESTDIR)$(incdir)
 
 uninstall:
 	rm -f $(DESTDIR)$(syslibdir)/$(LIBS)
diff --git a/libmpathpersist/mpath_updatepr.c b/libmpathpersist/mpath_updatepr.c
index dce580f..bda8991 100644
--- a/libmpathpersist/mpath_updatepr.c
+++ b/libmpathpersist/mpath_updatepr.c
@@ -12,9 +12,9 @@
 #include <sys/poll.h>
 #include <errno.h>
 #include <debug.h>
+#include <mpath_cmd.h>
+#include <uxsock.h>
 #include "memory.h"
-#include "../libmultipath/uxsock.h"
-#include "../libmultipath/defaults.h"
 
 unsigned long mem_allocated;    /* Total memory used in Bytes */
 
@@ -23,10 +23,9 @@ int update_prflag(char * arg1, char * arg2, int noisy)
 	int fd;
 	char str[64];
 	char *reply;
-	size_t len;
 	int ret = 0;
 
-	fd = ux_socket_connect(DEFAULT_SOCKET);
+	fd = mpath_connect();
 	if (fd == -1) {
 		condlog (0, "ux socket connect error");
 		return 1 ;
@@ -34,10 +33,10 @@ int update_prflag(char * arg1, char * arg2, int noisy)
 
 	snprintf(str,sizeof(str),"map %s %s", arg1, arg2);
 	condlog (2, "%s: pr flag message=%s", arg1, str);
-	send_packet(fd, str, strlen(str) + 1);
-	ret = recv_packet(fd, &reply, &len, DEFAULT_UXSOCK_TIMEOUT);
+	send_packet(fd, str);
+	ret = recv_packet(fd, &reply, DEFAULT_REPLY_TIMEOUT);
 	if (ret < 0) {
-		condlog(2, "%s: message=%s error=%d", arg1, str, -ret);
+		condlog(2, "%s: message=%s error=%d", arg1, str, errno);
 		ret = -2;
 	} else {
 		condlog (2, "%s: message=%s reply=%s", arg1, str, reply);
@@ -51,5 +50,6 @@ int update_prflag(char * arg1, char * arg2, int noisy)
 	}
 
 	free(reply);
+	mpath_disconnect(fd);
 	return ret;
 }
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index fc0f3d6..11750c2 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -7,7 +7,8 @@ include ../Makefile.inc
 SONAME=0
 DEVLIB = libmultipath.so
 LIBS = $(DEVLIB).$(SONAME)
-LIBDEPS = -lpthread -ldl -ldevmapper -ludev
+CFLAGS += -I$(mpathcmddir)
+LIBDEPS = -lpthread -ldl -ldevmapper -ludev -L$(mpathcmddir) -lmpathcmd
 ifdef SYSTEMD
 	ifeq ($(shell test $(SYSTEMD) -gt 209 && echo 1), 1)
 		LIBDEPS += -lsystemd
diff --git a/libmultipath/config.c b/libmultipath/config.c
index 005252b..b038b47 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -24,6 +24,7 @@
 #include "defaults.h"
 #include "prio.h"
 #include "devmapper.h"
+#include "mpath_cmd.h"
 
 static int
 hwe_strmatch (struct hwentry *hwe1, struct hwentry *hwe2)
@@ -616,7 +617,7 @@ load_config (char * file, struct udev *udev)
 	conf->partition_delim = NULL;
 	conf->processed_main_config = 0;
 	conf->find_multipaths = DEFAULT_FIND_MULTIPATHS;
-	conf->uxsock_timeout = DEFAULT_UXSOCK_TIMEOUT;
+	conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
 	conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
 	conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
 	conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 3559c01..3c9badd 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -14,6 +14,7 @@
 #include <errno.h>
 #include <libdevmapper.h>
 #include <libudev.h>
+#include <mpath_cmd.h>
 
 #include "checkers.h"
 #include "vector.h"
@@ -708,16 +709,15 @@ int check_daemon(void)
 {
 	int fd;
 	char *reply;
-	size_t len;
 	int ret = 0;
 
-	fd = ux_socket_connect(DEFAULT_SOCKET);
+	fd = mpath_connect();
 	if (fd == -1)
 		return 0;
 
-	if (send_packet(fd, "show daemon", 12) != 0)
+	if (send_packet(fd, "show daemon") != 0)
 		goto out;
-	if (recv_packet(fd, &reply, &len, conf->uxsock_timeout) != 0)
+	if (recv_packet(fd, &reply, conf->uxsock_timeout) != 0)
 		goto out;
 
 	if (strstr(reply, "shutdown"))
@@ -728,7 +728,7 @@ int check_daemon(void)
 out_free:
 	FREE(reply);
 out:
-	close(fd);
+	mpath_disconnect(fd);
 	return ret;
 }
 
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 7ef5adf..7e847ae 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -20,7 +20,6 @@
 #define DEFAULT_DEFERRED_REMOVE DEFERRED_REMOVE_OFF
 #define DEFAULT_DELAY_CHECKS DELAY_CHECKS_OFF
 #define DEFAULT_UEVENT_STACKSIZE 256
-#define DEFAULT_UXSOCK_TIMEOUT  1000
 #define DEFAULT_RETRIGGER_DELAY 10
 #define DEFAULT_RETRIGGER_TRIES 3
 
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index e3bc67e..661456f 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -21,6 +21,7 @@
 #include "prio.h"
 #include "errno.h"
 #include <inttypes.h>
+#include <mpath_cmd.h>
 
 static int
 set_int(vector strvec, void *ptr)
@@ -1051,10 +1052,10 @@ def_uxsock_timeout_handler(vector strvec)
 		return 1;
 
 	if (sscanf(buff, "%u", &uxsock_timeout) == 1 &&
-	    uxsock_timeout > DEFAULT_UXSOCK_TIMEOUT)
+	    uxsock_timeout > DEFAULT_REPLY_TIMEOUT)
 		conf->uxsock_timeout = uxsock_timeout;
 	else
-		conf->uxsock_timeout = DEFAULT_UXSOCK_TIMEOUT;
+		conf->uxsock_timeout = DEFAULT_REPLY_TIMEOUT;
 
 	free(buff);
 	return 0;
@@ -1143,9 +1144,6 @@ declare_ble_handler(elist_property)
 static int
 snprint_def_uxsock_timeout(char * buff, int len, void * data)
 {
-	if (conf->uxsock_timeout == DEFAULT_UXSOCK_TIMEOUT)
-		return 0;
-
 	return snprintf(buff, len, "%u", conf->uxsock_timeout);
 }
 
diff --git a/libmultipath/uxsock.c b/libmultipath/uxsock.c
index 300d657..e91abd9 100644
--- a/libmultipath/uxsock.c
+++ b/libmultipath/uxsock.c
@@ -19,40 +19,11 @@
 #ifdef USE_SYSTEMD
 #include <systemd/sd-daemon.h>
 #endif
+#include <mpath_cmd.h>
 
 #include "memory.h"
 #include "uxsock.h"
 #include "debug.h"
-
-/*
- * connect to a unix domain socket
- */
-int ux_socket_connect(const char *name)
-{
-	int fd, len;
-	struct sockaddr_un addr;
-
-	memset(&addr, 0, sizeof(addr));
-	addr.sun_family = AF_LOCAL;
-	addr.sun_path[0] = '\0';
-	len = strlen(name) + 1 + sizeof(sa_family_t);
-	strncpy(&addr.sun_path[1], name, len);
-
-	fd = socket(AF_LOCAL, SOCK_STREAM, 0);
-	if (fd == -1) {
-		condlog(3, "Couldn't create ux_socket, error %d", errno);
-		return -1;
-	}
-
-	if (connect(fd, (struct sockaddr *)&addr, len) == -1) {
-		condlog(3, "Couldn't connect to ux_socket, error %d", errno);
-		close(fd);
-		return -1;
-	}
-
-	return fd;
-}
-
 /*
  * create a unix domain socket and start listening on it
  * return a file descriptor open on the socket
@@ -165,7 +136,7 @@ ssize_t read_all(int fd, void *buf, size_t len, unsigned int timeout)
 /*
  * send a packet in length prefix format
  */
-int send_packet(int fd, const char *buf, size_t len)
+int send_packet(int fd, const char *buf)
 {
 	int ret = 0;
 	sigset_t set, old;
@@ -175,10 +146,7 @@ int send_packet(int fd, const char *buf, size_t len)
 	sigaddset(&set, SIGPIPE);
 	pthread_sigmask(SIG_BLOCK, &set, &old);
 
-	if (write_all(fd, &len, sizeof(len)) != sizeof(len))
-		ret = -1;
-	if (!ret && write_all(fd, buf, len) != len)
-		ret = -1;
+	ret = mpath_send_cmd(fd, buf);
 
 	/* And unblock it again */
 	pthread_sigmask(SIG_SETMASK, &old, NULL);
@@ -189,34 +157,23 @@ int send_packet(int fd, const char *buf, size_t len)
 /*
  * receive a packet in length prefix format
  */
-int recv_packet(int fd, char **buf, size_t *len, unsigned int timeout)
+int recv_packet(int fd, char **buf, unsigned int timeout)
 {
-	ssize_t ret;
-
-	ret = read_all(fd, len, sizeof(*len), timeout);
-	if (ret < 0) {
-		(*buf) = NULL;
-		*len = 0;
-		return ret;
-	}
-	if (ret < sizeof(*len)) {
-		(*buf) = NULL;
-		*len = 0;
-		return -EIO;
-	}
-	if (len == 0) {
-		(*buf) = NULL;
-		return 0;
-	}
-	(*buf) = MALLOC(*len);
+	int err;
+	ssize_t len;
+
+	*buf = NULL;
+	len = mpath_recv_reply_len(fd, timeout);
+	if (len <= 0)
+		return len;
+	(*buf) = MALLOC(len);
 	if (!*buf)
 		return -ENOMEM;
-	ret = read_all(fd, *buf, *len, timeout);
-	if (ret != *len) {
+	err = mpath_recv_reply_data(fd, *buf, len, timeout);
+	if (err) {
 		FREE(*buf);
 		(*buf) = NULL;
-		*len = 0;
-		return ret < 0 ? ret : -EIO;
+		return err;
 	}
 	return 0;
 }
diff --git a/libmultipath/uxsock.h b/libmultipath/uxsock.h
index 94af8d8..c1cf81f 100644
--- a/libmultipath/uxsock.h
+++ b/libmultipath/uxsock.h
@@ -1,7 +1,6 @@
 /* some prototypes */
-int ux_socket_connect(const char *name);
 int ux_socket_listen(const char *name);
-int send_packet(int fd, const char *buf, size_t len);
-int recv_packet(int fd, char **buf, size_t *len, unsigned int timeout);
+int send_packet(int fd, const char *buf);
+int recv_packet(int fd, char **buf, unsigned int timeout);
 size_t write_all(int fd, const void *buf, size_t len);
 ssize_t read_all(int fd, void *buf, size_t len, unsigned int timeout);
diff --git a/mpathpersist/Makefile b/mpathpersist/Makefile
index ad8e607..6f7a5cf 100644
--- a/mpathpersist/Makefile
+++ b/mpathpersist/Makefile
@@ -5,7 +5,7 @@ include ../Makefile.inc
 OBJS = main.o 
 
 CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) 
-LDFLAGS += -lpthread -ldevmapper -L$(mpathpersistdir) -lmpathpersist -L$(multipathdir) -lmultipath -ludev
+LDFLAGS += -lpthread -ldevmapper -L$(mpathpersistdir) -lmpathpersist -L$(multipathdir) -L$(mpathcmddir) -lmpathcmd -lmultipath -ludev
 
 EXEC = mpathpersist
 
diff --git a/multipath/Makefile b/multipath/Makefile
index 7f18e9a..3707235 100644
--- a/multipath/Makefile
+++ b/multipath/Makefile
@@ -6,8 +6,9 @@ include ../Makefile.inc
 
 OBJS = main.o
 
-CFLAGS += -I$(multipathdir)
-LDFLAGS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath -ludev
+CFLAGS += -I$(multipathdir) -I$(mpathcmddir)
+LDFLAGS += -lpthread -ldevmapper -ldl -L$(multipathdir) -lmultipath -ludev \
+	-L$(mpathcmddir) -lmpathcmd
 
 EXEC = multipath
 
diff --git a/multipath/main.c b/multipath/main.c
index d761621..decb590 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -57,6 +57,7 @@
 #include <sys/resource.h>
 #include <wwids.h>
 #include <uxsock.h>
+#include <mpath_cmd.h>
 
 int logsink;
 
@@ -633,13 +634,13 @@ main (int argc, char *argv[])
 	    conf->dev_type == DEV_UEVENT) {
 		int fd;
 
-		fd = ux_socket_connect(DEFAULT_SOCKET);
+		fd = mpath_connect();
 		if (fd == -1) {
 			printf("%s is not a valid multipath device path\n",
 				conf->dev);
 			goto out;
 		}
-		close(fd);
+		mpath_disconnect(fd);
 	}
 	if (conf->cmd == CMD_REMOVE_WWID && !conf->dev) {
 		condlog(0, "the -w option requires a device");
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 901d1e4..9b0210f 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -5,7 +5,7 @@ include ../Makefile.inc
 #
 # basic flags setting
 #
-CFLAGS += -I$(multipathdir) -I$(mpathpersistdir)
+CFLAGS += -I$(multipathdir) -I$(mpathpersistdir) -I$(mpathcmddir)
 ifdef SYSTEMD
 	CFLAGS += -DUSE_SYSTEMD=$(SYSTEMD)
 endif
@@ -18,7 +18,8 @@ ifdef SYSTEMD
 	endif
 endif
 LDFLAGS += -ludev -ldl \
-	-L$(multipathdir) -lmultipath -L$(mpathpersistdir) -lmpathpersist
+	-L$(multipathdir) -lmultipath -L$(mpathpersistdir) -lmpathpersist \
+	-L$(mpathcmddir) -lmpathcmd
 
 #
 # debuging stuff
diff --git a/multipathd/main.c b/multipathd/main.c
index 687c697..21df7be 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -21,6 +21,7 @@
 #include <systemd/sd-daemon.h>
 #endif
 #include <semaphore.h>
+#include <mpath_cmd.h>
 #include <mpath_persist.h>
 #include <time.h>
 
diff --git a/multipathd/uxclnt.c b/multipathd/uxclnt.c
index 536579f..06c1bf8 100644
--- a/multipathd/uxclnt.c
+++ b/multipathd/uxclnt.c
@@ -18,6 +18,7 @@
 #include <readline/readline.h>
 #include <readline/history.h>
 
+#include <mpath_cmd.h>
 #include <uxsock.h>
 #include <memory.h>
 #include <defaults.h>
@@ -74,7 +75,6 @@ static void process(int fd, unsigned int timeout)
 	rl_readline_name = "multipathd";
 	rl_completion_entry_function = key_generator;
 	while ((line = readline("multipathd> "))) {
-		size_t len;
 		size_t llen = strlen(line);
 
 		if (!llen) {
@@ -85,8 +85,8 @@ static void process(int fd, unsigned int timeout)
 		if (need_quit(line, llen))
 			break;
 
-		if (send_packet(fd, line, llen + 1) != 0) break;
-		ret = recv_packet(fd, &reply, &len, timeout);
+		if (send_packet(fd, line) != 0) break;
+		ret = recv_packet(fd, &reply, timeout);
 		if (ret != 0) break;
 
 		print_reply(reply);
@@ -102,14 +102,13 @@ static void process(int fd, unsigned int timeout)
 static void process_req(int fd, char * inbuf, unsigned int timeout)
 {
 	char *reply;
-	size_t len;
 	int ret;
 
-	if (send_packet(fd, inbuf, strlen(inbuf) + 1) != 0) {
+	if (send_packet(fd, inbuf) != 0) {
 		printf("cannot send packet\n");
 		return;
 	}
-	ret = recv_packet(fd, &reply, &len, timeout);
+	ret = recv_packet(fd, &reply, timeout);
 	if (ret < 0) {
 		if (ret == -ETIMEDOUT)
 			printf("timeout receiving packet\n");
@@ -128,7 +127,7 @@ int uxclnt(char * inbuf, unsigned int timeout)
 {
 	int fd;
 
-	fd = ux_socket_connect(DEFAULT_SOCKET);
+	fd = mpath_connect();
 	if (fd == -1)
 		exit(1);
 
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index d5e0ba3..e5c5d90 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -31,6 +31,7 @@
 #include <uxsock.h>
 #include <defaults.h>
 #include <config.h>
+#include <mpath_cmd.h>
 
 #include "main.h"
 #include "cli.h"
@@ -128,7 +129,6 @@ void uxsock_cleanup(void *arg)
 void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 {
 	int ux_sock;
-	size_t len;
 	int rlen, timeout;
 	char *inbuf;
 	char *reply;
@@ -236,18 +236,15 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 				}
 				if (gettimeofday(&start_time, NULL) != 0)
 					start_time.tv_sec = 0;
-
-				if (recv_packet(c->fd, &inbuf, &len,
-						timeout) != 0) {
+				if (recv_packet(c->fd, &inbuf, timeout) != 0) {
 					dead_client(c);
 				} else {
-					inbuf[len - 1] = 0;
 					condlog(4, "Got request [%s]", inbuf);
 					uxsock_trigger(inbuf, &reply, &rlen,
 						       trigger_data);
 					if (reply) {
-						if (send_packet(c->fd, reply,
-								rlen) != 0) {
+						if (send_packet(c->fd,
+								reply) != 0) {
 							dead_client(c);
 						}
 						condlog(4, "Reply [%d bytes]",
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 05/17] libmultipath: add ignore_new_boot_devs option
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (3 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 04/17] Add libmpathcmd library and use it internally Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 06/17] libmultipath: fix PAD and PRINT macros Benjamin Marzinski
                   ` (11 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

When multipath relies on the wwids file to determine whether a device is
a multipath path (with "multipath -c"), it will fail the first time a
new multipathable device is discovered, since the wwid clearly won't be
in the wwids file.  This is usually fine.  Multipath will still set
itself up on the device, and add the wwid to the wwids file. However,
this causes a race, where multipath won't claim the path immediately,
and something else may.  Later multipath will try, and possibly succeed
at, setting itself up on that device.

I've seen cases where this can cause problems during boot on and
immediately after install, where multipath racing with LVM on an already
labelled device can get the machine into a state where boot fails. This
can be avoided if multipath simply doesn't set itself up on any devices
that it didn't claim (with "multipath -c") in the initramfs.  It can
still safely attempt to set itself up on these devices later in boot,
after the regular filesystem has been set up.

To allow this, this patch adds a new multipahtd commandline option, -n. When
multipathd is run with this set, it will not create multipath devices if
their wwid isn't already in the wwids file. This means that only devices
that are claimed by "multipath -c" will be used by multipathd.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.h    |  1 +
 libmultipath/configure.c |  3 +--
 libmultipath/wwids.c     | 19 ++++++++++++-------
 multipathd/main.c        |  9 ++++++---
 multipathd/multipathd.8  |  4 ++++
 5 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/libmultipath/config.h b/libmultipath/config.h
index 372eace..d6a1d4f 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -137,6 +137,7 @@ struct config {
 	int uxsock_timeout;
 	int retrigger_tries;
 	int retrigger_delay;
+	int ignore_new_devs;
 	unsigned int version[3];
 
 	char * dev;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 3c9badd..1ab3324 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -778,8 +778,7 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 			continue;
 
 		/* If find_multipaths was selected check if the path is valid */
-		if (conf->find_multipaths && !refwwid &&
-		    !should_multipath(pp1, pathvec)) {
+		if (!refwwid && !should_multipath(pp1, pathvec)) {
 			orphan_path(pp1, "only one path");
 			continue;
 		}
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index f6f8ea8..567c93d 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -266,14 +266,19 @@ should_multipath(struct path *pp1, vector pathvec)
 	int i;
 	struct path *pp2;
 
+	if (!conf->find_multipaths && !conf->ignore_new_devs)
+		return 1;
+
 	condlog(4, "checking if %s should be multipathed", pp1->dev);
-	vector_foreach_slot(pathvec, pp2, i) {
-		if (pp1->dev == pp2->dev)
-			continue;
-		if (strncmp(pp1->wwid, pp2->wwid, WWID_SIZE) == 0) {
-			condlog(3, "found multiple paths with wwid %s, "
-				"multipathing %s", pp1->wwid, pp1->dev);
-			return 1;
+	if (!conf->ignore_new_devs) {
+		vector_foreach_slot(pathvec, pp2, i) {
+			if (pp1->dev == pp2->dev)
+				continue;
+			if (strncmp(pp1->wwid, pp2->wwid, WWID_SIZE) == 0) {
+				condlog(3, "found multiple paths with wwid %s, "
+					"multipathing %s", pp1->wwid, pp1->dev);
+				return 1;
+			}
 		}
 	}
 	if (check_wwids_file(pp1->wwid, 0) < 0) {
diff --git a/multipathd/main.c b/multipathd/main.c
index 21df7be..8f4fb58 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -518,8 +518,7 @@ rescan:
 		mpp->flush_on_last_del = FLUSH_UNDEF;
 		mpp->action = ACT_RELOAD;
 	} else {
-		if (conf->find_multipaths &&
-		    !should_multipath(pp, vecs->pathvec)) {
+		if (!should_multipath(pp, vecs->pathvec)) {
 			orphan_path(pp, "only one path");
 			return 0;
 		}
@@ -1572,6 +1571,7 @@ reconfigure (struct vectors * vecs)
 		dm_drv_version(conf->version, TGT_MPATH);
 		conf->verbosity = old->verbosity;
 		conf->bindings_read_only = old->bindings_read_only;
+		conf->ignore_new_devs = old->ignore_new_devs;
 		conf->daemon = 1;
 		configure(vecs, 1);
 		free_config(old);
@@ -2076,7 +2076,7 @@ main (int argc, char *argv[])
 	if (!conf)
 		exit(1);
 
-	while ((arg = getopt(argc, argv, ":dsv:k::B")) != EOF ) {
+	while ((arg = getopt(argc, argv, ":dsv:k::Bn")) != EOF ) {
 	switch(arg) {
 		case 'd':
 			foreground = 1;
@@ -2102,6 +2102,9 @@ main (int argc, char *argv[])
 		case 'B':
 			conf->bindings_read_only = 1;
 			break;
+		case 'n':
+			conf->ignore_new_devs = 1;
+			break;
 		default:
 			;
 		}
diff --git a/multipathd/multipathd.8 b/multipathd/multipathd.8
index 7fe6597..77f6e72 100644
--- a/multipathd/multipathd.8
+++ b/multipathd/multipathd.8
@@ -35,6 +35,10 @@ will use its WWID as its alias.
 .TP
 .B -k 
 multipathd will enter interactive mode. From this mode, the available commands can be viewed by entering "help". When you are finished entering commands, press CTRL-D to quit.
+.TP
+.B -n
+ignore new devices. Multipathd will not create a multipath device unless the
+wwid for the device is already listed in the wwids file.
 
 .SH COMMANDS
 .TP
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 06/17] libmultipath: fix PAD and PRINT macros
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (4 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 05/17] libmultipath: add ignore_new_boot_devs option Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 07/17] libmultipath: Cut down on alua prioritizer ioctls Benjamin Marzinski
                   ` (10 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

The PAD and PRINT macros are multi-line macros that aren't enclosed in
braces. This means that if they are used as single line code blocks
with no braces, they won't work correctly. This is currently happening
with the PAD macro, but should be fixed in both.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/print.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 6215d0b..13d076a 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -32,14 +32,21 @@
 #define MAX(x,y) (x > y) ? x : y
 #define TAIL     (line + len - 1 - c)
 #define NOPAD    s = c
-#define PAD(x)   while ((int)(c - s) < (x) && (c < (line + len - 1))) \
-			*c++ = ' '; s = c
+#define PAD(x) \
+do { \
+	while ((int)(c - s) < (x) && (c < (line + len - 1))) \
+		*c++ = ' '; \
+	s = c; \
+} while (0)
+
 #define ENDLINE \
 		if (c > line) \
 			line[c - line - 1] = '\n'
-#define PRINT(var, size, format, args...)      \
-		fwd = snprintf(var, size, format, ##args); \
-		 c += (fwd >= size) ? size : fwd;
+#define PRINT(var, size, format, args...) \
+do { \
+	fwd = snprintf(var, size, format, ##args); \
+	c += (fwd >= size) ? size : fwd; \
+} while (0)
 
 /*
  * information printing helpers
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 07/17] libmultipath: Cut down on alua prioritizer ioctls
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (5 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 06/17] libmultipath: fix PAD and PRINT macros Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 08/17] multipathd: fail if pidfile can't be created Benjamin Marzinski
                   ` (9 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Currently, running the alua prioritizer on a path causes 5 ioctls on
many devices.  get_target_port_group_support() returns whether alua is
supported. get_target_port_group() gets the TPG id. This often takes two
ioctls because 128 bytes is not a large enough buffer size on many
devices. Finally, get_asymmetric_access_state() also often takes two
ioctls because of the buffer size. This can get to be problematic when
there are thousands of paths.  The goal of this patch to to cut this
down to one call in the usual case.

In order to do this, get_target_port_group_support() is now only called
when get_target_port_group() fails, to provide a more useful error
message. Also, before doing an SGIO ioctl to get the vpd83 data,
multipath first tries to read it from sysfs.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.h              |  2 +
 libmultipath/prioritizers/alua.c      | 24 ++++++------
 libmultipath/prioritizers/alua_rtpg.c | 69 ++++++++++++++++++++++++-----------
 libmultipath/prioritizers/alua_rtpg.h |  2 +-
 libmultipath/propsel.c                |  2 +-
 5 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index da7652c..5931bc6 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -45,6 +45,8 @@ int sysfs_set_scsi_tmo (struct multipath *mpp);
 int sysfs_get_timeout(struct path *pp, unsigned int *timeout);
 int sysfs_get_host_pci_name(struct path *pp, char *pci_name);
 int sysfs_get_iscsi_ip_address(struct path *pp, char *ip_address);
+ssize_t sysfs_get_vpd (struct udev_device * udev, int pg, unsigned char * buff,
+		       size_t len);
 
 /*
  * discovery bitmask
diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 39ed1c8..0bd374f 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -51,24 +51,22 @@ static const char *aas_print_string(int rc)
 }
 
 int
-get_alua_info(int fd)
+get_alua_info(struct path * pp)
 {
 	int	rc;
 	int	tpg;
 
-	rc = get_target_port_group_support(fd);
-	if (rc < 0)
-		return -ALUA_PRIO_TPGS_FAILED;
-
-	if (rc == TPGS_NONE)
-		return -ALUA_PRIO_NOT_SUPPORTED;
-
-	tpg = get_target_port_group(fd);
-	if (tpg < 0)
+	tpg = get_target_port_group(pp);
+	if (tpg < 0) {
+		rc = get_target_port_group_support(pp->fd);
+		if (rc < 0)
+			return -ALUA_PRIO_TPGS_FAILED;
+		if (rc == TPGS_NONE)
+			return -ALUA_PRIO_NOT_SUPPORTED;
 		return -ALUA_PRIO_RTPG_FAILED;
-
+	}
 	condlog(3, "reported target port group is %i", tpg);
-	rc = get_asymmetric_access_state(fd, tpg);
+	rc = get_asymmetric_access_state(pp->fd, tpg);
 	if (rc < 0)
 		return -ALUA_PRIO_GETAAS_FAILED;
 
@@ -86,7 +84,7 @@ int getprio (struct path * pp, char * args)
 	if (pp->fd < 0)
 		return -ALUA_PRIO_NO_INFORMATION;
 
-	rc = get_alua_info(pp->fd);
+	rc = get_alua_info(pp);
 	if (rc >= 0) {
 		aas = (rc & 0x0f);
 		priopath = (rc & 0x80);
diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index 6d04fc1..636aae5 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -17,15 +17,18 @@
 #include <string.h>
 #include <sys/ioctl.h>
 #include <inttypes.h>
+#include <libudev.h>
 
 #define __user
 #include <scsi/sg.h>
 
+#include "../structs.h"
 #include "../prio.h"
+#include "../discovery.h"
 #include "alua_rtpg.h"
 
 #define SENSE_BUFF_LEN  32
-#define DEF_TIMEOUT     60000
+#define SGIO_TIMEOUT     60000
 
 /*
  * Macro used to print debug messaged.
@@ -135,7 +138,7 @@ do_inquiry(int fd, int evpd, unsigned int codepage, void *resp, int resplen)
 	hdr.dxfer_len		= resplen;
 	hdr.sbp			= sense;
 	hdr.mx_sb_len		= sizeof(sense);
-	hdr.timeout		= get_prio_timeout(DEF_TIMEOUT);
+	hdr.timeout		= get_prio_timeout(SGIO_TIMEOUT);
 
 	if (ioctl(fd, SG_IO, &hdr) < 0) {
 		PRINT_DEBUG("do_inquiry: IOCTL failed!\n");
@@ -170,8 +173,27 @@ get_target_port_group_support(int fd)
 	return rc;
 }
 
+static int
+get_sysfs_pg83(struct path *pp, unsigned char *buff, int buflen)
+{
+	struct udev_device *parent = pp->udev;
+
+	while (parent) {
+		const char *subsys = udev_device_get_subsystem(parent);
+		if (subsys && !strncmp(subsys, "scsi", 4))
+			break;
+		parent = udev_device_get_parent(parent);
+	}
+
+	if (!parent || sysfs_get_vpd(parent, 0x83, buff, buflen) <= 0) {
+		PRINT_DEBUG("failed to read sysfs vpd pg83\n");
+		return -1;
+	}
+	return 0;
+}
+
 int
-get_target_port_group(int fd)
+get_target_port_group(struct path * pp)
 {
 	unsigned char		*buf;
 	struct vpd83_data *	vpd83;
@@ -179,7 +201,7 @@ get_target_port_group(int fd)
 	int			rc;
 	int			buflen, scsi_buflen;
 
-	buflen = 128; /* Lets start from 128 */
+	buflen = 4096;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG("malloc failed: could not allocate"
@@ -188,24 +210,29 @@ get_target_port_group(int fd)
 	}
 
 	memset(buf, 0, buflen);
-	rc = do_inquiry(fd, 1, 0x83, buf, buflen);
-	if (rc < 0)
-		goto out;
 
-	scsi_buflen = (buf[2] << 8 | buf[3]) + 4;
-	if (buflen < scsi_buflen) {
-		free(buf);
-		buf = (unsigned char *)malloc(scsi_buflen);
-		if (!buf) {
-			PRINT_DEBUG("malloc failed: could not allocate"
-				     "%u bytes\n", scsi_buflen);
-			return -RTPG_RTPG_FAILED;
-		}
-		buflen = scsi_buflen;
-		memset(buf, 0, buflen);
-		rc = do_inquiry(fd, 1, 0x83, buf, buflen);
+	rc = get_sysfs_pg83(pp, buf, buflen);
+
+	if (rc < 0) {
+		rc = do_inquiry(pp->fd, 1, 0x83, buf, buflen);
 		if (rc < 0)
 			goto out;
+
+		scsi_buflen = (buf[2] << 8 | buf[3]) + 4;
+		if (buflen < scsi_buflen) {
+			free(buf);
+			buf = (unsigned char *)malloc(scsi_buflen);
+			if (!buf) {
+				PRINT_DEBUG("malloc failed: could not allocate"
+					    "%u bytes\n", scsi_buflen);
+				return -RTPG_RTPG_FAILED;
+			}
+			buflen = scsi_buflen;
+			memset(buf, 0, buflen);
+			rc = do_inquiry(pp->fd, 1, 0x83, buf, buflen);
+			if (rc < 0)
+				goto out;
+		}
 	}
 
 	vpd83 = (struct vpd83_data *) buf;
@@ -254,7 +281,7 @@ do_rtpg(int fd, void* resp, long resplen)
 	hdr.dxfer_len		= resplen;
 	hdr.mx_sb_len		= sizeof(sense);
 	hdr.sbp			= sense;
-	hdr.timeout		= get_prio_timeout(DEF_TIMEOUT);
+	hdr.timeout		= get_prio_timeout(SGIO_TIMEOUT);
 
 	if (ioctl(fd, SG_IO, &hdr) < 0)
 		return -RTPG_RTPG_FAILED;
@@ -278,7 +305,7 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
 	int			buflen;
 	uint32_t		scsi_buflen;
 
-	buflen = 128; /* Initial value from old code */
+	buflen = 4096;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG ("malloc failed: could not allocate"
diff --git a/libmultipath/prioritizers/alua_rtpg.h b/libmultipath/prioritizers/alua_rtpg.h
index c43e0a9..91a15a4 100644
--- a/libmultipath/prioritizers/alua_rtpg.h
+++ b/libmultipath/prioritizers/alua_rtpg.h
@@ -23,7 +23,7 @@
 #define RTPG_TPG_NOT_FOUND			4
 
 int get_target_port_group_support(int fd);
-int get_target_port_group(int fd);
+int get_target_port_group(struct path * pp);
 int get_asymmetric_access_state(int fd, unsigned int tpg);
 
 #endif /* __RTPG_H__ */
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 890d4b1..8abe360 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -378,7 +378,7 @@ detect_prio(struct path * pp)
 	if ((tpgs = get_target_port_group_support(pp->fd)) <= 0)
 		return;
 	pp->tpgs = tpgs;
-	ret = get_target_port_group(pp->fd);
+	ret = get_target_port_group(pp);
 	if (ret < 0)
 		return;
 	if (get_asymmetric_access_state(pp->fd, ret) < 0)
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 08/17] multipathd: fail if pidfile can't be created
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (6 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 07/17] libmultipath: Cut down on alua prioritizer ioctls Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 09/17] libmultipath: check correct function for define Benjamin Marzinski
                   ` (8 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

Right now, multipathd ignores failures from pidfile_create.  This means
that multiple multipathd processes can be running at the same time. If
someone runs "multipathd" and doesn't add a command after it, a new
process will be created, even if one is already running. To avoid this,
multipathd needs to actually fail if the pidfile can't be created or
locked. Since we only really need the pidfile to keep from launching
multiple processes, we can create in earlier in startup. This patch
moves pidfile_create to as soon as we can be sure that we can log an
error message, and fails multipathd if the create fails.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/main.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 8f4fb58..06876b9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1768,7 +1768,7 @@ child (void * param)
 #ifdef USE_SYSTEMD
 	unsigned long checkint;
 #endif
-	int rc, pid_rc;
+	int rc;
 	char *envp;
 
 	mlockall(MCL_CURRENT | MCL_FUTURE);
@@ -1786,6 +1786,12 @@ child (void * param)
 		log_thread_start(&log_attr);
 		pthread_attr_destroy(&log_attr);
 	}
+	if (pidfile_create(DEFAULT_PIDFILE, daemon_pid)) {
+		condlog(1, "failed to create pidfile");
+		if (logsink == 1)
+			log_thread_stop();
+		exit(1);
+	}
 
 	running_state = DAEMON_START;
 
@@ -1905,10 +1911,6 @@ child (void * param)
 	}
 	pthread_attr_destroy(&misc_attr);
 
-	/* Startup complete, create logfile */
-	pid_rc = pidfile_create(DEFAULT_PIDFILE, daemon_pid);
-	/* Ignore errors, we can live without */
-
 	running_state = DAEMON_RUNNING;
 #ifdef USE_SYSTEMD
 	sd_notify(0, "READY=1\nSTATUS=running");
@@ -1959,10 +1961,8 @@ child (void * param)
 	dm_lib_exit();
 
 	/* We're done here */
-	if (!pid_rc) {
-		condlog(3, "unlink pidfile");
-		unlink(DEFAULT_PIDFILE);
-	}
+	condlog(3, "unlink pidfile");
+	unlink(DEFAULT_PIDFILE);
 
 	condlog(2, "--------shut down-------");
 
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 09/17] libmultipath: check correct function for define
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (7 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 08/17] multipathd: fail if pidfile can't be created Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 10/17] multipathd: delay reloads during creation Benjamin Marzinski
                   ` (7 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

multipath was setting LIBUDEV_API_RECVBUF in the makefile if
udev_monitor_set_resolve_buffer_size existed. However the correct
fuction name (and the one used in the code) is
udev_monitor_set_receive_buffer_size. As a result, multipath was never
setting the receive buffer size. This patch simply fixes the Makefile
define.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index 11750c2..1ee968e 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -37,7 +37,7 @@ ifneq ($(strip $(LIBDM_API_COOKIE)),0)
 	CFLAGS += -DLIBDM_API_COOKIE
 endif
 
-LIBUDEV_API_RECVBUF = $(shell grep -Ecs '^[a-z]*[[:space:]]+udev_monitor_set_resolve_buffer_size' /usr/include/libudev.h)
+LIBUDEV_API_RECVBUF = $(shell grep -Ecs '^[a-z]*[[:space:]]+udev_monitor_set_receive_buffer_size' /usr/include/libudev.h)
 
 ifneq ($(strip $(LIBUDEV_API_RECVBUF)),0)
 	CFLAGS += -DLIBUDEV_API_RECVBUF
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 10/17] multipathd: delay reloads during creation
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (8 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 09/17] libmultipath: check correct function for define Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29 14:02   ` John Stoffel
  2016-03-29  3:13 ` [PATCH 11/17] multipath: Fix minor text issues Benjamin Marzinski
                   ` (6 subsequent siblings)
  16 siblings, 1 reply; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

lvm needs PV devices to not be suspended while the udev rules are
running, for them to be correctly identified as PVs. However, multipathd
will often be in a situation where it will create a multipath device
upon seeing a path, and then immediately reload the device upon seeing
another path.  If multipath is reloading a device while processing the
udev event from its creation, lvm can fail to identify it as a PV. This
can cause systems to fail to boot. Unfortunately, using udev
synchronization cookies to solve this issue would cause a host of other
issues that could only be avoided by a pretty substantial change in how
multipathd does locking and event processing. The good news is that
multipathd is already listening to udev events itself, and can make sure
that it isn't reloading when it shouldn't be.

This patch makes multipathd delay or refuse any reloads that would
happen between the time when it creates a device, and when it receives
the change uevent from the device creation. The only reloads that it
refuses are from the multipathd interactive commands that make no sense
on a not fully started device.  Otherwise, it processes the event or
command, and sets a flag to either mark that device for an update, or
to signal that multipathd needs a reconfigure. When the udev event for
the creation arrives, multipath will reload the device if necessary. If
a reconfigure has been requested, and no devices are currently being
created, multipathd will also do the reconfigure then.

Also this patch adds a configurable timer "missing_uev_msg_delay"
defaulting to 30 seconds. If the udev creation event has not arrived
after this timeout has triggered, multipathd will start printing
messages alerting the user of this every "missing_uev_msg_delay"
seconds.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c      |   1 +
 libmultipath/config.h      |   2 +
 libmultipath/configure.c   |   4 ++
 libmultipath/defaults.h    |   1 +
 libmultipath/dict.c        |   4 ++
 libmultipath/structs.h     |   2 +
 multipath.conf.defaults    |   1 +
 multipath/multipath.conf.5 |   8 +++
 multipathd/cli_handlers.c  |  65 +++++++++++++++++++++----
 multipathd/main.c          | 119 +++++++++++++++++++++++++++++++++++++++++++--
 multipathd/main.h          |   1 +
 11 files changed, 195 insertions(+), 13 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index b038b47..b06d1eb 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -621,6 +621,7 @@ load_config (char * file, struct udev *udev)
 	conf->uid_attribute = set_default(DEFAULT_UID_ATTRIBUTE);
 	conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
 	conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
+	conf->uev_msg_delay = DEFAULT_UEV_MSG_DELAY;
 
 	/*
 	 * preload default hwtable
diff --git a/libmultipath/config.h b/libmultipath/config.h
index d6a1d4f..93acfa4 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -138,6 +138,8 @@ struct config {
 	int retrigger_tries;
 	int retrigger_delay;
 	int ignore_new_devs;
+	int delayed_reconfig;
+	int uev_msg_delay;
 	unsigned int version[3];
 
 	char * dev;
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 1ab3324..8d5ed2f 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -676,6 +676,10 @@ domap (struct multipath * mpp, char * params)
 			 */
 			if (mpp->action != ACT_CREATE)
 				mpp->action = ACT_NOTHING;
+			else {
+				mpp->wait_for_udev = 1;
+				mpp->uev_msg_tick = conf->uev_msg_delay;
+			}
 		}
 		dm_setgeometry(mpp);
 		return DOMAP_OK;
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 7e847ae..24a0495 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -22,6 +22,7 @@
 #define DEFAULT_UEVENT_STACKSIZE 256
 #define DEFAULT_RETRIGGER_DELAY 10
 #define DEFAULT_RETRIGGER_TRIES 3
+#define DEFAULT_UEV_MSG_DELAY 30
 
 #define DEFAULT_CHECKINT	5
 #define MAX_CHECKINT(a)		(a << 2)
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 661456f..192495c 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -396,6 +396,9 @@ declare_def_snprint(retrigger_tries, print_int)
 declare_def_handler(retrigger_delay, set_int)
 declare_def_snprint(retrigger_delay, print_int)
 
+declare_def_handler(uev_msg_delay, set_int)
+declare_def_snprint(uev_msg_delay, print_int)
+
 static int
 def_config_dir_handler(vector strvec)
 {
@@ -1371,6 +1374,7 @@ init_keywords(void)
 	install_keyword("uxsock_timeout", &def_uxsock_timeout_handler, &snprint_def_uxsock_timeout);
 	install_keyword("retrigger_tries", &def_retrigger_tries_handler, &snprint_def_retrigger_tries);
 	install_keyword("retrigger_delay", &def_retrigger_delay_handler, &snprint_def_retrigger_delay);
+	install_keyword("missing_uev_msg_delay", &def_uev_msg_delay_handler, &snprint_def_uev_msg_delay);
 	__deprecated install_keyword("default_selector", &def_selector_handler, NULL);
 	__deprecated install_keyword("default_path_grouping_policy", &def_pgpolicy_handler, NULL);
 	__deprecated install_keyword("default_uid_attribute", &def_uid_attribute_handler, NULL);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index c56221b..b313fca 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -226,6 +226,8 @@ struct multipath {
 	int bestpg;
 	int queuedio;
 	int action;
+	int wait_for_udev;
+	int uev_msg_tick;
 	int pgfailback;
 	int failback_tick;
 	int rr_weight;
diff --git a/multipath.conf.defaults b/multipath.conf.defaults
index 6adff28..1ab58e4 100644
--- a/multipath.conf.defaults
+++ b/multipath.conf.defaults
@@ -29,6 +29,7 @@
 #	config_dir "/etc/multipath/conf.d"
 #	delay_watch_checks no
 #	delay_wait_checks no
+#	missing_uev_msg_delay 30
 #}
 #blacklist {
 #	devnode "^(ram|raw|loop|fd|md|dm-|sr|scd|st)[0-9]*"
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 6980010..aac95fd 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -488,6 +488,14 @@ can be processed. This will result in errors like
 In these cases it is recommended to increase the CLI timeout to avoid
 those issues. The default is
 .I 1000
+.TP
+.B missing_uev_msg_delay
+Controls how long multipathd will wait, after a new multipath device is created,
+to receive a change event from udev for the device, before printing a warning
+message. This warning message will print every
+.I missing_uev_msg_delay
+seconds until the uevent is received. the default is
+.I 30
 .
 .SH "blacklist section"
 The
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index a44281f..21fe00e 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -623,6 +623,11 @@ cli_reload(void *v, char **reply, int *len, void *data)
 		condlog(0, "%s: invalid map name. cannot reload", mapname);
 		return 1;
 	}
+	if (mpp->wait_for_udev) {
+		condlog(2, "%s: device not fully created, failing reload",
+			mpp->alias);
+		return 1;
+	}
 
 	return reload_map(vecs, mpp, 0);
 }
@@ -669,6 +674,12 @@ cli_resize(void *v, char **reply, int *len, void *data)
 		return 1;
 	}
 
+	if (mpp->wait_for_udev) {
+		condlog(2, "%s: device not fully created, failing resize",
+			mpp->alias);
+		return 1;
+	}
+
 	pgp = VECTOR_SLOT(mpp->pg, 0);
 
 	if (!pgp){
@@ -833,6 +844,12 @@ cli_reconfigure(void * v, char ** reply, int * len, void * data)
 {
 	struct vectors * vecs = (struct vectors *)data;
 
+	if (need_to_delay_reconfig(vecs)) {
+		conf->delayed_reconfig = 1;
+		condlog(2, "delaying reconfigure (operator)");
+		return 0;
+	}
+
 	condlog(2, "reconfigure (operator)");
 
 	return reconfigure(vecs);
@@ -843,17 +860,25 @@ cli_suspend(void * v, char ** reply, int * len, void * data)
 {
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
-	int r = dm_simplecmd_noflush(DM_DEVICE_SUSPEND, param, 0, 0);
+	int r;
+	struct multipath * mpp;
 
 	param = convert_dev(param, 0);
-	condlog(2, "%s: suspend (operator)", param);
+	mpp = find_mp_by_alias(vecs->mpvec, param);
+	if (!mpp)
+		return 1;
 
-	if (!r) /* error */
+	if (mpp->wait_for_udev) {
+		condlog(2, "%s: device not fully created, failing suspend",
+			mpp->alias);
 		return 1;
+	}
 
-	struct multipath * mpp = find_mp_by_alias(vecs->mpvec, param);
+	r = dm_simplecmd_noflush(DM_DEVICE_SUSPEND, param, 0, 0);
 
-	if (!mpp)
+	condlog(2, "%s: suspend (operator)", param);
+
+	if (!r) /* error */
 		return 1;
 
 	dm_get_info(param, &mpp->dmi);
@@ -865,17 +890,25 @@ cli_resume(void * v, char ** reply, int * len, void * data)
 {
 	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
-	int r = dm_simplecmd_noflush(DM_DEVICE_RESUME, param, 0, 0);
+	int r;
+	struct multipath * mpp;
 
 	param = convert_dev(param, 0);
-	condlog(2, "%s: resume (operator)", param);
+	mpp = find_mp_by_alias(vecs->mpvec, param);
+	if (!mpp)
+		return 1;
 
-	if (!r) /* error */
+	if (mpp->wait_for_udev) {
+		condlog(2, "%s: device not fully created, failing resume",
+			mpp->alias);
 		return 1;
+	}
 
-	struct multipath * mpp = find_mp_by_alias(vecs->mpvec, param);
+	r = dm_simplecmd_noflush(DM_DEVICE_RESUME, param, 0, 0);
 
-	if (!mpp)
+	condlog(2, "%s: resume (operator)", param);
+
+	if (!r) /* error */
 		return 1;
 
 	dm_get_info(param, &mpp->dmi);
@@ -908,9 +941,21 @@ cli_reinstate(void * v, char ** reply, int * len, void * data)
 int
 cli_reassign (void * v, char ** reply, int * len, void * data)
 {
+	struct vectors * vecs = (struct vectors *)data;
 	char * param = get_keyparam(v, MAP);
+	struct multipath *mpp;
 
 	param = convert_dev(param, 0);
+	mpp = find_mp_by_alias(vecs->mpvec, param);
+	if (!mpp)
+		return 1;
+
+	if (mpp->wait_for_udev) {
+		condlog(2, "%s: device not fully created, failing reassign",
+			mpp->alias);
+		return 1;
+	}
+
 	condlog(3, "%s: reset devices (operator)", param);
 
 	dm_reassign(param);
diff --git a/multipathd/main.c b/multipathd/main.c
index 06876b9..ea14e03 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -262,6 +262,47 @@ flush_map(struct multipath * mpp, struct vectors * vecs, int nopaths)
 	return 0;
 }
 
+int
+update_map (struct multipath *mpp, struct vectors *vecs)
+{
+	int retries = 3;
+	char params[PARAMS_SIZE] = {0};
+
+retry:
+	condlog(4, "%s: updating new map", mpp->alias);
+	if (adopt_paths(vecs->pathvec, mpp, 1)) {
+		condlog(0, "%s: failed to adopt paths for new map update",
+			mpp->alias);
+		retries = -1;
+		goto fail;
+	}
+	verify_paths(mpp, vecs);
+	mpp->flush_on_last_del = FLUSH_UNDEF;
+	mpp->action = ACT_RELOAD;
+
+	if (setup_map(mpp, params, PARAMS_SIZE)) {
+		condlog(0, "%s: failed to setup new map in update", mpp->alias);
+		retries = -1;
+		goto fail;
+	}
+	if (domap(mpp, params) <= 0 && retries-- > 0) {
+		condlog(0, "%s: map_udate sleep", mpp->alias);
+		sleep(1);
+		goto retry;
+	}
+	dm_lib_release();
+
+fail:
+	if (setup_multipath(vecs, mpp))
+		return 1;
+
+	sync_map_state(mpp);
+
+	if (retries < 0)
+		condlog(0, "%s: failed reload in new map update", mpp->alias);
+	return 0;
+}
+
 static int
 uev_add_map (struct uevent * uev, struct vectors * vecs)
 {
@@ -304,6 +345,20 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
 	mpp = find_mp_by_alias(vecs->mpvec, alias);
 
 	if (mpp) {
+		if (mpp->wait_for_udev > 1) {
+			if (update_map(mpp, vecs))
+			/* setup multipathd removed the map */
+				return 1;
+		}
+		if (mpp->wait_for_udev) {
+			mpp->wait_for_udev = 0;
+			if (conf->delayed_reconfig &&
+			    !need_to_delay_reconfig(vecs)) {
+				condlog(2, "reconfigure (delayed)");
+				reconfigure(vecs);
+				return 0;
+			}
+		}
 		/*
 		 * Not really an error -- we generate our own uevent
 		 * if we create a multipath mapped device as a result
@@ -495,7 +550,14 @@ ev_add_path (struct path * pp, struct vectors * vecs)
 		condlog(0, "%s: failed to get path uid", pp->dev);
 		goto fail; /* leave path added to pathvec */
 	}
-	mpp = pp->mpp = find_mp_by_wwid(vecs->mpvec, pp->wwid);
+	mpp = find_mp_by_wwid(vecs->mpvec, pp->wwid);
+	if (mpp && mpp->wait_for_udev) {
+		mpp->wait_for_udev = 2;
+		orphan_path(pp, "waiting for create to complete");
+		return 0;
+	}
+
+	pp->mpp = mpp;
 rescan:
 	if (mpp) {
 		if (mpp->size != pp->size) {
@@ -678,6 +740,12 @@ ev_remove_path (struct path *pp, struct vectors * vecs)
 				" removal of path %s", mpp->alias, pp->dev);
 			goto fail;
 		}
+
+		if (mpp->wait_for_udev) {
+			mpp->wait_for_udev = 2;
+			goto out;
+		}
+
 		/*
 		 * reload the map
 		 */
@@ -735,6 +803,11 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 		condlog(2, "%s: update path write_protect to '%d' (uevent)",
 			uev->kernel, ro);
 		if (pp->mpp) {
+			if (pp->mpp->wait_for_udev) {
+				pp->mpp->wait_for_udev = 2;
+				return 0;
+			}
+
 			retval = reload_map(vecs, pp->mpp, 0);
 
 			condlog(2, "%s: map %s reloaded (retval %d)",
@@ -1075,6 +1148,20 @@ followover_should_failback(struct path * pp)
 }
 
 static void
+missing_uev_message_tick(vector mpvec)
+{
+	struct multipath * mpp;
+	unsigned int i;
+
+	vector_foreach_slot (mpvec, mpp, i) {
+		if (mpp->wait_for_udev && --mpp->uev_msg_tick <= 0) {
+			condlog(0, "%s: startup incomplete. Still waiting on udev", mpp->alias);
+			mpp->uev_msg_tick = conf->uev_msg_delay;
+		}
+	}
+}
+
+static void
 defered_failback_tick (vector mpvec)
 {
 	struct multipath * mpp;
@@ -1378,6 +1465,9 @@ check_path (struct vectors * vecs, struct path * pp)
 
 	pp->state = newstate;
 
+
+	if (pp->mpp->wait_for_udev)
+		return 1;
 	/*
 	 * path prio refreshing
 	 */
@@ -1440,6 +1530,7 @@ checkerloop (void *ap)
 		if (vecs->mpvec) {
 			defered_failback_tick(vecs->mpvec);
 			retry_count_tick(vecs->mpvec);
+			missing_uev_message_tick(vecs->mpvec);
 		}
 		if (count)
 			count--;
@@ -1545,6 +1636,22 @@ configure (struct vectors * vecs, int start_waiters)
 }
 
 int
+need_to_delay_reconfig(struct vectors * vecs)
+{
+	struct multipath *mpp;
+	int i;
+
+	if (!VECTOR_SIZE(vecs->mpvec))
+		return 0;
+
+	vector_foreach_slot(vecs->mpvec, mpp, i) {
+		if (mpp->wait_for_udev)
+			return 1;
+	}
+	return 0;
+}
+
+int
 reconfigure (struct vectors * vecs)
 {
 	struct config * old = conf;
@@ -1633,12 +1740,18 @@ void
 handle_signals(void)
 {
 	if (reconfig_sig && running_state == DAEMON_RUNNING) {
-		condlog(2, "reconfigure (signal)");
 		pthread_cleanup_push(cleanup_lock,
 				&gvecs->lock);
 		lock(gvecs->lock);
 		pthread_testcancel();
-		reconfigure(gvecs);
+		if (need_to_delay_reconfig(gvecs)) {
+			conf->delayed_reconfig = 1;
+			condlog(2, "delaying reconfigure (signal)");
+		}
+		else {
+			condlog(2, "reconfigure (signal)");
+			reconfigure(gvecs);
+		}
 		lock_cleanup_pop(gvecs->lock);
 	}
 	if (log_reset_sig) {
diff --git a/multipathd/main.h b/multipathd/main.h
index 10378ef..2f706d2 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -18,6 +18,7 @@ extern pid_t daemon_pid;
 
 void exit_daemon(void);
 const char * daemon_status(void);
+int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
 int ev_add_path (struct path *, struct vectors *);
 int ev_remove_path (struct path *, struct vectors *);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 11/17] multipath: Fix minor text issues
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (9 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 10/17] multipathd: delay reloads during creation Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 12/17] kpartx: verify partition devices Benjamin Marzinski
                   ` (5 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

The multipath.conf man page gave the incorrect default for
queue_without_daemon. The multipath usage output listed the -p option
twice. And multipath was misspelled in an mpathpersist error message.
This patch fixes these issues.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c | 2 +-
 multipath/main.c                | 1 -
 multipath/multipath.conf.5      | 2 +-
 3 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 7a2249f..b23e116 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -852,7 +852,7 @@ int update_map_pr(struct multipath *mpp)
 	if (!mpp->reservation_key)
 	{
 		/* Nothing to do. Assuming pr mgmt feature is disabled*/
-		condlog(3, "%s: reservation_key not set in multiapth.conf", mpp->alias);
+		condlog(3, "%s: reservation_key not set in multipath.conf", mpp->alias);
 		return MPATH_PR_SUCCESS;
 	}
 
diff --git a/multipath/main.c b/multipath/main.c
index decb590..d14a913 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -107,7 +107,6 @@ usage (char * progname)
 		"  -r      force devmap reload\n" \
 		"  -i      ignore wwids file\n" \
 		"  -B      treat the bindings file as read only\n" \
-		"  -p      policy failover|multibus|group_by_serial|group_by_prio\n" \
 		"  -b fil  bindings file location\n" \
 		"  -w      remove a device from the wwids file\n" \
 		"  -W      reset the wwids file include only the current devices\n" \
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index aac95fd..01adf4a 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -389,7 +389,7 @@ multipathd running, access to the paths cannot be restored, and the kernel
 cannot be told to stop queueing IO. Setting queue_without_daemon to
 .I no
 , avoids this problem. Default is
-.I yes
+.I no
 .TP
 .B bindings_file
 The full pathname of the binding file to be used when the user_friendly_names option is set. Defaults to
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 12/17] kpartx: verify partition devices
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (10 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 11/17] multipath: Fix minor text issues Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 13/17] multipath: add exclusive_pref_bit for alua prio Benjamin Marzinski
                   ` (4 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

When kpartx is modifies devices (either by removing them, or
reloading them), it doesn't first verify that the device really
is a partition of the expected device.  If a dm device just happens
to have the same name as a kpartx created device would, kpartx can
either delete that device or remap it, causing all sorts of problems.

This patch makes kpartx check the uuid to verify that the device it is
modifying really is a partition device for the correct dm device.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/devmapper.c | 17 ++++++++++---
 kpartx/devmapper.h |  2 +-
 kpartx/kpartx.c    | 70 ++++++++++++++++++++++++++++++++++++++++++++++++++----
 3 files changed, 81 insertions(+), 8 deletions(-)

diff --git a/kpartx/devmapper.c b/kpartx/devmapper.c
index 82be990..e006bc3 100644
--- a/kpartx/devmapper.c
+++ b/kpartx/devmapper.c
@@ -167,12 +167,16 @@ addout:
 }
 
 extern int
-dm_map_present (char * str)
+dm_map_present (char * str, char **uuid)
 {
 	int r = 0;
 	struct dm_task *dmt;
+	const char *uuidtmp;
 	struct dm_info info;
 
+	if (uuid)
+		*uuid = NULL;
+
 	if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
 		return 0;
 
@@ -187,8 +191,15 @@ dm_map_present (char * str)
 	if (!dm_task_get_info(dmt, &info))
 		goto out;
 
-	if (info.exists)
-		r = 1;
+	if (!info.exists)
+		goto out;
+
+	r = 1;
+	if (uuid) {
+		uuidtmp = dm_task_get_uuid(dmt);
+		if (uuidtmp && strlen(uuidtmp))
+			*uuid = strdup(uuidtmp);
+	}
 out:
 	dm_task_destroy(dmt);
 	return r;
diff --git a/kpartx/devmapper.h b/kpartx/devmapper.h
index ac1d5d9..436efe1 100644
--- a/kpartx/devmapper.h
+++ b/kpartx/devmapper.h
@@ -13,7 +13,7 @@ int dm_prereq (char *, int, int, int);
 int dm_simplecmd (int, const char *, int, uint16_t);
 int dm_addmap (int, const char *, const char *, const char *, uint64_t,
 	       int, const char *, int, mode_t, uid_t, gid_t);
-int dm_map_present (char *);
+int dm_map_present (char *, char **);
 char * dm_mapname(int major, int minor);
 dev_t dm_get_first_dep(char *devname);
 char * dm_mapuuid(int major, int minor);
diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index 8047698..e8c35d4 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -191,6 +191,21 @@ get_hotplug_device(void)
 	return device;
 }
 
+static int
+check_uuid(char *uuid, char *part_uuid, char **err_msg) {
+	char *map_uuid = strchr(part_uuid, '-');
+	if (!map_uuid || strncmp(part_uuid, "part", 4) != 0) {
+		*err_msg = "not a kpartx partition";
+		return -1;
+	}
+	map_uuid++;
+	if (strcmp(uuid, map_uuid) != 0) {
+		*err_msg = "a partition of a different device";
+		return -1;
+	}
+	return 0;
+}
+
 int
 main(int argc, char **argv){
 	int i, j, m, n, op, off, arg, c, d, ro=0;
@@ -432,6 +447,8 @@ main(int argc, char **argv){
 
 		case DELETE:
 			for (j = MAXSLICES-1; j >= 0; j--) {
+				char *part_uuid, *reason;
+
 				if (safe_sprintf(partname, "%s%s%d",
 					     mapname, delim, j+1)) {
 					fprintf(stderr, "partname too small\n");
@@ -439,9 +456,18 @@ main(int argc, char **argv){
 				}
 				strip_slash(partname);
 
-				if (!dm_map_present(partname))
+				if (!dm_map_present(partname, &part_uuid))
 					continue;
 
+				if (part_uuid && uuid) {
+					if (check_uuid(uuid, part_uuid, &reason) != 0) {
+						fprintf(stderr, "%s is %s. Not removing\n", partname, reason);
+						free(part_uuid);
+						continue;
+					}
+					free(part_uuid);
+				}
+
 				if (!dm_simplecmd(DM_DEVICE_REMOVE, partname,
 						  0, 0)) {
 					r++;
@@ -466,6 +492,8 @@ main(int argc, char **argv){
 		case UPDATE:
 			/* ADD and UPDATE share the same code that adds new partitions. */
 			for (j = 0, c = 0; j < n; j++) {
+				char *part_uuid, *reason;
+
 				if (slices[j].size == 0)
 					continue;
 
@@ -488,9 +516,19 @@ main(int argc, char **argv){
 					exit(1);
 				}
 
-				op = (dm_map_present(partname) ?
+				op = (dm_map_present(partname, &part_uuid) ?
 					DM_DEVICE_RELOAD : DM_DEVICE_CREATE);
 
+				if (part_uuid && uuid) {
+					if (check_uuid(uuid, part_uuid, &reason) != 0) {
+						fprintf(stderr, "%s is already in use, and %s\n", partname, reason);
+						r++;
+						free(part_uuid);
+						continue;
+					}
+					free(part_uuid);
+				}
+
 				if (!dm_addmap(op, partname, DM_TARGET, params,
 					       slices[j].size, ro, uuid, j+1,
 					       buf.st_mode & 0777, buf.st_uid,
@@ -498,6 +536,7 @@ main(int argc, char **argv){
 					fprintf(stderr, "create/reload failed on %s\n",
 						partname);
 					r++;
+					continue;
 				}
 				if (op == DM_DEVICE_RELOAD &&
 				    !dm_simplecmd(DM_DEVICE_RESUME, partname,
@@ -505,6 +544,7 @@ main(int argc, char **argv){
 					fprintf(stderr, "resume failed on %s\n",
 						partname);
 					r++;
+					continue;
 				}
 
 				dm_devn(partname, &slices[j].major,
@@ -520,6 +560,7 @@ main(int argc, char **argv){
 			d = c;
 			while (c) {
 				for (j = 0; j < n; j++) {
+					char *part_uuid, *reason;
 					int k = slices[j].container - 1;
 
 					if (slices[j].size == 0)
@@ -552,9 +593,19 @@ main(int argc, char **argv){
 						exit(1);
 					}
 
-					op = (dm_map_present(partname) ?
+					op = (dm_map_present(partname,
+							     &part_uuid) ?
 					      DM_DEVICE_RELOAD : DM_DEVICE_CREATE);
 
+					if (part_uuid && uuid) {
+						if (check_uuid(uuid, part_uuid, &reason) != 0) {
+							fprintf(stderr, "%s is already in use, and %s\n", partname, reason);
+							free(part_uuid);
+							continue;
+						}
+						free(part_uuid);
+					}
+
 					dm_addmap(op, partname, DM_TARGET, params,
 						  slices[j].size, ro, uuid, j+1,
 						  buf.st_mode & 0777,
@@ -584,6 +635,7 @@ main(int argc, char **argv){
 			}
 
 			for (j = MAXSLICES-1; j >= 0; j--) {
+				char *part_uuid, *reason;
 				if (safe_sprintf(partname, "%s%s%d",
 					     mapname, delim, j+1)) {
 					fprintf(stderr, "partname too small\n");
@@ -591,9 +643,19 @@ main(int argc, char **argv){
 				}
 				strip_slash(partname);
 
-				if (slices[j].size || !dm_map_present(partname))
+				if (slices[j].size ||
+				    !dm_map_present(partname, &part_uuid))
 					continue;
 
+				if (part_uuid && uuid) {
+					if (check_uuid(uuid, part_uuid, &reason) != 0) {
+						fprintf(stderr, "%s is %s. Not removing\n", partname, reason);
+						free(part_uuid);
+						continue;
+					}
+					free(part_uuid);
+				}
+
 				if (!dm_simplecmd(DM_DEVICE_REMOVE,
 						  partname, 1, 0)) {
 					r++;
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 13/17] multipath: add exclusive_pref_bit for alua prio
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (11 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 12/17] kpartx: verify partition devices Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 14/17] multipathd: print "fail" when remove fails Benjamin Marzinski
                   ` (3 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

The SCSI spec is pretty vague on exactly what the tpgs pref bit should
guarantee. In the past, the alua prioritizer has put paths with the pref
bit set in their own priority group, and some users complained.
Currently, the alua prioritizer puts paths with the tpgs pref bit set in
the highest priority path group. However, if the path with the pref bit
set is active/optimized, it will be grouped with other active/optimized
paths. Other users complained about this. The only good solution is to
allow users to configure what setting the pref bit does.

This patch allows user to set

prio_args "exclusive_pref_bit"

for the alua prioritizer.  If this is set, a paths with the pref bit set
will never get grouped with paths that don't have it set. If this is
not set, multipath will continue to work as it currently does, where
they may be grouped when all paths are active/optimized.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/prioritizers/alua.c | 20 +++++++++++++++++++-
 multipath/multipath.conf.5       | 19 ++++++++++++++++---
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 0bd374f..cd4aafc 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -75,15 +75,33 @@ get_alua_info(struct path * pp)
 	return rc;
 }
 
+int get_exclusive_perf_arg(char *args)
+{
+	char *ptr;
+
+	if (args == NULL)
+		return 0;
+	ptr = strstr(args, "exclusive_pref_bit");
+	if (!ptr)
+		return 0;
+	if (ptr[18] != '\0' && ptr[18] != ' ' && ptr[18] != '\t')
+		return 0;
+	if (ptr != args && ptr[-1] != ' ' && ptr[-1] != '\t')
+		return 0;
+	return 1;
+}
+
 int getprio (struct path * pp, char * args)
 {
 	int rc;
 	int aas;
 	int priopath;
+	int exclusive_perf;
 
 	if (pp->fd < 0)
 		return -ALUA_PRIO_NO_INFORMATION;
 
+	exclusive_perf = get_exclusive_perf_arg(args);
 	rc = get_alua_info(pp);
 	if (rc >= 0) {
 		aas = (rc & 0x0f);
@@ -104,7 +122,7 @@ int getprio (struct path * pp, char * args)
 			default:
 				rc = 0;
 		}
-		if (priopath && aas != AAS_OPTIMIZED)
+		if (priopath && (aas != AAS_OPTIMIZED || exclusive_perf))
 			rc += 80;
 	} else {
 		switch(-rc) {
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 01adf4a..0ea8c47 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -193,7 +193,9 @@ Return a constant priority of \fI1\fR.
 Generate the path priority for EMC arrays.
 .TP
 .B alua
-Generate the path priority based on the SCSI-3 ALUA settings.
+Generate the path priority based on the SCSI-3 ALUA settings. This prioritizer
+accepts the optional prio_arg
+.I exclusive_pref_bit
 .TP
 .B ontap
 Generate the path priority for NetApp arrays.
@@ -219,14 +221,25 @@ Default value is \fBnone\fR.
 .RE
 .TP
 .B prio_args
-Arguments to pass to to the prio function.  Currently only used with
-.I weighted, which needs a value of the form
+Arguments to pass to to the prio function. This only applies to certain
+prioritizers
+.RS
+.TP 12
+.B weighted
+Needs a value of the form
 .I "<hbtl|devname> <regex1> <prio1> <regex2> <prio2> ..."
 .I hbtl
 regex can be of SCSI H:B:T:L format  Ex: 1:0:.:. , *:0:0:.
 .I devname
 regex can be of device name format  Ex: sda , sd.e
 .TP
+.B alua
+If
+.I exclusive_pref_bit
+is set, paths with the TPGS pref bit set will always be in their own path
+group.
+.RE
+.TP
 .B features
 Specify any device-mapper features to be used. Syntax is
 .I num list
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 14/17] multipathd: print "fail" when remove fails
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (12 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 13/17] multipath: add exclusive_pref_bit for alua prio Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 15/17] multipath: check partitions unused before removing Benjamin Marzinski
                   ` (2 subsequent siblings)
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

When multipathd fails to remove a path or a map, it was printing "ok"
instead of "fail", and exitting with a 0 return code.  Now it prints
"fail" and exits with a 1, like it does when other commands fail.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/cli_handlers.c | 8 ++++----
 multipathd/main.c         | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 21fe00e..168b872 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -511,7 +511,7 @@ cli_del_path (void * v, char ** reply, int * len, void * data)
 	pp = find_path_by_dev(vecs->pathvec, param);
 	if (!pp) {
 		condlog(0, "%s: path already removed", param);
-		return 0;
+		return 1;
 	}
 	return ev_remove_path(pp, vecs);
 }
@@ -585,19 +585,19 @@ cli_del_map (void * v, char ** reply, int * len, void * data)
 	minor = dm_get_minor(param);
 	if (minor < 0) {
 		condlog(2, "%s: not a device mapper table", param);
-		return 0;
+		return 1;
 	}
 	major = dm_get_major(param);
 	if (major < 0) {
 		condlog(2, "%s: not a device mapper table", param);
-		return 0;
+		return 1;
 	}
 	sprintf(dev_path,"dm-%d", minor);
 	alias = dm_mapname(major, minor);
 	if (!alias) {
 		condlog(2, "%s: mapname not found for %d:%d",
 			param, major, minor);
-		return 0;
+		return 1;
 	}
 	rc = ev_remove_map(param, alias, minor, vecs);
 	FREE(alias);
diff --git a/multipathd/main.c b/multipathd/main.c
index ea14e03..0128326 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -448,12 +448,12 @@ ev_remove_map (char * devname, char * alias, int minor, struct vectors * vecs)
 	if (!mpp) {
 		condlog(2, "%s: devmap not registered, can't remove",
 			devname);
-		return 0;
+		return 1;
 	}
 	if (strcmp(mpp->alias, alias)) {
 		condlog(2, "%s: minor number mismatch (map %d, event %d)",
 			mpp->alias, mpp->dmi->minor, minor);
-		return 0;
+		return 1;
 	}
 	return flush_map(mpp, vecs, 0);
 }
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 15/17] multipath: check partitions unused before removing
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (13 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 14/17] multipathd: print "fail" when remove fails Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 16/17] multipathd.service: remove blk-availability Requires Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 17/17] multipathd: use 64-bit int for command key Benjamin Marzinski
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

If kpartx partition devices are in-use, multipath will not be able to
perform a non-deferred remove of the multipath device.  So, before
starting the remove, multipath should verify that none of the partition
devices are currently in-use.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 46 ++++++++++++++++++++++++++++++++++++++--------
 libmultipath/devmapper.h |  2 +-
 2 files changed, 39 insertions(+), 9 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index cef8522..36c1a20 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -36,6 +36,10 @@
 static int dm_cancel_remove_partmaps(const char * mapname);
 #endif
 
+static int do_foreach_partmaps(const char * mapname,
+			       int (*partmap_func)(const char *, void *),
+			       void *data);
+
 #ifndef LIBDM_API_COOKIE
 static inline int dm_task_set_cookie(struct dm_task *dmt, uint32_t *c, int a)
 {
@@ -735,6 +739,26 @@ out:
 	return r;
 }
 
+static int
+partmap_in_use(const char *name, void *data)
+{
+	int part_count, *ret_count = (int *)data;
+	int open_count = dm_get_opencount(name);
+
+	if (ret_count)
+		(*ret_count)++;
+	part_count = 0;
+	if (open_count) {
+		if (do_foreach_partmaps(name, partmap_in_use, &part_count))
+			return 1;
+		if (open_count != part_count) {
+			condlog(2, "%s: map in use", name);
+			return 1;
+		}
+	}
+	return 0;
+}
+
 extern int
 _dm_flush_map (const char * mapname, int need_sync, int deferred_remove)
 {
@@ -743,6 +767,11 @@ _dm_flush_map (const char * mapname, int need_sync, int deferred_remove)
 	if (!dm_is_mpath(mapname))
 		return 0; /* nothing to do */
 
+	/* If you aren't doing a deferred remove, make sure that no
+	 * devices are in use */
+	if (!do_deferred(deferred_remove) && partmap_in_use(mapname, NULL))
+			return 1;
+
 	if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
 		return 1;
 
@@ -851,7 +880,7 @@ dm_flush_maps (void)
 }
 
 int
-dm_message(char * mapname, char * message)
+dm_message(const char * mapname, char * message)
 {
 	int r = 1;
 	struct dm_task *dmt;
@@ -1092,7 +1121,8 @@ bad:
 }
 
 static int
-do_foreach_partmaps (const char * mapname, int (*partmap_func)(char *, void *),
+do_foreach_partmaps (const char * mapname,
+		     int (*partmap_func)(const char *, void *),
 		     void *data)
 {
 	struct dm_task *dmt;
@@ -1165,7 +1195,7 @@ struct remove_data {
 };
 
 static int
-remove_partmap(char *name, void *data)
+remove_partmap(const char *name, void *data)
 {
 	struct remove_data *rd = (struct remove_data *)data;
 
@@ -1192,7 +1222,7 @@ dm_remove_partmaps (const char * mapname, int need_sync, int deferred_remove)
 #ifdef LIBDM_API_DEFERRED
 
 static int
-cancel_remove_partmap (char *name, void *unused)
+cancel_remove_partmap (const char *name, void *unused)
 {
 	if (dm_get_opencount(name))
 		dm_cancel_remove_partmaps(name);
@@ -1312,13 +1342,13 @@ out:
 }
 
 struct rename_data {
-	char *old;
+	const char *old;
 	char *new;
 	char *delim;
 };
 
 static int
-rename_partmap (char *name, void *data)
+rename_partmap (const char *name, void *data)
 {
 	char buff[PARAMS_SIZE];
 	int offset;
@@ -1335,7 +1365,7 @@ rename_partmap (char *name, void *data)
 }
 
 int
-dm_rename_partmaps (char * old, char * new)
+dm_rename_partmaps (const char * old, char * new)
 {
 	struct rename_data rd;
 
@@ -1352,7 +1382,7 @@ dm_rename_partmaps (char * old, char * new)
 }
 
 int
-dm_rename (char * old, char * new)
+dm_rename (const char * old, char * new)
 {
 	int r = 0;
 	struct dm_task *dmt;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 1752045..0d27723 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -46,7 +46,7 @@ int dm_remove_partmaps (const char * mapname, int need_sync,
 			int deferred_remove);
 int dm_get_uuid(char *name, char *uuid);
 int dm_get_info (char * mapname, struct dm_info ** dmi);
-int dm_rename (char * old, char * new);
+int dm_rename (const char * old, char * new);
 int dm_reassign(const char * mapname);
 int dm_reassign_table(const char *name, char *old, char *new);
 int dm_setgeometry(struct multipath *mpp);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 16/17] multipathd.service: remove blk-availability Requires
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (14 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 15/17] multipath: check partitions unused before removing Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-03-29  3:13 ` [PATCH 17/17] multipathd: use 64-bit int for command key Benjamin Marzinski
  16 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

multipathd.service can start up and run just fine without
blk-availability.service. It is only necessary to coordinate shutdown
order in certain multipath setups (over iscsi for instance). Thus,
instead of "Requires", multipathd.service should use "Wants"

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/multipathd.service | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index fc2e009..d5f8606 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,11 +1,10 @@
 [Unit]
 Description=Device-Mapper Multipath Device Controller
-Requires=blk-availability.service
 Before=iscsi.service iscsid.service lvm2-activation-early.service
 Before=local-fs-pre.target
 After=multipathd.socket
 DefaultDependencies=no
-Wants=local-fs-pre.target multipathd.socket
+Wants=local-fs-pre.target multipathd.socket blk-availability.service
 Conflicts=shutdown.target
 
 [Service]
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 17/17] multipathd: use 64-bit int for command key
  2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
                   ` (15 preceding siblings ...)
  2016-03-29  3:13 ` [PATCH 16/17] multipathd.service: remove blk-availability Requires Benjamin Marzinski
@ 2016-03-29  3:13 ` Benjamin Marzinski
  2016-04-07  2:10   ` multipathd: segfault in multipathd cli_add_map() Zhangguanghui
  16 siblings, 1 reply; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-29  3:13 UTC (permalink / raw)
  To: device-mapper development; +Cc: Christophe Varoqui

There are now more than 32 keywords for the multipathd client commands.
Since these are represented by a bit flags, multipathd needs more than
32 bits for the client command keycodes. However, multipath is currently
using unsigned long for these. This makes some client commands work
incorrectly on 32-bit systems. This patch switches all the keys to use
uint64_t instead.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/cli.c | 22 +++++++++++-----------
 multipathd/cli.h | 20 +++++++++++---------
 2 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/multipathd/cli.c b/multipathd/cli.c
index e0a6bec..6a5c6db 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -26,7 +26,7 @@ alloc_handler (void)
 }
 
 static int
-add_key (vector vec, char * str, unsigned long code, int has_param)
+add_key (vector vec, char * str, uint64_t code, int has_param)
 {
 	struct key * kw;
 
@@ -57,7 +57,7 @@ out:
 }
 
 int
-add_handler (unsigned long fp, int (*fn)(void *, char **, int *, void *))
+add_handler (uint64_t fp, int (*fn)(void *, char **, int *, void *))
 {
 	struct handler * h;
 
@@ -79,7 +79,7 @@ add_handler (unsigned long fp, int (*fn)(void *, char **, int *, void *))
 }
 
 static struct handler *
-find_handler (unsigned long fp)
+find_handler (uint64_t fp)
 {
 	int i;
 	struct handler *h;
@@ -92,7 +92,7 @@ find_handler (unsigned long fp)
 }
 
 int
-set_handler_callback (unsigned long fp, int (*fn)(void *, char **, int *, void *))
+set_handler_callback (uint64_t fp, int (*fn)(void *, char **, int *, void *))
 {
 	struct handler * h = find_handler(fp);
 
@@ -293,11 +293,11 @@ out:
 	return r;
 }
 
-static unsigned long 
+static uint64_t
 fingerprint(vector vec)
 {
 	int i;
-	unsigned long fp = 0;
+	uint64_t fp = 0;
 	struct key * kw;
 
 	if (!vec)
@@ -343,7 +343,7 @@ static int
 do_genhelp(char *reply, int maxlen) {
 	int len = 0;
 	int i, j;
-	unsigned long fp;
+	uint64_t fp;
 	struct handler * h;
 	struct key * kw;
 
@@ -442,7 +442,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data)
 }
 
 char *
-get_keyparam (vector v, unsigned long code)
+get_keyparam (vector v, uint64_t code)
 {
 	struct key * kw;
 	int i;
@@ -516,7 +516,7 @@ void cli_exit(void)
 }
 
 static int
-key_match_fingerprint (struct key * kw, unsigned long fp)
+key_match_fingerprint (struct key * kw, uint64_t fp)
 {
 	if (!fp)
 		return 0;
@@ -531,7 +531,7 @@ char *
 key_generator (const char * str, int state)
 {
 	static int index, len, has_param;
-	static unsigned long rlfp;	
+	static uint64_t rlfp;
 	struct key * kw;
 	int i;
 	struct handler *h;
@@ -601,7 +601,7 @@ key_generator (const char * str, int state)
 			 * nfp is the candidate fingerprint we try to
 			 * validate against all known command fingerprints.
 			 */
-			unsigned long nfp = rlfp | kw->code;
+			uint64_t nfp = rlfp | kw->code;
 			vector_foreach_slot(handlers, h, i) {
 				if (!rlfp || ((h->fingerprint & nfp) == nfp)) {
 					/*
diff --git a/multipathd/cli.h b/multipathd/cli.h
index f6d2726..2aa19d5 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -1,3 +1,5 @@
+#include <stdint.h>
+
 enum {
 	__LIST,
 	__ADD,
@@ -68,10 +70,10 @@ enum {
 #define WILDCARDS	(1 << __WILDCARDS)
 #define QUIT		(1 << __QUIT)
 #define SHUTDOWN	(1 << __SHUTDOWN)
-#define GETPRSTATUS	(1UL << __GETPRSTATUS)
-#define SETPRSTATUS	(1UL << __SETPRSTATUS)
-#define UNSETPRSTATUS	(1UL << __UNSETPRSTATUS)
-#define FMT		(1UL << __FMT)
+#define GETPRSTATUS	(1ULL << __GETPRSTATUS)
+#define SETPRSTATUS	(1ULL << __SETPRSTATUS)
+#define UNSETPRSTATUS	(1ULL << __UNSETPRSTATUS)
+#define FMT		(1ULL << __FMT)
 
 #define INITIAL_REPLY_LEN	1200
 
@@ -92,21 +94,21 @@ enum {
 struct key {
 	char * str;
 	char * param;
-	unsigned long code;
+	uint64_t code;
 	int has_param;
 };
 
 struct handler {
-	unsigned long fingerprint;
+	uint64_t fingerprint;
 	int (*fn)(void *, char **, int *, void *);
 };
 
 int alloc_handlers (void);
-int add_handler (unsigned long fp, int (*fn)(void *, char **, int *, void *));
-int set_handler_callback (unsigned long fp, int (*fn)(void *, char **, int *, void *));
+int add_handler (uint64_t fp, int (*fn)(void *, char **, int *, void *));
+int set_handler_callback (uint64_t fp, int (*fn)(void *, char **, int *, void *));
 int parse_cmd (char * cmd, char ** reply, int * len, void *);
 int load_keys (void);
-char * get_keyparam (vector v, unsigned long code);
+char * get_keyparam (vector v, uint64_t code);
 void free_keys (vector vec);
 void free_handlers (void);
 int cli_init (void);
-- 
1.8.3.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* Re: [PATCH 01/17] multipathd: use /run instead of /var/run
  2016-03-29  3:12 ` [PATCH 01/17] multipathd: use /run instead of /var/run Benjamin Marzinski
@ 2016-03-29 13:57   ` John Stoffel
  2016-03-30  0:41     ` Benjamin Marzinski
  0 siblings, 1 reply; 24+ messages in thread
From: John Stoffel @ 2016-03-29 13:57 UTC (permalink / raw)
  To: Benjamin Marzinski; +Cc: device-mapper development, Christophe Varoqui


Benjamin> /var/run is now usually a symlink to /run.  If /var is on a separate
Benjamin> filesytem, when multipathd starts up, it might end up writing to
Benjamin> /var/run before the /var filesytem is mounted and thus not have its
Benjamin> pidfile accessible at /var/run afterwards.  On most distrubutions /run
Benjamin> is now a tmpfs and should always be available before multipathd is
Benjamin> started, so multipath should just write there directly, instead of
Benjamin> through the symlink.

I'm not sure I agree with this, it really smells more like a
distribution problem, than a multipathd problem.  If multipathd starts
up in an initramfs, how does it get reset to the proper /var/run or
/run directory then?

And especially if it's a symlink, we shouldn't care at all either.

What distros are using /run instead of /var/run these days?

John


Benjamin> If /var/run is not a symlink, continue using it.

Benjamin> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Benjamin> ---
Benjamin>  Makefile.inc                    | 10 +++++++++-
Benjamin>  libmultipath/defaults.h         |  2 +-
Benjamin>  multipathd/multipathd.init.suse |  2 +-
Benjamin>  3 files changed, 11 insertions(+), 3 deletions(-)

Benjamin> diff --git a/Makefile.inc b/Makefile.inc
Benjamin> index c3ed73f..357dd33 100644
Benjamin> --- a/Makefile.inc
Benjamin> +++ b/Makefile.inc
Benjamin> @@ -21,6 +21,14 @@ ifndef LIB
Benjamin>  	endif
Benjamin>  endif
 
Benjamin> +ifndef RUN
Benjamin> +	ifeq ($(shell test -L /var/run -o ! -d /var/run && echo 1),1)
Benjamin> +		RUN=run
Benjamin> +	else
Benjamin> +		RUN=var/run
Benjamin> +	endif
Benjamin> +endif
Benjamin> +
Benjamin>  ifndef SYSTEMD
Benjamin>  	ifeq ($(shell systemctl --version > /dev/null 2>&1 && echo 1), 1)
Benjamin>  		SYSTEMD = $(shell systemctl --version 2> /dev/null |  sed -n 's/systemd \([0-9]*\)/\1/p')
Benjamin> @@ -54,7 +62,7 @@ ifndef RPM_OPT_FLAGS
Benjamin>  endif
 
Benjamin>  OPTFLAGS     = $(RPM_OPT_FLAGS) -Wunused -Wstrict-prototypes
Benjamin> -CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\"
Benjamin> +CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
Benjamin>  SHARED_FLAGS = -shared
 
Benjamin>  %.o:	%.c
Benjamin> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
Benjamin> index 8902f40..cf1d5be 100644
Benjamin> --- a/libmultipath/defaults.h
Benjamin> +++ b/libmultipath/defaults.h
Benjamin> @@ -26,7 +26,7 @@
Benjamin>  #define MAX_CHECKINT(a)		(a << 2)
 
Benjamin>  #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
Benjamin> -#define DEFAULT_PIDFILE		"/var/run/multipathd.pid"
Benjamin> +#define DEFAULT_PIDFILE		"/" RUN_DIR "/multipathd.pid"
Benjamin>  #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
Benjamin>  #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
Benjamin>  #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
Benjamin> diff --git a/multipathd/multipathd.init.suse b/multipathd/multipathd.init.suse
Benjamin> index d1319b1..ed699fa 100644
Benjamin> --- a/multipathd/multipathd.init.suse
Benjamin> +++ b/multipathd/multipathd.init.suse
Benjamin> @@ -17,7 +17,7 @@
 
Benjamin>  PATH=/bin:/usr/bin:/sbin:/usr/sbin
Benjamin>  DAEMON=/sbin/multipathd
Benjamin> -PIDFILE=/var/run/multipathd.pid
Benjamin> +PIDFILE=/run/multipathd.pid
Benjamin>  MPATH_INIT_TIMEOUT=10
Benjamin>  ARGS=""
 
Benjamin> -- 
Benjamin> 1.8.3.1

Benjamin> --
Benjamin> dm-devel mailing list
Benjamin> dm-devel@redhat.com
Benjamin> https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 10/17] multipathd: delay reloads during creation
  2016-03-29  3:13 ` [PATCH 10/17] multipathd: delay reloads during creation Benjamin Marzinski
@ 2016-03-29 14:02   ` John Stoffel
  2016-03-30  0:57     ` Benjamin Marzinski
  0 siblings, 1 reply; 24+ messages in thread
From: John Stoffel @ 2016-03-29 14:02 UTC (permalink / raw)
  To: Benjamin Marzinski; +Cc: device-mapper development, Christophe Varoqui

>>>>> "Benjamin" == Benjamin Marzinski <bmarzins@redhat.com> writes:

Benjamin> lvm needs PV devices to not be suspended while the udev
Benjamin> rules are running, for them to be correctly identified as
Benjamin> PVs. However, multipathd will often be in a situation where
Benjamin> it will create a multipath device upon seeing a path, and
Benjamin> then immediately reload the device upon seeing another path.
Benjamin> If multipath is reloading a device while processing the udev
Benjamin> event from its creation, lvm can fail to identify it as a
Benjamin> PV. This can cause systems to fail to boot. Unfortunately,
Benjamin> using udev synchronization cookies to solve this issue would
Benjamin> cause a host of other issues that could only be avoided by a
Benjamin> pretty substantial change in how multipathd does locking and
Benjamin> event processing. The good news is that multipathd is
Benjamin> already listening to udev events itself, and can make sure
Benjamin> that it isn't reloading when it shouldn't be.

Benjamin> This patch makes multipathd delay or refuse any reloads that
Benjamin> would happen between the time when it creates a device, and
Benjamin> when it receives the change uevent from the device
Benjamin> creation. The only reloads that it refuses are from the
Benjamin> multipathd interactive commands that make no sense on a not
Benjamin> fully started device.  Otherwise, it processes the event or
Benjamin> command, and sets a flag to either mark that device for an
Benjamin> update, or to signal that multipathd needs a
Benjamin> reconfigure. When the udev event for the creation arrives,
Benjamin> multipath will reload the device if necessary. If a
Benjamin> reconfigure has been requested, and no devices are currently
Benjamin> being created, multipathd will also do the reconfigure then.

Benjamin> Also this patch adds a configurable timer
Benjamin> "missing_uev_msg_delay" defaulting to 30 seconds. If the
Benjamin> udev creation event has not arrived after this timeout has
Benjamin> triggered, multipathd will start printing messages alerting
Benjamin> the user of this every "missing_uev_msg_delay" seconds.

Should this really keep printing this message every 30 seconds for
eternity?  I would think that having it give up after 30 * N seconds
would be better instead.  I'm worried that this might block or slow
down system boots forever, instead of at least failing and falling
through so that maybe something can be recovered here.

Basically, what can the user do if they start getting these messages?
We should prompt them with a possible cause/solution if at all
possible.

John

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 01/17] multipathd: use /run instead of /var/run
  2016-03-29 13:57   ` John Stoffel
@ 2016-03-30  0:41     ` Benjamin Marzinski
  2016-03-30 16:06       ` John Stoffel
  0 siblings, 1 reply; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-30  0:41 UTC (permalink / raw)
  To: John Stoffel; +Cc: device-mapper development, Christophe Varoqui

On Tue, Mar 29, 2016 at 09:57:25AM -0400, John Stoffel wrote:
> 
> Benjamin> /var/run is now usually a symlink to /run.  If /var is on a separate
> Benjamin> filesytem, when multipathd starts up, it might end up writing to
> Benjamin> /var/run before the /var filesytem is mounted and thus not have its
> Benjamin> pidfile accessible at /var/run afterwards.  On most distrubutions /run
> Benjamin> is now a tmpfs and should always be available before multipathd is
> Benjamin> started, so multipath should just write there directly, instead of
> Benjamin> through the symlink.
> 
> I'm not sure I agree with this, it really smells more like a
> distribution problem, than a multipathd problem.  If multipathd starts
> up in an initramfs, how does it get reset to the proper /var/run or
> /run directory then?

Huh? /run is a tmpfs on almost all distros now. It gets remounted
when you switch the root from the initramfs, but it doesn't go away. So
there isn't a "proper" /run that appears later. /var/run just gets
symlinked to it. 

> And especially if it's a symlink, we shouldn't care at all either.

But /var/run isn't a symlink before /var is mounted.  That's the problem
that this is supposed to fix.

> What distros are using /run instead of /var/run these days?

Debian, Suse, Ubuntu, Fedora and some others. Pretty much everyone who
is using systemd made or is planning to make this switch. See
https://lists.fedoraproject.org/pipermail/devel/2011-March/150031.html

We could call it a distribution problem, but it will be a problem that
almost all distributions will face. To turn your question around, which
distribution do you think will be negatively impacted by this?

-Ben

> 
> John
> 
> 
> Benjamin> If /var/run is not a symlink, continue using it.
> 
> Benjamin> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> Benjamin> ---
> Benjamin>  Makefile.inc                    | 10 +++++++++-
> Benjamin>  libmultipath/defaults.h         |  2 +-
> Benjamin>  multipathd/multipathd.init.suse |  2 +-
> Benjamin>  3 files changed, 11 insertions(+), 3 deletions(-)
> 
> Benjamin> diff --git a/Makefile.inc b/Makefile.inc
> Benjamin> index c3ed73f..357dd33 100644
> Benjamin> --- a/Makefile.inc
> Benjamin> +++ b/Makefile.inc
> Benjamin> @@ -21,6 +21,14 @@ ifndef LIB
> Benjamin>  	endif
> Benjamin>  endif
>  
> Benjamin> +ifndef RUN
> Benjamin> +	ifeq ($(shell test -L /var/run -o ! -d /var/run && echo 1),1)
> Benjamin> +		RUN=run
> Benjamin> +	else
> Benjamin> +		RUN=var/run
> Benjamin> +	endif
> Benjamin> +endif
> Benjamin> +
> Benjamin>  ifndef SYSTEMD
> Benjamin>  	ifeq ($(shell systemctl --version > /dev/null 2>&1 && echo 1), 1)
> Benjamin>  		SYSTEMD = $(shell systemctl --version 2> /dev/null |  sed -n 's/systemd \([0-9]*\)/\1/p')
> Benjamin> @@ -54,7 +62,7 @@ ifndef RPM_OPT_FLAGS
> Benjamin>  endif
>  
> Benjamin>  OPTFLAGS     = $(RPM_OPT_FLAGS) -Wunused -Wstrict-prototypes
> Benjamin> -CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\"
> Benjamin> +CFLAGS	     = $(OPTFLAGS) -fPIC -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
> Benjamin>  SHARED_FLAGS = -shared
>  
> Benjamin>  %.o:	%.c
> Benjamin> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> Benjamin> index 8902f40..cf1d5be 100644
> Benjamin> --- a/libmultipath/defaults.h
> Benjamin> +++ b/libmultipath/defaults.h
> Benjamin> @@ -26,7 +26,7 @@
> Benjamin>  #define MAX_CHECKINT(a)		(a << 2)
>  
> Benjamin>  #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
> Benjamin> -#define DEFAULT_PIDFILE		"/var/run/multipathd.pid"
> Benjamin> +#define DEFAULT_PIDFILE		"/" RUN_DIR "/multipathd.pid"
> Benjamin>  #define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
> Benjamin>  #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
> Benjamin>  #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
> Benjamin> diff --git a/multipathd/multipathd.init.suse b/multipathd/multipathd.init.suse
> Benjamin> index d1319b1..ed699fa 100644
> Benjamin> --- a/multipathd/multipathd.init.suse
> Benjamin> +++ b/multipathd/multipathd.init.suse
> Benjamin> @@ -17,7 +17,7 @@
>  
> Benjamin>  PATH=/bin:/usr/bin:/sbin:/usr/sbin
> Benjamin>  DAEMON=/sbin/multipathd
> Benjamin> -PIDFILE=/var/run/multipathd.pid
> Benjamin> +PIDFILE=/run/multipathd.pid
> Benjamin>  MPATH_INIT_TIMEOUT=10
> Benjamin>  ARGS=""
>  
> Benjamin> -- 
> Benjamin> 1.8.3.1
> 
> Benjamin> --
> Benjamin> dm-devel mailing list
> Benjamin> dm-devel@redhat.com
> Benjamin> https://www.redhat.com/mailman/listinfo/dm-devel

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 10/17] multipathd: delay reloads during creation
  2016-03-29 14:02   ` John Stoffel
@ 2016-03-30  0:57     ` Benjamin Marzinski
  0 siblings, 0 replies; 24+ messages in thread
From: Benjamin Marzinski @ 2016-03-30  0:57 UTC (permalink / raw)
  To: John Stoffel; +Cc: device-mapper development, Christophe Varoqui

On Tue, Mar 29, 2016 at 10:02:41AM -0400, John Stoffel wrote:
> >>>>> "Benjamin" == Benjamin Marzinski <bmarzins@redhat.com> writes:
> 
> Benjamin> lvm needs PV devices to not be suspended while the udev
> Benjamin> rules are running, for them to be correctly identified as
> Benjamin> PVs. However, multipathd will often be in a situation where
> Benjamin> it will create a multipath device upon seeing a path, and
> Benjamin> then immediately reload the device upon seeing another path.
> Benjamin> If multipath is reloading a device while processing the udev
> Benjamin> event from its creation, lvm can fail to identify it as a
> Benjamin> PV. This can cause systems to fail to boot. Unfortunately,
> Benjamin> using udev synchronization cookies to solve this issue would
> Benjamin> cause a host of other issues that could only be avoided by a
> Benjamin> pretty substantial change in how multipathd does locking and
> Benjamin> event processing. The good news is that multipathd is
> Benjamin> already listening to udev events itself, and can make sure
> Benjamin> that it isn't reloading when it shouldn't be.
> 
> Benjamin> This patch makes multipathd delay or refuse any reloads that
> Benjamin> would happen between the time when it creates a device, and
> Benjamin> when it receives the change uevent from the device
> Benjamin> creation. The only reloads that it refuses are from the
> Benjamin> multipathd interactive commands that make no sense on a not
> Benjamin> fully started device.  Otherwise, it processes the event or
> Benjamin> command, and sets a flag to either mark that device for an
> Benjamin> update, or to signal that multipathd needs a
> Benjamin> reconfigure. When the udev event for the creation arrives,
> Benjamin> multipath will reload the device if necessary. If a
> Benjamin> reconfigure has been requested, and no devices are currently
> Benjamin> being created, multipathd will also do the reconfigure then.
> 
> Benjamin> Also this patch adds a configurable timer
> Benjamin> "missing_uev_msg_delay" defaulting to 30 seconds. If the
> Benjamin> udev creation event has not arrived after this timeout has
> Benjamin> triggered, multipathd will start printing messages alerting
> Benjamin> the user of this every "missing_uev_msg_delay" seconds.
> 
> Should this really keep printing this message every 30 seconds for
> eternity?  I would think that having it give up after 30 * N seconds
> would be better instead.  I'm worried that this might block or slow
> down system boots forever, instead of at least failing and falling
> through so that maybe something can be recovered here.

Fair enough. I should probably lower that timer, and after (timer *
some_max_retries) seconds have passed, just stop waiting, and let the
reloads go ahead.  However, as this is, it isn't too likely to interfere
with bootup. It won't do anything to stop the multipath devices from
being created, just reloaded.

> Basically, what can the user do if they start getting these messages?
> We should prompt them with a possible cause/solution if at all
> possible.

All that the user could do would be to override the waiting, which is
just what multipathd could do automatically. If we've already waited a
while for the event and haven't received it, we're unlikely to have it
come through while we are reloading.  Also, the risk is the same for a
person manually fixing it and for multipathd automatically doing it, so
it seems like multipath should just automatically just stop waiting in
this case.

-Ben

> John

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [PATCH 01/17] multipathd: use /run instead of /var/run
  2016-03-30  0:41     ` Benjamin Marzinski
@ 2016-03-30 16:06       ` John Stoffel
  0 siblings, 0 replies; 24+ messages in thread
From: John Stoffel @ 2016-03-30 16:06 UTC (permalink / raw)
  To: Benjamin Marzinski
  Cc: device-mapper development, John Stoffel, Christophe Varoqui

>>>>> "Benjamin" == Benjamin Marzinski <bmarzins@redhat.com> writes:

Benjamin> On Tue, Mar 29, 2016 at 09:57:25AM -0400, John Stoffel wrote:
>> 
Benjamin> /var/run is now usually a symlink to /run.  If /var is on a separate
Benjamin> filesytem, when multipathd starts up, it might end up writing to
Benjamin> /var/run before the /var filesytem is mounted and thus not have its
Benjamin> pidfile accessible at /var/run afterwards.  On most distrubutions /run
Benjamin> is now a tmpfs and should always be available before multipathd is
Benjamin> started, so multipath should just write there directly, instead of
Benjamin> through the symlink.
>> 
>> I'm not sure I agree with this, it really smells more like a
>> distribution problem, than a multipathd problem.  If multipathd starts
>> up in an initramfs, how does it get reset to the proper /var/run or
>> /run directory then?

Benjamin> Huh? /run is a tmpfs on almost all distros now. It gets
Benjamin> remounted when you switch the root from the initramfs, but
Benjamin> it doesn't go away. So there isn't a "proper" /run that
Benjamin> appears later. /var/run just gets symlinked to it.

Gah!  I apologize for the noise, I did a quick check on some RHEL5/6
systems (which are ancient I agree!) and didn't look at newer
debian/mint systems as well.  And I do see that /run is there.  

John

^ permalink raw reply	[flat|nested] 24+ messages in thread

* multipathd: segfault in multipathd cli_add_map()
  2016-03-29  3:13 ` [PATCH 17/17] multipathd: use 64-bit int for command key Benjamin Marzinski
@ 2016-04-07  2:10   ` Zhangguanghui
  0 siblings, 0 replies; 24+ messages in thread
From: Zhangguanghui @ 2016-04-07  2:10 UTC (permalink / raw)
  To: dm-devel-bounces, dm-devel; +Cc: Christophe Varoqui


[-- Attachment #1.1: Type: text/plain, Size: 1903 bytes --]

--- multipath-tools-ecf84a8/multipathd/cli_handlers.c 2016-04-07 09:28:46.693622941 +0800
+++ patch/cli_handlers.c 2016-04-07 09:43:36.237622835 +0800
@@ -523,7 +523,8 @@
char * param = get_keyparam(v, MAP);
int major, minor;
char dev_path[PATH_SIZE];
- char *alias, *refwwid;
+ char *alias = NULL;
+ char *refwwid = NULL;
int rc, count = 0;

param = convert_dev(param, 0);
@@ -565,8 +566,10 @@
return 1;
}
rc = ev_add_map(dev_path, alias, vecs);
- FREE(alias);
- FREE(refwwid);
+ if (alias)
+ FREE(alias);
+ if (refwwid)
+ FREE(refwwid);
return rc;
}

@@ -577,7 +580,7 @@
char * param = get_keyparam(v, MAP);
int major, minor;
char dev_path[PATH_SIZE];
- char *alias;
+ char *alias = NULL;
int rc;

param = convert_dev(param, 0);
@@ -600,7 +603,8 @@
return 0;
}
rc = ev_remove_map(param, alias, minor, vecs);
- FREE(alias);
+ if (alias)
+ FREE(alias);
return rc;
}


-------------------------------------------------------------------------------------------------------------------------------------
本邮件及其附件含有杭州华三通信技术有限公司的保密信息,仅限于发送给上面地址中列出
的个人或群组。禁止任何其他人以任何形式使用(包括但不限于全部或部分地泄露、复制、
或散发)本邮件中的信息。如果您错收了本邮件,请您立即电话或邮件通知发件人并删除本
邮件!
This e-mail and its attachments contain confidential information from H3C, which is
intended only for the person or entity whose address is listed above. Any use of the
information contained herein in any way (including, but not limited to, total or partial
disclosure, reproduction, or dissemination) by persons other than the intended
recipient(s) is prohibited. If you receive this e-mail in error, please notify the sender
by phone or email immediately and delete it!

[-- Attachment #1.2: Type: text/html, Size: 3176 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2016-04-07  2:10 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-29  3:12 [PATCH 00/17] Multipath patch sync Benjamin Marzinski
2016-03-29  3:12 ` [PATCH 01/17] multipathd: use /run instead of /var/run Benjamin Marzinski
2016-03-29 13:57   ` John Stoffel
2016-03-30  0:41     ` Benjamin Marzinski
2016-03-30 16:06       ` John Stoffel
2016-03-29  3:12 ` [PATCH 02/17] retrigger uevents to try and get the uid through udev Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 03/17] Fix issues with user_friendly_names initramfs bindings Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 04/17] Add libmpathcmd library and use it internally Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 05/17] libmultipath: add ignore_new_boot_devs option Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 06/17] libmultipath: fix PAD and PRINT macros Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 07/17] libmultipath: Cut down on alua prioritizer ioctls Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 08/17] multipathd: fail if pidfile can't be created Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 09/17] libmultipath: check correct function for define Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 10/17] multipathd: delay reloads during creation Benjamin Marzinski
2016-03-29 14:02   ` John Stoffel
2016-03-30  0:57     ` Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 11/17] multipath: Fix minor text issues Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 12/17] kpartx: verify partition devices Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 13/17] multipath: add exclusive_pref_bit for alua prio Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 14/17] multipathd: print "fail" when remove fails Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 15/17] multipath: check partitions unused before removing Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 16/17] multipathd.service: remove blk-availability Requires Benjamin Marzinski
2016-03-29  3:13 ` [PATCH 17/17] multipathd: use 64-bit int for command key Benjamin Marzinski
2016-04-07  2:10   ` multipathd: segfault in multipathd cli_add_map() Zhangguanghui

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.