[PATCH 00/18] multipath patch sync

[PATCH 00/18] multipath patch sync

[Benjamin Marzinski]

Here are a number of my patches that haven't been applied upstream yet.
All of them except the last four are just resends of patches I've
submitted earlier. Out of the last four, only the very last is a new
feature, which was discussed a couple of months ago.

Benjamin Marzinski (18):
  libmultipath: Add prioritizer context data
  fix memory leaks on realloc failures
  multipathd: use /run instead of /var/run
  retrigger uevents to try and get the uid through udev
  update multipath rules to deal with partition devices
  change order of multipath.rules
  make kpartx -d remove all partitions
  Fix issues with user_friendly_names initramfs bindings
  Make multipath deactivate devices before iscsi shutdown
  resize reply buffer for mutipathd help message
  Add libmpathcmd library and use it internally
  add raw format multipathd commands
  libmultipath: add ignore_new_boot_devs option
  Make use of /run depend on systemd
  libmpathpersist: uninstall man page correctly
  Increase host buffer size
  Fix sun partition numbering
  Make multipath ignore devices without mpath prefix

 Makefile                                 |   1 +
 Makefile.inc                             |   2 +
 kpartx/kpartx.c                          |   4 +-
 kpartx/sun.c                             |   2 -
 libmpathcmd/Makefile                     |  30 ++++++
 libmpathcmd/mpath_cmd.c                  | 178 +++++++++++++++++++++++++++++++
 libmpathcmd/mpath_cmd.h                  | 125 ++++++++++++++++++++++
 libmpathpersist/Makefile                 |  13 +--
 libmpathpersist/mpath_persist.c          |   4 +-
 libmpathpersist/mpath_updatepr.c         |  14 +--
 libmultipath/Makefile                    |   3 +-
 libmultipath/config.c                    |  10 +-
 libmultipath/config.h                    |   4 +
 libmultipath/configure.c                 |  16 +--
 libmultipath/defaults.h                  |  10 +-
 libmultipath/devmapper.c                 |  67 ++++++++----
 libmultipath/devmapper.h                 |   1 +
 libmultipath/dict.c                      |  24 ++++-
 libmultipath/discovery.c                 |  28 ++---
 libmultipath/dmparser.c                  |   6 +-
 libmultipath/print.c                     |  25 +++--
 libmultipath/print.h                     |   4 +-
 libmultipath/prio.c                      |  34 +++++-
 libmultipath/prio.h                      |   7 ++
 libmultipath/prioritizers/alua.c         |  58 +++++++---
 libmultipath/prioritizers/alua_rtpg.c    |  22 +++-
 libmultipath/prioritizers/alua_rtpg.h    |   4 +-
 libmultipath/prioritizers/const.c        |   4 +
 libmultipath/prioritizers/datacore.c     |   3 +
 libmultipath/prioritizers/def_func.h     |  11 ++
 libmultipath/prioritizers/emc.c          |   4 +
 libmultipath/prioritizers/hds.c          |   4 +
 libmultipath/prioritizers/hp_sw.c        |   4 +
 libmultipath/prioritizers/iet.c          |   4 +
 libmultipath/prioritizers/ontap.c        |   4 +
 libmultipath/prioritizers/random.c       |   4 +
 libmultipath/prioritizers/rdac.c         |   4 +
 libmultipath/prioritizers/weightedpath.c |   3 +
 libmultipath/propsel.c                   |   4 +-
 libmultipath/structs.h                   |  10 +-
 libmultipath/util.c                      |  30 ++++++
 libmultipath/util.h                      |   1 +
 libmultipath/uxsock.c                    |  73 +++----------
 libmultipath/uxsock.h                    |   5 +-
 libmultipath/wwids.c                     |  21 ++--
 mpathpersist/Makefile                    |   2 +-
 multipath/Makefile                       |   9 +-
 multipath/main.c                         |  14 ++-
 multipath/multipath.rules                |  16 ++-
 multipathd/Makefile                      |   5 +-
 multipathd/cli.c                         |  93 +++++++++++-----
 multipathd/cli.h                         |  22 +++-
 multipathd/cli_handlers.c                | 108 +++++++++----------
 multipathd/cli_handlers.h                |   2 +
 multipathd/main.c                        |  57 ++++++----
 multipathd/multipathd.init.suse          |   2 +-
 multipathd/multipathd.service            |   1 +
 multipathd/uxclnt.c                      |  13 ++-
 multipathd/uxlsnr.c                      |  30 ++++--
 59 files changed, 951 insertions(+), 312 deletions(-)
 create mode 100644 libmpathcmd/Makefile
 create mode 100644 libmpathcmd/mpath_cmd.c
 create mode 100644 libmpathcmd/mpath_cmd.h
 create mode 100644 libmultipath/prioritizers/def_func.h

-- 
1.8.3.1

[PATCH 01/18] libmultipath: Add prioritizer context data

[Benjamin Marzinski]

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, I've added a context pointer to the prio structure,
similar to what exists for the checker structure, and initprio() and
freeprio() functions to the prioritizers. The only one that currently uses
these is the alua prioritizer. It caches the type of alua support, the TPG
id, and the necessary buffer size.  The only thing I'm worried about with
this patch is whether the first two values could change.  In order to deal
with that possibility, whenever a path gets a change event, or becomes
valid again after a failure, it resets the context structure values, which
forces all of them to get checked the next time the prioritizer is called.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/prio.c                      | 34 ++++++++++++++++++-
 libmultipath/prio.h                      |  7 ++++
 libmultipath/prioritizers/alua.c         | 58 +++++++++++++++++++++++++-------
 libmultipath/prioritizers/alua_rtpg.c    | 22 +++++++++---
 libmultipath/prioritizers/alua_rtpg.h    |  4 +--
 libmultipath/prioritizers/const.c        |  4 +++
 libmultipath/prioritizers/datacore.c     |  3 ++
 libmultipath/prioritizers/def_func.h     | 11 ++++++
 libmultipath/prioritizers/emc.c          |  4 +++
 libmultipath/prioritizers/hds.c          |  4 +++
 libmultipath/prioritizers/hp_sw.c        |  4 +++
 libmultipath/prioritizers/iet.c          |  4 +++
 libmultipath/prioritizers/ontap.c        |  4 +++
 libmultipath/prioritizers/random.c       |  4 +++
 libmultipath/prioritizers/rdac.c         |  4 +++
 libmultipath/prioritizers/weightedpath.c |  3 ++
 libmultipath/propsel.c                   |  4 +--
 multipathd/main.c                        | 24 ++++++++-----
 18 files changed, 171 insertions(+), 31 deletions(-)
 create mode 100644 libmultipath/prioritizers/def_func.h

diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index ab8eca9..c8fe252 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -115,9 +115,24 @@ struct prio * add_prio (char * name)
 	p->getprio = (int (*)(struct path *, char *)) dlsym(p->handle, "getprio");
 	errstr = dlerror();
 	if (errstr != NULL)
-		condlog(0, "A dynamic linking error occurred: (%s)", errstr);
+		condlog(0, "A dynamic linking error occurred with getprio: (%s)", errstr);
 	if (!p->getprio)
 		goto out;
+
+	p->initprio = (int (*)(struct prio *)) dlsym(p->handle, "initprio");
+	errstr = dlerror();
+	if (errstr != NULL)
+		condlog(0, "A dynamic linking error occurred with initprio: (%s)", errstr);
+	if (!p->initprio)
+		goto out;
+
+	p->freeprio = (int (*)(struct prio *)) dlsym(p->handle, "freeprio");
+	errstr = dlerror();
+	if (errstr != NULL)
+		condlog(0, "A dynamic linking error occurred with freeprio: (%s)", errstr);
+	if (!p->freeprio)
+		goto out;
+
 	list_add(&p->node, &prioritizers);
 	return p;
 out:
@@ -125,6 +140,13 @@ out:
 	return NULL;
 }
 
+int prio_init (struct prio * p)
+{
+	if (!p || !p->initprio)
+		return 1;
+	return p->initprio(p);
+}
+
 int prio_getprio (struct prio * p, struct path * pp)
 {
 	return p->getprio(pp, p->args);
@@ -159,8 +181,16 @@ void prio_get (struct prio * dst, char * name, char * args)
 	strncpy(dst->name, src->name, PRIO_NAME_LEN);
 	if (args)
 		strncpy(dst->args, args, PRIO_ARGS_LEN);
+	dst->initprio = src->initprio;
 	dst->getprio = src->getprio;
+	dst->freeprio = src->freeprio;
 	dst->handle = NULL;
+	dst->context = NULL;
+
+	if (dst->initprio(dst) != 0){
+		memset(dst, 0x0, sizeof(struct prio));
+		return;
+	}
 
 	src->refcount++;
 }
@@ -173,6 +203,8 @@ void prio_put (struct prio * dst)
 		return;
 
 	src = prio_lookup(dst->name);
+	if (dst->freeprio)
+		dst->freeprio(dst);
 	memset(dst, 0x0, sizeof(struct prio));
 	free_prio(src);
 }
diff --git a/libmultipath/prio.h b/libmultipath/prio.h
index 495688f..6d57ecd 100644
--- a/libmultipath/prio.h
+++ b/libmultipath/prio.h
@@ -46,9 +46,15 @@ struct prio {
 	void *handle;
 	int refcount;
 	struct list_head node;
+	void * context;
 	char name[PRIO_NAME_LEN];
 	char args[PRIO_ARGS_LEN];
+	int (*initprio)(struct prio * p);
+	/* You are allowed to call initprio multiple times without calling
+	 * freeprio. Doing so will reinitialize it (possibly skipping
+	 * allocations) */
 	int (*getprio)(struct path *, char *);
+	int (*freeprio)(struct prio * p);
 };
 
 unsigned int get_prio_timeout(unsigned int default_timeout);
@@ -57,6 +63,7 @@ void cleanup_prio (void);
 struct prio * add_prio (char *);
 struct prio * prio_lookup (char *);
 int prio_getprio (struct prio *, struct path *);
+int prio_init (struct prio *);
 void prio_get (struct prio *, char *, char *);
 void prio_put (struct prio *);
 int prio_selected (struct prio *);
diff --git a/libmultipath/prioritizers/alua.c b/libmultipath/prioritizers/alua.c
index 39ed1c8..7691156 100644
--- a/libmultipath/prioritizers/alua.c
+++ b/libmultipath/prioritizers/alua.c
@@ -37,6 +37,12 @@ static const char * aas_string[] = {
 	[AAS_TRANSITIONING]	= "transitioning between states",
 };
 
+struct alua_context {
+	int tpg_support;
+	int tpg;
+	int buflen;
+};
+
 static const char *aas_print_string(int rc)
 {
 	rc &= 0x7f;
@@ -51,24 +57,25 @@ static const char *aas_print_string(int rc)
 }
 
 int
-get_alua_info(int fd)
+get_alua_info(int fd, struct alua_context *ct)
 {
 	int	rc;
-	int	tpg;
 
-	rc = get_target_port_group_support(fd);
-	if (rc < 0)
-		return -ALUA_PRIO_TPGS_FAILED;
+	if (ct->tpg_support <= 0 || ct->tpg < 0) {
+		ct->tpg_support = get_target_port_group_support(fd);
+		if (ct->tpg_support < 0)
+			return -ALUA_PRIO_TPGS_FAILED;
 
-	if (rc == TPGS_NONE)
-		return -ALUA_PRIO_NOT_SUPPORTED;
+		if (ct->tpg_support == TPGS_NONE)
+			return -ALUA_PRIO_NOT_SUPPORTED;
 
-	tpg = get_target_port_group(fd);
-	if (tpg < 0)
-		return -ALUA_PRIO_RTPG_FAILED;
+		ct->tpg = get_target_port_group(fd, &ct->buflen);
+		if (ct->tpg < 0)
+			return -ALUA_PRIO_RTPG_FAILED;
+	}
 
-	condlog(3, "reported target port group is %i", tpg);
-	rc = get_asymmetric_access_state(fd, tpg);
+	condlog(3, "reported target port group is %i", ct->tpg);
+	rc = get_asymmetric_access_state(fd, ct->tpg, &ct->buflen);
 	if (rc < 0)
 		return -ALUA_PRIO_GETAAS_FAILED;
 
@@ -86,7 +93,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->fd, pp->prio.context);
 	if (rc >= 0) {
 		aas = (rc & 0x0f);
 		priopath = (rc & 0x80);
@@ -126,3 +133,28 @@ int getprio (struct path * pp, char * args)
 	}
 	return rc;
 }
