[PATCH 00/14] Various multipath-tools fixes

[PATCH 00/14] Various multipath-tools fixes

[Martin Wilck]

Hi Christophe,

here is a series of small fixes that has accumulated at SUSE during
the last months. The first three were already posted here, but you
didn't merge them yet, so I took the freedom to re-post them.
The others should be mostly non-controversial, I hope.
Reviews are of course welcome.

Regards,
Martin

Gris Ge (1):
  multipathd daemon: Fix incorrect use of CLOCK_MONOTONIC in pthread

Martin Wilck (13):
  libmultipath: don't try to set hwhandler if it is retained
  multipath: delegate dangerous commands to multipathd
  libmultipath: condlog: log to stderr
  libmultipath: fix return code of sysfs_get_timeout
  libmultipath: fix return code of sgio_get_vpd()
  libmultipath: sgio_get_vpd: add page argument
  libmultipath: get_vpd_sgio: support VPD 0xc9
  libmultipath: select ALUA prioritizer for RDAC arrays only
  libmultipath: hwtable: multibus for NetApp NVMe-FC
  multipath -C: decrease log level
  kpartx.rules: fix by-id/scsi-* for user_friendly_names
  multipathd.socket: add WantedBy=sockets.target
  multipathd.service: drop Before=lvm2-lvmetad.service

 Makefile.inc                  |  2 +-
 kpartx/dm-parts.rules         |  4 +--
 kpartx/kpartx_id              |  8 +++++-
 libmultipath/configure.c      |  8 +++++-
 libmultipath/debug.c          |  4 +--
 libmultipath/discovery.c      | 16 +++++++----
 libmultipath/hwtable.c        | 13 +++++++++
 libmultipath/propsel.c        | 37 +++++++++++++++++++++++-
 multipath/11-dm-mpath.rules   |  3 +-
 multipath/main.c              | 66 ++++++++++++++++++++++++++++++++++++++++++-
 multipathd/cli.c              |  2 +-
 multipathd/multipathd.service |  2 +-
 multipathd/multipathd.socket  |  3 ++
 13 files changed, 150 insertions(+), 18 deletions(-)

-- 
2.15.1

[PATCH 01/14] multipathd daemon: Fix incorrect use of CLOCK_MONOTONIC in pthread

[Martin Wilck]

From: Gris Ge <fge@redhat.com>

Issue:
    When multipathd is starting up, it will reply "timeout\n" immediatly
    when got any IPC command from socket. The expected way is to wait
    uxsock_timeout/1000 seconds.

Root cause:
    pthread_mutex_timedlock() is expecting a CLOCK_REALTIME time.

Fix:
    Use CLOCK_REALTIME for pthread_mutex_timedlock().

Signed-off-by: Gris Ge <fge@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/cli.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/cli.c b/multipathd/cli.c
index deb72cbe6430..f10f862cd14f 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -480,7 +480,7 @@ parse_cmd (char * cmd, char ** reply, int * len, void * data, int timeout )
 	/*
 	 * execute handler
 	 */
