Skip to contentSkip to conversation

SLES resync

PATCH In review Hannes Reinecke 2 participants 34 messages Jul 15, 2013 – Jul 16, 2013 Device mapper

lore
Share & tools
lore
#0 lore raw

cover SLES resync

Hi Christophe,

here as some patches which accumulated in my SLES repository. Most are plain bugfixes, but there are some nasty issues fixed with this:
- The conversion to libudev was a bit overzealous; originally we'd be skipping all devices which don't have a SCSI ID. With libudev we'd be including even those, making multipath claiming even USB flash disks :-(
So I've implemented a new 'property' blacklist which will allow us to blacklist devices based on the udev property.
- dev_loss_tmo setting continues to be tricky, but this time I think we've got it sorted
- When using libudev to read attribute variables we'd be getting an automated caching from libudev. Which is nice, unless the data changed in between. So I'd have to go back an roll our own accessors, skipping libudev here.
- Ripping out 'getuid_callout' rendered all old configuration files still using in unworkable. So I've re-introduced it by marked it 'obsolete'.

As usual, comments etc are welcome.

The patchset can also be pulled from
git://github.com/hreinecke/multipath-tools.git
branch sles-resync

Hannes Reinecke (27):
multipath: bind lifetime of udev context to main thread Document 'infinity' as possible value for dev_loss_tmo
alua: Do not add preferred path priority for active/optimized
multipath: Increase dev_loss_tmo prior to fast_io_fail
libmultipath: return PATH_DOWN for quiesced paths
libmultipath: Implement PATH_TIMEOUT
Deprecate pg_timeout
kpartx: create correct symlinks for PATH_FAILED events
multipath: Deprecate 'getuid' configuration variable
multipath: Add 'Datacore Virtual Disk' to internal hardware table Minor fixes for priority handling
Check return value from pathinfo()
Read directly from sysfs when checking the device size multipath.conf.annotated: Document rr_min_io_rq Correctly print out 'max' for max_fds
Correctly set max_fds in case of failure
Update multipath.conf.defaults
Correctly set pgfailback
multipath.conf.5: clarify 'no_path_retry' default setting multipath.conf.annotated: remove 'udev_dir'
multipath: Implement 'property' blacklist
Do not print error when rport is blocked
multipath: reference the udev context when starting event queue
multipathd: valgrind fixes
multipathd: increase stacksize for uevent listener Specify checker_timeout in seconds
multipath: reset queue_if_no_path if flush failed

Petr Uzel (2):
kpartx: support disk with non-512B sectors
multipath: fix setting of fast_io_fail_tmo

 kpartx/dos.c                         |   17 +--
 kpartx/gpt.c                         |   20 +---
 kpartx/kpartx.c                      |   12 ++
 kpartx/kpartx.h                      |    8 ++
 kpartx/kpartx.rules                  |   18 ++-
 libmpathpersist/mpath_persist.c      |    7 +-
 libmpathpersist/mpath_persist.h      |    2 +-
 libmpathpersist/mpath_pr_ioctl.c     |    5 +-
 libmultipath/Makefile                |    2 +-
 libmultipath/blacklist.c             |  105 +++++++++++++----
 libmultipath/blacklist.h             |   19 +--
 libmultipath/callout.c               |  216 ++++++++++++++++++++++++++++++++++
 libmultipath/callout.h               |    7 ++
 libmultipath/checkers.c              |    3 +-
 libmultipath/checkers.h              |    5 +
 libmultipath/checkers/emc_clariion.c |    2 +-
 libmultipath/checkers/hp_sw.c        |    4 +-
 libmultipath/checkers/libsg.c        |    2 +-
 libmultipath/checkers/rdac.c         |    4 +-
 libmultipath/checkers/tur.c          |    4 +-
 libmultipath/config.c                |   44 +++++--
 libmultipath/config.h                |   10 +-
 libmultipath/configure.c             |   25 ++--
 libmultipath/devmapper.c             |   31 ++---
 libmultipath/devmapper.h             |    3 +-
 libmultipath/dict.c                  |  195 ++++++++++++++++--------------
 libmultipath/discovery.c             |  170 +++++++++++++++++++-------
 libmultipath/discovery.h             |    2 +-
 libmultipath/hwtable.c               |   13 ++
 libmultipath/print.c                 |   35 ++++++
 libmultipath/prioritizers/alua.c     |    2 +-
 libmultipath/propsel.c               |   57 +++------
 libmultipath/propsel.h               |    1 -
 libmultipath/structs.h               |    2 +-
 libmultipath/structs_vec.c           |    2 +-
 libmultipath/sysfs.c                 |   86 ++++++++++++--
 libmultipath/sysfs.h                 |    2 +
 libmultipath/uevent.c                |   16 ++-
 libmultipath/uevent.h                |    4 +-
 mpathpersist/main.c                  |   11 +-
 multipath.conf.annotated             |   40 +++++--
 multipath.conf.defaults              |  112 ++++++++++--------
 multipath/main.c                     |   27 +++--
 multipath/multipath.conf.5           |   27 ++++-
 multipathd/main.c                    |   19 ++-
 45 files changed, 1007 insertions(+), 391 deletions(-)
 create mode 100644 libmultipath/callout.c
 create mode 100644 libmultipath/callout.h
Signature · 1 line
1.7.10.4
#1 lore raw

PATCH 1/29 multipath: bind lifetime of udev context to main thread

We have to tie the lifetime of the udev context to the thread or program. The current approach by creating it on config_load() will invalidate the context during reconfiguration, thereby causing all still existent objects to refer to an invalid pointer. And resulting in a nice crash.

---
 libmpathpersist/mpath_persist.c  |    7 ++++---
 libmpathpersist/mpath_persist.h  |    2 +-
 libmpathpersist/mpath_pr_ioctl.c |    5 +++--
 libmultipath/config.c            |    9 +++------
 libmultipath/config.h            |    2 +-
 mpathpersist/main.c              |   11 +++++++----
 multipath/main.c                 |   12 ++++++++----
 multipathd/main.c                |   11 ++++++++---
 8 files changed, 35 insertions(+), 24 deletions(-)
libmpathpersist/mpath_persist.c +4 -3
index 3041089..bd30125 100644
@@ -1,4 +1,3 @@
-#include "mpath_persist.h"
 #include <libdevmapper.h>
 #include <defaults.h>
 #include <sys/stat.h>
@@ -8,6 +7,7 @@
 #include <checkers.h>
 #include <structs.h>
 #include <structs_vec.h>
+#include <libudev.h>
 
 #include <prio.h>
 #include <unistd.h>
@@ -20,6 +20,7 @@
 #include <ctype.h>
 #include <propsel.h>
 
+#include "mpath_persist.h"
 #include "mpathpr.h"
 #include "mpath_pr_ioctl.h"
 
@@ -32,9 +33,9 @@
 
 
 int
-mpath_lib_init (void)
+mpath_lib_init (struct udev *udev)
 {
-	if (load_config(DEFAULT_CONFIGFILE)){
+	if (load_config(DEFAULT_CONFIGFILE, udev)){
 		condlog(0, "Failed to initialize multipath config.");
 		return 1;
 	}
libmpathpersist/mpath_persist.h +1 -1
index d8ff6f2..a5e7868 100644
@@ -174,7 +174,7 @@ struct prout_param_descriptor { 	/* PROUT parameter descriptor */
  *
  * RETURNS: 0->Success, 1->Failed.
  */
-extern int mpath_lib_init (void );
+extern int mpath_lib_init (struct udev *udev);
 
 
 /*
libmpathpersist/mpath_pr_ioctl.c +3 -2
index de3292e..c85fd10 100644
@@ -10,8 +10,9 @@
 #include <string.h>
 #include <sys/ioctl.h>
 #include <unistd.h>
-#include "mpath_pr_ioctl.h" 
-#include <mpath_persist.h> 
+#include <libudev.h>
+#include "mpath_pr_ioctl.h"
+#include <mpath_persist.h>
 
 #include <debug.h>
 
libmultipath/config.c +3 -6
index da676df..a322dfe 100644
@@ -466,9 +466,6 @@ free_config (struct config * conf)
 	if (conf->dev)
 		FREE(conf->dev);
 
-	if (conf->udev)
-		udev_unref(conf->udev);
-
 	if (conf->multipath_dir)
 		FREE(conf->multipath_dir);
 
@@ -518,12 +515,12 @@ free_config (struct config * conf)
 }
 
 int
-load_config (char * file)
+load_config (char * file, struct udev *udev)
 {
 	if (!conf)
 		conf = alloc_config();
 
-	if (!conf)
+	if (!conf || !udev)
 		return 1;
 
 	/*
@@ -532,7 +529,7 @@ load_config (char * file)
 	if (!conf->verbosity)
 		conf->verbosity = DEFAULT_VERBOSITY;
 
-	conf->udev = udev_new();
+	conf->udev = udev;
 	dm_drv_version(conf->version, TGT_MPATH);
 	conf->dev_type = DEV_NONE;
 	conf->minio = DEFAULT_MINIO;
libmultipath/config.h +1 -1
index 4ade355..405a8f3 100644
@@ -157,7 +157,7 @@ void free_mptable (vector mptable);
 
 int store_hwe (vector hwtable, struct hwentry *);
 
-int load_config (char * file);
+int load_config (char * file, struct udev * udev);
 struct config * alloc_config (void);
 void free_config (struct config * conf);
 
mpathpersist/main.c +7 -4
index 465fcb1..e3484b5 100644
@@ -7,6 +7,7 @@
 #include <vector.h>
 #include <structs.h>
 #include <getopt.h>
+#include <libudev.h>
 #include <mpath_persist.h>
 #include "main.h"
 #include <pthread.h>
@@ -68,7 +69,8 @@ int main (int argc, char * argv[])
 	int noisy = 0;
 	int num_transport =0;
 	void *resp = NULL;
-	struct transportid * tmp; 
+	struct transportid * tmp;
+	struct udev *udev = NULL;
 
 	if (optind == argc)
 	{
@@ -84,8 +86,8 @@ int main (int argc, char * argv[])
 		exit (1);
 	}
 
-
-	mpath_lib_init();
+	udev = udev_new();
+	mpath_lib_init(udev);
 	memset(transportids,0,MPATH_MX_TIDS);
 
 	while (1)
@@ -461,12 +463,13 @@ int main (int argc, char * argv[])
 	if (res < 0)
 	{
 		mpath_lib_exit();
+		udev_unref(udev);
 		return MPATH_PR_FILE_ERROR;
 	}
 
 out :
 	mpath_lib_exit();
-
+	udev_unref(udev);
 	return (ret >= 0) ? ret : MPATH_PR_OTHER;
 }
 
multipath/main.c +8 -4
index f1b3ec9..4a8b966 100644
@@ -27,6 +27,7 @@
 #include <stdio.h>
 #include <unistd.h>
 #include <ctype.h>
+#include <libudev.h>
 
 #include <checkers.h>
 #include <prio.h>
@@ -435,6 +436,7 @@ convert_dev(char *dev)
 int
 main (int argc, char *argv[])
 {
+	struct udev *udev;
 	int arg;
 	extern char *optarg;
 	extern int optind;
@@ -445,10 +447,12 @@ main (int argc, char *argv[])
 		exit(1);
 	}
 
+	udev = udev_new();
+
 	if (dm_prereq())
 		exit(1);
 
-	if (load_config(DEFAULT_CONFIGFILE))
+	if (load_config(DEFAULT_CONFIGFILE, udev))
 		exit(1);
 
 	while ((arg = getopt(argc, argv, ":dchl::FfM:v:p:b:BrtqwW")) != EOF ) {
@@ -560,11 +564,11 @@ main (int argc, char *argv[])
 
 	if (init_checkers()) {
 		condlog(0, "failed to initialize checkers");
-		exit(1);
+		goto out;
 	}
 	if (init_prio()) {
 		condlog(0, "failed to initialize prioritizers");
-		exit(1);
+		goto out;
 	}
 	dm_init();
 
@@ -628,7 +632,7 @@ out:
 	 */
 	free_config(conf);
 	conf = NULL;
-
+	udev_unref(udev);
 #ifdef _DEBUG_
 	dbg_free_final(NULL);
 #endif
multipathd/main.c +8 -3
index 0fa1b17..aa5a298 100644
@@ -93,6 +93,8 @@ static sem_t exit_sem;
  */
 struct vectors * gvecs;
 
+struct udev * udev;
+
 static int
 need_switch_pathgroup (struct multipath * mpp, int refresh)
 {
@@ -1403,7 +1405,7 @@ reconfigure (struct vectors * vecs)
 	vecs->pathvec = NULL;
 	conf = NULL;
 
-	if (!load_config(DEFAULT_CONFIGFILE)) {
+	if (!load_config(DEFAULT_CONFIGFILE, udev)) {
 		conf->verbosity = old->verbosity;
 		conf->daemon = 1;
 		configure(vecs, 1);
@@ -1588,6 +1590,8 @@ child (void * param)
 	sem_init(&exit_sem, 0, 0);
 	signal_init();
 
+	udev = udev_new();
+
 	setup_thread_attr(&misc_attr, 64 * 1024, 1);
 	setup_thread_attr(&waiter_attr, 32 * 1024, 1);
 
@@ -1602,7 +1606,7 @@ child (void * param)
 	condlog(2, "--------start up--------");
 	condlog(2, "read " DEFAULT_CONFIGFILE);
 
-	if (load_config(DEFAULT_CONFIGFILE))
+	if (load_config(DEFAULT_CONFIGFILE, udev))
 		exit(1);
 
 	if (init_checkers()) {
@@ -1752,7 +1756,8 @@ child (void * param)
 	 */
 	free_config(conf);
 	conf = NULL;
-
+	udev_unref(udev);
+	udev = NULL;
 #ifdef _DEBUG_
 	dbg_free_final(NULL);
 #endif
Signature · 1 line
1.7.10.4
#2 lore raw

PATCH 2/29 Document 'infinity' as possible value for dev_loss_tmo

In reply to Hannes Reinecke

infinity is a valid value for dev_loss_tmo, so we should be documenting it.

---
 multipath.conf.annotated |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
multipath.conf.annotated +2 -1
index 488720c..09ed632 100644
@@ -248,9 +248,10 @@
 #	# desc    : The number of seconds the scsi layer will wait after a
 #	#           problem has been detected on a FC remote port before
 #	#           removing it from the system.
-#	# values  : n > 0
+#	# values  : infinity | n > 0
 #	# default : determined by the OS
 #	dev_loss_tmo 600
+#
 #	# name    : bindings_file
 #	# scope   : multipath
 #	# desc    : The location of the bindings file that is used with
Signature · 1 line
1.7.10.4
#3 lore raw

PATCH 3/29 alua: Do not add preferred path priority for active/optimized

In reply to Hannes Reinecke

When a path is in active/optimized we should disregard the 'preferred path' bit when calculating the priority. Otherwise we'll end up with having two different priorities (one for 'active/optimized (preferred)' and one for 'active/optimized (non-preferred)').
Which will result in two different path groups and a sub-optimal path usage.

---
 libmultipath/prioritizers/alua.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libmultipath/prioritizers/alua.c +1 -1
index b9493a4..4165ec6 100644
@@ -108,7 +108,7 @@ int getprio (struct path * pp, char * args)
 			default:
 				rc = 0;
 		}
-		if (priopath)
+		if (priopath && aas != AAS_OPTIMIZED)
 			rc += 80;
 	} else {
 		switch(-rc) {
Signature · 1 line
1.7.10.4
#4 lore raw

PATCH 4/29 multipath: Increase dev_loss_tmo prior to fast_io_fail

In reply to Hannes Reinecke

There are several constraints when setting fast_io_fail and dev_loss_tmo.
dev_loss_tmo will be capped automatically when fast_io_fail is not set. And fast_io_fail can not be raised beyond dev_loss_tmo.

So to increase dev_loss_tmo and fast_io_fail we first need to increase dev_loss_tmo to the given fast_io_fail setting, then set fast_io_fail, and then set dev_loss_tmo to the final setting.

---
 libmultipath/discovery.c |   66 +++++++++++++++++++++++++++++++++++-----------
 libmultipath/sysfs.c     |   64 ++++++++++++++++++++++++++++++++++++++++++--
 libmultipath/sysfs.h     |    2 ++
 3 files changed, 115 insertions(+), 17 deletions(-)
libmultipath/discovery.c +51 -15
index 26983ca..18f72ec 100644
@@ -317,6 +317,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	struct udev_device *rport_dev = NULL;
 	char value[11];
 	char rport_id[32];
+	unsigned long long tmo;
 
 	sprintf(rport_id, "rport-%d:%d-%d",
 		pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id);
@@ -330,23 +331,51 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	condlog(4, "target%d:%d:%d -> %s", pp->sg_id.host_no,
 		pp->sg_id.channel, pp->sg_id.scsi_id, rport_id);
 
-	snprintf(value, 11, "%u", mpp->dev_loss);
-	if (mpp->dev_loss &&
-	    sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
-		if ((mpp->fast_io_fail == MP_FAST_IO_FAIL_UNSET ||
-		     mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
-		    && mpp->dev_loss > 600) {
-			condlog(3, "%s: limiting dev_loss_tmo to 600, since "
-				"fast_io_fail is not set", mpp->alias);
-			snprintf(value, 11, "%u", 600);
-			if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-						 value, 11) <= 0)
-				condlog(0, "%s failed to set dev_loss_tmo",
-					mpp->alias);
+	/*
+	 * This is tricky.
+	 * dev_loss_tmo will be limited to 600 if fast_io_fail
+	 * is _not_ set.
+	 * fast_io_fail will be limited by the current dev_loss_tmo
+	 * setting.
+	 * So to get everything right we first need to increase
+	 * dev_loss_tmo to the fast_io_fail setting (if present),
+	 * then set fast_io_fail, and _then_ set dev_loss_tmo
+	 * to the correct value.
+	 */
+	value[0] = '\0';
+	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
+	    mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
+		/* Check if we need to temporarily increase dev_loss_tmo */
+		if (sysfs_attr_get_value(rport_dev, "dev_loss_tmo",
+					 value, 16) <= 0) {
+			condlog(0, "%s: failed to read dev_loss_tmo value, "
+				"error %d", pp->dev, errno);
+			goto out;
+		}
+		if (sscanf(value, "%llu\n", &tmo) != 1) {
+			condlog(0, "%s: Cannot parse dev_loss_tmo "
+				"attribute '%s'",pp->dev, value);
 			goto out;
 		}
+		if (mpp->fast_io_fail >= tmo) {
+			snprintf(value, 11, "%u", mpp->fast_io_fail);
+		} else {
+			tmo = 0;
+		}
+	} else if (mpp->dev_loss > 600) {
+		condlog(3, "%s: limiting dev_loss_tmo to 600, since "
+			"fast_io_fail is not set", pp->dev);
+		snprintf(value, 11, "%u", 600);
+	} else {
+		snprintf(value, 11, "%u", mpp->dev_loss);
 	}
-	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET){
+	if (strlen(value) &&
+	    sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
+		condlog(0, "%s failed to set dev_loss_tmo",
+			mpp->alias);
+		goto out;
+	}
+	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
 		if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
 			sprintf(value, "off");
 		else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
@@ -359,6 +388,13 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 				mpp->alias);
 		}
 	}
+	if (tmo > 0) {
+		snprintf(value, 11, "%u", mpp->dev_loss);
+		if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
+					 value, 11) <= 0)
+			condlog(0, "%s failed to set dev_loss_tmo",
+				mpp->alias);
+	}
 out:
 	udev_device_unref(rport_dev);
 }
@@ -394,7 +430,7 @@ sysfs_set_session_tmo(struct multipath *mpp, struct path *pp)
 		} else {
 			snprintf(value, 11, "%u", mpp->fast_io_fail);
 			if (sysfs_attr_set_value(session_dev, "recovery_tmo",
-						 value, 11)) {
+						 value, 11) <= 0) {
 				condlog(3, "%s: Failed to set recovery_tmo, "
 					" error %d", pp->dev, errno);
 			}