+
+int initprio(struct prio *p)
+{
+	if (!p->context) {
+		struct alua_context *ct;
+
+		ct = malloc(sizeof(struct alua_context));
+		if (!ct)
+			return 1;
+		p->context = ct;
+	}
+	memset(p->context, 0, sizeof(struct alua_context));
+	return 0;
+}
+
+
+int freeprio(struct prio *p)
+{
+	if (p->context) {
+		free(p->context);
+		p->context = NULL;
+	}
+	return 0;
+}
+
diff --git a/libmultipath/prioritizers/alua_rtpg.c b/libmultipath/prioritizers/alua_rtpg.c
index 6d04fc1..7ef2ef5 100644
--- a/libmultipath/prioritizers/alua_rtpg.c
+++ b/libmultipath/prioritizers/alua_rtpg.c
@@ -171,7 +171,7 @@ get_target_port_group_support(int fd)
 }
 
 int
-get_target_port_group(int fd)
+get_target_port_group(int fd, int *buflen_ptr)
 {
 	unsigned char		*buf;
 	struct vpd83_data *	vpd83;
@@ -179,7 +179,12 @@ get_target_port_group(int fd)
 	int			rc;
 	int			buflen, scsi_buflen;
 
-	buflen = 128; /* Lets start from 128 */
+	if (!buflen_ptr || *buflen_ptr == 0) {
+		buflen = 128; /* Lets start from 128 */
+		if (buflen_ptr)
+			*buflen_ptr = 128;
+	} else
+		buflen = *buflen_ptr;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG("malloc failed: could not allocate"
@@ -202,6 +207,8 @@ get_target_port_group(int fd)
 			return -RTPG_RTPG_FAILED;
 		}
 		buflen = scsi_buflen;
+		if (buflen_ptr)
+			*buflen_ptr = buflen;
 		memset(buf, 0, buflen);
 		rc = do_inquiry(fd, 1, 0x83, buf, buflen);
 		if (rc < 0)
@@ -269,7 +276,7 @@ do_rtpg(int fd, void* resp, long resplen)
 }
 
 int