-	if (clock_gettime(CLOCK_MONOTONIC, &tmo) == 0) {
+	if (clock_gettime(CLOCK_REALTIME, &tmo) == 0) {
 		tmo.tv_sec += timeout;
 	} else {
 		tmo.tv_sec = 0;
-- 
2.15.1

[PATCH 02/14] libmultipath: don't try to set hwhandler if it is retained

[Martin Wilck]

Setting a device handler only works if retain_attached_hw_handler
is 'no', or if the kernel didn't auto-assign a handler. If this
is not the case, don't even attempt to set a different handler.

This requires reading the sysfs "dh_state" path attribute.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c |  8 +++++++-
 libmultipath/propsel.c   | 35 +++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+), 1 deletion(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 09821e84e62a..bc0840ac893e 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -275,15 +275,21 @@ int setup_map(struct multipath *mpp, char *params, int params_size)
 
 	/*
 	 * properties selectors
+	 *
+	 * Ordering matters for some properties:
+	 * - features after no_path_retry and retain_hwhandler
+	 * - hwhandler after retain_hwhandler
+	 * No guarantee that this list is complete, check code in
+	 * propsel.c if in doubt.
 	 */
 	conf = get_multipath_config();
 	select_pgfailback(conf, mpp);
 	select_pgpolicy(conf, mpp);
 	select_selector(conf, mpp);
-	select_hwhandler(conf, mpp);
 	select_no_path_retry(conf, mpp);
 	select_retain_hwhandler(conf, mpp);
 	select_features(conf, mpp);
+	select_hwhandler(conf, mpp);
 	select_rr_weight(conf, mpp);
 	select_minio(conf, mpp);
 	select_mode(conf, mpp);
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 0d29ed289389..d08954ac8464 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -19,9 +19,11 @@
 #include "discovery.h"
 #include "dict.h"
 #include "util.h"
+#include "sysfs.h"
 #include "prioritizers/alua_rtpg.h"
 #include "prkey.h"
 #include <inttypes.h>
+#include <libudev.h>
 
 pgpolicyfn *pgpolicies[] = {
 	NULL,
@@ -353,9 +355,42 @@ out:
 	return 0;
 }
 
+static int get_dh_state(struct path *pp, char *value, size_t value_len)
+{
+	struct udev_device *ud;
+
+	if (pp->udev == NULL)
+		return -1;
+
+	ud = udev_device_get_parent_with_subsystem_devtype(
+		pp->udev, "scsi", "scsi_device");
+	if (ud == NULL)
+		return -1;
+
+	return sysfs_attr_get_value(ud, "dh_state", value, value_len);
+}
+
 int select_hwhandler(struct config *conf, struct multipath *mp)
 {
 	char *origin;
+	struct path *pp;
+	/* dh_state is no longer than "detached" */
+	char handler[12];
+	char *dh_state;
+	int i;
+
+	dh_state = &handler[2];
+	if (mp->retain_hwhandler != RETAIN_HWHANDLER_OFF) {
+		vector_foreach_slot(mp->paths, pp, i) {
+			if (get_dh_state(pp, dh_state, sizeof(handler) - 2) > 0
+			    && strcmp(dh_state, "detached")) {
+				memcpy(handler, "1 ", 2);
+				mp->hwhandler = handler;
+				origin = "(setting: retained by kernel driver)";
+				goto out;
+			}
+		}
+	}
 
 	mp_set_hwe(hwhandler);
 	mp_set_conf(hwhandler);
-- 
2.15.1

[PATCH 03/14] multipath: delegate dangerous commands to multipathd

[Martin Wilck]

Some multipath commands are dangerous to run while multipathd is running.
For example, "multipath -r" may apply a modified configuration to the kernel,
while multipathd is still using the old configuration, leading to
inconsistent state between multipathd and the kernel.

It is safer to use equivalent multipathd client commands instead.
For now, do this only for "multipath -r", but other invocations
may be added in the future. Perhaps some day, all "multipath"
commands will be mapped to multipathd actions.

Note that with delegation, "multipath -r" will not produce any
terminal output, so this may affect users who capture "multipath -r"
output for parsing it. It would be very hard to produce compatible output
to the normal multipath command for different verbosity levels. I
considered running "show topology" after "reconfigure", but the output
would have slightly different format and would only match -v2, anyway.

I plan to convert more multipath commands, but that needs some more
thought. Some additional multipathd client commands need to be
implemented first.

Changes wrt v2:
 - use libmpathcmd rather than exec'ing multipathd (Ben Marzinski)
 - pass more parameters from main program, preparing for other commands

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 Makefile.inc     |  2 +-
 multipath/main.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 1 deletion(-)

diff --git a/Makefile.inc b/Makefile.inc
index 29c290a22e8f..d012b3d7feb4 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -90,7 +90,7 @@ OPTFLAGS	= -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int \
 		  -Wp,-D_FORTIFY_SOURCE=2 $(STACKPROT) \
 		  --param=ssp-buffer-size=4
 
-CFLAGS		= $(OPTFLAGS) -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
+CFLAGS		= $(OPTFLAGS) -D BIN_DIR=\"$(bindir)\" -DLIB_STRING=\"${LIB}\" -DRUN_DIR=\"${RUN}\"
 BIN_CFLAGS	= -fPIE -DPIE
 LIB_CFLAGS	= -fPIC
 SHARED_FLAGS	= -shared
diff --git a/multipath/main.c b/multipath/main.c
index bffe0653d905..25162a0e9a7c 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -578,6 +578,64 @@ get_dev_type(char *dev) {
 	return DEV_NONE;
 }
 
+/*
+ * Some multipath commands are dangerous to run while multipathd is running.
+ * For example, "multipath -r" may apply a modified configuration to the kernel,
+ * while multipathd is still using the old configuration, leading to
+ * inconsistent state.
+ *
+ * It is safer to use equivalent multipathd client commands instead.
+ */
+int delegate_to_multipathd(enum mpath_cmds cmd, const char *dev,
+			   enum devtypes dev_type, const struct config *conf)
+{
+	int fd;
+	char command[1024], *p, *reply;
+	int n, r = 0;
+
+	fd = mpath_connect();
+	if (fd == -1)
+		return 0;
+
+	p = command;
+	*p = '\0';
+	n = sizeof(command);
+
+	if (cmd == CMD_CREATE && conf->force_reload == FORCE_RELOAD_YES) {
+		p += snprintf(p, n, "reconfigure");
+	}
+	/* Add other translations here */
+
+	if (strlen(command) == 0)
+		/* No command found, no need to delegate */
+		return 0;
+	else if (p >= command + sizeof(command)) {
+		condlog(0, "internal error - command buffer overflow");
+		r = -1;
+		goto out;
+	}
+
+	condlog(3, "delegating command to multipathd");
+	r = mpath_process_cmd(fd, command, &reply, conf->uxsock_timeout);
+
+	if (r == -1) {
+		condlog(1, "error in multipath command %s: %s",
+			command, strerror(errno));
+		goto out;
+	}
+
+	if (reply != NULL && *reply != '\0' && strcmp(reply, "ok\n"))
+		printf("%s", reply);
+	r = 1;
+
+out:
+	FREE(reply);
+	close(fd);
+	if (r < 0)
+		exit(1);
+	return r;
+}
+
 int
 main (int argc, char *argv[])
 {
@@ -781,10 +839,15 @@ main (int argc, char *argv[])
 		} else
 			mpath_disconnect(fd);
 	}
+
 	if (cmd == CMD_REMOVE_WWID && !dev) {
 		condlog(0, "the -w option requires a device");
 		goto out;
 	}
+
+	if (delegate_to_multipathd(cmd, dev, dev_type, conf))
+		exit(0);
+
 	if (cmd == CMD_RESET_WWIDS) {
 		struct multipath * mpp;
 		int i;
-- 
2.15.1

[PATCH 04/14] libmultipath: condlog: log to stderr

[Martin Wilck]

Calling 'multipath' might result in various error messages, all
of which should be directed to stderr.
Having them intermixed with the actual output on stdout makes
parsing really hard.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/debug.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libmultipath/debug.c b/libmultipath/debug.c
index f89b26443ea5..f95a3e5f97f5 100644
--- a/libmultipath/debug.c
+++ b/libmultipath/debug.c
@@ -37,9 +37,9 @@ void dlog (int sink, int prio, const char * fmt, ...)
 					 "%b %d %H:%M:%S", tb);
 				buff[sizeof(buff)-1] = '\0';
 
-				fprintf(stdout, "%s | ", buff);
+				fprintf(stderr, "%s | ", buff);
 			}
-			vfprintf(stdout, fmt, ap);
+			vfprintf(stderr, fmt, ap);
 		}
 		else
 			log_safe(prio + 3, fmt, ap);