libmultipath/sysfs.c +62 -2

80 lines not on this page · raw message

libmultipath/sysfs.h +2 -0
index 13d7545..34f3e00 100644
@@ -7,6 +7,8 @@
 
 ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 			     char * value, size_t value_len);
+ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
+			     char * value, size_t value_len);
 int sysfs_get_size (struct path *pp, unsigned long long * size);
 int sysfs_check_holders(char * check_devt, char * new_devt);
 #endif
Signature · 1 line
1.7.10.4
#5 lore raw

PATCH 5/29 libmultipath: return PATH_DOWN for quiesced paths

In reply to Hannes Reinecke

When a SCSI device is quiesced we cannot send any I/O requests to it, so we should be returning 'PATH_DOWN' here.

---
 libmultipath/discovery.c |    1 +
 1 file changed, 1 insertion(+)
libmultipath/discovery.c +1 -0
index 18f72ec..04c6029 100644
@@ -835,6 +835,7 @@ path_offline (struct path * pp)
 	condlog(3, "%s: path state = %s", pp->dev, buff);
 
 	if (!strncmp(buff, "offline", 7) ||
+	    !strncmp(buff, "quiesce", 7) ||
 	    !strncmp(buff, "transport-offline", 17)) {
 		pp->offline = 1;
 		return PATH_DOWN;
Signature · 1 line
1.7.10.4
#6 lore raw

PATCH 6/29 libmultipath: Implement PATH_TIMEOUT

In reply to Hannes Reinecke

The tur checker might run into a timeout, eg when a command is sent but the checker hasn't been able to receive a reply in time. Use a specific 'PATH_TIMEOUT' state for these cases.

---
 libmultipath/checkers.c     |    3 ++-
 libmultipath/checkers.h     |    5 +++++
 libmultipath/checkers/tur.c |    2 +-
 libmultipath/discovery.c    |    2 ++
 libmultipath/print.c        |    4 ++++
 5 files changed, 14 insertions(+), 2 deletions(-)
libmultipath/checkers.c +2 -1
index 01dafdd..47f5c68 100644
@@ -16,7 +16,8 @@ char *checker_state_names[] = {
       "up",
       "shaky",
       "ghost",
-      "pending"
+      "pending",
+      "timeout",
 };
 
 static LIST_HEAD(checkers);
libmultipath/checkers.h +5 -0
index 5a96165..1b6c22d 100644
@@ -46,6 +46,10 @@
  * PATH_PENDING:
  * - Use: All async checkers
  * - Description: Indicates a check IO is in flight.
+ *
+ * PATH_TIMEOUT:
+ * - Use: Only tur checker
+ * - Description: Command timed out
  */
 enum path_check_state {
 	PATH_WILD,
@@ -55,6 +59,7 @@ enum path_check_state {
 	PATH_SHAKY,
 	PATH_GHOST,
 	PATH_PENDING,
+	PATH_TIMEOUT,
 	PATH_MAX_STATE
 };
 
libmultipath/checkers/tur.c +1 -1
index 9c141aa..6f5d4d9 100644
@@ -297,7 +297,7 @@ libcheck_check (struct checker * c)
 				pthread_cancel(ct->thread);
 				ct->running = 0;
 				MSG(c, MSG_TUR_TIMEOUT);
-				tur_status = PATH_DOWN;
+				tur_status = PATH_TIMEOUT;
 				ct->state = PATH_UNCHECKED;
 			} else {
 				condlog(3, "%d:%d: tur checker not finished",
libmultipath/discovery.c +2 -0
index 04c6029..6af5083 100644
@@ -1065,6 +1065,8 @@ pathinfo (struct path *pp, vector hwtable, int mask)
 			if (pp->state == PATH_UNCHECKED ||
 			    pp->state == PATH_WILD)
 				goto blank;
+			if (pp->state == PATH_TIMEOUT)
+				pp->state = PATH_DOWN;
 		} else {
 			condlog(3, "%s: path inaccessible", pp->dev);
 			pp->chkrstate = pp->state = path_state;
libmultipath/print.c +4 -0
index 274f5e7..b6d08b7 100644
@@ -335,6 +335,10 @@ snprint_chk_state (char * buff, size_t len, struct path * pp)
 		return snprintf(buff, len, "shaky");
 	case PATH_GHOST:
 		return snprintf(buff, len, "ghost");
+	case PATH_PENDING:
+		return snprintf(buff, len, "i/o pending");
+	case PATH_TIMEOUT:
+		return snprintf(buff, len, "i/o timeout");
 	default:
 		return snprintf(buff, len, "undef");
 	}
Signature · 1 line
1.7.10.4
#7 lore raw

PATCH 7/29 Deprecate pg_timeout

In reply to Hannes Reinecke

pg_timeout has been removed from dm-multipath, so deprecate it from userspace, too.

---
 libmultipath/config.c      |    2 --
 libmultipath/config.h      |    3 --
 libmultipath/configure.c   |   15 +--------
 libmultipath/devmapper.c   |   10 ------
 libmultipath/devmapper.h   |    1 -
 libmultipath/dict.c        |   75 +++-----------------------------------------
 libmultipath/propsel.c     |   39 -----------------------
 libmultipath/propsel.h     |    1 -
 libmultipath/structs.h     |    1 -
 libmultipath/structs_vec.c |    1 -
 multipath.conf.defaults    |    1 -
 11 files changed, 5 insertions(+), 144 deletions(-)
libmultipath/config.c +0 -2
index a322dfe..138e6b4 100644
@@ -327,7 +327,6 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
 	merge_num(no_path_retry);
 	merge_num(minio);
 	merge_num(minio_rq);
-	merge_num(pg_timeout);
 	merge_num(flush_on_last_del);
 	merge_num(fast_io_fail);
 	merge_num(dev_loss);
@@ -397,7 +396,6 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
 	hwe->no_path_retry = dhwe->no_path_retry;
 	hwe->minio = dhwe->minio;
 	hwe->minio_rq = dhwe->minio_rq;
-	hwe->pg_timeout = dhwe->pg_timeout;
 	hwe->flush_on_last_del = dhwe->flush_on_last_del;
 	hwe->fast_io_fail = dhwe->fast_io_fail;
 	hwe->dev_loss = dhwe->dev_loss;
libmultipath/config.h +0 -3
index 405a8f3..f1578a8 100644
@@ -41,7 +41,6 @@ struct hwentry {
 	int no_path_retry;
 	int minio;
 	int minio_rq;
-	int pg_timeout;
 	int flush_on_last_del;
 	int fast_io_fail;
 	unsigned int dev_loss;
@@ -67,7 +66,6 @@ struct mpentry {
 	int no_path_retry;
 	int minio;
 	int minio_rq;
-	int pg_timeout;
 	int flush_on_last_del;
 	int attribute_flags;
 	int user_friendly_names;
@@ -93,7 +91,6 @@ struct config {
 	int no_path_retry;
 	int user_friendly_names;
 	int bindings_read_only;
-	int pg_timeout;
 	int max_fds;
 	int force_reload;
 	int queue_without_daemon;
libmultipath/configure.c +1 -14
index e5048eb..1cf46cc 100644
@@ -69,7 +69,6 @@ setup_map (struct multipath * mpp, char * params, int params_size)
 	select_rr_weight(mpp);
 	select_minio(mpp);
 	select_no_path_retry(mpp);
-	select_pg_timeout(mpp);
 	select_mode(mpp);
 	select_uid(mpp);
 	select_gid(mpp);
@@ -210,7 +209,7 @@ select_action (struct multipath * mpp, vector curmp, int force_reload)
 			mpp->alias);
 		return;
 	}
-	if (!mpp->no_path_retry && !mpp->pg_timeout &&
+	if (!mpp->no_path_retry &&
 	    (strlen(cmpp->features) != strlen(mpp->features) ||
 	     strcmp(cmpp->features, mpp->features))) {
 		mpp->action =  ACT_RELOAD;
@@ -627,12 +626,6 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 						    "queue_if_no_path");
 			}
 		}
-		if (mpp->pg_timeout != PGTIMEOUT_UNDEF) {
-			if (mpp->pg_timeout == -PGTIMEOUT_NONE)
-				dm_set_pg_timeout(mpp->alias,  0);
-			else
-				dm_set_pg_timeout(mpp->alias, mpp->pg_timeout);
-		}
 
 		if (!conf->daemon && mpp->action != ACT_NOTHING)
 			print_multipath_topology(mpp, conf->verbosity);
@@ -821,12 +814,6 @@ extern int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh)
 		else
 			dm_queue_if_no_path(mpp->alias, 1);
 	}
-	if (mpp->pg_timeout != PGTIMEOUT_UNDEF) {
-		if (mpp->pg_timeout == -PGTIMEOUT_NONE)
-			dm_set_pg_timeout(mpp->alias,  0);
-		else
-			dm_set_pg_timeout(mpp->alias, mpp->pg_timeout);
-	}
 
 	return 0;
 }
libmultipath/devmapper.c +0 -10
index 67481c4..69f475c 100644
@@ -806,16 +806,6 @@ dm_queue_if_no_path(char *mapname, int enable)
 	return dm_message(mapname, message);
 }
 
-int
-dm_set_pg_timeout(char *mapname, int timeout_val)
-{
-	char message[24];
-
-	if (snprintf(message, 24, "set_pg_timeout %d", timeout_val) >= 24)
-		return 1;
-	return dm_message(mapname, message);
-}
-
 static int
 dm_groupmsg (char * msg, char * mapname, int index)
 {
libmultipath/devmapper.h +0 -1
index bf8ee91..8ea9dd3 100644
@@ -27,7 +27,6 @@ int dm_flush_maps (void);
 int dm_fail_path(char * mapname, char * path);
 int dm_reinstate_path(char * mapname, char * path);
 int dm_queue_if_no_path(char *mapname, int enable);
-int dm_set_pg_timeout(char *mapname, int timeout_val);
 int dm_switchgroup(char * mapname, int index);
 int dm_enablegroup(char * mapname, int index);
 int dm_disablegroup(char * mapname, int index);
libmultipath/dict.c +4 -71

130 lines not on this page · raw message

libmultipath/propsel.c +0 -39
index a7e1fc2..613dafc 100644
@@ -541,45 +541,6 @@ select_minio (struct multipath * mp)
 }
 
 extern int