-get_asymmetric_access_state(int fd, unsigned int tpg)
+get_asymmetric_access_state(int fd, unsigned int tpg, int *buflen_ptr)
 {
 	unsigned char		*buf;
 	struct rtpg_data *	tpgd;
@@ -278,7 +285,12 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
 	int			buflen;
 	uint32_t		scsi_buflen;
 
-	buflen = 128; /* Initial value from old code */
+	if (!buflen_ptr || *buflen_ptr == 0) {
+		buflen = 128; /* Initial value from old code */
+		if (buflen_ptr)
+			*buflen_ptr = 128;
+	} else
+		buflen = *buflen_ptr;
 	buf = (unsigned char *)malloc(buflen);
 	if (!buf) {
 		PRINT_DEBUG ("malloc failed: could not allocate"
@@ -299,6 +311,8 @@ get_asymmetric_access_state(int fd, unsigned int tpg)
 			return -RTPG_RTPG_FAILED;
 		}
 		buflen = scsi_buflen;
+		if (buflen_ptr)
+			*buflen_ptr = buflen;
 		memset(buf, 0, buflen);
 		rc = do_rtpg(fd, buf, buflen);
 		if (rc < 0)
diff --git a/libmultipath/prioritizers/alua_rtpg.h b/libmultipath/prioritizers/alua_rtpg.h
index c43e0a9..cade64d 100644
--- a/libmultipath/prioritizers/alua_rtpg.h
+++ b/libmultipath/prioritizers/alua_rtpg.h
@@ -23,8 +23,8 @@
 #define RTPG_TPG_NOT_FOUND			4
 
 int get_target_port_group_support(int fd);
-int get_target_port_group(int fd);
-int get_asymmetric_access_state(int fd, unsigned int tpg);
+int get_target_port_group(int fd, int *buflen_ptr);
+int get_asymmetric_access_state(int fd, unsigned int tpg, int *buflen_ptr);
 
 #endif /* __RTPG_H__ */
 
diff --git a/libmultipath/prioritizers/const.c b/libmultipath/prioritizers/const.c
index bf689cd..362d4e1 100644
--- a/libmultipath/prioritizers/const.c
+++ b/libmultipath/prioritizers/const.c
@@ -1,8 +1,12 @@
 #include <stdio.h>
 
 #include <prio.h>
+#include "def_func.h"
 
 int getprio (struct path * pp, char * args)
 {
 	return 1;
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/datacore.c b/libmultipath/prioritizers/datacore.c
index e3e6a51..c2f0d0b 100644
--- a/libmultipath/prioritizers/datacore.c
+++ b/libmultipath/prioritizers/datacore.c
@@ -25,6 +25,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define INQ_REPLY_LEN 255
 #define INQ_CMD_CODE 0x12
@@ -111,3 +112,5 @@ int getprio (struct path * pp, char * args)
         return datacore_prio(pp->dev, pp->fd, args);
 }
 
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/def_func.h b/libmultipath/prioritizers/def_func.h
new file mode 100644
index 0000000..8976b3f
--- /dev/null
+++ b/libmultipath/prioritizers/def_func.h
@@ -0,0 +1,11 @@
+#ifndef _DEF_FUNC_H
+#define _DEF_FUNC_H
+
+#include "prio.h"
+
+#define declare_nop_prio(name)						\
+int name (struct prio *p)						\
+{									\
+	return 0;							\
+}
+#endif /* _DEF_FUNC_H */
diff --git a/libmultipath/prioritizers/emc.c b/libmultipath/prioritizers/emc.c
index e49809c..448528f 100644
--- a/libmultipath/prioritizers/emc.c
+++ b/libmultipath/prioritizers/emc.c
@@ -6,6 +6,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define INQUIRY_CMD     0x12
 #define INQUIRY_CMDLEN  6
@@ -85,3 +86,6 @@ int getprio (struct path * pp, char * args)
 {
 	return emc_clariion_prio(pp->dev, pp->fd);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/hds.c b/libmultipath/prioritizers/hds.c
index 8043b5b..292fe1c 100644
--- a/libmultipath/prioritizers/hds.c
+++ b/libmultipath/prioritizers/hds.c
@@ -76,6 +76,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define INQ_REPLY_LEN 255
 #define INQ_CMD_CODE 0x12
@@ -170,3 +171,6 @@ int getprio (struct path * pp, char * args)
 {
 	return hds_modular_prio(pp->dev, pp->fd);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/hp_sw.c b/libmultipath/prioritizers/hp_sw.c
index 4950cf7..371d766 100644
--- a/libmultipath/prioritizers/hp_sw.c
+++ b/libmultipath/prioritizers/hp_sw.c
@@ -16,6 +16,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define TUR_CMD_LEN		6
 #define SCSI_CHECK_CONDITION	0x2
@@ -99,3 +100,6 @@ int getprio (struct path * pp, char * args)
 {
 	return hp_sw_prio(pp->dev, pp->fd);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/iet.c b/libmultipath/prioritizers/iet.c
index 0bcc48b..a6b73d5 100644
--- a/libmultipath/prioritizers/iet.c
+++ b/libmultipath/prioritizers/iet.c
@@ -9,6 +9,7 @@
 #include <debug.h>
 #include <unistd.h>
 #include <structs.h>
+#include "def_func.h"
 
 //
 // This prioritizer suits iSCSI needs, makes it possible to prefer one path.
@@ -141,3 +142,6 @@ int getprio(struct path * pp, char * args)
 {
 	return iet_prio(pp->dev, args);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/ontap.c b/libmultipath/prioritizers/ontap.c
index 5e82a17..c556e3b 100644
--- a/libmultipath/prioritizers/ontap.c
+++ b/libmultipath/prioritizers/ontap.c
@@ -23,6 +23,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define INQUIRY_CMD	0x12
 #define INQUIRY_CMDLEN	6
@@ -245,3 +246,6 @@ int getprio (struct path * pp, char * args)
 {
 	return ontap_prio(pp->dev, pp->fd);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/random.c b/libmultipath/prioritizers/random.c
index 281a0d1..c38b0f6 100644
--- a/libmultipath/prioritizers/random.c
+++ b/libmultipath/prioritizers/random.c
@@ -4,6 +4,7 @@
 #include <time.h>
 
 #include <prio.h>
+#include "def_func.h"
 
 int getprio (struct path * pp, char * args)
 {
@@ -13,3 +14,6 @@ int getprio (struct path * pp, char * args)
 	srand((unsigned int)tv.tv_usec);
 	return 1+(int) (10.0*rand()/(RAND_MAX+1.0));
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/rdac.c b/libmultipath/prioritizers/rdac.c
index a210055..1fc302f 100644
--- a/libmultipath/prioritizers/rdac.c
+++ b/libmultipath/prioritizers/rdac.c
@@ -6,6 +6,7 @@
 #include <debug.h>
 #include <prio.h>
 #include <structs.h>
+#include "def_func.h"
 
 #define INQUIRY_CMD     0x12
 #define INQUIRY_CMDLEN  6
@@ -95,3 +96,6 @@ int getprio (struct path * pp, char * args)
 {
 	return rdac_prio(pp->dev, pp->fd);
 }
+
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/prioritizers/weightedpath.c b/libmultipath/prioritizers/weightedpath.c
index 13bc52f..1c3f821 100644
--- a/libmultipath/prioritizers/weightedpath.c
+++ b/libmultipath/prioritizers/weightedpath.c
@@ -32,6 +32,7 @@
 #include <memory.h>
 #include <debug.h>
 #include <regex.h>
+#include "def_func.h"
 
 char *get_next_string(char **temp, char *split_char)
 {
@@ -104,3 +105,5 @@ int getprio(struct path *pp, char *args)
 	return prio_path_weight(pp, args);
 }
 
+declare_nop_prio(initprio)
+declare_nop_prio(freeprio)
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index f64d5e4..347cc66 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -376,10 +376,10 @@ detect_prio(struct path * pp)
 
 	if (get_target_port_group_support(pp->fd) <= 0)
 		return;
-	ret = get_target_port_group(pp->fd);
+	ret = get_target_port_group(pp->fd, NULL);
 	if (ret < 0)
 		return;
-	if (get_asymmetric_access_state(pp->fd, ret) < 0)
+	if (get_asymmetric_access_state(pp->fd, ret, NULL) < 0)
 		return;
 	prio_get(p, PRIO_ALUA, DEFAULT_PRIO_ARGS);
 }
diff --git a/multipathd/main.c b/multipathd/main.c
index f876258..360a584 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -721,20 +721,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;
+	}
+	/* reinit the prio values on change event, in case something is
+	 * different */
+	prio_init(&pp->prio);
 
 	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);
 
@@ -1272,6 +1275,7 @@ check_path (struct vectors * vecs, struct path * pp)
 		}
 
 		if(newstate == PATH_UP || newstate == PATH_GHOST){
+			prio_init(&pp->prio);
 			if ( pp->mpp && pp->mpp->prflag ){
 				/*
 				 * Check Persistent Reservation.
@@ -1308,7 +1312,9 @@ check_path (struct vectors * vecs, struct path * pp)
 
 		/*
 		 * if at least one path is up in a group, and
-		 * the group is disabled, re-enable it
+		 * the group is disabled, re-enable it.
+		 * Reinitialize the prioritizer, in case something
+		 * changed.
 		 */
 		if (newstate == PATH_UP)
 			enable_group(pp);
-- 
1.8.3.1

Re: [PATCH 01/18] libmultipath: Add prioritizer context data

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> 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, I've added a context pointer to the prio structure,
> similar to what exists for the checker structure, and initprio() and
> freeprio() functions to the prioritizers. The only one that currently uses
> these is the alua prioritizer. It caches the type of alua support, the TPG
> id, and the necessary buffer size.  The only thing I'm worried about with
> this patch is whether the first two values could change.  In order to deal
> with that possibility, whenever a path gets a change event, or becomes
> valid again after a failure, it resets the context structure values, which
> forces all of them to get checked the next time the prioritizer is called.
> 
Hmm. What about reading /sys/block/sdX/device/vpg_pg83 ?
That carries the same information, and you would need to call the
ioctl only once ...

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 01/18] libmultipath: Add prioritizer context data

[Benjamin Marzinski]

On Mon, Oct 12, 2015 at 08:35:22AM +0200, Hannes Reinecke wrote:
> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> > 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, I've added a context pointer to the prio structure,
> > similar to what exists for the checker structure, and initprio() and
> > freeprio() functions to the prioritizers. The only one that currently uses
> > these is the alua prioritizer. It caches the type of alua support, the TPG
> > id, and the necessary buffer size.  The only thing I'm worried about with
> > this patch is whether the first two values could change.  In order to deal
> > with that possibility, whenever a path gets a change event, or becomes
> > valid again after a failure, it resets the context structure values, which
> > forces all of them to get checked the next time the prioritizer is called.
> > 
> Hmm. What about reading /sys/block/sdX/device/vpg_pg83 ?
> That carries the same information, and you would need to call the
> ioctl only once ...

Sure. If you want to write a patch, that would be fine by me. But I
still think caching the result, so you don't need to rerun this makes
sense.

-Ben

> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		               zSeries & Storage
> hare@suse.de			               +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

Re: [PATCH 01/18] libmultipath: Add prioritizer context data

[Hannes Reinecke]

On 10/22/2015 12:16 AM, Benjamin Marzinski wrote:
> On Mon, Oct 12, 2015 at 08:35:22AM +0200, Hannes Reinecke wrote:
>> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
>>> 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, I've added a context pointer to the prio structure,
>>> similar to what exists for the checker structure, and initprio() and
>>> freeprio() functions to the prioritizers. The only one that currently uses
>>> these is the alua prioritizer. It caches the type of alua support, the TPG
>>> id, and the necessary buffer size.  The only thing I'm worried about with
>>> this patch is whether the first two values could change.  In order to deal
>>> with that possibility, whenever a path gets a change event, or becomes
>>> valid again after a failure, it resets the context structure values, which
>>> forces all of them to get checked the next time the prioritizer is called.
>>>
>> Hmm. What about reading /sys/block/sdX/device/vpg_pg83 ?
>> That carries the same information, and you would need to call the
>> ioctl only once ...
> 
> Sure. If you want to write a patch, that would be fine by me. But I
> still think caching the result, so you don't need to rerun this makes
> sense.
> 
Well ... the information is already cached in sysfs; I doubt read()
is that much of an overhead.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 02/18] fix memory leaks on realloc failures

[Benjamin Marzinski]

When realloc fails, it doesn't free the existing memory.  In many places,
multipath was just forgetting about that that memory. It needs to free
up the existing memory, or to reset back to using it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/dmparser.c   |  6 ++++--
 multipath/main.c          |  9 ++++++---
 multipathd/cli_handlers.c | 50 +++++++++++++----------------------------------
 multipathd/uxlsnr.c       | 19 +++++++++++++++++-
 4 files changed, 42 insertions(+), 42 deletions(-)

diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 2562ba1..2209b2d 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -20,14 +20,16 @@
 static int
 merge_words (char ** dst, char * word, int space)
 {
-	char * p;
+	char * p = *dst;
 	int len;
 
 	len = strlen(*dst) + strlen(word) + space;
 	*dst = REALLOC(*dst, len + 1);
 
-	if (!*dst)
+	if (!*dst) {
+		free(p);
 		return 1;
+	}
 
 	p = *dst;
 
diff --git a/multipath/main.c b/multipath/main.c
index f62dcb1..d761621 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -395,7 +395,7 @@ out:
 static int
 dump_config (void)
 {
-	char * c;
+	char * c, * tmp = NULL;
 	char * reply;
 	unsigned int maxlen = 256;
 	int again = 1;
@@ -403,9 +403,12 @@ dump_config (void)
 	reply = MALLOC(maxlen);
 
 	while (again) {
-		if (!reply)
+		if (!reply) {
+			if (tmp)
+				free(tmp);
 			return 1;
-		c = reply;
+		}
+		c = tmp = reply;
 		c += snprint_defaults(c, reply + maxlen - c);
 		again = ((c - reply) == maxlen);
 		if (again) {
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index dc96c45..305cd7f 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -26,11 +26,14 @@
 #define REALLOC_REPLY(r, a, m)					\
 	do {							\
 		if ((a)) {					\
+			char *tmp = (r);			\
 			(r) = REALLOC((r), (m) * 2);		\
 			if ((r)) {				\
 				memset((r) + (m), 0, (m));	\
 				(m) *= 2;			\
 			}					\
+			else					\
+				free(tmp);			\
 		}						\
 	} while (0)
 
@@ -173,7 +176,7 @@ show_config (char ** r, int * len)
 	unsigned int maxlen = INITIAL_REPLY_LEN;
 	int again = 1;
 
-	reply = MALLOC(maxlen);
+	c = reply = MALLOC(maxlen);
 
 	while (again) {
 		if (!reply)
@@ -181,54 +184,29 @@ show_config (char ** r, int * len)
 		c = reply;
 		c += snprint_defaults(c, reply + maxlen - c);
 		again = ((c - reply) == maxlen);
-		if (again) {
-			reply = REALLOC(reply, maxlen * 2);
-			if (!reply)
-				return 1;
-			memset(reply + maxlen, 0, maxlen);
-			maxlen *= 2;
+		REALLOC_REPLY(reply, again, maxlen);
+		if (again)
 			continue;
-		}
 		c += snprint_blacklist(c, reply + maxlen - c);
 		again = ((c - reply) == maxlen);
-		if (again) {
-			reply = REALLOC(reply, maxlen * 2);
-			if (!reply)
-				return 1;
-			memset(reply + maxlen, 0, maxlen);
-			maxlen *= 2;
+		REALLOC_REPLY(reply, again, maxlen);
+		if (again)
 			continue;
-		}
 		c += snprint_blacklist_except(c, reply + maxlen - c);
 		again = ((c - reply) == maxlen);
-		if (again) {
-			reply = REALLOC(reply, maxlen * 2);
-			if (!reply)
-				return 1;
-			memset(reply + maxlen, 0, maxlen);
-			maxlen *= 2;
+		REALLOC_REPLY(reply, again, maxlen);
+		if (again)
 			continue;
-		}
 		c += snprint_hwtable(c, reply + maxlen - c, conf->hwtable);
 		again = ((c - reply) == maxlen);
-		if (again) {
-			reply = REALLOC(reply, maxlen * 2);
-			if (!reply)
-				return 1;
-			memset(reply + maxlen, 0, maxlen);
-			maxlen *= 2;
+		REALLOC_REPLY(reply, again, maxlen);
+		if (again)
 			continue;
-		}
 		c += snprint_overrides(c, reply + maxlen - c, conf->overrides);
 		again = ((c - reply) == maxlen);
-		if (again) {
-			reply = REALLOC(reply, maxlen * 2);
-			if (!reply)
-				return 1;
-			memset(reply + maxlen, 0, maxlen);
-			maxlen *= 2;
+		REALLOC_REPLY(reply, again, maxlen);
+		if (again)
 			continue;
-		}
 		if (VECTOR_SIZE(conf->mptable) > 0) {
 			c += snprint_mptable(c, reply + maxlen - c,
 					     conf->mptable);
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 61ba49a..64bd35c 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -65,6 +65,10 @@ static void new_client(int ux_sock)
 		return;
 
 	c = (struct client *)MALLOC(sizeof(*c));
+	if (!c) {
+		close(fd);
+		return;
+	}
 	memset(c, 0, sizeof(*c));
 	INIT_LIST_HEAD(&c->node);
 	c->fd = fd;
@@ -151,6 +155,7 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 	sigdelset(&mask, SIGHUP);
 	sigdelset(&mask, SIGUSR1);
 	while (1) {
+		struct pollfd *new;
 		struct client *c, *tmp;
 		int i, poll_count, num_clients;
 
@@ -168,7 +173,19 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
 		list_for_each_entry(c, &clients, node) {
 			num_clients++;
 		}
-		polls = REALLOC(polls, (1+num_clients) * sizeof(*polls));
+		new = REALLOC(polls, (1+num_clients) * sizeof(*polls));
+		/* If we can't allocate poliing space for the new client,
+		 * close it */
+		if (!new) {
+			if (!num_clients) {
+				condlog(1, "can't listen for new clients");
+				return NULL;
+			}
+			dead_client(list_entry(clients.prev,
+					       typeof(struct client), node));
+		}
+		else
+			polls = new;		
 		polls[0].fd = ux_sock;
 		polls[0].events = POLLIN;
 
-- 
1.8.3.1

Re: [PATCH 02/18] fix memory leaks on realloc failures

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> When realloc fails, it doesn't free the existing memory.  In many places,
> multipath was just forgetting about that that memory. It needs to free
> up the existing memory, or to reset back to using it.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 03/18] multipathd: use /run instead of /var/run

[Benjamin Marzinski]

/var/run is 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.  /run is a tmpfs and
should always be available before multipathd is started, so
multipath should just write there directly, instead of through the
symlink.

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

diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 8902f40..79d6b91 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/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

Re: [PATCH 03/18] multipathd: use /run instead of /var/run

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> /var/run is 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.  /run is a tmpfs and
> should always be available before multipathd is started, so
> multipath should just write there directly, instead of through the
> symlink.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
???

What now?

First we've been told by the systemd folks that using / directly is
evil, and everything needs to be moved to /var or /usr.

Now we should have to move it back?

Is there a reference for this?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 04/18] retrigger uevents to try and get the uid through udev

[Benjamin Marzinski]

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        | 25 ++++++++++++++++++-------
 7 files changed, 56 insertions(+), 19 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 79d6b91..025a016 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 c1fce04..85a8fdc 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;
@@ -201,6 +208,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 360a584..ea67324 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -457,11 +457,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;
@@ -729,6 +724,10 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
 			uev->kernel);
 		return 1;
 	}
+
+	if (pp->initialized == INIT_REQUESTED_UDEV)
+		return uev_add_path(uev, vecs);
+
 	/* reinit the prio values on change event, in case something is
 	 * different */
 	prio_init(&pp->prio);
@@ -1164,12 +1163,24 @@ check_path (struct vectors * vecs, struct path * pp)
 	int add_active;
 	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
@@ -1196,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

Re: [PATCH 04/18] retrigger uevents to try and get the uid through udev

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> 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
> retriggering the uevent would be useful.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Hmm. Yes, this 'udev killing worker after a certain time' is a major
pain. And we've tried to work around it, too.
With various degrees of success.
But I'm not sure if retriggering is a good idea here; we simply have
no idea if the failure is legit or not.

Can't we work with the udev/systemd folks to add a variable telling
us that udev killed the worker?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 04/18] retrigger uevents to try and get the uid through udev

[Benjamin Marzinski]

On Mon, Oct 12, 2015 at 08:40:42AM +0200, Hannes Reinecke wrote:
> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> > 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
> > retriggering the uevent would be useful.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> Hmm. Yes, this 'udev killing worker after a certain time' is a major
> pain. And we've tried to work around it, too.
> With various degrees of success.
> But I'm not sure if retriggering is a good idea here; we simply have
> no idea if the failure is legit or not.
> 
> Can't we work with the udev/systemd folks to add a variable telling
> us that udev killed the worker?

If we can get a variable to know when retriggering is a good idea, I'd
be happy to use it, like I said above. But as it stands, I don't have a
better way to solve this right now, and it's causing real issues right
now. If you have a better idea, please post it.

-Ben

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		               zSeries & Storage
> hare@suse.de			               +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 05/18] update multipath rules to deal with partition devices

[Benjamin Marzinski]

Partition devices should inherit the DM_MULTIPATH_DEVICE_PATH
state of their parents, and if they are part of multipath path devices,
they shouldn't have their own ID_FS_TYPE

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/multipath.rules | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/multipath/multipath.rules b/multipath/multipath.rules
index c76e6b8..c8fb7e6 100644
--- a/multipath/multipath.rules
+++ b/multipath/multipath.rules
@@ -1,14 +1,22 @@
 # Set DM_MULTIPATH_DEVICE_PATH if the device should be handled by multipath
 SUBSYSTEM!="block", GOTO="end_mpath"
+ACTION!="add|change", GOTO="end_mpath"
+KERNEL!="sd*|dasd*", GOTO="end_mpath"
+
+ENV{DEVTYPE}!="partition", GOTO="test_dev"
+IMPORT{parent}="DM_MULTIPATH_DEVICE_PATH"
+ENV{DM_MULTIPATH_DEVICE_PATH}=="1", ENV{ID_FS_TYPE}="none", \
+	ENV{SYSTEMD_READY}="0"
+GOTO="end_mpath"
+
+LABEL="test_dev"
 
 ENV{MPATH_SBIN_PATH}="/sbin"
 TEST!="$env{MPATH_SBIN_PATH}/multipath", ENV{MPATH_SBIN_PATH}="/usr/sbin"
 
-SUBSYSTEM=="block", ACTION=="add|change", KERNEL=="sd*|dasd*", \
-	ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
+ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
 	PROGRAM=="$env{MPATH_SBIN_PATH}/multipath -u %k", \
-	ENV{DM_MULTIPATH_DEVICE_PATH}="1" \
-	ENV{ID_FS_TYPE}="none" \
+	ENV{DM_MULTIPATH_DEVICE_PATH}="1", ENV{ID_FS_TYPE}="none", \
 	ENV{SYSTEMD_READY}="0"
 
 LABEL="end_mpath"
-- 
1.8.3.1

Re: [PATCH 05/18] update multipath rules to deal with partition devices

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> Partition devices should inherit the DM_MULTIPATH_DEVICE_PATH
> state of their parents, and if they are part of multipath path devices,
> they shouldn't have their own ID_FS_TYPE
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  multipath/multipath.rules | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/multipath/multipath.rules b/multipath/multipath.rules
> index c76e6b8..c8fb7e6 100644
> --- a/multipath/multipath.rules
> +++ b/multipath/multipath.rules
> @@ -1,14 +1,22 @@
>  # Set DM_MULTIPATH_DEVICE_PATH if the device should be handled by multipath
>  SUBSYSTEM!="block", GOTO="end_mpath"
> +ACTION!="add|change", GOTO="end_mpath"
> +KERNEL!="sd*|dasd*", GOTO="end_mpath"
> +
> +ENV{DEVTYPE}!="partition", GOTO="test_dev"
> +IMPORT{parent}="DM_MULTIPATH_DEVICE_PATH"
> +ENV{DM_MULTIPATH_DEVICE_PATH}=="1", ENV{ID_FS_TYPE}="none", \
> +	ENV{SYSTEMD_READY}="0"
> +GOTO="end_mpath"
> +
> +LABEL="test_dev"
>  
>  ENV{MPATH_SBIN_PATH}="/sbin"
>  TEST!="$env{MPATH_SBIN_PATH}/multipath", ENV{MPATH_SBIN_PATH}="/usr/sbin"
>  
> -SUBSYSTEM=="block", ACTION=="add|change", KERNEL=="sd*|dasd*", \
> -	ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
> +ENV{DM_MULTIPATH_DEVICE_PATH}!="1", \
>  	PROGRAM=="$env{MPATH_SBIN_PATH}/multipath -u %k", \
> -	ENV{DM_MULTIPATH_DEVICE_PATH}="1" \
> -	ENV{ID_FS_TYPE}="none" \
> +	ENV{DM_MULTIPATH_DEVICE_PATH}="1", ENV{ID_FS_TYPE}="none", \
>  	ENV{SYSTEMD_READY}="0"
>  
>  LABEL="end_mpath"
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 06/18] change order of multipath.rules

[Benjamin Marzinski]

At least for RedHat, the rule that calls scsi_id is
60-persistent-storage.rules, so the multipath rule needs to come
after this.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipath/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/multipath/Makefile b/multipath/Makefile
index 7f18e9a..4b3de81 100644
--- a/multipath/Makefile
+++ b/multipath/Makefile
@@ -23,7 +23,7 @@ install:
 	$(INSTALL_PROGRAM) -m 755 $(EXEC) $(DESTDIR)$(bindir)/
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
 	$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
-	$(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
+	$(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)
 	$(INSTALL_PROGRAM) -m 644 $(EXEC).8.gz $(DESTDIR)$(mandir)
 	$(INSTALL_PROGRAM) -d $(DESTDIR)$(man5dir)
@@ -32,7 +32,7 @@ install:
 uninstall:
 	rm $(DESTDIR)$(bindir)/$(EXEC)
 	rm $(DESTDIR)$(udevrulesdir)/11-dm-mpath.rules
-	rm $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
+	rm $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
 	rm $(DESTDIR)$(mandir)/$(EXEC).8.gz
 	rm $(DESTDIR)$(man5dir)/$(EXEC).conf.5.gz
 
-- 
1.8.3.1

Re: [PATCH 06/18] change order of multipath.rules

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> At least for RedHat, the rule that calls scsi_id is
> 60-persistent-storage.rules, so the multipath rule needs to come
> after this.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  multipath/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/multipath/Makefile b/multipath/Makefile
> index 7f18e9a..4b3de81 100644
> --- a/multipath/Makefile
> +++ b/multipath/Makefile
> @@ -23,7 +23,7 @@ install:
>  	$(INSTALL_PROGRAM) -m 755 $(EXEC) $(DESTDIR)$(bindir)/
>  	$(INSTALL_PROGRAM) -d $(DESTDIR)$(udevrulesdir)
>  	$(INSTALL_PROGRAM) -m 644 11-dm-mpath.rules $(DESTDIR)$(udevrulesdir)
> -	$(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
> +	$(INSTALL_PROGRAM) -m 644 $(EXEC).rules $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
>  	$(INSTALL_PROGRAM) -d $(DESTDIR)$(mandir)
>  	$(INSTALL_PROGRAM) -m 644 $(EXEC).8.gz $(DESTDIR)$(mandir)
>  	$(INSTALL_PROGRAM) -d $(DESTDIR)$(man5dir)
> @@ -32,7 +32,7 @@ install:
>  uninstall:
>  	rm $(DESTDIR)$(bindir)/$(EXEC)
>  	rm $(DESTDIR)$(udevrulesdir)/11-dm-mpath.rules
> -	rm $(DESTDIR)$(libudevdir)/rules.d/56-multipath.rules
> +	rm $(DESTDIR)$(libudevdir)/rules.d/62-multipath.rules
>  	rm $(DESTDIR)$(mandir)/$(EXEC).8.gz
>  	rm $(DESTDIR)$(man5dir)/$(EXEC).conf.5.gz
>  
> 
NACK.

56-multipath.rules has been placed bevor persistent storage so that
it can override settings in 60-persistent-storage.rules.

If you need some additional settings to fixup things after that
please add a new rules file.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 07/18] make kpartx -d remove all partitions

[Benjamin Marzinski]

Currently kpartx -d only removes the partitions that have entries in the
partition table.  If you remove a partition from the device, and then
run kpartx -d, it will not delete that partition (kpartx -u will).  I don't
see any value in leaving partition devices for partitions that don't even
exist anymore, so this patch makes -d delete all partitions.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/kpartx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
index d69f9af..a9d4c98 100644
--- a/kpartx/kpartx.c
+++ b/kpartx/kpartx.c
@@ -426,7 +426,7 @@ main(int argc, char **argv){
 			break;
 
 		case DELETE:
-			for (j = n-1; j >= 0; j--) {
+			for (j = MAXSLICES-1; j >= 0; j--) {
 				if (safe_sprintf(partname, "%s%s%d",
 					     mapname, delim, j+1)) {
 					fprintf(stderr, "partname too small\n");
@@ -434,7 +434,7 @@ main(int argc, char **argv){
 				}
 				strip_slash(partname);
 
-				if (!slices[j].size || !dm_map_present(partname))
+				if (!dm_map_present(partname))
 					continue;
 
 				if (!dm_simplecmd(DM_DEVICE_REMOVE, partname,
-- 
1.8.3.1

Re: [PATCH 07/18] make kpartx -d remove all partitions

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> Currently kpartx -d only removes the partitions that have entries in the
> partition table.  If you remove a partition from the device, and then
> run kpartx -d, it will not delete that partition (kpartx -u will).  I don't
> see any value in leaving partition devices for partitions that don't even
> exist anymore, so this patch makes -d delete all partitions.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/kpartx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/kpartx/kpartx.c b/kpartx/kpartx.c
> index d69f9af..a9d4c98 100644
> --- a/kpartx/kpartx.c
> +++ b/kpartx/kpartx.c
> @@ -426,7 +426,7 @@ main(int argc, char **argv){
>  			break;
>  
>  		case DELETE:
> -			for (j = n-1; j >= 0; j--) {
> +			for (j = MAXSLICES-1; j >= 0; j--) {
>  				if (safe_sprintf(partname, "%s%s%d",
>  					     mapname, delim, j+1)) {
>  					fprintf(stderr, "partname too small\n");
> @@ -434,7 +434,7 @@ main(int argc, char **argv){
>  				}
>  				strip_slash(partname);
>  
> -				if (!slices[j].size || !dm_map_present(partname))
> +				if (!dm_map_present(partname))
>  					continue;
>  
>  				if (!dm_simplecmd(DM_DEVICE_REMOVE, partname,
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 08/18] Fix issues with user_friendly_names initramfs bindings

[Benjamin Marzinski]

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 adds a new option to help avoid the problem in the first
place.  There actually already is a solution to this issue. If
multipathd is started with -B, it makes the bindings file read-only. If
multipathd is started this way in the initramfs, it will never make new
bindings for a device. If the device doesn't already have a binding, it
will simply be named with its wwid. The problem with this method is that
AFAICT it causes a maintenance problem with dracut.  At least on RedHat,
dracut is simply copying the regular multipathd.service file into the
initramfs. I don't see a way in multipathd.service to only start
multipathd with the -B option in the initramfs (If there is a way, I'd
be happy to use that). So, the only alternative that lets us use -B is
to have a separate multipathd.service file that dracut uses for the
initramfs.  This seems like more work than it's worth.

Instead, this patch pulls some code from systemd to detect if multipathd
is running in the initramfs.  If it is, and if new_bindings_in_boot is
not set, multipathd will set the bindings file to read-only, just like
the -B option does.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c    |  4 ++++
 libmultipath/config.h    |  1 +
 libmultipath/configure.c |  3 +++
 libmultipath/dict.c      |  4 ++++
 libmultipath/util.c      | 30 ++++++++++++++++++++++++++++++
 libmultipath/util.h      |  1 +
 6 files changed, 43 insertions(+)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index cfcc685..f0aae53 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->new_bindings_in_boot = 0;
 
 	/*
 	 * preload default hwtable
@@ -735,6 +736,9 @@ load_config (char * file, struct udev *udev)
 	    !conf->wwids_file)
 		goto out;
 
+	 if (conf->new_bindings_in_boot == 0 && in_initrd())
+		conf->bindings_read_only = 1;
+
 	return 0;
 out:
 	free_config(conf);
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 372eace..05f1f6d 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 new_bindings_in_boot;
 	unsigned int version[3];
 
 	char * dev;
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/libmultipath/dict.c b/libmultipath/dict.c
index e3bc67e..1952b1e 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -395,6 +395,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(new_bindings_in_boot, set_yes_no)
+declare_def_snprint(new_bindings_in_boot, print_yes_no)
+
 static int
 def_config_dir_handler(vector strvec)
 {
@@ -1373,6 +1376,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("new_bindings_in_boot", &def_new_bindings_in_boot_handler, &snprint_def_new_bindings_in_boot);
 	__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/util.c b/libmultipath/util.c
index ac0d1b2..fcab5fc 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -3,6 +3,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#include <sys/vfs.h>
+#include <linux/magic.h>
 
 #include "debug.h"
 #include "memory.h"
@@ -258,3 +260,31 @@ dev_t parse_devt(const char *dev_t)
 
 	return makedev(maj, min);
 }
+
+/* This define was taken from systemd. src/shared/macro.h */
+#define F_TYPE_EQUAL(a, b) (a == (typeof(a)) b)
+
+/* This function was taken from systemd. src/shared/util.c */
+int in_initrd(void) {
+	static int saved = -1;
+	struct statfs s;
+
+	if (saved >= 0)
+		return saved;
+
+	/* We make two checks here:
+	 *
+	 * 1. the flag file /etc/initrd-release must exist
+	 * 2. the root file system must be a memory file system
+	 * The second check is extra paranoia, since misdetecting an
+	 * initrd can have bad bad consequences due the initrd
+	 * emptying when transititioning to the main systemd.
+	 */
+
+	saved = access("/etc/initrd-release", F_OK) >= 0 &&
+		statfs("/", &s) >= 0 &&
+		(F_TYPE_EQUAL(s.f_type, TMPFS_MAGIC) ||
+		 F_TYPE_EQUAL(s.f_type, RAMFS_MAGIC));
+
+	return saved;
+}
diff --git a/libmultipath/util.h b/libmultipath/util.h
index 257912c..a1404f2 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -10,6 +10,7 @@ size_t strlcat(char *dst, const char *src, size_t size);
 int devt2devname (char *, int, char *);
 dev_t parse_devt(const char *dev_t);
 char *convert_dev(char *dev, int is_path_device);
+int in_initrd(void);
 
 #define safe_sprintf(var, format, args...)	\
 	snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
-- 
1.8.3.1

Re: [PATCH 08/18] Fix issues with user_friendly_names initramfs bindings

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> 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 adds a new option to help avoid the problem in the first
> place.  There actually already is a solution to this issue. If
> multipathd is started with -B, it makes the bindings file read-only. If
> multipathd is started this way in the initramfs, it will never make new
> bindings for a device. If the device doesn't already have a binding, it
> will simply be named with its wwid. The problem with this method is that
> AFAICT it causes a maintenance problem with dracut.  At least on RedHat,
> dracut is simply copying the regular multipathd.service file into the
> initramfs. I don't see a way in multipathd.service to only start
> multipathd with the -B option in the initramfs (If there is a way, I'd
> be happy to use that). So, the only alternative that lets us use -B is
> to have a separate multipathd.service file that dracut uses for the
> initramfs.  This seems like more work than it's worth.
> 
> Instead, this patch pulls some code from systemd to detect if multipathd
> is running in the initramfs.  If it is, and if new_bindings_in_boot is
> not set, multipathd will set the bindings file to read-only, just like
> the -B option does.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

Please split off this patch. The first issue is a real fix, it should
really be a separate patch.

For the second one it actually quite simple to inject a different
service file for dracut (I did that actually in our dracut package).
So I'd rather see this happening instead of second-guessing by looking
at magic files. Which will fail for anyone _not_ using dracut.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		      zSeries & Storage
hare@suse.de			      +49 911 74053 688
SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)

Re: [PATCH 08/18] Fix issues with user_friendly_names initramfs bindings

[Benjamin Marzinski]

On Thu, Oct 15, 2015 at 10:52:31AM +0200, Hannes Reinecke wrote:
> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> > 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 adds a new option to help avoid the problem in the first
> > place.  There actually already is a solution to this issue. If
> > multipathd is started with -B, it makes the bindings file read-only. If
> > multipathd is started this way in the initramfs, it will never make new
> > bindings for a device. If the device doesn't already have a binding, it
> > will simply be named with its wwid. The problem with this method is that
> > AFAICT it causes a maintenance problem with dracut.  At least on RedHat,
> > dracut is simply copying the regular multipathd.service file into the
> > initramfs. I don't see a way in multipathd.service to only start
> > multipathd with the -B option in the initramfs (If there is a way, I'd
> > be happy to use that). So, the only alternative that lets us use -B is
> > to have a separate multipathd.service file that dracut uses for the
> > initramfs.  This seems like more work than it's worth.
> > 
> > Instead, this patch pulls some code from systemd to detect if multipathd
> > is running in the initramfs.  If it is, and if new_bindings_in_boot is
> > not set, multipathd will set the bindings file to read-only, just like
> > the -B option does.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> Please split off this patch. The first issue is a real fix, it should
> really be a separate patch.
> 
> For the second one it actually quite simple to inject a different
> service file for dracut (I did that actually in our dracut package).
> So I'd rather see this happening instead of second-guessing by looking
> at magic files. Which will fail for anyone _not_ using dracut.

Fine. I'd rather only manage one multipathd.service file, but your point
that my solution is dracut specific is fair.

-Ben
 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		      zSeries & Storage
> hare@suse.de			      +49 911 74053 688
> SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: J. Hawn, J. Guild, F. Imendörffer, HRB 16746 (AG Nürnberg)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 09/18] Make multipath deactivate devices before iscsi shutdown

[Benjamin Marzinski]

The multipath devices need to be deactivated before iscsi is shutdown,
otherwise the multipath devices will generate a lot of error messages
when it loses its iscsi path devices.

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

diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index b5b755b..fc2e009 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,5 +1,6 @@
 [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
-- 
1.8.3.1

Re: [PATCH 09/18] Make multipath deactivate devices before iscsi shutdown

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> The multipath devices need to be deactivated before iscsi is shutdown,
> otherwise the multipath devices will generate a lot of error messages
> when it loses its iscsi path devices.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  multipathd/multipathd.service | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
> index b5b755b..fc2e009 100644
> --- a/multipathd/multipathd.service
> +++ b/multipathd/multipathd.service
> @@ -1,5 +1,6 @@
>  [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
> 

This will limit multipath to run on root-on-iSCSI scenarios. You
cannot run multipath on non-iSCSI and have a non-root share on iSCSI
after this change.
Is that intended?

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 09/18] Make multipath deactivate devices before iscsi shutdown

[Benjamin Marzinski]

On Mon, Oct 12, 2015 at 09:05:07AM +0200, Hannes Reinecke wrote:
> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> > The multipath devices need to be deactivated before iscsi is shutdown,
> > otherwise the multipath devices will generate a lot of error messages
> > when it loses its iscsi path devices.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  multipathd/multipathd.service | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
> > index b5b755b..fc2e009 100644
> > --- a/multipathd/multipathd.service
> > +++ b/multipathd/multipathd.service
> > @@ -1,5 +1,6 @@
> >  [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
> > 
> 
> This will limit multipath to run on root-on-iSCSI scenarios. You
> cannot run multipath on non-iSCSI and have a non-root share on iSCSI
> after this change.
> Is that intended?

Huh? This doesn't stop multipath from being set up on non-root iscsi
devices. It just means that before systemd shuts down iscsi in the
shutdown initramfs, it will attempt to disassemble the multipath device,
if possible.  I'm not sure what your objection here is. Am I missing
something?

-Ben

> 
> Cheers,
> 
> Hannes
> -- 
> Dr. Hannes Reinecke		               zSeries & Storage
> hare@suse.de			               +49 911 74053 688
> SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
> GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
> HRB 21284 (AG Nürnberg)
> 
> --
> dm-devel mailing list
> dm-devel@redhat.com
> https://www.redhat.com/mailman/listinfo/dm-devel

[PATCH 10/18] resize reply buffer for mutipathd help message

[Benjamin Marzinski]

Unlike the other reply messages from multipathd, the generic help
message code couldn't resize the buffer, and the reply had grown too big
for the initial buffer size. This was causing memory corruption and
crashing multipathd instead of printing a help message. This patch
increases the size of the initial reply buffer, and makes the help
message code able to resize the buffer so this doesn't happen in the
future.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/cli.c          | 88 +++++++++++++++++++++++++++++++++--------------
 multipathd/cli.h          | 16 ++++++++-
 multipathd/cli_handlers.c | 14 --------
 3 files changed, 78 insertions(+), 40 deletions(-)

diff --git a/multipathd/cli.c b/multipathd/cli.c
index acc4249..8d26956 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -320,52 +320,90 @@ alloc_handlers (void)
 }
 
 static int
-genhelp_sprint_aliases (char * reply, vector keys, struct key * refkw)
+genhelp_sprint_aliases (char * reply, int maxlen, vector keys,
+			struct key * refkw)
 {
-	int i, fwd = 0;
+	int i, len = 0;
 	struct key * kw;
 
-	vector_foreach_slot (keys, kw, i)
-		if (kw->code == refkw->code && kw != refkw)
-			fwd += sprintf(reply, "|%s", kw->str);
+	vector_foreach_slot (keys, kw, i) {
+		if (kw->code == refkw->code && kw != refkw) {
+			len += snprintf(reply + len, maxlen - len,
+					"|%s", kw->str);
+			if (len >= maxlen)
+				return len;
+		}
+	}
 
-	return fwd;
+	return len;
 }
 
-static char *
-genhelp_handler (void)
-{
+static int
+do_genhelp(char *reply, int maxlen) {
+	int len = 0;
 	int i, j;
 	unsigned long fp;
 	struct handler * h;
 	struct key * kw;
-	char * reply;
-	char * p;
-
-	reply = MALLOC(INITIAL_REPLY_LEN);
 
-	if (!reply)
-		return NULL;
-
-	p = reply;
-	p += sprintf(p, VERSION_STRING);
-	p += sprintf(p, "CLI commands reference:\n");
+	len += snprintf(reply + len, maxlen - len, VERSION_STRING);
+	if (len >= maxlen)
+		goto out;
+	len += snprintf(reply + len, maxlen - len, "CLI commands reference:\n");
+	if (len >= maxlen)
+		goto out;
 
 	vector_foreach_slot (handlers, h, i) {
 		fp = h->fingerprint;
 		vector_foreach_slot (keys, kw, j) {
 			if ((kw->code & fp)) {
 				fp -= kw->code;
-				p += sprintf(p, " %s", kw->str);
-				p += genhelp_sprint_aliases(p, keys, kw);
-
-				if (kw->has_param)
-					p += sprintf(p, " $%s", kw->str);
+				len += snprintf(reply + len , maxlen - len,
+						" %s", kw->str);
+				if (len >= maxlen)
+					goto out;
+				len += genhelp_sprint_aliases(reply + len,
+							      maxlen - len,
+							      keys, kw);
+				if (len >= maxlen)
+					goto out;
+
+				if (kw->has_param) {
+					len += snprintf(reply + len,
+							maxlen - len,
+							" $%s", kw->str);
+					if (len >= maxlen)
+						goto out;
+				}
 			}
 		}
-		p += sprintf(p, "\n");
+		len += snprintf(reply + len, maxlen - len, "\n");
+		if (len >= maxlen)
+			goto out;
 	}
+out:
+	return len;
+}
+
 
+static char *
+genhelp_handler (void)
+{
+	char * reply;
+	char * p = NULL;
+	int maxlen = INITIAL_REPLY_LEN;
+	int again = 1;
+
+	reply = MALLOC(maxlen);
+
+	while (again) {
+		if (!reply)
+			return NULL;
+		p = reply;
+		p += do_genhelp(reply, maxlen);
+		again = ((p - reply) >= maxlen);
+		REALLOC_REPLY(reply, again, maxlen);
+	}
 	return reply;
 }
 
diff --git a/multipathd/cli.h b/multipathd/cli.h
index 09fdc68..2e0e1da 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -71,7 +71,21 @@ enum {
 #define SETPRSTATUS	(1UL << __SETPRSTATUS)
 #define UNSETPRSTATUS	(1UL << __UNSETPRSTATUS)
 
-#define INITIAL_REPLY_LEN	1100
+#define INITIAL_REPLY_LEN	1200
+
+#define REALLOC_REPLY(r, a, m)					\
+	do {							\
+		if ((a)) {					\
+			char *tmp = (r);			\
+			(r) = REALLOC((r), (m) * 2);		\
+			if ((r)) {				\
+				memset((r) + (m), 0, (m));	\
+				(m) *= 2;			\
+			}					\
+			else					\
+				free(tmp);			\
+		}						\
+	} while (0)
 
 struct key {
 	char * str;
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 305cd7f..8cde810 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -23,20 +23,6 @@
 #include "cli.h"
 #include "uevent.h"
 
-#define REALLOC_REPLY(r, a, m)					\
-	do {							\
-		if ((a)) {					\
-			char *tmp = (r);			\
-			(r) = REALLOC((r), (m) * 2);		\
-			if ((r)) {				\
-				memset((r) + (m), 0, (m));	\
-				(m) *= 2;			\
-			}					\
-			else					\
-				free(tmp);			\
-		}						\
-	} while (0)
-
 int
 show_paths (char ** r, int * len, struct vectors * vecs, char * style)
 {
-- 
1.8.3.1

Re: [PATCH 10/18] resize reply buffer for mutipathd help message

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> Unlike the other reply messages from multipathd, the generic help
> message code couldn't resize the buffer, and the reply had grown too big
> for the initial buffer size. This was causing memory corruption and
> crashing multipathd instead of printing a help message. This patch
> increases the size of the initial reply buffer, and makes the help
> message code able to resize the buffer so this doesn't happen in the
> future.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 11/18] Add libmpathcmd library and use it internally

[Benjamin Marzinski]

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          |   2 -
 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(+), 107 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 c3ed73f..2a13b42 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -42,9 +42,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 c4ec1c5..7dcfd26 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 f0aae53..746459c 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)
@@ -617,7 +618,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 025a016..e2d2779 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
 
@@ -29,7 +28,6 @@
 
 #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
 #define DEFAULT_PIDFILE		"/run/multipathd.pid"
-#define DEFAULT_SOCKET		"/org/kernel/linux/storage/multipathd"
 #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
 #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
 #define DEFAULT_WWIDS_FILE	"/etc/multipath/wwids"
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 1952b1e..1fabc54 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)
@@ -1054,10 +1055,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;
@@ -1146,9 +1147,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 4b3de81..b5bcb3b 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 ea67324..f17b7da 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 64bd35c..740d1e7 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

Re: [PATCH 11/18] Add libmpathcmd library and use it internally

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> 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>
> ---

Good idea.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 12/18] add raw format multipathd commands

[Benjamin Marzinski]

This patch adds the following multipathd interactive commands

show paths raw format $fmt
show maps raw format $fmt

These commands work just like the regular "show ... format"
commands, except that they don't print a header, and don't add
any extra padding that isn't in the format string.  This should
make it easier for programs wanting to parse the output from
these commands.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/print.c      | 25 +++++++++++++-----------
 libmultipath/print.h      |  4 ++--
 multipathd/cli.c          |  5 ++++-
 multipathd/cli.h          |  6 ++++--
 multipathd/cli_handlers.c | 50 +++++++++++++++++++++++++++++++++++------------
 multipathd/cli_handlers.h |  2 ++
 multipathd/main.c         |  2 ++
 7 files changed, 65 insertions(+), 29 deletions(-)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 5d63ed3..831a062 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -732,7 +732,7 @@ snprint_multipath_header (char * line, int len, char * format)
 
 int
 snprint_multipath (char * line, int len, char * format,
-	     struct multipath * mpp)
+	     struct multipath * mpp, int pad)
 {
 	char * c = line;   /* line cursor */
 	char * s = line;   /* for padding */
@@ -759,7 +759,8 @@ snprint_multipath (char * line, int len, char * format,
 
 		data->snprint(buff, MAX_FIELD_LEN, mpp);
 		PRINT(c, TAIL, "%s", buff);
-		PAD(data->width);
+		if (pad)
+			PAD(data->width);
 		buff[0] = '\0';
 	} while (*f++);
 
@@ -802,7 +803,7 @@ snprint_path_header (char * line, int len, char * format)
 
 int
 snprint_path (char * line, int len, char * format,
-	     struct path * pp)
+	     struct path * pp, int pad)
 {
 	char * c = line;   /* line cursor */
 	char * s = line;   /* for padding */
@@ -829,7 +830,8 @@ snprint_path (char * line, int len, char * format,
 
 		data->snprint(buff, MAX_FIELD_LEN, pp);
 		PRINT(c, TAIL, "%s", buff);
-		PAD(data->width);
+		if (pad)
+			PAD(data->width);
 	} while (*f++);
 
 	ENDLINE;
@@ -921,7 +923,7 @@ snprint_multipath_topology (char * buff, int len, struct multipath * mpp,
 	reset_multipath_layout();
 
 	if (verbosity == 1)
-		return snprint_multipath(buff, len, "%n", mpp);
+		return snprint_multipath(buff, len, "%n", mpp, 1);
 
 	if(isatty(1))
 		c += sprintf(c, "%c[%dm", 0x1B, 1); /* bold on */
@@ -940,10 +942,11 @@ snprint_multipath_topology (char * buff, int len, struct multipath * mpp,
 	if(isatty(1))
 		c += sprintf(c, "%c[%dm", 0x1B, 0); /* bold off */
 
-	fwd += snprint_multipath(buff + fwd, len - fwd, style, mpp);
+	fwd += snprint_multipath(buff + fwd, len - fwd, style, mpp, 1);
 	if (fwd > len)
 		return len;
-	fwd += snprint_multipath(buff + fwd, len - fwd, PRINT_MAP_PROPS, mpp);
+	fwd += snprint_multipath(buff + fwd, len - fwd, PRINT_MAP_PROPS, mpp,
+				 1);
 	if (fwd > len)
 		return len;
 
@@ -970,7 +973,7 @@ snprint_multipath_topology (char * buff, int len, struct multipath * mpp,
 				strcpy(f, " |- " PRINT_PATH_INDENT);
 			else
 				strcpy(f, " `- " PRINT_PATH_INDENT);
-			fwd += snprint_path(buff + fwd, len - fwd, fmt, pp);
+			fwd += snprint_path(buff + fwd, len - fwd, fmt, pp, 1);
 			if (fwd > len)
 				return len;
 		}
@@ -1502,7 +1505,7 @@ snprint_devices (char * buff, int len, struct vectors *vecs)
 			if (r > 0)
 				fwd += snprintf(buff + fwd, len - fwd,
 						" devnode blacklisted, unmonitored");
-			else if (r < 0)
+			else if (r <= 0)
 				fwd += snprintf(buff + fwd, len - fwd,
 						" devnode whitelisted, unmonitored");
 		} else
@@ -1532,7 +1535,7 @@ print_path (struct path * pp, char * style)
 	char line[MAX_LINE_LEN];
 
 	memset(&line[0], 0, MAX_LINE_LEN);
-	snprint_path(&line[0], MAX_LINE_LEN, style, pp);
+	snprint_path(&line[0], MAX_LINE_LEN, style, pp, 1);
 	printf("%s", line);
 }
 
@@ -1542,7 +1545,7 @@ print_multipath (struct multipath * mpp, char * style)
 	char line[MAX_LINE_LEN];
 
 	memset(&line[0], 0, MAX_LINE_LEN);
-	snprint_multipath(&line[0], MAX_LINE_LEN, style, mpp);
+	snprint_multipath(&line[0], MAX_LINE_LEN, style, mpp, 1);
 	printf("%s", line);
 }
 
diff --git a/libmultipath/print.h b/libmultipath/print.h
index a3c3319..9344271 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -37,8 +37,8 @@ void get_path_layout (vector pathvec, int header);
 void get_multipath_layout (vector mpvec, int header);
 int snprint_path_header (char *, int, char *);
 int snprint_multipath_header (char *, int, char *);
-int snprint_path (char *, int, char *, struct path *);
-int snprint_multipath (char *, int, char *, struct multipath *);
+int snprint_path (char *, int, char *, struct path *, int);
+int snprint_multipath (char *, int, char *, struct multipath *, int);
 int snprint_multipath_topology (char *, int, struct multipath * mpp,
 				int verbosity);
 int snprint_defaults (char *, int);
diff --git a/multipathd/cli.c b/multipathd/cli.c
index 8d26956..e0a6bec 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -180,7 +180,7 @@ load_keys (void)
 	r += add_key(keys, "config", CONFIG, 0);
 	r += add_key(keys, "blacklist", BLACKLIST, 0);
 	r += add_key(keys, "devices", DEVICES, 0);
-	r += add_key(keys, "format", FMT, 1);
+	r += add_key(keys, "raw", RAW, 0);
 	r += add_key(keys, "wildcards", WILDCARDS, 0);
 	r += add_key(keys, "quit", QUIT, 0);
 	r += add_key(keys, "exit", QUIT, 0);
@@ -188,6 +188,7 @@ load_keys (void)
 	r += add_key(keys, "getprstatus", GETPRSTATUS, 0);
 	r += add_key(keys, "setprstatus", SETPRSTATUS, 0);
 	r += add_key(keys, "unsetprstatus", UNSETPRSTATUS, 0);
+	r += add_key(keys, "format", FMT, 1);
 
 	if (r) {
 		free_keys(keys);
@@ -463,6 +464,7 @@ cli_init (void) {
 
 	add_handler(LIST+PATHS, NULL);
 	add_handler(LIST+PATHS+FMT, NULL);
+	add_handler(LIST+PATHS+RAW+FMT, NULL);
 	add_handler(LIST+PATH, NULL);
 	add_handler(LIST+STATUS, NULL);
 	add_handler(LIST+DAEMON, NULL);
@@ -470,6 +472,7 @@ cli_init (void) {
 	add_handler(LIST+MAPS+STATUS, NULL);
 	add_handler(LIST+MAPS+STATS, NULL);
 	add_handler(LIST+MAPS+FMT, NULL);
+	add_handler(LIST+MAPS+RAW+FMT, NULL);
 	add_handler(LIST+MAPS+TOPOLOGY, NULL);
 	add_handler(LIST+TOPOLOGY, NULL);
 	add_handler(LIST+MAP+TOPOLOGY, NULL);
diff --git a/multipathd/cli.h b/multipathd/cli.h
index 2e0e1da..f6d2726 100644
--- a/multipathd/cli.h
+++ b/multipathd/cli.h
@@ -26,13 +26,14 @@ enum {
 	__CONFIG,
 	__BLACKLIST,
 	__DEVICES,
-	__FMT,
+	__RAW,
 	__WILDCARDS,
 	__QUIT,
 	__SHUTDOWN,
 	__GETPRSTATUS,
 	__SETPRSTATUS,
 	__UNSETPRSTATUS,
+	__FMT,
 };
 
 #define LIST		(1 << __LIST)
@@ -62,7 +63,7 @@ enum {
 #define CONFIG		(1 << __CONFIG)
 #define BLACKLIST	(1 << __BLACKLIST)
 #define DEVICES		(1 << __DEVICES)
-#define FMT		(1 << __FMT)
+#define RAW		(1 << __RAW)
 #define COUNT		(1 << __COUNT)
 #define WILDCARDS	(1 << __WILDCARDS)
 #define QUIT		(1 << __QUIT)
@@ -70,6 +71,7 @@ enum {
 #define GETPRSTATUS	(1UL << __GETPRSTATUS)
 #define SETPRSTATUS	(1UL << __SETPRSTATUS)
 #define UNSETPRSTATUS	(1UL << __UNSETPRSTATUS)
+#define FMT		(1UL << __FMT)
 
 #define INITIAL_REPLY_LEN	1200
 
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 8cde810..ff4c2d1 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -24,7 +24,8 @@
 #include "uevent.h"
 
 int
-show_paths (char ** r, int * len, struct vectors * vecs, char * style)
+show_paths (char ** r, int * len, struct vectors * vecs, char * style,
+	    int pretty)
 {
 	int i;
 	struct path * pp;
@@ -42,13 +43,13 @@ show_paths (char ** r, int * len, struct vectors * vecs, char * style)
 
 		c = reply;
 
-		if (VECTOR_SIZE(vecs->pathvec) > 0)
+		if (pretty && VECTOR_SIZE(vecs->pathvec) > 0)
 			c += snprint_path_header(c, reply + maxlen - c,
 						 style);
 
 		vector_foreach_slot(vecs->pathvec, pp, i)
 			c += snprint_path(c, reply + maxlen - c,
-					  style, pp);
+					  style, pp, pretty);
 
 		again = ((c - reply) == (maxlen - 1));
 
@@ -77,7 +78,7 @@ show_path (char ** r, int * len, struct vectors * vecs, struct path *pp,
 
 		c = reply;
 
-		c += snprint_path(c, reply + maxlen - c, style, pp);
+		c += snprint_path(c, reply + maxlen - c, style, pp, 0);
 
 		again = ((c - reply) == (maxlen - 1));
 
@@ -220,7 +221,7 @@ cli_list_paths (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list paths (operator)");
 
-	return show_paths(reply, len, vecs, PRINT_PATH_CHECKER);
+	return show_paths(reply, len, vecs, PRINT_PATH_CHECKER, 1);
 }
 
 int
@@ -231,7 +232,18 @@ cli_list_paths_fmt (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list paths (operator)");
 
-	return show_paths(reply, len, vecs, fmt);
+	return show_paths(reply, len, vecs, fmt, 1);
+}
+
+int
+cli_list_paths_raw (void * v, char ** reply, int * len, void * data)
+{
+	struct vectors * vecs = (struct vectors *)data;
+	char * fmt = get_keyparam(v, FMT);
+
+	condlog(3, "list paths (operator)");
+
+	return show_paths(reply, len, vecs, fmt, 0);
 }
 
 int
@@ -337,7 +349,8 @@ show_daemon (char ** r, int *len)
 }
 
 int
-show_maps (char ** r, int *len, struct vectors * vecs, char * style)
+show_maps (char ** r, int *len, struct vectors * vecs, char * style,
+	   int pretty)
 {
 	int i;
 	struct multipath * mpp;
@@ -354,13 +367,13 @@ show_maps (char ** r, int *len, struct vectors * vecs, char * style)
 			return 1;
 
 		c = reply;
-		if (VECTOR_SIZE(vecs->mpvec) > 0)
+		if (pretty && VECTOR_SIZE(vecs->mpvec) > 0)
 			c += snprint_multipath_header(c, reply + maxlen - c,
 						      style);
 
 		vector_foreach_slot(vecs->mpvec, mpp, i)
 			c += snprint_multipath(c, reply + maxlen - c,
-					       style, mpp);
+					       style, mpp, pretty);
 
 		again = ((c - reply) == (maxlen - 1));
 
@@ -379,7 +392,18 @@ cli_list_maps_fmt (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list maps (operator)");
 
-	return show_maps(reply, len, vecs, fmt);
+	return show_maps(reply, len, vecs, fmt, 1);
+}
+
+int
+cli_list_maps_raw (void * v, char ** reply, int * len, void * data)
+{
+	struct vectors * vecs = (struct vectors *)data;
+	char * fmt = get_keyparam(v, FMT);
+
+	condlog(3, "list maps (operator)");
+
+	return show_maps(reply, len, vecs, fmt, 0);
 }
 
 int
@@ -389,7 +413,7 @@ cli_list_maps (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list maps (operator)");
 
-	return show_maps(reply, len, vecs, PRINT_MAP_NAMES);
+	return show_maps(reply, len, vecs, PRINT_MAP_NAMES, 1);
 }
 
 int
@@ -409,7 +433,7 @@ cli_list_maps_status (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list maps status (operator)");
 
-	return show_maps(reply, len, vecs, PRINT_MAP_STATUS);
+	return show_maps(reply, len, vecs, PRINT_MAP_STATUS, 1);
 }
 
 int
@@ -419,7 +443,7 @@ cli_list_maps_stats (void * v, char ** reply, int * len, void * data)
 
 	condlog(3, "list maps stats (operator)");
 
-	return show_maps(reply, len, vecs, PRINT_MAP_STATS);
+	return show_maps(reply, len, vecs, PRINT_MAP_STATS, 1);
 }
 
 int
diff --git a/multipathd/cli_handlers.h b/multipathd/cli_handlers.h
index c4636d2..799f8da 100644
--- a/multipathd/cli_handlers.h
+++ b/multipathd/cli_handlers.h
@@ -1,10 +1,12 @@
 int cli_list_paths (void * v, char ** reply, int * len, void * data);
 int cli_list_paths_fmt (void * v, char ** reply, int * len, void * data);
+int cli_list_paths_raw (void * v, char ** reply, int * len, void * data);
 int cli_list_path (void * v, char ** reply, int * len, void * data);
 int cli_list_status (void * v, char ** reply, int * len, void * data);
 int cli_list_daemon (void * v, char ** reply, int * len, void * data);
 int cli_list_maps (void * v, char ** reply, int * len, void * data);
 int cli_list_maps_fmt (void * v, char ** reply, int * len, void * data);
+int cli_list_maps_raw (void * v, char ** reply, int * len, void * data);
 int cli_list_maps_status (void * v, char ** reply, int * len, void * data);
 int cli_list_maps_stats (void * v, char ** reply, int * len, void * data);
 int cli_list_map_topology (void * v, char ** reply, int * len, void * data);
diff --git a/multipathd/main.c b/multipathd/main.c
index f17b7da..09defc9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -904,6 +904,7 @@ uxlsnrloop (void * ap)
 
 	set_handler_callback(LIST+PATHS, cli_list_paths);
 	set_handler_callback(LIST+PATHS+FMT, cli_list_paths_fmt);
+	set_handler_callback(LIST+PATHS+RAW+FMT, cli_list_paths_raw);
 	set_handler_callback(LIST+PATH, cli_list_path);
 	set_handler_callback(LIST+MAPS, cli_list_maps);
 	set_handler_callback(LIST+STATUS, cli_list_status);
@@ -911,6 +912,7 @@ uxlsnrloop (void * ap)
 	set_handler_callback(LIST+MAPS+STATUS, cli_list_maps_status);
 	set_handler_callback(LIST+MAPS+STATS, cli_list_maps_stats);
 	set_handler_callback(LIST+MAPS+FMT, cli_list_maps_fmt);
+	set_handler_callback(LIST+MAPS+RAW+FMT, cli_list_maps_raw);
 	set_handler_callback(LIST+MAPS+TOPOLOGY, cli_list_maps_topology);
 	set_handler_callback(LIST+TOPOLOGY, cli_list_maps_topology);
 	set_handler_callback(LIST+MAP+TOPOLOGY, cli_list_map_topology);
-- 
1.8.3.1

Re: [PATCH 12/18] add raw format multipathd commands

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> This patch adds the following multipathd interactive commands
> 
> show paths raw format $fmt
> show maps raw format $fmt
> 
> These commands work just like the regular "show ... format"
> commands, except that they don't print a header, and don't add
> any extra padding that isn't in the format string.  This should
> make it easier for programs wanting to parse the output from
> these commands.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 13/18] libmultipath: add ignore_new_boot_devs option

[Benjamin Marzinski]

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 option, ignore_new_boot_devs.  When
enabled, this patch simply keeps multipath from being set up on devices
that aren't already in the wwids file (along with all the other checks
that multipath already does). This means that only devices that are
claimed by "multipath -c" will be used by multipath.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c    |  1 +
 libmultipath/config.h    |  1 +
 libmultipath/configure.c |  3 +--
 libmultipath/dict.c      |  4 ++++
 libmultipath/wwids.c     | 21 ++++++++++++++-------
 multipathd/main.c        |  3 +--
 6 files changed, 22 insertions(+), 11 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 746459c..773a17a 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -623,6 +623,7 @@ load_config (char * file, struct udev *udev)
 	conf->retrigger_tries = DEFAULT_RETRIGGER_TRIES;
 	conf->retrigger_delay = DEFAULT_RETRIGGER_DELAY;
 	conf->new_bindings_in_boot = 0;
+	conf->ignore_new_boot_devs = 0;
 
 	/*
 	 * preload default hwtable
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 05f1f6d..65a5978 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -138,6 +138,7 @@ struct config {
 	int retrigger_tries;
 	int retrigger_delay;
 	int new_bindings_in_boot;
+	int ignore_new_boot_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/dict.c b/libmultipath/dict.c
index 1fabc54..e3a33c6 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -399,6 +399,9 @@ declare_def_snprint(retrigger_delay, print_int)
 declare_def_handler(new_bindings_in_boot, set_yes_no)
 declare_def_snprint(new_bindings_in_boot, print_yes_no)
 
+declare_def_handler(ignore_new_boot_devs, set_yes_no)
+declare_def_snprint(ignore_new_boot_devs, print_yes_no)
+
 static int
 def_config_dir_handler(vector strvec)
 {
@@ -1375,6 +1378,7 @@ init_keywords(void)
 	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("new_bindings_in_boot", &def_new_bindings_in_boot_handler, &snprint_def_new_bindings_in_boot);
+	install_keyword("ignore_new_boot_devs", &def_ignore_new_boot_devs_handler, &snprint_def_ignore_new_boot_devs);
 	__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/wwids.c b/libmultipath/wwids.c
index f6f8ea8..d862d9f 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -15,6 +15,7 @@
 #include "wwids.h"
 #include "defaults.h"
 #include "config.h"
+#include "util.h"
 
 /*
  * Copyright (c) 2010 Benjamin Marzinski, Redhat
@@ -265,15 +266,21 @@ should_multipath(struct path *pp1, vector pathvec)
 {
 	int i;
 	struct path *pp2;
+	int ignore_new_devs = (conf->ignore_new_boot_devs && in_initrd());
+
+	if (!conf->find_multipaths && !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 (!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 09defc9..6c315ad 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -517,8 +517,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;
 		}
-- 
1.8.3.1

Re: [PATCH 13/18] libmultipath: add ignore_new_boot_devs option

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> 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 option, ignore_new_boot_devs.  When
> enabled, this patch simply keeps multipath from being set up on devices
> that aren't already in the wwids file (along with all the other checks
> that multipath already does). This means that only devices that are
> claimed by "multipath -c" will be used by multipath.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Well, as SUSE is not relying on /etc/wwids during boot we don't have
this issue :-)

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 14/18] Make use of /run depend on systemd

[Benjamin Marzinski]

While there are other ways to decide whether to use /run or /var/run,
I believe this should be good enough, and it uses a Makefile define
that already exists.  If it doesn't work for everyone, I'll switch it
to actually test the directories and add a new define to CFLAGS.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/defaults.h | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index e2d2779..a313d75 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -27,7 +27,11 @@
 #define MAX_CHECKINT(a)		(a << 2)
 
 #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
+#ifdef USE_SYSTEMD
 #define DEFAULT_PIDFILE		"/run/multipathd.pid"
+#else
+#define DEFAULT_PIDFILE         "/var/run/multipathd.pid"
+#endif
 #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
 #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
 #define DEFAULT_WWIDS_FILE	"/etc/multipath/wwids"
-- 
1.8.3.1

Re: [PATCH 14/18] Make use of /run depend on systemd

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> While there are other ways to decide whether to use /run or /var/run,
> I believe this should be good enough, and it uses a Makefile define
> that already exists.  If it doesn't work for everyone, I'll switch it
> to actually test the directories and add a new define to CFLAGS.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/defaults.h | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index e2d2779..a313d75 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -27,7 +27,11 @@
>  #define MAX_CHECKINT(a)		(a << 2)
>  
>  #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
> +#ifdef USE_SYSTEMD
>  #define DEFAULT_PIDFILE		"/run/multipathd.pid"
> +#else
> +#define DEFAULT_PIDFILE         "/var/run/multipathd.pid"
> +#endif
>  #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
>  #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
>  #define DEFAULT_WWIDS_FILE	"/etc/multipath/wwids"
> 
Hmpf. Again.

But yeah, I guess it's okay.

If only systemd would stop moving things around ...

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

Re: [PATCH 14/18] Make use of /run depend on systemd

[Zdenek Kabelac]

Dne 12.10.2015 v 09:09 Hannes Reinecke napsal(a):
> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
>> While there are other ways to decide whether to use /run or /var/run,
>> I believe this should be good enough, and it uses a Makefile define
>> that already exists.  If it doesn't work for everyone, I'll switch it
>> to actually test the directories and add a new define to CFLAGS.
>>
>> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
>> ---
>>   libmultipath/defaults.h | 4 ++++
>>   1 file changed, 4 insertions(+)
>>
>> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
>> index e2d2779..a313d75 100644
>> --- a/libmultipath/defaults.h
>> +++ b/libmultipath/defaults.h
>> @@ -27,7 +27,11 @@
>>   #define MAX_CHECKINT(a)		(a << 2)
>>
>>   #define MAX_DEV_LOSS_TMO	0x7FFFFFFF
>> +#ifdef USE_SYSTEMD
>>   #define DEFAULT_PIDFILE		"/run/multipathd.pid"
>> +#else
>> +#define DEFAULT_PIDFILE         "/var/run/multipathd.pid"
>> +#endif
>>   #define DEFAULT_CONFIGFILE	"/etc/multipath.conf"
>>   #define DEFAULT_BINDINGS_FILE	"/etc/multipath/bindings"
>>   #define DEFAULT_WWIDS_FILE	"/etc/multipath/wwids"
>>
> Hmpf. Again.
>
> But yeah, I guess it's okay.
>
> If only systemd would stop moving things around ...
>

I assume things like this should be detected at configure time and
used  via  'AC_DEFINE()' m4 macro.
(Check i.e. lvm2 configure.in file)
(As user may wish to even use completely different directory, to
make a parallel usable binary)

Zdenek

Re: [PATCH 14/18] Make use of /run depend on systemd

[Hannes Reinecke]

On 10/12/2015 09:34 AM, Zdenek Kabelac wrote:
> Dne 12.10.2015 v 09:09 Hannes Reinecke napsal(a):
>> On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
>>> While there are other ways to decide whether to use /run or
>>> /var/run,
>>> I believe this should be good enough, and it uses a Makefile define
>>> that already exists.  If it doesn't work for everyone, I'll
>>> switch it
>>> to actually test the directories and add a new define to CFLAGS.
>>>
>>> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
>>> ---
>>>   libmultipath/defaults.h | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
>>> index e2d2779..a313d75 100644
>>> --- a/libmultipath/defaults.h
>>> +++ b/libmultipath/defaults.h
>>> @@ -27,7 +27,11 @@
>>>   #define MAX_CHECKINT(a)        (a << 2)
>>>
>>>   #define MAX_DEV_LOSS_TMO    0x7FFFFFFF
>>> +#ifdef USE_SYSTEMD
>>>   #define DEFAULT_PIDFILE        "/run/multipathd.pid"
>>> +#else
>>> +#define DEFAULT_PIDFILE         "/var/run/multipathd.pid"
>>> +#endif
>>>   #define DEFAULT_CONFIGFILE    "/etc/multipath.conf"
>>>   #define DEFAULT_BINDINGS_FILE    "/etc/multipath/bindings"
>>>   #define DEFAULT_WWIDS_FILE    "/etc/multipath/wwids"
>>>
>> Hmpf. Again.
>>
>> But yeah, I guess it's okay.
>>
>> If only systemd would stop moving things around ...
>>
> 
> I assume things like this should be detected at configure time and
> used  via  'AC_DEFINE()' m4 macro.
> (Check i.e. lvm2 configure.in file)
> (As user may wish to even use completely different directory, to
> make a parallel usable binary)
> 
Yes, please!

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 15/18] libmpathpersist: uninstall man page correctly

[Benjamin Marzinski]

the libmpathpersist man pages were being installed to
/usr/share/man/man3, but uninstalled from /usr/share/man/man8.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
index 7dcfd26..e49cdb9 100644
--- a/libmpathpersist/Makefile
+++ b/libmpathpersist/Makefile
@@ -40,8 +40,8 @@ install: $(LIBS)
 
 uninstall:
 	rm -f $(DESTDIR)$(syslibdir)/$(LIBS)
-	rm $(DESTDIR)$(mandir)/mpath_persistent_reserve_in.3.gz	
-	rm $(DESTDIR)$(mandir)/mpath_persistent_reserve_out.3.gz	
+	rm $(DESTDIR)$(man3dir)/mpath_persistent_reserve_in.3.gz	
+	rm $(DESTDIR)$(man3dir)/mpath_persistent_reserve_out.3.gz	
 
 clean:
 	rm -f core *.a *.o 
-- 
1.8.3.1

Re: [PATCH 15/18] libmpathpersist: uninstall man page correctly

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> the libmpathpersist man pages were being installed to
> /usr/share/man/man3, but uninstalled from /usr/share/man/man8.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathpersist/Makefile | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libmpathpersist/Makefile b/libmpathpersist/Makefile
> index 7dcfd26..e49cdb9 100644
> --- a/libmpathpersist/Makefile
> +++ b/libmpathpersist/Makefile
> @@ -40,8 +40,8 @@ install: $(LIBS)
>  
>  uninstall:
>  	rm -f $(DESTDIR)$(syslibdir)/$(LIBS)
> -	rm $(DESTDIR)$(mandir)/mpath_persistent_reserve_in.3.gz	
> -	rm $(DESTDIR)$(mandir)/mpath_persistent_reserve_out.3.gz	
> +	rm $(DESTDIR)$(man3dir)/mpath_persistent_reserve_in.3.gz	
> +	rm $(DESTDIR)$(man3dir)/mpath_persistent_reserve_out.3.gz	
>  
>  clean:
>  	rm -f core *.a *.o 
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 16/18] Increase host buffer size

[Benjamin Marzinski]

Currently the host buffer only has space for 7 characters, this means
on systems with many scsi hosts (1000 or more), multipath will overflow
this buffer. This can happen pretty easily if there are a large number
of iscsi devices.  This patch increases the host buffer to hold 15
characters, which can deal with 100000000000000 scsi hosts.

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

diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 85a8fdc..99b6aae 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -15,7 +15,7 @@
 #define BLK_DEV_SIZE		33
 #define PATH_SIZE		512
 #define NAME_SIZE		512
-#define HOST_NAME_LEN		8
+#define HOST_NAME_LEN		16
 #define SLOT_NAME_SIZE		40
 
 #define SCSI_VENDOR_SIZE	9
-- 
1.8.3.1

Re: [PATCH 16/18] Increase host buffer size

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> Currently the host buffer only has space for 7 characters, this means
> on systems with many scsi hosts (1000 or more), multipath will overflow
> this buffer. This can happen pretty easily if there are a large number
> of iscsi devices.  This patch increases the host buffer to hold 15
> characters, which can deal with 100000000000000 scsi hosts.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/structs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index 85a8fdc..99b6aae 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -15,7 +15,7 @@
>  #define BLK_DEV_SIZE		33
>  #define PATH_SIZE		512
>  #define NAME_SIZE		512
> -#define HOST_NAME_LEN		8
> +#define HOST_NAME_LEN		16
>  #define SLOT_NAME_SIZE		40
>  
>  #define SCSI_VENDOR_SIZE	9
> 
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 17/18] Fix sun partition numbering

[Benjamin Marzinski]

On Sun labeled devices, the kpartx partition numbers weren't agreeing
with the underlying device partition numbers.  This is because kpartx
doesn't count slices with no sectors, when determining the numbering.
This patch counts these empty slices, so the numbering comes out the
same.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/sun.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kpartx/sun.c b/kpartx/sun.c
index 3d88b21..b8c023b 100644
--- a/kpartx/sun.c
+++ b/kpartx/sun.c
@@ -82,8 +82,6 @@ read_sun_pt(int fd, struct slice all, struct slice *sp, int ns) {
 	for(i=0, n=0; i<SUN_DISK_MAXPARTITIONS; i++) {
 		s = &l->partitions[i];
 
-		if (s->num_sectors == 0)
-			continue;
 		if (n < ns) {
 			sp[n].start = offset +
 				be32_to_cpu(s->start_cylinder) * be16_to_cpu(l->nsect) * be16_to_cpu(l->ntrks);
-- 
1.8.3.1

Re: [PATCH 17/18] Fix sun partition numbering

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> On Sun labeled devices, the kpartx partition numbers weren't agreeing
> with the underlying device partition numbers.  This is because kpartx
> doesn't count slices with no sectors, when determining the numbering.
> This patch counts these empty slices, so the numbering comes out the
> same.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/sun.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/kpartx/sun.c b/kpartx/sun.c
> index 3d88b21..b8c023b 100644
> --- a/kpartx/sun.c
> +++ b/kpartx/sun.c
> @@ -82,8 +82,6 @@ read_sun_pt(int fd, struct slice all, struct slice *sp, int ns) {
>  	for(i=0, n=0; i<SUN_DISK_MAXPARTITIONS; i++) {
>  		s = &l->partitions[i];
>  
> -		if (s->num_sectors == 0)
> -			continue;
>  		if (n < ns) {
>  			sp[n].start = offset +
>  				be32_to_cpu(s->start_cylinder) * be16_to_cpu(l->nsect) * be16_to_cpu(l->ntrks);
> 
A-ha. If you say so.

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)

[PATCH 18/18] Make multipath ignore devices without mpath prefix

[Benjamin Marzinski]

Multpathd currently assumes that it controls all devices of "multipath"
type. In order to allow people to create multipath type devices outside
of the multipath-tools, this patch makes multipathd only modify devices
if they have a uuid that starts with "mpath-". This was a suggestion
from Hannes a couple of months ago.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c |  4 +--
 libmultipath/devmapper.c        | 67 ++++++++++++++++++++++++++++++-----------
 libmultipath/devmapper.h        |  1 +
 multipathd/main.c               |  2 +-
 4 files changed, 53 insertions(+), 21 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index eba26a2..113cf7f 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -168,7 +168,7 @@ int mpath_persistent_reserve_in (int fd, int rq_servact,
 
 	condlog(3, "alias = %s", alias);
 	map_present = dm_map_present(alias);
-	if (map_present && dm_type(alias, TGT_MPATH) <= 0){
+	if (map_present && !dm_is_mpath(alias)){
 		condlog( 0, "%s: not a multipath device.", alias);
 		ret = MPATH_PR_DMMP_ERROR;
 		goto out;
@@ -258,7 +258,7 @@ int mpath_persistent_reserve_out ( int fd, int rq_servact, int rq_scope,
 	condlog(3, "alias = %s", alias);
 	map_present = dm_map_present(alias);
 
-	if (map_present && dm_type(alias, TGT_MPATH) <= 0){
+	if (map_present && !dm_is_mpath(alias)){
 		condlog(3, "%s: not a multipath device.", alias);
 		ret = MPATH_PR_DMMP_ERROR;
 		goto out;
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 560d418..cef8522 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -581,6 +581,48 @@ out:
 	return r;
 }
 
+extern int
+dm_is_mpath(const char * name)
+{
+	int r = 0;
+	struct dm_task *dmt;
+	struct dm_info info;
+	uint64_t start, length;
+	char *target_type = NULL;
+	char *params;
+	const char *uuid;
+
+	if (!(dmt = dm_task_create(DM_DEVICE_TABLE)))
+		return 0;
+
+	if (!dm_task_set_name(dmt, name))
+		goto out;
+
+	dm_task_no_open_count(dmt);
+
+	if (!dm_task_run(dmt))
+		goto out;
+
+	if (!dm_task_get_info(dmt, &info) || !info.exists)
+		goto out;
+
+	uuid = dm_task_get_uuid(dmt);
+
+	if (!uuid || strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN) != 0)
+		goto out;
+
+	/* Fetch 1st target */
+	dm_get_next_target(dmt, NULL, &start, &length, &target_type, &params);
+
+	if (!target_type || strcmp(target_type, TGT_MPATH) != 0)
+		goto out;
+
+	r = 1;
+out:
+	dm_task_destroy(dmt);
+	return r;
+}
+
 static int
 dm_dev_t (const char * mapname, char * dev_t, int len)
 {
@@ -698,10 +740,7 @@ _dm_flush_map (const char * mapname, int need_sync, int deferred_remove)
 {
 	int r;
 
-	if (!dm_map_present(mapname))
-		return 0;
-
-	if (dm_type(mapname, TGT_MPATH) <= 0)
+	if (!dm_is_mpath(mapname))
 		return 0; /* nothing to do */
 
 	if (dm_remove_partmaps(mapname, need_sync, deferred_remove))
@@ -751,10 +790,7 @@ dm_suspend_and_flush_map (const char * mapname)
 	unsigned long long mapsize;
 	char params[PARAMS_SIZE] = {0};
 
-	if (!dm_map_present(mapname))
-		return 0;
-
-	if (dm_type(mapname, TGT_MPATH) <= 0)
+	if (!dm_is_mpath(mapname))
 		return 0; /* nothing to do */
 
 	if (!dm_get_map(mapname, &mapsize, params)) {
@@ -915,7 +951,6 @@ dm_get_maps (vector mp)
 {
 	struct multipath * mpp;
 	int r = 1;
-	int info;
 	struct dm_task *dmt;
 	struct dm_names *names;
 	unsigned next = 0;
@@ -940,9 +975,7 @@ dm_get_maps (vector mp)
 	}
 
 	do {
-		info = dm_type(names->name, TGT_MPATH);
-
-		if (info <= 0)
+		if (!dm_is_mpath(names->name))
 			goto next;
 
 		mpp = alloc_multipath();
@@ -955,13 +988,11 @@ dm_get_maps (vector mp)
 		if (!mpp->alias)
 			goto out1;
 
-		if (info > 0) {
-			if (dm_get_map(names->name, &mpp->size, NULL))
-				goto out1;
+		if (dm_get_map(names->name, &mpp->size, NULL))
+			goto out1;
 
-			dm_get_uuid(names->name, mpp->wwid);
-			dm_get_info(names->name, &mpp->dmi);
-		}
+		dm_get_uuid(names->name, mpp->wwid);
+		dm_get_info(names->name, &mpp->dmi);
 
 		if (!vector_alloc_slot(mp))
 			goto out1;
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 2613503..1752045 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -23,6 +23,7 @@ int dm_map_present (const char *);
 int dm_get_map(const char *, unsigned long long *, char *);
 int dm_get_status(char *, char *);
 int dm_type(const char *, char *);
+int dm_is_mpath(const char *);
 int _dm_flush_map (const char *, int, int);
 int dm_flush_map_nopaths(const char * mapname, int deferred_remove);
 #define dm_flush_map(mapname) _dm_flush_map(mapname, 1, 0)
diff --git a/multipathd/main.c b/multipathd/main.c
index 6c315ad..14afe0c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -295,7 +295,7 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
 
 	map_present = dm_map_present(alias);
 
-	if (map_present && dm_type(alias, TGT_MPATH) <= 0) {
+	if (map_present && !dm_is_mpath(alias)) {
 		condlog(4, "%s: not a multipath map", alias);
 		return 0;
 	}
-- 
1.8.3.1

Re: [PATCH 18/18] Make multipath ignore devices without mpath prefix

[Hannes Reinecke]

On 10/08/2015 09:44 PM, Benjamin Marzinski wrote:
> Multpathd currently assumes that it controls all devices of "multipath"
> type. In order to allow people to create multipath type devices outside
> of the multipath-tools, this patch makes multipathd only modify devices
> if they have a uuid that starts with "mpath-". This was a suggestion
> from Hannes a couple of months ago.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke		               zSeries & Storage
hare@suse.de			               +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)