-- 
2.15.1

[PATCH 05/14] libmultipath: fix return code of sysfs_get_timeout

[Martin Wilck]

This function should return 1 on success, as the other sysfs_get_XXX
functions. The callers actually expect this - therefore timeout values
from sysfs are never used.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index cadf4617894f..3ae934261b65 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -276,7 +276,7 @@ sysfs_get_timeout(struct path *pp, unsigned int *timeout)
 	}
 	*timeout = t;
 
-	return 0;
+	return 1;
 }
 
 int
-- 
2.15.1

[PATCH 06/14] libmultipath: fix return code of sgio_get_vpd()

[Martin Wilck]

This function must return the length of data received in success
case.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 3ae934261b65..da7bf5652a53 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -852,7 +852,7 @@ retry:
 			return len;
 		if (len > DEFAULT_SGIO_LEN)
 			goto retry;
-		return 0;
+		return len;
 	}
 	return -1;
 }
-- 
2.15.1

[PATCH 07/14] libmultipath: sgio_get_vpd: add page argument

[Martin Wilck]

get_vpd_sgio() assumes to be able to send different VPD inquires.
This requires passing the pg argument to sgio_get_vpd().

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index da7bf5652a53..cb7fbb54883e 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -837,7 +837,7 @@ detect_alua(struct path * pp, struct config *conf)
 #define DEFAULT_SGIO_LEN 254
 
 static int