-select_pg_timeout(struct multipath *mp)
-{
-	if (mp->mpe && mp->mpe->pg_timeout != PGTIMEOUT_UNDEF) {
-		mp->pg_timeout = mp->mpe->pg_timeout;
-		if (mp->pg_timeout > 0)
-			condlog(3, "%s: pg_timeout = %d (multipath setting)",
-				mp->alias, mp->pg_timeout);
-		else
-			condlog(3, "%s: pg_timeout = NONE (multipath setting)",
-				mp->alias);
-		return 0;
-	}
-	if (mp->hwe && mp->hwe->pg_timeout != PGTIMEOUT_UNDEF) {
-		mp->pg_timeout = mp->hwe->pg_timeout;
-		if (mp->pg_timeout > 0)
-			condlog(3, "%s: pg_timeout = %d (controller setting)",
-				mp->alias, mp->pg_timeout);
-		else
-			condlog(3, "%s: pg_timeout = NONE (controller setting)",
-				mp->alias);
-		return 0;
-	}
-	if (conf->pg_timeout != PGTIMEOUT_UNDEF) {
-		mp->pg_timeout = conf->pg_timeout;
-		if (mp->pg_timeout > 0)
-			condlog(3, "%s: pg_timeout = %d (config file default)",
-				mp->alias, mp->pg_timeout);
-		else
-			condlog(3,
-				"%s: pg_timeout = NONE (config file default)",
-				mp->alias);
-		return 0;
-	}
-	mp->pg_timeout = PGTIMEOUT_UNDEF;
-	condlog(3, "%s: pg_timeout = NONE (internal default)", mp->alias);
-	return 0;
-}
-
-extern int
 select_fast_io_fail(struct multipath *mp)
 {
 	if (mp->hwe && mp->hwe->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
libmultipath/propsel.h +0 -1
index eb1e534..05c6a4e 100644
@@ -9,7 +9,6 @@ int select_checker(struct path *pp);
 int select_getuid (struct path * pp);
 int select_prio (struct path * pp);
 int select_no_path_retry(struct multipath *mp);
-int select_pg_timeout(struct multipath *mp);
 int select_flush_on_last_del(struct multipath *mp);
 int select_minio(struct multipath *mp);
 int select_mode(struct multipath *mp);
libmultipath/structs.h +0 -1
index 59387d6..ef3d76f 100644
@@ -201,7 +201,6 @@ struct multipath {
 	int no_path_retry; /* number of retries after all paths are down */
 	int retry_tick;    /* remaining times for retries */
 	int minio;
-	int pg_timeout;
 	int flush_on_last_del;
 	int attribute_flags;
 	int fast_io_fail;
libmultipath/structs_vec.c +0 -1
index 72622b0..186cd0e 100644
@@ -363,7 +363,6 @@ __setup_multipath (struct vectors * vecs, struct multipath * mpp, int reset)
 		select_rr_weight(mpp);
 		select_pgfailback(mpp);
 		set_no_path_retry(mpp);
-		select_pg_timeout(mpp);
 		select_flush_on_last_del(mpp);
 	}
 
multipath.conf.defaults +0 -1
index ad32343..cd13369 100644
@@ -17,7 +17,6 @@
 #	max_fds 1048576
 #	rr_weight "uniform"
 #	queue_without_daemon "yes"
-#	pg_timeout "none"
 #	flush_on_last_del "no"
 #	user_friendly_names "no"
 #	fast_io_fail_tmo 5
Signature · 1 line
1.7.10.4
#8 lore raw

PATCH 8/29 kpartx: create correct symlinks for PATH_FAILED events

In reply to Hannes Reinecke

The kernel is sending out PATH_FAILED/PATH_REINSTATED events, upon each of which we need to regenerate the existing symlinks. However, for an all-paths-down scenario we cannot read from the disk, so we cannot execute blkid or kpartx.
For these cases we need to import the existing data from the udev database.

---
 kpartx/kpartx.rules |   18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)
kpartx/kpartx.rules +17 -1
index b96ec89..5c0d0ff 100644
@@ -8,11 +8,16 @@ KERNEL!="dm-*", GOTO="kpartx_end"
 ACTION=="remove", GOTO="kpartx_end"
 
 ENV{DM_TABLE_STATE}!="LIVE", GOTO="kpartx_end"
-ENV{DM_ACTION}=="PATH_FAILED|PATH_REINSTATED", GOTO="kpartx_end"
 ENV{DM_DEPS}=="0", GOTO="kpartx_end"
 
 ENV{DM_UUID}=="?*", IMPORT{program}=="kpartx_id %M %m $env{DM_UUID}"
 
+ENV{DM_ACTION}=="PATH_FAILED", ENV{DM_NR_VALID_PATHS}=="0", \
+	GOTO="blkid_end"
+ENV{DM_UUID}=="mpath-*", IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
+ENV{DM_PART}=="?*", IMPORT{program}="/sbin/blkid -o udev -p $tempnode"
+LABEL="blkid_end"
+
 OPTIONS="link_priority=50"
 
 # Create persistent links for multipath tables
@@ -31,7 +36,18 @@ ENV{DM_MPATH}=="?*", ENV{DM_PART}=="?*", \
 ENV{DM_WWN}=="?*", ENV{DM_PART}=="?*", \
 	SYMLINK+="disk/by-id/wwn-$env{DM_WWN}-part$env{DM_PART}"
 
+# Create persistent by-label/by-uuid links
+ENV{ID_FS_USAGE}=="?*", IMPORT{db}="ID_FS_USAGE"
+ENV{ID_FS_UUID_ENC}=="?*", IMPORT{db}="ID_FS_UUID_ENC"
+ENV{ID_FS_USAGE}=="filesystem|other|crypto", ENV{ID_FS_UUID_ENC}=="?*", \
+	SYMLINK+="disk/by-uuid/$env{ID_FS_UUID_ENC}"
+ENV{ID_FS_LABEL_ENC}=="?*", IMPORT{db}="ID_FS_LABEL_ENC"
+ENV{ID_FS_USAGE}=="filesystem|other", ENV{ID_FS_LABEL_ENC}=="?*", \
+	SYMLINK+="disk/by-label/$env{ID_FS_LABEL_ENC}"
+
 # Create dm tables for partitions
+ENV{DM_ACTION}=="PATH_FAILED", ENV{DM_NR_VALID_PATHS}=="0", \
+	GOTO="kpartx_end"
 ENV{DM_STATE}!="SUSPENDED", ENV{DM_UUID}=="mpath-*", \
         RUN+="/sbin/kpartx -u -p -part /dev/$name"
 
Signature · 1 line
1.7.10.4
#9 lore raw

PATCH 9/29 multipath: Deprecate 'getuid' configuration variable

In reply to Hannes Reinecke

Older versions of multipath-tools used the 'getuid_callout' configuration variable to generate the WWID. So for compatibility we should be accepting existing configurations, but mark the variable as 'deprecated'.

---
 libmultipath/Makefile      |    2 +-
 libmultipath/callout.c     |  216 ++++++++++++++++++++++++++++++++++++++++++++
 libmultipath/callout.h     |    7 ++
 libmultipath/config.c      |   13 +++
 libmultipath/config.h      |    3 +
 libmultipath/dict.c        |   47 ++++++++++
 libmultipath/discovery.c   |   55 +++++++----
 libmultipath/propsel.c     |   12 +++
 libmultipath/structs.h     |    1 +
 libmultipath/structs_vec.c |    1 +
 multipath.conf.annotated   |   12 ++-
 multipath/multipath.conf.5 |    5 +
 12 files changed, 354 insertions(+), 20 deletions(-)
 create mode 100644 libmultipath/callout.c
 create mode 100644 libmultipath/callout.h
libmultipath/Makefile +1 -1
index 22d3844..ae1d8a3 100644
@@ -9,7 +9,7 @@ DEVLIB = libmultipath.so
 LIBS = $(DEVLIB).$(SONAME)
 LIBDEPS = -lpthread -ldl -ldevmapper -ludev
 
-OBJS = memory.o parser.o vector.o devmapper.o \
+OBJS = memory.o parser.o vector.o devmapper.o callout.o \
        hwtable.o blacklist.o util.o dmparser.o config.o \
        structs.o discovery.o propsel.o dict.o \
        pgpolicies.o debug.o regex.o defaults.o uevent.o \
libmultipath/callout.c new file +216 -0

216 lines not on this page · raw message

libmultipath/callout.h new file +7 -0
new file mode 100644
index 0000000..ab648e8
@@ -0,0 +1,7 @@
+#ifndef _CALLOUT_H
+#define _CALLOUT_H
+
+int execute_program(char *, char *, int);
+int apply_format (char *, char *, struct path *);
+
+#endif /* _CALLOUT_H */
libmultipath/config.c +13 -0
index 138e6b4..cc44244 100644
@@ -167,6 +167,9 @@ free_hwe (struct hwentry * hwe)
 	if (hwe->revision)
 		FREE(hwe->revision);
 
+	if (hwe->getuid)
+		FREE(hwe->getuid);
+
 	if (hwe->uid_attribute)
 		FREE(hwe->uid_attribute);
 
@@ -224,6 +227,9 @@ free_mpe (struct mpentry * mpe)
 	if (mpe->selector)
 		FREE(mpe->selector);
 
+	if (mpe->getuid)
+		FREE(mpe->getuid);
+
 	if (mpe->uid_attribute)
 		FREE(mpe->uid_attribute);
 
@@ -312,6 +318,7 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
 	merge_str(vendor);
 	merge_str(product);
 	merge_str(revision);
+	merge_str(getuid);
 	merge_str(uid_attribute);
 	merge_str(features);
 	merge_str(hwhandler);
@@ -369,6 +376,9 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
 	if (dhwe->uid_attribute && !(hwe->uid_attribute = set_param_str(dhwe->uid_attribute)))
 		goto out;
 
+	if (dhwe->getuid && !(hwe->getuid = set_param_str(dhwe->getuid)))
+		goto out;
+
 	if (dhwe->features && !(hwe->features = set_param_str(dhwe->features)))
 		goto out;
 
@@ -473,6 +483,9 @@ free_config (struct config * conf)
 	if (conf->uid_attribute)
 		FREE(conf->uid_attribute);
 
+	if (conf->getuid)
+		FREE(conf->getuid);
+
 	if (conf->features)
 		FREE(conf->features);
 
libmultipath/config.h +3 -0
index f1578a8..ca65f88 100644
@@ -27,6 +27,7 @@ struct hwentry {
 	char * product;
 	char * revision;
 	char * uid_attribute;
+	char * getuid;
 	char * features;
 	char * hwhandler;
 	char * selector;
@@ -54,6 +55,7 @@ struct mpentry {
 	char * wwid;
 	char * alias;
 	char * uid_attribute;
+	char * getuid;
 	char * selector;
 	char * features;
 
@@ -116,6 +118,7 @@ struct config {
 	char * multipath_dir;
 	char * selector;
 	char * uid_attribute;
+	char * getuid;
 	char * features;
 	char * hwhandler;
 	char * bindings_file;
libmultipath/dict.c +47 -0

89 lines not on this page · raw message

libmultipath/discovery.c +37 -18

83 lines not on this page · raw message

libmultipath/propsel.c +12 -0
index 613dafc..65c2c7f 100644
@@ -371,12 +371,24 @@ select_getuid (struct path * pp)
 			pp->dev, pp->uid_attribute);
 		return 0;
 	}
+	if (pp->hwe && pp->hwe->getuid) {
+		pp->getuid = pp->hwe->getuid;
+		condlog(3, "%s: getuid = %s (deprecated) (controller setting)",
+			pp->dev, pp->getuid);
+		return 0;
+	}
 	if (conf->uid_attribute) {
 		pp->uid_attribute = conf->uid_attribute;
 		condlog(3, "%s: uid_attribute = %s (config file default)",
 			pp->dev, pp->uid_attribute);
 		return 0;
 	}
+	if (conf->getuid) {
+		pp->getuid = conf->getuid;
+		condlog(3, "%s: getuid = %s (deprecated) (config file default)",
+			pp->dev, pp->getuid);
+		return 0;
+	}
 	pp->uid_attribute = STRDUP(DEFAULT_UID_ATTRIBUTE);
 	condlog(3, "%s: uid_attribute = %s (internal default)",
 		pp->dev, pp->uid_attribute);
libmultipath/structs.h +1 -0
index ef3d76f..64de06e 100644
@@ -173,6 +173,7 @@ struct path {
 	int pgindex;
 	int detect_prio;
 	char * uid_attribute;
+	char * getuid;
 	struct prio prio;
 	char * prio_args;
 	struct checker checker;
libmultipath/structs_vec.c +1 -0
index 186cd0e..993d3e0 100644
@@ -82,6 +82,7 @@ orphan_path (struct path * pp)
 	pp->mpp = NULL;
 	pp->dmstate = PSTATE_UNDEF;
 	pp->uid_attribute = NULL;
+	pp->getuid = NULL;
 	prio_put(&pp->prio);
 	checker_put(&pp->checker);
 	if (pp->fd >= 0)
multipath.conf.annotated +11 -1
index 09ed632..15ec468 100644
@@ -59,10 +59,20 @@
 #	path_grouping_policy	multibus
 #
 #	#
+#	# name    : uid_attribute
+#	# scope   : multipath & multipathd
+#	# desc    : the default udev attribute from which the path
+#	# 	    identifier should be generated.
+#	# default : ID_SERIAL
+#	#
+#	uid_attribute	"ID_SERIAL"
+#
+#	#
 #	# name    : getuid_callout
 #	# scope   : multipath & multipathd
 #	# desc    : the default program and args to callout to obtain a unique 
-#	#           path identifier. Absolute path required
+#	#           path identifier. This parameter is deprecated.
+#	#           This parameter is deprecated, superseded by uid_attribute
 #	# default : /lib/udev/scsi_id --whitelisted --device=/dev/%n
 #	#
 #	getuid_callout	"/lib/udev/scsi_id --whitelisted --device=/dev/%n"
multipath/multipath.conf.5 +5 -0
index 70d15f3..ade9885 100644
@@ -144,6 +144,11 @@ The udev attribute providing a unique path
 identifier. Default value is
 .I ID_SERIAL
 .TP
+.B getuid_callout
+The default program and args to callout to obtain a unique path
+identifier. Should be specified with an absolute path.
+This parameter is deprecated; \fIuid_attribute\fR should be used instead.
+.TP
 .B prio
 The name of the path priority routine. The specified routine
 should return a numeric value specifying the relative priority
Signature · 1 line
1.7.10.4
#10 lore raw

PATCH 10/29 kpartx: support disk with non-512B sectors

In reply to Hannes Reinecke

From: Petr Uzel <[email protected]>

libdevmapper expects sector size to be recalculated to 512B, so we need to teach kpartx to do so if the underlying DM device has different sector size (for GPT and msods partition tables).

---
 kpartx/dos.c    |   17 ++++++++++-------
 kpartx/gpt.c    |   20 +-------------------
 kpartx/kpartx.c |   12 ++++++++++++
 kpartx/kpartx.h |    8 ++++++++
 4 files changed, 31 insertions(+), 26 deletions(-)
kpartx/dos.c +10 -7
index a1a9961..0e57f0e 100644
@@ -26,7 +26,9 @@ read_extended_partition(int fd, struct partition *ep, int en,
 	int moretodo = 1;
 	int i, n=0;
 
-	next = start = le32_to_cpu(ep->start_sect);
+	int sector_size_mul = get_sector_size(fd)/512;
+
+	next = start = sector_size_mul * le32_to_cpu(ep->start_sect);
 
 	while (moretodo) {
 		here = next;
@@ -45,14 +47,14 @@ read_extended_partition(int fd, struct partition *ep, int en,
 			memcpy(&p, bp + 0x1be + i * sizeof (p), sizeof (p));
 			if (is_extended(p.sys_type)) {
 				if (p.nr_sects && !moretodo) {
-					next = start + le32_to_cpu(p.start_sect);
+					next = start + sector_size_mul * le32_to_cpu(p.start_sect);
 					moretodo = 1;
 				}
 				continue;
 			}
 			if (n < ns) {
-				sp[n].start = here + le32_to_cpu(p.start_sect);
-				sp[n].size = le32_to_cpu(p.nr_sects);
+				sp[n].start = here + sector_size_mul * le32_to_cpu(p.start_sect);
+				sp[n].size = sector_size_mul * le32_to_cpu(p.nr_sects);
 				sp[n].container = en + 1;
 				n++;
 			} else {
@@ -77,6 +79,7 @@ read_dos_pt(int fd, struct slice all, struct slice *sp, int ns) {
 	unsigned long offset = all.start;
 	int i, n=4;
 	unsigned char *bp;
+	int sector_size_mul = get_sector_size(fd)/512;
 
 	bp = (unsigned char *)getblock(fd, offset);
 	if (bp == NULL)
@@ -90,15 +93,15 @@ read_dos_pt(int fd, struct slice all, struct slice *sp, int ns) {
 		if (is_gpt(p.sys_type))
 			return 0;
 		if (i < ns) {
-			sp[i].start =  le32_to_cpu(p.start_sect);
-			sp[i].size = le32_to_cpu(p.nr_sects);
+			sp[i].start =  sector_size_mul * le32_to_cpu(p.start_sect);
+			sp[i].size = sector_size_mul * le32_to_cpu(p.nr_sects);
 		} else {
 			fprintf(stderr,
 				"dos_partition: too many slices\n");
 			break;
 		}
 		if (is_extended(p.sys_type)) {
-			sp[i].size = 2; /* extended partitions only get two
+			sp[i].size = sector_size_mul * 2; /* extended partitions only get two
 					   sectors mapped for LILO to install */
 			n += read_extended_partition(fd, &p, i, sp+n, ns-n);
 		}
kpartx/gpt.c +1 -19
index 0a22927..5a54970 100644
@@ -38,6 +38,7 @@
 #include <byteswap.h>
 #include <linux/fs.h>
 #include "crc32.h"
+#include "kpartx.h"
 
 #if BYTE_ORDER == LITTLE_ENDIAN
 #  define __le16_to_cpu(x) (x)
@@ -116,25 +117,6 @@ is_pmbr_valid(legacy_mbr *mbr)
 
 
 /************************************************************
- * get_sector_size
- * Requires:
- *  - filedes is an open file descriptor, suitable for reading
- * Modifies: nothing
- * Returns:
- *  sector size, or 512.
- ************************************************************/
-static int
-get_sector_size(int filedes)
-{
-	int rc, sector_size = 512;
-
-	rc = ioctl(filedes, BLKSSZGET, &sector_size);
-	if (rc)
-		sector_size = 512;
-	return sector_size;
-}
-
-/************************************************************
  * _get_num_sectors
  * Requires:
  *  - filedes is an open file descriptor, suitable for reading
kpartx/kpartx.c +12 -0
index 98d88c0..1369542 100644
@@ -26,6 +26,7 @@
 #include <string.h>
 #include <unistd.h>
 #include <stdint.h>
+#include <sys/ioctl.h>
 #include <sys/stat.h>
 #include <sys/types.h>
 #include <ctype.h>
@@ -695,3 +696,14 @@ getblock (int fd, unsigned int secnr) {
 
 	return bp->block;
 }
+
+int
+get_sector_size(int filedes)
+{
+	int rc, sector_size = 512;
+
+	rc = ioctl(filedes, BLKSSZGET, &sector_size);
+	if (rc)
+		sector_size = 512;
+	return sector_size;
+}
kpartx/kpartx.h +8 -0
index 61d31b6..a55c211 100644
@@ -2,6 +2,7 @@
 #define _KPARTX_H
 
 #include <stdint.h>
+#include <sys/ioctl.h>
 
 /*
  * For each partition type there is a routine that takes
@@ -18,6 +19,13 @@
 #define safe_sprintf(var, format, args...)	\
 	snprintf(var, sizeof(var), format, ##args) >= sizeof(var)
 
+#ifndef BLKSSZGET
+#define BLKSSZGET  _IO(0x12,104)	/* get block device sector size */
+#endif
+
+int
+get_sector_size(int filedes);
+
 /*
  * units: 512 byte sectors
  */
Signature · 1 line
1.7.10.4
#11 lore raw

PATCH 11/29 multipath: Add 'Datacore Virtual Disk' to internal hardware table

In reply to Hannes Reinecke

Add 'Datacore Virtual Disk' to internal hardware table.

---
 libmultipath/hwtable.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)
libmultipath/hwtable.c +13 -0
index c317cde..ce12cbb 100644
@@ -1107,6 +1107,19 @@ static struct hwentry default_hw[] = {
 		.prio_name     = PRIO_ALUA,
 		.prio_args     = NULL,
 	},
+	{
+		.vendor	       = "DataCore",
+		.product       = "Virtual Disk",
+		.features      = DEFAULT_FEATURES,
+		.hwhandler     = DEFAULT_HWHANDLER,
+		.pgpolicy      = GROUP_BY_PRIO,
+		.pgfailback    = -FAILBACK_IMMEDIATE,
+		.rr_weight     = RR_WEIGHT_NONE,
+		.no_path_retry = NO_PATH_RETRY_QUEUE,
+		.checker_name  = TUR,
+		.prio_name     = PRIO_ALUA,
+		.prio_args     = NULL,
+	},
 	/*
 	 * EOL
 	 */
Signature · 1 line
1.7.10.4
#12 lore raw

PATCH 12/29 Minor fixes for priority handling

In reply to Hannes Reinecke

When no prio handler was selected we should be setting the priority to 'PRIO_UNDEF'.
Also fixup a typo in the logging message when selecting the default prioritizer.

---
 libmultipath/discovery.c |    1 +
 libmultipath/propsel.c   |    2 +-
 2 files changed, 2 insertions(+), 1 deletion(-)
libmultipath/discovery.c +1 -0
index fcb8e4f..a482c53 100644
@@ -957,6 +957,7 @@ get_prio (struct path * pp)
 		select_prio(pp);
 		if (!prio_selected(p)) {
 			condlog(3, "%s: no prio selected", pp->dev);
+			pp->priority = PRIO_UNDEF;
 			return 1;
 		}
 	}
libmultipath/propsel.c +1 -1
index 65c2c7f..cea8c1c 100644
@@ -447,7 +447,7 @@ select_prio (struct path * pp)
 	prio_get(p, DEFAULT_PRIO, DEFAULT_PRIO_ARGS);
 	condlog(3, "%s: prio = %s (internal default)",
 		pp->dev, DEFAULT_PRIO);
-	condlog(3, "%s: prio = %s (internal default)",
+	condlog(3, "%s: prio args = %s (internal default)",
 		pp->dev, DEFAULT_PRIO_ARGS);
 	return 0;
 }
Signature · 1 line
1.7.10.4
#13 lore raw

PATCH 13/29 Check return value from pathinfo()

In reply to Hannes Reinecke

pathinfo() has a return value, which should be checked to catch any abnormal behaviour.

---
 libmultipath/configure.c |   10 ++++++++--
 libmultipath/discovery.c |    6 ++++--
 multipath/main.c         |   15 ++++++++++-----
 3 files changed, 22 insertions(+), 9 deletions(-)
libmultipath/configure.c +8 -2
index 1cf46cc..7f5e065 100644
@@ -793,8 +793,14 @@ extern int reload_map(struct vectors *vecs, struct multipath *mpp, int refresh)
 
 	update_mpp_paths(mpp, vecs->pathvec);
 	if (refresh) {
-		vector_foreach_slot (mpp->paths, pp, i)
-			pathinfo(pp, conf->hwtable, DI_PRIO);
+		vector_foreach_slot (mpp->paths, pp, i) {
+			r = pathinfo(pp, conf->hwtable, DI_PRIO);
+			if (r) {
+				condlog(2, "%s: failed to refresh pathinfo",
+					mpp->alias);
+				return 1;
+			}
+		}
 	}
 	if (setup_map(mpp, params, PARAMS_SIZE)) {
 		condlog(0, "%s: failed to setup map", mpp->alias);
libmultipath/discovery.c +4 -2
index a482c53..96fe985 100644
@@ -125,8 +125,10 @@ path_discovery (vector pathvec, struct config * conf, int flag)
 			continue;
 		}
 		devtype = udev_device_get_devtype(udevice);
-		if(devtype && !strncmp(devtype, "disk", 4))
-			r += path_discover(pathvec, conf, udevice, flag);
+		if(devtype && !strncmp(devtype, "disk", 4)) {
+			r += path_discover(pathvec, conf,
+						   udevice, flag);
+		}
 		udev_device_unref(udevice);
 	}
 	udev_enumerate_unref(udev_iter);
multipath/main.c +10 -5
index 4a8b966..842a787 100644
@@ -152,16 +152,21 @@ update_paths (struct multipath * mpp)
 					continue;
 				}
 				pp->mpp = mpp;
-				pathinfo(pp, conf->hwtable, DI_ALL);
+				if (pathinfo(pp, conf->hwtable, DI_ALL))
+					pp->state = PATH_UNCHECKED;
 				continue;
 			}
 			pp->mpp = mpp;
 			if (pp->state == PATH_UNCHECKED ||
-			    pp->state == PATH_WILD)
-				pathinfo(pp, conf->hwtable, DI_CHECKER);
+			    pp->state == PATH_WILD) {
+				if (pathinfo(pp, conf->hwtable, DI_CHECKER))
+					pp->state = PATH_UNCHECKED;
+			}
 
-			if (pp->priority == PRIO_UNDEF)
-				pathinfo(pp, conf->hwtable, DI_PRIO);
+			if (pp->priority == PRIO_UNDEF) {
+				if (pathinfo(pp, conf->hwtable, DI_PRIO))
+					pp->priority = PRIO_UNDEF;
+			}
 		}
 	}
 	return 0;
Signature · 1 line
1.7.10.4
#14 lore raw

PATCH 14/29 Read directly from sysfs when checking the device size

In reply to Hannes Reinecke

Device sizes might change, so we need to be reading from sysfs directly to avoid udev still caching the old value.

---
 libmultipath/discovery.c |    5 ++---
 libmultipath/sysfs.c     |    8 +++-----
 2 files changed, 5 insertions(+), 8 deletions(-)
libmultipath/discovery.c +2 -3
index 96fe985..82f086b 100644
@@ -320,7 +320,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	struct udev_device *rport_dev = NULL;
 	char value[11];
 	char rport_id[32];
-	unsigned long long tmo;
+	unsigned long long tmo = 0;
 
 	sprintf(rport_id, "rport-%d:%d-%d",
 		pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id);
@@ -347,6 +347,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	 */
 	value[0] = '\0';
 	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
+	    mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
 		/* Check if we need to temporarily increase dev_loss_tmo */
 		if (sysfs_attr_get_value(rport_dev, "dev_loss_tmo",
@@ -362,8 +363,6 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		}
 		if (mpp->fast_io_fail >= tmo) {
 			snprintf(value, 11, "%u", mpp->fast_io_fail);
-		} else {
-			tmo = 0;
 		}
 	} else if (mpp->dev_loss > 600) {
 		condlog(3, "%s: limiting dev_loss_tmo to 600, since "
libmultipath/sysfs.c +3 -5
index 22b73b1..bab3837 100644
@@ -149,14 +149,13 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 int
 sysfs_get_size (struct path *pp, unsigned long long * size)
 {
-	const char * attr;
+	char attr[255];
 	int r;
 
 	if (!pp->udev)
 		return 1;
 
-	attr = udev_device_get_sysattr_value(pp->udev, "size");
-	if (!attr) {
+	if (sysfs_attr_get_value(pp->udev, "size", attr, 255) == 0) {
 		condlog(3, "%s: No size attribute in sysfs", pp->dev);
 		return 1;
 	}
@@ -164,8 +163,7 @@ sysfs_get_size (struct path *pp, unsigned long long * size)
 	r = sscanf(attr, "%llu\n", size);
 
 	if (r != 1) {
-		condlog(3, "%s: Cannot parse size attribute '%s'",
-			pp->dev, attr);
+		condlog(3, "%s: Cannot parse size attribute", pp->dev);
 		return 1;
 	}
 
Signature · 1 line
1.7.10.4
#15 lore raw

PATCH 15/29 multipath.conf.annotated: Document rr_min_io_rq

In reply to Hannes Reinecke

rr_min_io_rq wasn't documented in multipath.conf.annotated.

---
 multipath.conf.annotated |   18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)
multipath.conf.annotated +16 -2
index 15ec468..7ed62de 100644
@@ -128,10 +128,24 @@
 #	# name    : rr_min_io
 #	# scope   : multipath & multipathd
 #	# desc    : the number of IO to route to a path before switching
-#	#           to the next in the same path group
+#	#           to the next in the same path group for the bio-based
+#	#           multipath implementation. This parameter is used for
+#	#           kernels version up to 2.6.31; newer kernel version
+#	#           use the parameter rr_min_io_rq
 #	# default : 1000
 #	#
-#	rr_min_io	100
+#	rr_min_io	1000
+#
+#	#
+#	# name    : rr_min_io_rq
+#	# scope   : multipath & multipathd
+#	# desc    : the number of IO to route to a path before switching
+#	#           to the next in the same path group for the request-based
+#	#           multipath implementation. This parameter is used for
+#	#           kernels versions later than 2.6.31.
+#	# default : 1
+#	#
+#	rr_min_io_rq	1
 #
 #	#
 #	# name    : flush_on_last_del
Signature · 1 line
1.7.10.4
#16 lore raw

PATCH 16/29 Correctly print out 'max' for max_fds

In reply to Hannes Reinecke

If the value specified for 'max_fds' is the system-wide limit we should be printing out 'max', and not that value.

---
 libmultipath/dict.c |    8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
libmultipath/dict.c +7 -1
index b747fcd..6a58fa2 100644
@@ -2565,10 +2565,16 @@ snprint_def_rr_min_io_rq (char * buff, int len, void * data)
 static int
 snprint_max_fds (char * buff, int len, void * data)
 {
+	int r = 0, max_fds;
+
 	if (!conf->max_fds)
 		return 0;
 
-	return snprintf(buff, len, "%d", conf->max_fds);
+	r = get_sys_max_fds(&max_fds);
+	if (!r && max_fds == conf->max_fds)
+		return snprintf(buff, len, "\"max\"");
+	else
+		return snprintf(buff, len, "%d", conf->max_fds);
 }
 
 static int
Signature · 1 line
1.7.10.4
#17 lore raw

PATCH 17/29 Correctly set max_fds in case of failure

In reply to Hannes Reinecke

If we fail to get the system limit for max_fds we should assume a safe value of 4096.

---
 libmultipath/dict.c |   15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)
libmultipath/dict.c +12 -3
index 6a58fa2..4b840de 100644
@@ -290,18 +290,27 @@ static int
 max_fds_handler(vector strvec)
 {
 	char * buff;
-	int r = 0;
+	int r = 0, max_fds;
 
 	buff = set_value(strvec);
 
 	if (!buff)
 		return 1;
 
+	r = get_sys_max_fds(&max_fds);
+	if (r) {
+		/* Assume safe limit */
+		max_fds = 4096;
+	}
 	if (strlen(buff) == 3 &&
 	    !strcmp(buff, "max"))
-		r = get_sys_max_fds(&conf->max_fds);
+		conf->max_fds = max_fds;
 	else
 		conf->max_fds = atoi(buff);
+
+	if (conf->max_fds > max_fds)
+		conf->max_fds = max_fds;
+
 	FREE(buff);
 
 	return r;
@@ -2571,7 +2580,7 @@ snprint_max_fds (char * buff, int len, void * data)
 		return 0;
 
 	r = get_sys_max_fds(&max_fds);
-	if (!r && max_fds == conf->max_fds)
+	if (!r && conf->max_fds >= max_fds)
 		return snprintf(buff, len, "\"max\"");
 	else
 		return snprintf(buff, len, "%d", conf->max_fds);
Signature · 1 line
1.7.10.4
#18 lore raw

PATCH 18/29 Update multipath.conf.defaults

In reply to Hannes Reinecke
---
 multipath.conf.defaults |  110 ++++++++++++++++++++++++++---------------------
 1 file changed, 62 insertions(+), 48 deletions(-)
multipath.conf.defaults +62 -48
index cd13369..f278f1f 100644
@@ -4,6 +4,7 @@
 #defaults {
 #	verbosity 2
 #	polling_interval 5
+#	multipath_dir "/lib64/multipath"
 #	path_selector "service-time 0"
 #	path_grouping_policy "failover"
 #	uid_attribute "ID_SERIAL"
@@ -12,9 +13,10 @@
 #	features "0"
 #	path_checker "directio"
 #	alias_prefix "mpath"
+#	failback "manual"
 #	rr_min_io 1000
 #	rr_min_io_rq 1
-#	max_fds 1048576
+#	max_fds "max"
 #	rr_weight "uniform"
 #	queue_without_daemon "yes"
 #	flush_on_last_del "no"
@@ -78,7 +80,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "const"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -143,7 +145,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 12
 #		rr_min_io 100
@@ -156,7 +158,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 12
 #		rr_min_io 100
@@ -169,7 +171,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "const"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 18
 #		rr_min_io 100
@@ -182,7 +184,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 18
 #		rr_min_io 100
@@ -195,7 +197,7 @@
 #		features "0"
 #		hardware_handler "1 alua"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 12
 #		rr_min_io 100
@@ -219,7 +221,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 18
 #		rr_min_io 100
@@ -254,7 +256,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "1 emc"
 #		prio "emc"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 60
 #	}
@@ -288,7 +290,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 10
 #	}
@@ -310,7 +312,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "hds"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -332,7 +334,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 300
 #	}
@@ -345,7 +347,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 300
 #	}
@@ -358,7 +360,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 300
 #	}
@@ -371,7 +373,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -384,7 +386,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -397,7 +399,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -410,7 +412,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -423,7 +425,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -436,7 +438,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -478,7 +480,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -499,7 +501,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -534,7 +536,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "1 alua"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -545,7 +547,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #		rr_min_io 100
@@ -570,7 +572,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "const"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 60
 #	}
@@ -582,7 +584,7 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "const"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 60
 #	}
@@ -594,7 +596,7 @@
 #		features "0"
 #		hardware_handler "1 alua"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 60
 #	}
@@ -607,7 +609,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -620,7 +622,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -633,7 +635,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -646,7 +648,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -658,13 +660,13 @@
 #		features "3 queue_if_no_path pg_init_retries 50"
 #		hardware_handler "0"
 #		prio "ontap"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		rr_min_io 128
 #		flush_on_last_del "yes"
 #		dev_loss_tmo "infinity"
-#		retain_attached_hw_handler yes
-#		detect_prio yes
+#		retain_attached_hw_handler "yes"
+#		detect_prio "yes"
 #	}
 #	device {
 #		vendor "NEXENTA"
@@ -674,7 +676,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "const"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 30
 #		rr_min_io 128
@@ -687,7 +689,7 @@
 #		features "1 queue_if_no_path"
 #		hardware_handler "0"
 #		prio "ontap"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		rr_min_io 128
 #	}
@@ -720,7 +722,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -733,7 +735,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -745,7 +747,7 @@
 #		features "0"
 #		hardware_handler "1 alua"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -757,7 +759,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -778,7 +780,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #	}
 #	device {
@@ -811,7 +813,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -824,7 +826,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -836,7 +838,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #		rr_min_io 1000
@@ -851,7 +853,7 @@
 #		features "2 pg_init_retries 50"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry 15
 #	}
@@ -864,7 +866,7 @@
 #		features "0"
 #		hardware_handler "1 rdac"
 #		prio "rdac"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -876,7 +878,7 @@
 #		features "0"
 #		hardware_handler "1 alua"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
@@ -888,7 +890,19 @@
 #		features "0"
 #		hardware_handler "0"
 #		prio "alua"
-#		failback immediate
+#		failback "immediate"
+#		rr_weight "uniform"
+#		no_path_retry "queue"
+#	}
+#	device {
+#		vendor "DataCore"
+#		product "Virtual Disk"
+#		path_grouping_policy "group_by_prio"
+#		path_checker "tur"
+#		features "0"
+#		hardware_handler "0"
+#		prio "alua"
+#		failback "immediate"
 #		rr_weight "uniform"
 #		no_path_retry "queue"
 #	}
Signature · 1 line
1.7.10.4
#19 lore raw

PATCH 19/29 Correctly set pgfailback

In reply to Hannes Reinecke

Something weird happened to pgfailback; no default was assigned when loading the configuration, but then it got set (wrongly) to the default value when printing the configuration.

---
 libmultipath/config.c |    1 +
 libmultipath/dict.c   |   16 +++++-----------
 2 files changed, 6 insertions(+), 11 deletions(-)
libmultipath/config.c +1 -0
index cc44244..8013a07 100644
@@ -556,6 +556,7 @@ load_config (char * file, struct udev *udev)
 	conf->reassign_maps = DEFAULT_REASSIGN_MAPS;
 	conf->checkint = DEFAULT_CHECKINT;
 	conf->max_checkint = MAX_CHECKINT(conf->checkint);
+	conf->pgfailback = DEFAULT_FAILBACK;
 	conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
 	conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
 	conf->detect_prio = DEFAULT_DETECT_PRIO;
libmultipath/dict.c +5 -11
index 4b840de..0408e03 100644
@@ -1869,12 +1869,11 @@ snprint_mp_failback (char * buff, int len, void * data)
 {
 	struct mpentry * mpe = (struct mpentry *)data;
 
-	if (!mpe->pgfailback)
+	if (mpe->pgfailback == FAILBACK_UNDEF ||
+	    mpe->pgfailback == DEFAULT_FAILBACK)
 		return 0;
 
 	switch(mpe->pgfailback) {
-	case  FAILBACK_UNDEF:
-		break;
 	case -FAILBACK_MANUAL:
 		return snprintf(buff, len, "\"manual\"");
 	case -FAILBACK_IMMEDIATE:
@@ -2247,12 +2246,11 @@ snprint_hw_failback (char * buff, int len, void * data)
 {
 	struct hwentry * hwe = (struct hwentry *)data;
 
-	if (!hwe->pgfailback)
+	if (hwe->pgfailback == FAILBACK_UNDEF ||
+	    hwe->pgfailback == DEFAULT_FAILBACK)
 		return 0;
 
 	switch(hwe->pgfailback) {
-	case  FAILBACK_UNDEF:
-		break;
 	case -FAILBACK_MANUAL:
 		return snprintf(buff, len, "\"manual\"");
 	case -FAILBACK_IMMEDIATE:
@@ -2534,13 +2532,9 @@ snprint_def_path_checker (char * buff, int len, void * data)
 static int
 snprint_def_failback (char * buff, int len, void * data)
 {
-	int pgfailback = conf->pgfailback;
-	if (!pgfailback)
-		pgfailback = DEFAULT_FAILBACK;
-
 	switch(conf->pgfailback) {
 	case  FAILBACK_UNDEF:
-		break;
+		return snprintf(buff, len, "\"undef\"");
 	case -FAILBACK_MANUAL:
 		return snprintf(buff, len, "\"manual\"");
 	case -FAILBACK_IMMEDIATE:
Signature · 1 line
1.7.10.4
#20 lore raw

PATCH 20/29 multipath.conf.5: clarify 'no_path_retry' default setting

In reply to Hannes Reinecke

The default setting for 'no_path_retry' is 'unset', so we should be stating this in the man page.

---
 multipath/multipath.conf.5 |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
multipath/multipath.conf.5 +2 -1
index ade9885..a937db9 100644
@@ -295,7 +295,8 @@ Specify the number of retries until disable queueing, or
 .I fail
 for immediate failure (no queueing),
 .I queue
-for never stop queueing. Default is 0.
+for never stop queueing. If unset no queueing is attempted.
+Default is unset.
 .TP
 .B user_friendly_names
 If set to 
Signature · 1 line
1.7.10.4
#21 lore raw

PATCH 21/29 multipath.conf.annotated: remove 'udev_dir'

In reply to Hannes Reinecke

Deprecated, remove it.

---
 multipath.conf.annotated |    7 -------
 1 file changed, 7 deletions(-)
multipath.conf.annotated +0 -7
index 7ed62de..a20302c 100644
@@ -9,13 +9,6 @@
 ##
 #defaults {
 #	#
-#	# name    : udev_dir
-#	# desc    : directory where udev creates its device nodes
-#	# default : /dev
-#	#
-#	udev_dir	/dev
-#
-#	#
 #	# name    : polling_interval
 #	# scope   : multipathd
 #	# desc    : interval between two path checks in seconds. For
Signature · 1 line
1.7.10.4
#22 lore raw

PATCH 22/29 multipath: Implement 'property' blacklist

In reply to Hannes Reinecke

Multipath can only handle device properly which support the VPD page 0x83. Originally this was ensured by 'scsi_id', which would not present an ID_SERIAL value in these cases.
With the move to udev 'ID_SERIAL' is now always present, so multipath would try to attach to _all_ SCSI devices. This patch implements an 'property' blacklist, which allows to blacklist a device based on the existence of udev properties. Any device not providing the udev property from the whitelist will be ignored.
The default whitelist is set to '(ID_WWN|ID_SCSI_VPD)'.

---
 libmultipath/blacklist.c   |  105 +++++++++++++++++++++++++++++++++++---------
 libmultipath/blacklist.h   |   19 +++++---
 libmultipath/config.c      |   19 +++++++-
 libmultipath/config.h      |    2 +
 libmultipath/dict.c        |   36 ++++++++++++++-
 libmultipath/discovery.c   |    3 ++
 libmultipath/print.c       |   31 +++++++++++++
 multipath.conf.defaults    |    1 +
 multipath/multipath.conf.5 |   19 +++++++-
 9 files changed, 203 insertions(+), 32 deletions(-)
libmultipath/blacklist.c +85 -20
index 49a40f9..d597350 100644
@@ -2,6 +2,7 @@
  * Copyright (c) 2004, 2005 Christophe Varoqui
  */
 #include <stdio.h>
+#include <libudev.h>
 
 #include "checkers.h"
 #include "memory.h"
@@ -96,7 +97,7 @@ set_ble_device (vector blist, char * vendor, char * product, int origin)
 }
 
 int
-_blacklist_exceptions (vector elist, char * str)
+_blacklist_exceptions (vector elist, const char * str)
 {
 	int i;
 	struct blentry * ele;
@@ -109,7 +110,7 @@ _blacklist_exceptions (vector elist, char * str)
 }
 
 int
-_blacklist (vector blist, char * str)
+_blacklist (vector blist, const char * str)
 {
 	int i;
 	struct blentry * ble;
@@ -175,6 +176,12 @@ setup_default_blist (struct config * conf)
 	if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
 		return 1;
 
+	str = STRDUP("(ID_SCSI_VPD|ID_WWN)");
+	if (!str)
+		return 1;
+	if (store_ble(conf->elist_property, str, ORIGIN_DEFAULT))
+		return 1;
+
 	vector_foreach_slot (conf->hwtable, hwe, i) {
 		if (hwe->bl_product) {
 			if (_blacklist_device(conf->blist_device, hwe->vendor,
@@ -196,16 +203,20 @@ setup_default_blist (struct config * conf)
 	return 0;
 }
 
-#define LOG_BLIST(M) \
-	if (vendor && product)						 \
-		condlog(3, "%s: (%s:%s) %s", dev, vendor, product, (M)); \
-	else if (wwid)							 \
-		condlog(3, "%s: (%s) %s", dev, wwid, (M));		 \
-	else								 \
-		condlog(3, "%s: %s", dev, (M))
+#define LOG_BLIST(M,S)							\
+	if (vendor && product)						\
+		condlog(3, "%s: (%s:%s) %s %s",				\
+			dev, vendor, product, (M), (S));		\
+	else if (wwid)							\
+		condlog(3, "%s: %s %s %s", dev, (M), wwid, (S));	\
+	else if (env)							\
+		condlog(3, "%s: %s %s %s", dev, (M), env, (S));		\
+	else								\
+		condlog(3, "%s: %s %s", dev, (M), (S))
 
 void
-log_filter (char *dev, char *vendor, char *product, char *wwid, int r)
+log_filter (const char *dev, char *vendor, char *product, char *wwid,
+	    const char *env, int r)
 {
 	/*
 	 * Try to sort from most likely to least.
@@ -214,22 +225,31 @@ log_filter (char *dev, char *vendor, char *product, char *wwid, int r)
 	case MATCH_NOTHING:
 		break;
 	case MATCH_DEVICE_BLIST:
-		LOG_BLIST("vendor/product blacklisted");
+		LOG_BLIST("vendor/product", "blacklisted");
 		break;
 	case MATCH_WWID_BLIST:
-		LOG_BLIST("wwid blacklisted");
+		LOG_BLIST("wwid", "blacklisted");
 		break;
 	case MATCH_DEVNODE_BLIST:
-		LOG_BLIST("device node name blacklisted");
+		LOG_BLIST("device node name", "blacklisted");
+		break;
+	case MATCH_PROPERTY_BLIST:
+		LOG_BLIST("udev property", "blacklisted");
 		break;
 	case MATCH_DEVICE_BLIST_EXCEPT:
-		LOG_BLIST("vendor/product whitelisted");
+		LOG_BLIST("vendor/product", "whitelisted");
 		break;
 	case MATCH_WWID_BLIST_EXCEPT:
-		LOG_BLIST("wwid whitelisted");
+		LOG_BLIST("wwid", "whitelisted");
 		break;
 	case MATCH_DEVNODE_BLIST_EXCEPT:
-		LOG_BLIST("device node name whitelisted");
+		LOG_BLIST("device node name", "whitelisted");
+		break;
+	case MATCH_PROPERTY_BLIST_EXCEPT:
+		LOG_BLIST("udev property", "whitelisted");
+		break;
+	case MATCH_PROPERTY_BLIST_MISSING:
+		LOG_BLIST("blacklisted,", "udev property missing");
 		break;
 	}
 }
@@ -250,7 +270,7 @@ int
 filter_device (vector blist, vector elist, char * vendor, char * product)
 {
 	int r = _filter_device(blist, elist, vendor, product);
-	log_filter(NULL, vendor, product, NULL, r);
+	log_filter(NULL, vendor, product, NULL, NULL, r);
 	return r;
 }
 
@@ -270,7 +290,7 @@ int
 filter_devnode (vector blist, vector elist, char * dev)
 {
 	int r = _filter_devnode(blist, elist, dev);
-	log_filter(dev, NULL, NULL, NULL, r);
+	log_filter(dev, NULL, NULL, NULL, NULL, r);
 	return r;
 }
 
@@ -290,7 +310,7 @@ int
 filter_wwid (vector blist, vector elist, char * wwid)
 {
 	int r = _filter_wwid(blist, elist, wwid);
-	log_filter(NULL, NULL, NULL, wwid, r);
+	log_filter(NULL, NULL, NULL, wwid, NULL, r);
 	return r;
 }
 
@@ -314,10 +334,55 @@ int
 filter_path (struct config * conf, struct path * pp)
 {
 	int r=_filter_path(conf, pp);
-	log_filter(pp->dev, pp->vendor_id, pp->product_id, pp->wwid, r);
+	log_filter(pp->dev, pp->vendor_id, pp->product_id, pp->wwid, NULL, r);
 	return r;
 }
 
+int
+_filter_property (struct config *conf, const char *env)
+{
+	if (_blacklist_exceptions(conf->elist_property, env))
+		return MATCH_PROPERTY_BLIST_EXCEPT;
+	if (_blacklist(conf->blist_property, env))
+		return MATCH_PROPERTY_BLIST;
+
+	return 0;
+}
+
+int
+filter_property(struct config * conf, struct udev_device * udev)
+{
+	const char *devname = udev_device_get_sysname(udev);
+	struct udev_list_entry *list_entry;
+	int r;
+
+	if (!udev)
+		return 0;
+
+	udev_list_entry_foreach(list_entry,
+				udev_device_get_properties_list_entry(udev)) {
+		const char *env;
+
+		env = udev_list_entry_get_name(list_entry);
+		if (!env)
+			continue;
+
+		r = _filter_property(conf, env);
+		if (r) {
+			log_filter(devname, NULL, NULL, NULL, env, r);
+			return r;
+		}
+	}
+
+	/*
+	 * This is the inverse of the 'normal' matching;
+	 * the environment variable _has_ to match.
+	 */
+	log_filter(devname, NULL, NULL, NULL, NULL,
+		   MATCH_PROPERTY_BLIST_MISSING);
+	return MATCH_PROPERTY_BLIST_MISSING;
+}
+
 void
 free_blacklist (vector blist)
 {
libmultipath/blacklist.h +12 -7
index cdbebef..0e90e9a 100644
@@ -1,15 +1,19 @@
 #ifndef _BLACKLIST_H
 #define _BLACKLIST_H
 
+#include <libudev.h>
 #include "regex.h"
 
-#define MATCH_NOTHING       0
-#define MATCH_WWID_BLIST    1
-#define MATCH_DEVICE_BLIST  2
-#define MATCH_DEVNODE_BLIST 3
-#define MATCH_WWID_BLIST_EXCEPT    -MATCH_WWID_BLIST
-#define MATCH_DEVICE_BLIST_EXCEPT  -MATCH_DEVICE_BLIST
-#define MATCH_DEVNODE_BLIST_EXCEPT -MATCH_DEVNODE_BLIST
+#define MATCH_NOTHING        0
+#define MATCH_WWID_BLIST     1
+#define MATCH_DEVICE_BLIST   2
+#define MATCH_DEVNODE_BLIST  3
+#define MATCH_PROPERTY_BLIST 4
+#define MATCH_PROPERTY_BLIST_MISSING 5
+#define MATCH_WWID_BLIST_EXCEPT     -MATCH_WWID_BLIST
+#define MATCH_DEVICE_BLIST_EXCEPT   -MATCH_DEVICE_BLIST
+#define MATCH_DEVNODE_BLIST_EXCEPT  -MATCH_DEVNODE_BLIST
+#define MATCH_PROPERTY_BLIST_EXCEPT -MATCH_PROPERTY_BLIST
 
 struct blentry {
 	char * str;
@@ -31,6 +35,7 @@ int filter_devnode (vector, vector, char *);
 int filter_wwid (vector, vector, char *);
 int filter_device (vector, vector, char *, char *);
 int filter_path (struct config *, struct path *);
+int filter_property(struct config *, struct udev_device *);
 int store_ble (vector, char *, int);
 int set_ble_device (vector, char *, char *, int);
 void free_blacklist (vector);
libmultipath/config.c +17 -2
index 8013a07..9b7adda 100644
@@ -513,10 +513,12 @@ free_config (struct config * conf)
 
 	free_blacklist(conf->blist_devnode);
 	free_blacklist(conf->blist_wwid);
+	free_blacklist(conf->blist_property);
 	free_blacklist_device(conf->blist_device);
 
 	free_blacklist(conf->elist_devnode);
 	free_blacklist(conf->elist_wwid);
+	free_blacklist(conf->elist_property);
 	free_blacklist_device(conf->elist_device);
 
 	free_mptable(conf->mptable);
@@ -619,8 +621,12 @@ load_config (char * file, struct udev *udev)
 		if (!conf->blist_device)
 			goto out;
 	}
-	if (setup_default_blist(conf))
-		goto out;
+	if (conf->blist_property == NULL) {
+		conf->blist_property = vector_alloc();
+
+		if (!conf->blist_property)
+			goto out;
+	}
 
 	if (conf->elist_devnode == NULL) {
 		conf->elist_devnode = vector_alloc();
@@ -642,6 +648,15 @@ load_config (char * file, struct udev *udev)
 			goto out;
 	}
 
+	if (conf->elist_property == NULL) {
+		conf->elist_property = vector_alloc();
+
+		if (!conf->elist_property)
+			goto out;
+	}
+	if (setup_default_blist(conf))
+		goto out;
+
 	if (conf->mptable == NULL) {
 		conf->mptable = vector_alloc();
 		if (!conf->mptable)
libmultipath/config.h +2 -0
index ca65f88..9c467e8 100644
@@ -136,9 +136,11 @@ struct config {
 	vector blist_devnode;
 	vector blist_wwid;
 	vector blist_device;
+	vector blist_property;
 	vector elist_devnode;
 	vector elist_wwid;
 	vector elist_device;
+	vector elist_property;
 };
 
 struct config * conf;
libmultipath/dict.c +34 -2
index 0408e03..0bf9587 100644
@@ -691,8 +691,10 @@ blacklist_handler(vector strvec)
 	conf->blist_devnode = vector_alloc();
 	conf->blist_wwid = vector_alloc();
 	conf->blist_device = vector_alloc();
+	conf->blist_property = vector_alloc();
 
-	if (!conf->blist_devnode || !conf->blist_wwid || !conf->blist_device)
+	if (!conf->blist_devnode || !conf->blist_wwid ||
+	    !conf->blist_device || !conf->blist_property)
 		return 1;
 
 	return 0;
@@ -704,8 +706,10 @@ blacklist_exceptions_handler(vector strvec)
 	conf->elist_devnode = vector_alloc();
 	conf->elist_wwid = vector_alloc();
 	conf->elist_device = vector_alloc();
+	conf->elist_property = vector_alloc();
 
-	if (!conf->elist_devnode || !conf->elist_wwid || !conf->elist_device)
+	if (!conf->elist_devnode || !conf->elist_wwid ||
+	    !conf->elist_device || !conf->elist_property)
 		return 1;
 
 	return 0;
@@ -764,6 +768,32 @@ ble_except_wwid_handler(vector strvec)
 }
 
 static int
+ble_property_handler(vector strvec)
+{
+	char * buff;
+
+	buff = set_value(strvec);
+
+	if (!buff)
+		return 1;
+
+	return store_ble(conf->blist_property, buff, ORIGIN_CONFIG);
+}
+
+static int
+ble_except_property_handler(vector strvec)
+{
+	char * buff;
+
+	buff = set_value(strvec);
+
+	if (!buff)
+		return 1;
+
+	return store_ble(conf->elist_property, buff, ORIGIN_CONFIG);
+}
+
+static int
 ble_device_handler(vector strvec)
 {
 	return alloc_ble_device(conf->blist_device);
@@ -2830,6 +2860,7 @@ init_keywords(void)
 	install_keyword_root("blacklist", &blacklist_handler);
 	install_keyword_multi("devnode", &ble_devnode_handler, &snprint_ble_simple);
 	install_keyword_multi("wwid", &ble_wwid_handler, &snprint_ble_simple);
+	install_keyword_multi("property", &ble_property_handler, &snprint_ble_simple);
 	install_keyword_multi("device", &ble_device_handler, NULL);
 	install_sublevel();
 	install_keyword("vendor", &ble_vendor_handler, &snprint_bled_vendor);
@@ -2838,6 +2869,7 @@ init_keywords(void)
 	install_keyword_root("blacklist_exceptions", &blacklist_exceptions_handler);
 	install_keyword_multi("devnode", &ble_except_devnode_handler, &snprint_ble_simple);
 	install_keyword_multi("wwid", &ble_except_wwid_handler, &snprint_ble_simple);
+	install_keyword_multi("property", &ble_except_property_handler, &snprint_ble_simple);
 	install_keyword_multi("device", &ble_except_device_handler, NULL);
 	install_sublevel();
 	install_keyword("vendor", &ble_except_vendor_handler, &snprint_bled_vendor);
libmultipath/discovery.c +3 -0
index 82f086b..e9c6a50 100644
@@ -82,6 +82,9 @@ path_discover (vector pathvec, struct config * conf,
 	if (!devname)
 		return 0;
 
+	if (filter_property(conf, udevice) > 0)
+		return 0;
+
 	if (filter_devnode(conf->blist_devnode, conf->elist_devnode,
 			   (char *)devname) > 0)
 		return 0;
libmultipath/print.c +31 -0
index b6d08b7..7feeb26 100644
@@ -1078,6 +1078,19 @@ snprint_blacklist_report (char * buff, int len)
 
 	if ((len - fwd - threshold) <= 0)
 		return len;
+	fwd += snprintf(buff + fwd, len - fwd, "udev property rules:\n"
+					       "- blacklist:\n");
+	if (!snprint_blacklist_group(buff, len, &fwd, &conf->blist_property))
+		return len;
+
+	if ((len - fwd - threshold) <= 0)
+		return len;
+	fwd += snprintf(buff + fwd, len - fwd, "- exceptions:\n");
+	if (snprint_blacklist_group(buff, len, &fwd, &conf->elist_property) == 0)
+		return len;
+
+	if ((len - fwd - threshold) <= 0)
+		return len;
 	fwd += snprintf(buff + fwd, len - fwd, "wwid rules:\n"
 					       "- blacklist:\n");
 	if (snprint_blacklist_group(buff, len, &fwd, &conf->blist_wwid) == 0)
@@ -1143,6 +1156,15 @@ snprint_blacklist (char * buff, int len)
 		if (fwd > len)
 			return len;
 	}
+	vector_foreach_slot (conf->blist_property, ble, i) {
+		kw = find_keyword(rootkw->sub, "property");
+		if (!kw)
+			return 0;
+		fwd += snprint_keyword(buff + fwd, len - fwd, "\t%k %v\n",
+				       kw, ble);
+		if (fwd > len)
+			return len;
+	}
 	rootkw = find_keyword(rootkw->sub, "device");
 	if (!rootkw)
 		return 0;
@@ -1211,6 +1233,15 @@ snprint_blacklist_except (char * buff, int len)
 		if (fwd > len)
 			return len;
 	}
+	vector_foreach_slot (conf->elist_property, ele, i) {
+		kw = find_keyword(rootkw->sub, "property");
+		if (!kw)
+			return 0;
+		fwd += snprint_keyword(buff + fwd, len - fwd, "\t%k %v\n",
+				       kw, ele);
+		if (fwd > len)
+			return len;
+	}
 	rootkw = find_keyword(rootkw->sub, "device");
 	if (!rootkw)
 		return 0;
multipath.conf.defaults +1 -0
index f278f1f..636c1e6 100644
@@ -70,6 +70,7 @@
 #	}
 #}
 #blacklist_exceptions {
+#	property "(ID_SCSI_VPD|ID_WWN)"
 #}
 #devices {
 #	device {
multipath/multipath.conf.5 +18 -1
index a937db9..0fd3035 100644
@@ -422,6 +422,9 @@ The \fIWorld Wide Identification\fR of a device.
 .B devnode
 Regular expression of the device nodes to be excluded.
 .TP
+.B property
+Regular expresion of the udev property to be excluded.
+.TP
 .B device
 Subsection for the device description. This subsection recognizes the
 .I vendor
@@ -446,8 +449,12 @@ The following keywords are recognized:
 .B wwid
 The \fIWorld Wide Identification\fR of a device.
 .TP
+.B property
+Regular expresion of the udev property to be whitelisted. Defaults to
+.I (ID_WWN|ID_SCSI_VPD)
+.TP
 .B devnode
-Regular expression of the device nodes to be excluded.
+Regular expression of the device nodes to be whitelisted.
 .TP
 .B device
 Subsection for the device description. This subsection recognizes the
@@ -457,6 +464,16 @@ and
 keywords. For a full description of these keywords please see the
 .I devices
 section description.
+.LP
+The
+.I property
+blacklist and whitelist handling is different from the usual handling
+in the sense that the whitelist
+.B has
+to be set, otherwise the device will be blacklisted.
+In these cases the message
+.I blacklisted, udev property missing
+will be displayed.
 .SH "multipaths section"
 The only recognized attribute for the
 .B multipaths
Signature · 1 line
1.7.10.4
#23 lore raw

PATCH 23/29 Do not print error when rport is blocked

In reply to Hannes Reinecke

When an rport is blocked any write to the dev_loss_tmo attribute will fail with EBUSY. But that's perfectly normal and nothing to worry about, so decrease the logging priority for these cases.

---
 libmultipath/discovery.c |   52 ++++++++++++++++++++++++++++++----------------
 libmultipath/sysfs.c     |   20 +++++++++---------
 2 files changed, 44 insertions(+), 28 deletions(-)
libmultipath/discovery.c +34 -18
index e9c6a50..092930a 100644
@@ -324,6 +324,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	char value[11];
 	char rport_id[32];
 	unsigned long long tmo = 0;
+	int ret;
 
 	sprintf(rport_id, "rport-%d:%d-%d",
 		pp->sg_id.host_no, pp->sg_id.channel, pp->sg_id.transport_id);
@@ -353,15 +354,16 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
 		/* Check if we need to temporarily increase dev_loss_tmo */
-		if (sysfs_attr_get_value(rport_dev, "dev_loss_tmo",
-					 value, 16) <= 0) {
+		ret = sysfs_attr_get_value(rport_dev, "dev_loss_tmo",
+					   value, 16);
+		if (ret <= 0) {
 			condlog(0, "%s: failed to read dev_loss_tmo value, "
-				"error %d", pp->dev, errno);
+				"error %d", rport_id, -ret);
 			goto out;
 		}
 		if (sscanf(value, "%llu\n", &tmo) != 1) {
 			condlog(0, "%s: Cannot parse dev_loss_tmo "
-				"attribute '%s'",pp->dev, value);
+				"attribute '%s'", rport_id, value);
 			goto out;
 		}
 		if (mpp->fast_io_fail >= tmo) {
@@ -369,16 +371,21 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		}
 	} else if (mpp->dev_loss > 600) {
 		condlog(3, "%s: limiting dev_loss_tmo to 600, since "
-			"fast_io_fail is not set", pp->dev);
+			"fast_io_fail is not set", rport_id);
 		snprintf(value, 11, "%u", 600);
 	} else {
 		snprintf(value, 11, "%u", mpp->dev_loss);
 	}
-	if (strlen(value) &&
-	    sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11) <= 0) {
-		condlog(0, "%s failed to set dev_loss_tmo",
-			mpp->alias);
-		goto out;
+	if (strlen(value)) {
+		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11);
+		if (ret <= 0) {
+			if (ret == -EBUSY)
+				condlog(3, "%s: rport blocked", rport_id);
+			else
+				condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d",
+					rport_id, value, -ret);
+			goto out;
+		}
 	}
 	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET) {
 		if (mpp->fast_io_fail == MP_FAST_IO_FAIL_OFF)
@@ -387,18 +394,27 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 			sprintf(value, "0");
 		else
 			snprintf(value, 11, "%u", mpp->fast_io_fail);
-		if (sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
-					 value, 11) <= 0) {
-			condlog(0, "%s failed to set fast_io_fail_tmo",
-				mpp->alias);
+		ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
+					   value, 11);
+		if (ret <= 0) {
+			if (ret == -EBUSY)
+				condlog(3, "%s: rport blocked", rport_id);
+			else
+				condlog(0, "%s: failed to set fast_io_fail_tmo to %s, error %d",
+					rport_id, value, -ret);
 		}
 	}
 	if (tmo > 0) {
 		snprintf(value, 11, "%u", mpp->dev_loss);
-		if (sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-					 value, 11) <= 0)
-			condlog(0, "%s failed to set dev_loss_tmo",
-				mpp->alias);
+		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
+					   value, 11);
+		if (ret <= 0) {
+			if (ret == -EBUSY)
+				condlog(3, "%s: rport blocked", rport_id);
+			else
+				condlog(0, "%s: failed to set dev_loss_tmo to %s, error %d",
+					rport_id, value, -ret);
+		}
 	}
 out:
 	udev_device_unref(rport_dev);
libmultipath/sysfs.c +10 -10
index bab3837..0670e0a 100644
@@ -59,19 +59,19 @@ ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
 	condlog(4, "open '%s'", devpath);
 	if (stat(devpath, &statbuf) != 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
-		return 0;
+		return -errno;
 	}
 
 	/* skip directories */
 	if (S_ISDIR(statbuf.st_mode)) {
 		condlog(4, "%s is a directory", devpath);
-		return 0;
+		return -EISDIR;
 	}
 
 	/* skip non-writeable files */
 	if ((statbuf.st_mode & S_IRUSR) == 0) {
 		condlog(4, "%s is not readable", devpath);
-		return 0;
+		return -EPERM;
 	}
 
 	/* read attribute value */
@@ -79,12 +79,12 @@ ssize_t sysfs_attr_get_value(struct udev_device *dev, const char *attr_name,
 	if (fd < 0) {
 		condlog(4, "attribute '%s' can not be opened: %s",
 			devpath, strerror(errno));
-		return 0;
+		return -errno;
 	}
 	size = read(fd, value, value_len);
 	if (size < 0) {
 		condlog(4, "read from %s failed: %s", devpath, strerror(errno));
-		size = 0;
+		size = -errno;
 	} else if (size == value_len) {
 		condlog(4, "overflow while reading from %s", devpath);
 		size = 0;
@@ -110,19 +110,19 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 	condlog(4, "open '%s'", devpath);
 	if (stat(devpath, &statbuf) != 0) {
 		condlog(4, "stat '%s' failed: %s", devpath, strerror(errno));
-		return 0;
+		return -errno;
 	}
 
 	/* skip directories */
 	if (S_ISDIR(statbuf.st_mode)) {
 		condlog(4, "%s is a directory", devpath);
-		return 0;
+		return -EISDIR;
 	}
 
 	/* skip non-writeable files */
 	if ((statbuf.st_mode & S_IWUSR) == 0) {
 		condlog(4, "%s is not writeable", devpath);
-		return 0;
+		return -EPERM;
 	}
 
 	/* write attribute value */
@@ -130,12 +130,12 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 	if (fd < 0) {
 		condlog(4, "attribute '%s' can not be opened: %s",
 			devpath, strerror(errno));
-		return 0;
+		return -errno;
 	}
 	size = write(fd, value, value_len);
 	if (size < 0) {
 		condlog(4, "write to %s failed: %s", devpath, strerror(errno));
-		size = 0;
+		size = -errno;
 	} else if (size < value_len) {
 		condlog(4, "tried to write %ld to %s. Wrote %ld",
 			(long)value_len, devpath, (long)size);
Signature · 1 line
1.7.10.4
#24 lore raw

PATCH 24/29 multipath: reference the udev context when starting event queue

In reply to Hannes Reinecke

The uevent listener is running asynchronously, so it might still be active and receiving events when the main thread is already shut down. So it need to take a separate reference to the udev context to avoid the context becoming invalid while the listener is running.

---
 libmultipath/uevent.c |   16 ++++++++++++----
 libmultipath/uevent.h |    4 +++-
 multipathd/main.c     |    4 ++--
 3 files changed, 17 insertions(+), 7 deletions(-)
libmultipath/uevent.c +12 -4
index 0643e14..9ee3ade 100644
@@ -47,7 +47,6 @@
 #include "list.h"
 #include "uevent.h"
 #include "vector.h"
-#include "config.h"
 
 typedef int (uev_trigger)(struct uevent *, void * trigger_data);
 
@@ -127,11 +126,14 @@ service_uevq(struct list_head *tmpq)
 
 static void uevq_stop(void *arg)
 {
+	struct udev *udev = arg;
+
 	condlog(3, "Stopping uev queue");
 	pthread_mutex_lock(uevq_lockp);
 	my_uev_trigger = NULL;
 	pthread_cond_signal(uev_condp);
 	pthread_mutex_unlock(uevq_lockp);
+	udev_unref(udev);
 }
 
 void
@@ -399,7 +401,7 @@ exit:
 	return 1;
 }
 
-int uevent_listen(void)
+int uevent_listen(struct udev *udev)
 {
 	int err;
 	struct udev_monitor *monitor = NULL;
@@ -411,11 +413,17 @@ int uevent_listen(void)
 	 * thereby not getting to empty the socket's receive buffer queue
 	 * often enough.
 	 */
-	pthread_cleanup_push(uevq_stop, NULL);
+	if (!udev) {
+		condlog(1, "no udev context");
+		return 1;
+	}
+	udev_ref(udev);
+	pthread_cleanup_push(uevq_stop, udev);
 
-	monitor = udev_monitor_new_from_netlink(conf->udev, "udev");
+	monitor = udev_monitor_new_from_netlink(udev, "udev");
 	if (!monitor) {
 		condlog(2, "failed to create udev monitor");
+		err = 2;
 		goto out;
 	}
 #ifdef LIBUDEV_API_RECVBUF
libmultipath/uevent.h +3 -1
index 762595a..e5fdfcc 100644
@@ -13,6 +13,8 @@
 #define NETLINK_KOBJECT_UEVENT		15
 #endif
 
+struct udev;
+
 struct uevent {
 	struct list_head node;
 	struct udev_device *udev;
@@ -27,7 +29,7 @@ struct uevent {
 int is_uevent_busy(void);
 void setup_thread_attr(pthread_attr_t *attr, size_t stacksize, int detached);
 
-int uevent_listen(void);
+int uevent_listen(struct udev *udev);
 int uevent_dispatch(int (*store_uev)(struct uevent *, void * trigger_data),
 		    void * trigger_data);
 int uevent_get_major(struct uevent *uev);
multipathd/main.c +2 -2
index aa5a298..b41fc64 100644
@@ -835,7 +835,7 @@ out:
 static void *
 ueventloop (void * ap)
 {
-	if (uevent_listen())
+	if (uevent_listen(udev))
 		condlog(0, "error starting uevent listener");
 
 	return NULL;
@@ -1658,7 +1658,7 @@ child (void * param)
 	/*
 	 * Start uevent listener early to catch events
 	 */
-	if ((rc = pthread_create(&uevent_thr, &misc_attr, ueventloop, vecs))) {
+	if ((rc = pthread_create(&uevent_thr, &misc_attr, ueventloop, udev))) {
 		condlog(0, "failed to create uevent thread: %d", rc);
 		exit(1);
 	}
Signature · 1 line
1.7.10.4
#25 lore raw

PATCH 25/29 multipathd: valgrind fixes

In reply to Hannes Reinecke

valgrind complained about uninitialized memory.
As usual, valgrind was right, although the memory never was actually referenced.

---
 libmultipath/discovery.c |   27 +++++++++++++++++----------
 libmultipath/sysfs.c     |    6 ++++--
 2 files changed, 21 insertions(+), 12 deletions(-)
libmultipath/discovery.c +17 -10
index 092930a..c59dbc2 100644
@@ -321,7 +321,7 @@ static void
 sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 {
 	struct udev_device *rport_dev = NULL;
-	char value[11];
+	char value[16];
 	char rport_id[32];
 	unsigned long long tmo = 0;
 	int ret;
@@ -349,7 +349,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 	 * then set fast_io_fail, and _then_ set dev_loss_tmo
 	 * to the correct value.
 	 */
-	value[0] = '\0';
+	memset(value, 0, 16);
 	if (mpp->fast_io_fail != MP_FAST_IO_FAIL_UNSET &&
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_ZERO &&
 	    mpp->fast_io_fail != MP_FAST_IO_FAIL_OFF) {
@@ -367,17 +367,18 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 			goto out;
 		}
 		if (mpp->fast_io_fail >= tmo) {
-			snprintf(value, 11, "%u", mpp->fast_io_fail);
+			snprintf(value, 16, "%u", mpp->fast_io_fail);
 		}
 	} else if (mpp->dev_loss > 600) {
 		condlog(3, "%s: limiting dev_loss_tmo to 600, since "
 			"fast_io_fail is not set", rport_id);
-		snprintf(value, 11, "%u", 600);
+		snprintf(value, 16, "%u", 600);
 	} else {
-		snprintf(value, 11, "%u", mpp->dev_loss);
+		snprintf(value, 16, "%u", mpp->dev_loss);
 	}
 	if (strlen(value)) {
-		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo", value, 11);
+		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
+					   value, strlen(value));
 		if (ret <= 0) {
 			if (ret == -EBUSY)
 				condlog(3, "%s: rport blocked", rport_id);
@@ -393,9 +394,9 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		else if (mpp->fast_io_fail == MP_FAST_IO_FAIL_ZERO)
 			sprintf(value, "0");
 		else
-			snprintf(value, 11, "%u", mpp->fast_io_fail);
+			snprintf(value, 16, "%u", mpp->fast_io_fail);
 		ret = sysfs_attr_set_value(rport_dev, "fast_io_fail_tmo",
-					   value, 11);
+					   value, strlen(value));
 		if (ret <= 0) {
 			if (ret == -EBUSY)
 				condlog(3, "%s: rport blocked", rport_id);
@@ -405,9 +406,9 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 		}
 	}
 	if (tmo > 0) {
-		snprintf(value, 11, "%u", mpp->dev_loss);
+		snprintf(value, 16, "%u", mpp->dev_loss);
 		ret = sysfs_attr_set_value(rport_dev, "dev_loss_tmo",
-					   value, 11);
+					   value, strlen(value));
 		if (ret <= 0) {
 			if (ret == -EBUSY)
 				condlog(3, "%s: rport blocked", rport_id);
@@ -809,6 +810,9 @@ cciss_sysfs_pathinfo (struct path * pp)
 static int
 common_sysfs_pathinfo (struct path * pp)
 {
+	if (!pp)
+		return 1;
+
 	if (!pp->udev) {
 		condlog(4, "%s: udev not initialised", pp->dev);
 		return 1;
@@ -1059,6 +1063,9 @@ pathinfo (struct path *pp, vector hwtable, int mask)
 {
 	int path_state;
 
+	if (!pp)
+		return 1;
+
 	condlog(3, "%s: mask = 0x%x", pp->dev, mask);
 
 	/*
libmultipath/sysfs.c +4 -2
index 0670e0a..8ba27d4 100644
@@ -102,7 +102,7 @@ ssize_t sysfs_attr_set_value(struct udev_device *dev, const char *attr_name,
 	int fd;
 	ssize_t size = -1;
 
-	if (!dev || !attr_name || !value)
+	if (!dev || !attr_name || !value || !value_len)
 		return 0;
 
 	snprintf(devpath, PATH_SIZE, "%s/%s", udev_device_get_syspath(dev),
@@ -152,9 +152,10 @@ sysfs_get_size (struct path *pp, unsigned long long * size)
 	char attr[255];
 	int r;
 
-	if (!pp->udev)
+	if (!pp->udev || !size)
 		return 1;
 
+	attr[0] = '\0';
 	if (sysfs_attr_get_value(pp->udev, "size", attr, 255) == 0) {
 		condlog(3, "%s: No size attribute in sysfs", pp->dev);
 		return 1;
@@ -164,6 +165,7 @@ sysfs_get_size (struct path *pp, unsigned long long * size)
 
 	if (r != 1) {
 		condlog(3, "%s: Cannot parse size attribute", pp->dev);
+		*size = 0;
 		return 1;
 	}
 
Signature · 1 line
1.7.10.4
#26 lore raw

PATCH 26/29 multipathd: increase stacksize for uevent listener

In reply to Hannes Reinecke

libudev has quite some heavy stack usage, so the default stack size is not enough here.

---
 multipathd/main.c |    6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
multipathd/main.c +4 -2
index b41fc64..8681aa2 100644
@@ -1580,7 +1580,7 @@ static int
 child (void * param)
 {
 	pthread_t check_thr, uevent_thr, uxlsnr_thr, uevq_thr;
-	pthread_attr_t log_attr, misc_attr;
+	pthread_attr_t log_attr, misc_attr, uevent_attr;
 	struct vectors * vecs;
 	struct multipath * mpp;
 	int i;
@@ -1593,6 +1593,7 @@ child (void * param)
 	udev = udev_new();
 
 	setup_thread_attr(&misc_attr, 64 * 1024, 1);
+	setup_thread_attr(&uevent_attr, 128 * 1024, 1);
 	setup_thread_attr(&waiter_attr, 32 * 1024, 1);
 
 	if (logsink) {
@@ -1658,10 +1659,11 @@ child (void * param)
 	/*
 	 * Start uevent listener early to catch events
 	 */
-	if ((rc = pthread_create(&uevent_thr, &misc_attr, ueventloop, udev))) {
+	if ((rc = pthread_create(&uevent_thr, &uevent_attr, ueventloop, udev))) {
 		condlog(0, "failed to create uevent thread: %d", rc);
 		exit(1);
 	}
+	pthread_attr_destroy(&uevent_attr);
 	if ((rc = pthread_create(&uxlsnr_thr, &misc_attr, uxlsnrloop, vecs))) {
 		condlog(0, "failed to create cli listener: %d", rc);
 		exit(1);
Signature · 1 line
1.7.10.4
#27 lore raw

PATCH 27/29 Specify checker_timeout in seconds

In reply to Hannes Reinecke

Commit 8d3f07da changed the internal value for checker_timeout to be in milliseconds, which wasn't reflected in the tur checker. So better scale it back to seconds, and change the callers to scale it to milliseconds where appropriate.

---
 libmultipath/checkers/emc_clariion.c |    2 +-
 libmultipath/checkers/hp_sw.c        |    4 ++--
 libmultipath/checkers/libsg.c        |    2 +-
 libmultipath/checkers/rdac.c         |    4 ++--
 libmultipath/checkers/tur.c          |    2 +-
 libmultipath/discovery.c             |    2 +-
 libmultipath/discovery.h             |    2 +-
 libmultipath/propsel.c               |    4 ++--
 8 files changed, 11 insertions(+), 11 deletions(-)
libmultipath/checkers/emc_clariion.c +1 -1
index 5d87bfd..a797734 100644
@@ -114,7 +114,7 @@ int libcheck_check (struct checker * c)
 	io_hdr.dxferp = sense_buffer;
 	io_hdr.cmdp = inqCmdBlk;
 	io_hdr.sbp = sb;
-	io_hdr.timeout = c->timeout;
+	io_hdr.timeout = c->timeout * 1000;
 	io_hdr.pack_id = 0;
 	if (ioctl(c->fd, SG_IO, &io_hdr) < 0) {
 		MSG(c, "emc_clariion_checker: sending query command failed");
libmultipath/checkers/hp_sw.c +2 -2
index b50ac0c..fe5e0f9 100644
@@ -70,7 +70,7 @@ do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op,
 	io_hdr.dxferp = resp;
 	io_hdr.cmdp = inqCmdBlk;
 	io_hdr.sbp = sense_b;
-	io_hdr.timeout = timeout;
+	io_hdr.timeout = timeout * 1000;
 
 	if (ioctl(sg_fd, SG_IO, &io_hdr) < 0)
 		return 1;
@@ -111,7 +111,7 @@ do_tur (int fd, unsigned int timeout)
 	io_hdr.dxfer_direction = SG_DXFER_NONE;
 	io_hdr.cmdp = turCmdBlk;
 	io_hdr.sbp = sense_buffer;
-	io_hdr.timeout = timeout;
+	io_hdr.timeout = timeout * 1000;
 	io_hdr.pack_id = 0;
 
 	if (ioctl(fd, SG_IO, &io_hdr) < 0)
libmultipath/checkers/libsg.c +1 -1
index 5a989d3..0d3af1f 100644
@@ -53,7 +53,7 @@ sg_read (int sg_fd, unsigned char * buff, int buff_len,
 	io_hdr.dxferp = buff;
 	io_hdr.mx_sb_len = sense_len;
 	io_hdr.sbp = sense;
-	io_hdr.timeout = timeout;
+	io_hdr.timeout = timeout * 1000;
 	io_hdr.pack_id = (int)start_block;
 	if (diop && *diop)
 	io_hdr.flags |= SG_FLAG_DIRECT_IO;
libmultipath/checkers/rdac.c +2 -2
index 5f24f55..2e64dd3 100644
@@ -74,7 +74,7 @@ int libcheck_init (struct checker * c)
 	io_hdr.dxferp = &current;
 	io_hdr.cmdp = cmd;
 	io_hdr.sbp = sense_b;
-	io_hdr.timeout = c->timeout;
+	io_hdr.timeout = c->timeout * 1000;
 
 	if (ioctl(c->fd, SG_IO, &io_hdr) < 0)
 		goto out;
@@ -150,7 +150,7 @@ retry:
 	io_hdr.dxferp = resp;
 	io_hdr.cmdp = inqCmdBlk;
 	io_hdr.sbp = sense_b;
-	io_hdr.timeout = timeout;
+	io_hdr.timeout = timeout * 1000;
 
 	if (ioctl(sg_fd, SG_IO, &io_hdr) < 0)
 		return 1;
libmultipath/checkers/tur.c +1 -1
index 6f5d4d9..b76dcec 100644
@@ -116,7 +116,7 @@ tur_check(int fd, unsigned int timeout, char *msg)
 	io_hdr.dxfer_direction = SG_DXFER_NONE;
 	io_hdr.cmdp = turCmdBlk;
 	io_hdr.sbp = sense_buffer;
-	io_hdr.timeout = timeout;
+	io_hdr.timeout = timeout * 1000;
 	io_hdr.pack_id = 0;
 	if (ioctl(fd, SG_IO, &io_hdr) < 0) {
 		TUR_MSG(msg, MSG_TUR_DOWN);
libmultipath/discovery.c +1 -1
index c59dbc2..adfb1ba 100644
@@ -204,7 +204,7 @@ sysfs_get_timeout(struct path *pp, unsigned int *timeout)
 		return 1;
 	}
 
-	*timeout = t * 1000;
+	*timeout = t;
 
 	return 0;
 }
libmultipath/discovery.h +1 -1
index 1a614ee..d049ead 100644
@@ -14,7 +14,7 @@
 #endif
 
 #ifndef DEF_TIMEOUT
-#define DEF_TIMEOUT	300000
+#define DEF_TIMEOUT	30
 #endif
 
 /*
libmultipath/propsel.c +2 -2
index cea8c1c..e47d0ca 100644
@@ -347,8 +347,8 @@ select_checker(struct path *pp)
 		pp->dev, checker_name(c));
 out:
 	if (conf->checker_timeout) {
-		c->timeout = conf->checker_timeout * 1000;
-		condlog(3, "%s: checker timeout = %u ms (config file default)",
+		c->timeout = conf->checker_timeout;
+		condlog(3, "%s: checker timeout = %u s (config file default)",
 				pp->dev, c->timeout);
 	}
 	else if (pp->udev && sysfs_get_timeout(pp, &c->timeout) == 0)
Signature · 1 line
1.7.10.4
#28 lore raw

PATCH 28/29 multipath: fix setting of fast_io_fail_tmo

In reply to Hannes Reinecke

From: Petr Uzel <[email protected]>

Setting fast_io_fail_tmo to the same value as dev_loss_tmo is not allowed by the kernel. Increase dev_loss_tmo by 1 in such cases to make it strictly greated than fast_io_fail_tmo.

---
 libmultipath/discovery.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
libmultipath/discovery.c +1 -1
index adfb1ba..e2b4f56 100644
@@ -367,7 +367,7 @@ sysfs_set_rport_tmo(struct multipath *mpp, struct path *pp)
 			goto out;
 		}
 		if (mpp->fast_io_fail >= tmo) {
-			snprintf(value, 16, "%u", mpp->fast_io_fail);
+			snprintf(value, 16, "%u", mpp->fast_io_fail + 1);
 		}
 	} else if (mpp->dev_loss > 600) {
 		condlog(3, "%s: limiting dev_loss_tmo to 600, since "
Signature · 1 line
1.7.10.4
#29 lore raw

PATCH 29/29 multipath: reset queue_if_no_path if flush failed

In reply to Hannes Reinecke

When flushing a map failed the 'queue_if_no_path' setting is getting lost.

---
 libmultipath/devmapper.c |   21 +++++++++++++++++----
 libmultipath/devmapper.h |    2 +-
 2 files changed, 18 insertions(+), 5 deletions(-)
libmultipath/devmapper.c +17 -4
index 69f475c..4c8b923 100644
@@ -366,7 +366,7 @@ out:
 }
 
 extern int
-dm_get_map(char * name, unsigned long long * size, char * outparams)
+dm_get_map(const char * name, unsigned long long * size, char * outparams)
 {
 	int r = 1;
 	struct dm_task *dmt;
@@ -685,7 +685,9 @@ _dm_flush_map (const char * mapname, int need_sync)
 extern int
 dm_suspend_and_flush_map (const char * mapname)
 {
-	int s;
+	int s = 0, queue_if_no_path = 0;
+	unsigned long long mapsize;
+	char params[PARAMS_SIZE] = {0};
 
 	if (!dm_map_present(mapname))
 		return 0;
@@ -693,8 +695,17 @@ dm_suspend_and_flush_map (const char * mapname)
 	if (dm_type(mapname, TGT_MPATH) <= 0)
 		return 0; /* nothing to do */
 
-	s = dm_queue_if_no_path((char *)mapname, 0);
-	if (!s)
+	if (!dm_get_map(mapname, &mapsize, params)) {
+		if (strstr(params, "queue_if_no_path"))
+			queue_if_no_path = 1;
+	}
+
+	if (queue_if_no_path)
+		s = dm_queue_if_no_path((char *)mapname, 0);
+	/* Leave queue_if_no_path alone if unset failed */
+	if (s)
+		queue_if_no_path = 0;
+	else
 		s = dm_simplecmd_flush(DM_DEVICE_SUSPEND, mapname, 0);
 
 	if (!dm_flush_map(mapname)) {
@@ -703,6 +714,8 @@ dm_suspend_and_flush_map (const char * mapname)
 	}
 	condlog(2, "failed to remove multipath map %s", mapname);
 	dm_simplecmd_noflush(DM_DEVICE_RESUME, mapname);
+	if (queue_if_no_path)
+		s = dm_queue_if_no_path((char *)mapname, 1);
 	return 1;
 }
 
libmultipath/devmapper.h +1 -1
index 8ea9dd3..b27db56 100644
@@ -16,7 +16,7 @@ int dm_addmap_create_ro (struct multipath *mpp, char *params);
 int dm_addmap_reload (struct multipath *mpp, char *params);
 int dm_addmap_reload_ro (struct multipath *mpp, char *params);
 int dm_map_present (const char *);
-int dm_get_map(char *, unsigned long long *, 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_flush_map (const char *, int);
Signature · 1 line
1.7.10.4
#30 lore raw
In reply to Hannes Reinecke

On 15.07.2013 15:00, Hannes Reinecke wrote:
[...]

Quoted 4 lines

- When using libudev to read attribute variables we'd be getting an automated caching from libudev. Which is nice, unless the data changed in between. So I'd have to go back an roll our own accessors, skipping libudev here.

[...]

Hi Hannes,

nice, that's what I've already reported. Could you point me to the fix commit(s) for this please?

I couldn't find any related code in the series. Also here is still the cached behavior for the device state:
https://github.com/hreinecke/multipath-tools/blob/sles-resync/libmultipath/discovery.c

Or is this planned as future work?

Cheers,
Sebastian

#31 lore raw
Quoted 6 lines · Sebastian Riemer

On 15.07.2013 15:00, Hannes Reinecke wrote:
[...]

Quoted 4 lines

- When using libudev to read attribute variables we'd be getting an automated caching from libudev. Which is nice, unless the data changed in between. So I'd have to go back an roll our own accessors, skipping libudev here.

Quoted 7 lines

[...]

Hi Hannes,

nice, that's what I've already reported. Could you point me to the fix commit(s) for this please?

Gnaa. Merge error.
For some reason the patch _should_ be 0014, but then it contains random merge errors.

I'll be posting an additional patch for this. Christophe, I'll be probably have to rebase the branch, so wait a bit with pulling.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +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)
#32 lore raw
Quoted 15 lines · Hannes Reinecke
Quoted 6 lines · Sebastian Riemer

On 15.07.2013 15:00, Hannes Reinecke wrote:
[...]

Quoted 4 lines

- When using libudev to read attribute variables we'd be getting an automated caching from libudev. Which is nice, unless the data changed in between. So I'd have to go back an roll our own accessors, skipping libudev here.

Quoted 7 lines

[...]

Hi Hannes,

nice, that's what I've already reported. Could you point me to the fix commit(s) for this please?

Quoted 8 lines

Gnaa. Merge error.
For some reason the patch _should_ be 0014, but then it contains random merge errors.

I'll be posting an additional patch for this. Christophe, I'll be probably have to rebase the branch, so wait a bit with pulling.

Sigh. Sebastian, you've got me confused.
The patch you're talking about is 0004 (Increase dev_loss_tmo prior to fast_io_fail), which contains the code for reading from sysfs. 0014 is only related to that as it uses it in some more places. Only the 'state' attribute itself wasn't modified, so for that we'd still be using libudev.

So my patchset was in fact okay, but your point is valid, too.

Hence I've done a new patchset, identical to the first one, but with one additional patch reading the 'state' attribute directly from sysfs.

Cheers,

Hannes

Signature · 4 lines
Dr. Hannes Reinecke		      zSeries & Storage
[email protected]			      +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)
#33 lore raw
Quoted 31 lines · Hannes Reinecke
Quoted 15 lines · Hannes Reinecke
Quoted 6 lines · Sebastian Riemer

On 15.07.2013 15:00, Hannes Reinecke wrote:
[...]

Quoted 4 lines

- When using libudev to read attribute variables we'd be getting an automated caching from libudev. Which is nice, unless the data changed in between. So I'd have to go back an roll our own accessors, skipping libudev here.

Quoted 7 lines

[...]

Hi Hannes,

nice, that's what I've already reported. Could you point me to the fix commit(s) for this please?

Quoted 8 lines

Gnaa. Merge error.
For some reason the patch _should_ be 0014, but then it contains random merge errors.

I'll be posting an additional patch for this. Christophe, I'll be probably have to rebase the branch, so wait a bit with pulling.

Sigh. Sebastian, you've got me confused.
The patch you're talking about is 0004 (Increase dev_loss_tmo prior to fast_io_fail), which contains the code for reading from sysfs. 0014 is only related to that as it uses it in some more places. Only the 'state' attribute itself wasn't modified, so for that we'd still be using libudev.

Quoted 4 lines

So my patchset was in fact okay, but your point is valid, too.

Hence I've done a new patchset, identical to the first one, but with one additional patch reading the 'state' attribute directly from sysfs.

Hi Hannes!

Thanks for the explanation and adding the device state commit! Looks good for me now in terms of my casual review.

would have been good for the last patch of the new series. But okay.

Cheers,
Sebastian