-sgio_get_vpd (unsigned char * buff, int maxlen, int fd)
+sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int pg)
 {
 	int len = DEFAULT_SGIO_LEN;
 
@@ -846,7 +846,7 @@ sgio_get_vpd (unsigned char * buff, int maxlen, int fd)
 		return -1;
 	}
 retry:
-	if (0 == do_inq(fd, 0, 1, 0x83, buff, len)) {
+	if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
 		len = buff[3] + (buff[2] << 8);
 		if (len >= maxlen)
 			return len;
@@ -1099,7 +1099,7 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 	unsigned char buff[4096];
 
 	memset(buff, 0x0, 4096);
-	if (sgio_get_vpd(buff, 4096, fd) <= 0) {
+	if (sgio_get_vpd(buff, 4096, fd, pg) <= 0) {
 		condlog(3, "failed to issue vpd inquiry for pg%02x",
 			pg);
 		return -errno;
-- 
2.15.1

[PATCH 08/14] libmultipath: get_vpd_sgio: support VPD 0xc9

[Martin Wilck]

This VPD is needed to check for NetApp E-Series RDAC support.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index cb7fbb54883e..80ffab58d67f 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1118,7 +1118,11 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 		len = parse_vpd_pg80(buff, str, maxlen);
 	else if (pg == 0x83)
 		len = parse_vpd_pg83(buff, buff_len, str, maxlen);
-	else
+	else if (pg == 0xc9 && maxlen >= 8) {
+		len = buff_len < 8 ? -ENODATA :
+			(buff_len <= maxlen ? buff_len : maxlen);
+		memcpy (str, buff, len);
+	} else
 		len = -ENOSYS;
 
 	return len;
-- 
2.15.1

[PATCH 09/14] libmultipath: select ALUA prioritizer for RDAC arrays only

[Martin Wilck]

Since commit 7e2f46d3, multipathd with "detect_prio" setting (=default)
chooses ALUA prioritizer rather than sysfs prioritizer for arrays with
implicit TPGS. But the intention of that patch was to choose ALUA for
NetApp E-Series (RDAC) storage arrays *only*, not for every array
with implicit TPGS.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/propsel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index d08954ac8464..4dba4c50beb9 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -497,7 +497,7 @@ detect_prio(struct config *conf, struct path * pp)
 
 	if (pp->tpgs <= 0)
 		return;
-	if (pp->tpgs == 2 && !check_rdac(pp)) {
+	if (pp->tpgs == 2 || !check_rdac(pp)) {
 		if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
 			default_prio = PRIO_SYSFS;
 	}
-- 
2.15.1

[PATCH 10/14] libmultipath: hwtable: multibus for NetApp NVMe-FC

[Martin Wilck]

Use multibus policy for NetApp NVMe-FC namespace controllers.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/hwtable.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 40c4724fcd1b..bae280c835e6 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -1142,6 +1142,19 @@ static struct hwentry default_hw[] = {
 		.checker_name  = NONE,
 		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
 	},
+	/*
+	 * NetApp NVMe-FC namespace devices: MULTIBUS preferred
+	 *
+	 * The table is searched backwards, so place this after generic NVMe
+	 */
+	{
+		.vendor	       = "NVME",
+		.product       = "(NetApp |)ONTAP Controller)",
+		.uid_attribute = "ID_WWN",
+		.checker_name  = NONE,
+		.pgpolicy      = MULTIBUS,
+		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
+	},
 	/*
 	 * Dot Hill Systems - Seagate Technology
 	 */
-- 
2.15.1

[PATCH 11/14] multipath -C: decrease log level

[Martin Wilck]

Avoid that the positive case ("usable paths found") spams the
syslog on systems with many LUNs.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/multipath/main.c b/multipath/main.c
index 25162a0e9a7c..52bf1658bbca 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -325,7 +325,8 @@ static int check_usable_paths(struct config *conf,
 		}
 	}
 found:
-	condlog(2, "%s:%s usable paths found", devpath, r == 0 ? "" : " no");
+	condlog(r == 0 ? 3 : 2, "%s:%s usable paths found",
+		devpath, r == 0 ? "" : " no");
 free:
 	FREE(mapname);
 	free_multipath(mpp, FREE_PATHS);
-- 
2.15.1

[PATCH 12/14] kpartx.rules: fix by-id/scsi-* for user_friendly_names

[Martin Wilck]

With user_friendly names (or generally, with aliases), the udev
rules create /dev/disk/by-id/scsi-${ALIAS} rather than
/dev/disk/by-id/scsi-${ID_SERIAL}. Fix that.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 kpartx/dm-parts.rules       | 4 ++--
 kpartx/kpartx_id            | 8 +++++++-
 multipath/11-dm-mpath.rules | 3 ++-
 3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/kpartx/dm-parts.rules b/kpartx/dm-parts.rules
index 235642fd9ae3..b48b67c809d5 100644
--- a/kpartx/dm-parts.rules
+++ b/kpartx/dm-parts.rules
@@ -31,8 +31,8 @@ ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
 IMPORT{program}=="kpartx_id %M %m $env{DM_UUID}"
 
 # DM_TYPE only has a reasonable value for partitions on multipath.
-ENV{DM_UUID}=="*-mpath-*", ENV{DM_TYPE}=="?*" \
-	SYMLINK+="disk/by-id/$env{DM_TYPE}-$env{DM_NAME}"
+ENV{DM_UUID}=="*-mpath-*", ENV{DM_TYPE}=="?*", ENV{DM_SERIAL}=="?*" \
+	SYMLINK+="disk/by-id/$env{DM_TYPE}-$env{DM_SERIAL}-part$env{DM_PART}"
 ENV{DM_WWN}=="?*", ENV{DM_PART}=="?*", \
 	SYMLINK+="disk/by-id/wwn-$env{DM_WWN}-part$env{DM_PART}"
 
diff --git a/kpartx/kpartx_id b/kpartx/kpartx_id
index b7f802d9b455..c45db2f80489 100755
--- a/kpartx/kpartx_id
+++ b/kpartx/kpartx_id
@@ -42,6 +42,7 @@ fi
 dmuuid=${UUID#*-}
 dmtbl=${UUID%%-*}
 dmpart=${dmtbl#part}
+dmserial=
 # kpartx types are 'part<num>'
 if [ "$dmpart" = "$dmtbl" ] ; then
     dmpart=
@@ -59,10 +60,12 @@ if [ "$dmtbl" = "part" ] ; then
     case "$dmuuid" in
 	mpath-*)
 	    dmdeps=$($DMSETUP deps -u $dmuuid)
+	    dmserial=${dmuuid#mpath-}
 	    ;;
     esac
 elif [ "$dmtbl" = "mpath" ] ; then
     dmname="$dmuuid"
+    dmserial="$dmuuid"
     # We need the dependencies of the table to figure out the type
     dmdeps=$($DMSETUP deps -u $UUID)
 fi
@@ -84,11 +87,14 @@ if [ -n "$dmdeps" ] ; then
 	    ;;
 	*)
 	    echo "DM_TYPE=scsi"
-	    echo "DM_WWN=0x${dmname#?}"
+	    echo "DM_WWN=0x${dmserial#?}"
 	    ;;
     esac
 else
     echo "DM_TYPE=raid"
 fi
+if [[ $dmserial ]]; then
+    echo "DM_SERIAL=$dmserial"
+fi
 
 exit 0
diff --git a/multipath/11-dm-mpath.rules b/multipath/11-dm-mpath.rules
index 91d805877f0c..03ac5da22957 100644
--- a/multipath/11-dm-mpath.rules
+++ b/multipath/11-dm-mpath.rules
@@ -100,7 +100,8 @@ ENV{DM_UDEV_LOW_PRIORITY_FLAG}!="1", OPTIONS+="link_priority=50"
 TEST=="/usr/lib/udev/kpartx_id", \
 	IMPORT{program}=="kpartx_id %M %m $env{DM_UUID}"
 
-ENV{DM_TYPE}=="?*", SYMLINK+="disk/by-id/$env{DM_TYPE}-$env{DM_NAME}"
+ENV{DM_TYPE}=="?*", ENV{DM_SERIAL}=="?*", \
+	SYMLINK+="disk/by-id/$env{DM_TYPE}-$env{DM_SERIAL}"
 ENV{DM_WWN}=="?*", SYMLINK+="disk/by-id/wwn-$env{DM_WWN}"
 
 LABEL="mpath_end"
-- 
2.15.1

[PATCH 13/14] multipathd.socket: add WantedBy=sockets.target

[Martin Wilck]

Without this, "systemctl enable multipathd.service" spits out
an error message.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/multipathd.socket | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/multipathd/multipathd.socket b/multipathd/multipathd.socket
index 921706d5d333..0ed4a1f7958b 100644
--- a/multipathd/multipathd.socket
+++ b/multipathd/multipathd.socket
@@ -5,3 +5,6 @@ Before=sockets.target
 
 [Socket]
 ListenStream=@/org/kernel/linux/storage/multipathd
+
+[Install]
+WantedBy=sockets.target
-- 
2.15.1

[PATCH 14/14] multipathd.service: drop Before=lvm2-lvmetad.service

[Martin Wilck]

lvm2-lvmetad does not depend on multipathd. On the contrary,
when multipathd sets up a map that contains LVM PVs, it's good
if lvm2-lvmetad is already running.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/multipathd.service | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/multipathd.service b/multipathd/multipathd.service
index fd66cf62af17..0cf99b2f2df5 100644
--- a/multipathd/multipathd.service
+++ b/multipathd/multipathd.service
@@ -1,7 +1,7 @@
 [Unit]
 Description=Device-Mapper Multipath Device Controller
 Wants=systemd-udev-trigger.service systemd-udev-settle.service
-Before=iscsi.service iscsid.service lvm2-lvmetad.service lvm2-activation-early.service
+Before=iscsi.service iscsid.service lvm2-activation-early.service
 Before=local-fs-pre.target blk-availability.service
 After=multipathd.socket systemd-udev-trigger.service systemd-udev-settle.service
 DefaultDependencies=no
-- 
2.15.1

Re: [PATCH 10/14] libmultipath: hwtable: multibus for NetApp NVMe-FC

[Xose Vazquez Perez]

On 01/12/2018 11:07 PM, Martin Wilck wrote:

> Use multibus policy for NetApp NVMe-FC namespace controllers.
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/hwtable.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index 40c4724fcd1b..bae280c835e6 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -1142,6 +1142,19 @@ static struct hwentry default_hw[] = {
>  		.checker_name  = NONE,
>  		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
>  	},
> +	/*
> +	 * NetApp NVMe-FC namespace devices: MULTIBUS preferred
> +	 *
> +	 * The table is searched backwards, so place this after generic NVMe

Too tricky.

> +	 */
> +	{
> +		.vendor	       = "NVME",
> +		.product       = "(NetApp |)ONTAP Controller)",
                                          ^^ Is this correct?

> +		.uid_attribute = "ID_WWN",
> +		.checker_name  = NONE,
> +		.pgpolicy      = MULTIBUS,
> +		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
> +	},

Place it inside NetApp vendor entry, and remove:
        /*
         * Generic NVMe devices
         */
        {
                .vendor        = "NVME",
                .product       = ".*",
                .uid_attribute = "ID_WWN",
                .checker_name  = NONE,
                .retain_hwhandler = RETAIN_HWHANDLER_OFF,
        },


Thanks.

Re: [PATCH 00/14] Various multipath-tools fixes

[Xose Vazquez Perez]

On 01/12/2018 11:07 PM, Martin Wilck wrote:

> here is a series of small fixes that has accumulated at SUSE during
> the last months.

These are missing:
"test-kpartx: add test for mapping without UUID"
"multipathd.service: set TasksMax=infinity"

Thank you.

Re: [PATCH 10/14] libmultipath: hwtable: multibus for NetApp NVMe-FC

[Martin Wilck]

On Sat, 2018-01-13 at 00:42 +0100, Xose Vazquez Perez wrote:
> On 01/12/2018 11:07 PM, Martin Wilck wrote:
> 
> > Use multibus policy for NetApp NVMe-FC namespace controllers.
> > 
> > Signed-off-by: Martin Wilck <mwilck@suse.com>
> > ---
> >  libmultipath/hwtable.c | 13 +++++++++++++
> >  1 file changed, 13 insertions(+)
> > 
> > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> > index 40c4724fcd1b..bae280c835e6 100644
> > --- a/libmultipath/hwtable.c
> > +++ b/libmultipath/hwtable.c
> > @@ -1142,6 +1142,19 @@ static struct hwentry default_hw[] = {
> >  		.checker_name  = NONE,
> >  		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
> >  	},
> > +	/*
> > +	 * NetApp NVMe-FC namespace devices: MULTIBUS preferred
> > +	 *
> > +	 * The table is searched backwards, so place this after
> > generic NVMe
> 
> Too tricky.

Sorry? This is just utilizing the logic we've implemented in find_hwe()
to let user-specified entries override built-in ones. Nothing "tricky"
about it. Please read the code. I just added that comment there because
the way libmultipath handles this (generic entries first, specific
later) is a bit unusual - casual readers might think the ordering is
wrong, which it is not.

> 
> > +	 */
> > +	{
> > +		.vendor	       = "NVME",
> > +		.product       = "(NetApp |)ONTAP Controller)",
> 
>                                           ^^ Is this correct?

What do you mean? It's correct posix regexp syntax. And AFAIK it's also
the correct expression to identify the controllers in question.

> > +		.uid_attribute = "ID_WWN",
> > +		.checker_name  = NONE,
> > +		.pgpolicy      = MULTIBUS,
> > +		.retain_hwhandler = RETAIN_HWHANDLER_OFF,
> > +	},
> 
> Place it inside NetApp vendor entry, and remove:
>         /*
>          * Generic NVMe devices
>          */
>         {
>                 .vendor        = "NVME",
>                 .product       = ".*",
>                 .uid_attribute = "ID_WWN",
>                 .checker_name  = NONE,
>                 .retain_hwhandler = RETAIN_HWHANDLER_OFF,
>         },

That won't work. The NetApp vendor entry uses '.vendor = "NETAPP"',
while this device has '.vendor = "NVME"'. 

And sorry again, the generic entry needs to stay in place. All NVMe
controllers we know expect to be configured with the default "failover"
policy. The "ONTAP controller" is the first exception to this rule.
More may be added later. As explained earlier, because of the backward-
search logic in find_hwe(), specific entries need to be listed *after*
generic entries.

If it's important for you to have this Netapp specific entry in the
section commented "NetApp": that can be done, if we move the generic
NVMe entry to the top of hwtable.c. I personally don't care. I 

Martin


-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
HRB 21284 (AG Nürnberg)

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