All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/6] misc patches
@ 2017-02-27 18:26 Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

Here's a couple of minor features bugfixes and cleanups. The biggest
change is that the detect_checker option now sets the checker for
devices with detected ALUA to TUR. Also, udev wasn't correctly
disabling rules for multipath devices that lost their last usable
path through a table reload. This was causing lvmetad some problems
on failing multipath devices.

Differences from v3:
Rebased patches

Benjamin Marzinski (6):
  libmultipath: add detect_checker option
  libmultipath: cleanup orphan device states
  multipathd: don't update priority of failed paths
  multipathd: add messages on delayed path addition
  multipathd: allow resetting stats
  fix udev rules for failed multipath devices

 libmultipath/config.c                 |  3 ++
 libmultipath/config.h                 |  2 ++
 libmultipath/defaults.h               |  1 +
 libmultipath/devmapper.c              |  3 +-
 libmultipath/devmapper.h              |  6 ++++
 libmultipath/dict.c                   | 10 ++++++
 libmultipath/discovery.c              | 28 +++++++++++++--
 libmultipath/hwtable.c                |  1 +
 libmultipath/print.c                  |  4 +--
 libmultipath/prioritizers/alua_spc3.h |  1 +
 libmultipath/propsel.c                | 31 ++++++++++------
 libmultipath/propsel.h                |  1 +
 libmultipath/structs.c                |  2 ++
 libmultipath/structs.h                |  7 ++++
 multipath/11-dm-mpath.rules           | 67 ++++++++++++++++++++++++-----------
 multipath/multipath.conf.5            | 18 ++++++++++
 multipathd/cli.c                      |  2 ++
 multipathd/cli_handlers.c             | 44 +++++++++++++++++++++++
 multipathd/cli_handlers.h             |  2 ++
 multipathd/main.c                     |  8 ++++-
 20 files changed, 205 insertions(+), 36 deletions(-)

-- 
1.8.3.1

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

* [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-09-25 16:15   ` Xose Vazquez Perez
  2017-02-27 18:26 ` [PATCH v4 2/6] libmultipath: cleanup orphan device states Benjamin Marzinski
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

This patch adds a detect_checker option that works just like the
detect_prio option.  It currently only detects ALUA devices, and
if it finds ALUA support, it sets the priortizier to TUR. This is
useful for devices like the VNX2, where it should be using the
TUR checker when in ALUA mode (or so I have been told). It is set on by
default just like detect_prio and retain_attached_hw_handler.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/config.c                 |  3 +++
 libmultipath/config.h                 |  2 ++
 libmultipath/defaults.h               |  1 +
 libmultipath/dict.c                   | 10 ++++++++++
 libmultipath/discovery.c              | 28 ++++++++++++++++++++++++++--
 libmultipath/hwtable.c                |  1 +
 libmultipath/prioritizers/alua_spc3.h |  1 +
 libmultipath/propsel.c                | 31 +++++++++++++++++++++----------
 libmultipath/propsel.h                |  1 +
 libmultipath/structs.c                |  2 ++
 libmultipath/structs.h                |  7 +++++++
 multipath/multipath.conf.5            | 18 ++++++++++++++++++
 12 files changed, 93 insertions(+), 12 deletions(-)

diff --git a/libmultipath/config.c b/libmultipath/config.c
index 5837dc6..9d3f3e1 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -343,6 +343,7 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
 	merge_num(user_friendly_names);
 	merge_num(retain_hwhandler);
 	merge_num(detect_prio);
+	merge_num(detect_checker);
 	merge_num(deferred_remove);
 	merge_num(delay_watch_checks);
 	merge_num(delay_wait_checks);
@@ -423,6 +424,7 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
 	hwe->user_friendly_names = dhwe->user_friendly_names;
 	hwe->retain_hwhandler = dhwe->retain_hwhandler;
 	hwe->detect_prio = dhwe->detect_prio;
+	hwe->detect_checker = dhwe->detect_checker;
 
 	if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
 		goto out;
@@ -610,6 +612,7 @@ load_config (char * file)
 	conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
 	conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
 	conf->detect_prio = DEFAULT_DETECT_PRIO;
+	conf->detect_checker = DEFAULT_DETECT_CHECKER;
 	conf->force_sync = DEFAULT_FORCE_SYNC;
 	conf->partition_delim = DEFAULT_PARTITION_DELIM;
 	conf->processed_main_config = 0;
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 9e47894..9a90745 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -62,6 +62,7 @@ struct hwentry {
 	int user_friendly_names;
 	int retain_hwhandler;
 	int detect_prio;
+	int detect_checker;
 	int deferred_remove;
 	int delay_watch_checks;
 	int delay_wait_checks;
@@ -139,6 +140,7 @@ struct config {
 	int reassign_maps;
 	int retain_hwhandler;
 	int detect_prio;
+	int detect_checker;
 	int force_sync;
 	int deferred_remove;
 	int processed_main_config;
diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 3ef1579..db2b756 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -22,6 +22,7 @@
 #define DEFAULT_DEV_LOSS_TMO	600
 #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
 #define DEFAULT_DETECT_PRIO	DETECT_PRIO_ON
+#define DEFAULT_DETECT_CHECKER	DETECT_CHECKER_ON
 #define DEFAULT_DEFERRED_REMOVE	DEFERRED_REMOVE_OFF
 #define DEFAULT_DELAY_CHECKS	NU_NO
 #define DEFAULT_ERR_CHECKS	NU_NO
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 3521c78..bababdb 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -379,6 +379,13 @@ declare_ovr_snprint(detect_prio, print_yes_no_undef)
 declare_hw_handler(detect_prio, set_yes_no_undef)
 declare_hw_snprint(detect_prio, print_yes_no_undef)
 
+declare_def_handler(detect_checker, set_yes_no_undef)
+declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO)
+declare_ovr_handler(detect_checker, set_yes_no_undef)
+declare_ovr_snprint(detect_checker, print_yes_no_undef)
+declare_hw_handler(detect_checker, set_yes_no_undef)
+declare_hw_snprint(detect_checker, print_yes_no_undef)
+
 declare_def_handler(force_sync, set_yes_no)
 declare_def_snprint(force_sync, print_yes_no)
 
@@ -1419,6 +1426,7 @@ init_keywords(vector keywords)
 	install_keyword("reservation_key", &def_reservation_key_handler, &snprint_def_reservation_key);
 	install_keyword("retain_attached_hw_handler", &def_retain_hwhandler_handler, &snprint_def_retain_hwhandler);
 	install_keyword("detect_prio", &def_detect_prio_handler, &snprint_def_detect_prio);
+	install_keyword("detect_checker", &def_detect_checker_handler, &snprint_def_detect_checker);
 	install_keyword("force_sync", &def_force_sync_handler, &snprint_def_force_sync);
 	install_keyword("strict_timing", &def_strict_timing_handler, &snprint_def_strict_timing);
 	install_keyword("deferred_remove", &def_deferred_remove_handler, &snprint_def_deferred_remove);
@@ -1509,6 +1517,7 @@ init_keywords(vector keywords)
 	install_keyword("user_friendly_names", &hw_user_friendly_names_handler, &snprint_hw_user_friendly_names);
 	install_keyword("retain_attached_hw_handler", &hw_retain_hwhandler_handler, &snprint_hw_retain_hwhandler);
 	install_keyword("detect_prio", &hw_detect_prio_handler, &snprint_hw_detect_prio);
+	install_keyword("detect_checker", &hw_detect_checker_handler, &snprint_hw_detect_checker);
 	install_keyword("deferred_remove", &hw_deferred_remove_handler, &snprint_hw_deferred_remove);
 	install_keyword("delay_watch_checks", &hw_delay_watch_checks_handler, &snprint_hw_delay_watch_checks);
 	install_keyword("delay_wait_checks", &hw_delay_wait_checks_handler, &snprint_hw_delay_wait_checks);
@@ -1541,6 +1550,7 @@ init_keywords(vector keywords)
 	install_keyword("user_friendly_names", &ovr_user_friendly_names_handler, &snprint_ovr_user_friendly_names);
 	install_keyword("retain_attached_hw_handler", &ovr_retain_hwhandler_handler, &snprint_ovr_retain_hwhandler);
 	install_keyword("detect_prio", &ovr_detect_prio_handler, &snprint_ovr_detect_prio);
+	install_keyword("detect_checker", &ovr_detect_checker_handler, &snprint_ovr_detect_checker);
 	install_keyword("deferred_remove", &ovr_deferred_remove_handler, &snprint_ovr_deferred_remove);
 	install_keyword("delay_watch_checks", &ovr_delay_watch_checks_handler, &snprint_ovr_delay_watch_checks);
 	install_keyword("delay_wait_checks", &ovr_delay_wait_checks_handler, &snprint_ovr_delay_wait_checks);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 39dd92a..4e99845 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -30,6 +30,7 @@
 #include "discovery.h"
 #include "prio.h"
 #include "defaults.h"
+#include "prioritizers/alua_rtpg.h"
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
@@ -829,6 +830,25 @@ get_serial (char * str, int maxlen, int fd)
 	return 1;
 }
 
+static void
+detect_alua(struct path * pp, struct config *conf)
+{
+	int ret;
+	int tpgs;
+	unsigned int timeout = conf->checker_timeout;
+
+	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0) {
+		pp->tpgs = TPGS_NONE;
+		return;
+	}
+	ret = get_target_port_group(pp, timeout);
+	if (ret < 0 || get_asymmetric_access_state(pp->fd, ret, timeout) < 0) {
+		pp->tpgs = TPGS_NONE;
+		return;
+	}
+	pp->tpgs = tpgs;
+}
+
 #define DEFAULT_SGIO_LEN 254
 
 static int
@@ -1460,11 +1480,14 @@ sysfs_pathinfo(struct path * pp, vector hwtable)
 }
 
 static int
-scsi_ioctl_pathinfo (struct path * pp, int mask)
+scsi_ioctl_pathinfo (struct path * pp, struct config *conf, int mask)
 {
 	struct udev_device *parent;
 	const char *attr_path = NULL;
 
+	if (pp->tpgs == TPGS_UNDEF)
+		detect_alua(pp, conf);
+
 	if (!(mask & DI_SERIAL))
 		return 0;
 
@@ -1524,6 +1547,7 @@ get_state (struct path * pp, struct config *conf, int daemon)
 				return PATH_UNCHECKED;
 			}
 		}
+		select_detect_checker(conf, pp);
 		select_checker(conf, pp);
 		if (!checker_selected(c)) {
 			condlog(3, "%s: No checker selected", pp->dev);
@@ -1832,7 +1856,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 		get_geometry(pp);
 
 	if (path_state == PATH_UP && pp->bus == SYSFS_BUS_SCSI &&
-	    scsi_ioctl_pathinfo(pp, mask))
+	    scsi_ioctl_pathinfo(pp, conf, mask))
 		goto blank;
 
 	if (pp->bus == SYSFS_BUS_CCISS &&
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 8409261..c944015 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -1122,6 +1122,7 @@ static struct hwentry default_hw[] = {
 		.dev_loss      = 600,
 		.retain_hwhandler = RETAIN_HWHANDLER_ON,
 		.detect_prio   = DETECT_PRIO_ON,
+		.detect_checker = DETECT_CHECKER_ON,
 		.deferred_remove = DEFERRED_REMOVE_OFF,
 		.delay_watch_checks = DELAY_CHECKS_OFF,
 		.delay_wait_checks = DELAY_CHECKS_OFF,
diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
index 4d4969b..13a0924 100644
--- a/libmultipath/prioritizers/alua_spc3.h
+++ b/libmultipath/prioritizers/alua_spc3.h
@@ -109,6 +109,7 @@ inquiry_command_set_evpd(struct inquiry_command *ic)
 #define VERSION_SPC3					0x05
 
 /* Defined TPGS field values. */
+#define TPGS_UNDEF					 -1
 #define TPGS_NONE					0x0
 #define TPGS_IMPLICIT					0x1
 #define TPGS_EXPLICIT					0x2
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 1b27476..bba8194 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -312,6 +312,11 @@ int select_checker(struct config *conf, struct path *pp)
 	char *origin, *checker_name;
 	struct checker * c = &pp->checker;
 
+	if (pp->detect_checker == DETECT_CHECKER_ON && pp->tpgs > 0) {
+		checker_name = TUR;
+		origin = "(setting: array autodetected)";
+		goto out;
+	}
 	do_set(checker_name, conf->overrides, checker_name, "(setting: multipath.conf overrides section)");
 	do_set(checker_name, pp->hwe, checker_name, "(setting: array configuration)");
 	do_set(checker_name, conf, checker_name, "(setting: multipath.conf defaults/devices section)");
@@ -359,20 +364,11 @@ out:
 void
 detect_prio(struct config *conf, struct path * pp)
 {
-	int ret;
 	struct prio *p = &pp->prio;
-	int tpgs = 0;
-	unsigned int timeout = conf->checker_timeout;
 	char buff[512];
 	char *default_prio = PRIO_ALUA;
 
-	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0)
-		return;
-	pp->tpgs = tpgs;
-	ret = get_target_port_group(pp, timeout);
-	if (ret < 0)
-		return;
-	if (get_asymmetric_access_state(pp->fd, ret, timeout) < 0)
+	if (pp->tpgs <= 0)
 		return;
 	if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
 		default_prio = PRIO_SYSFS;
@@ -588,6 +584,21 @@ out:
 	return 0;
 }
 
+int select_detect_checker(struct config *conf, struct path *pp)
+{
+	char *origin;
+
+	pp_set_ovr(detect_checker);
+	pp_set_hwe(detect_checker);
+	pp_set_conf(detect_checker);
+	pp_set_default(detect_checker, DEFAULT_DETECT_CHECKER);
+out:
+	condlog(3, "%s: detect_checker = %s %s", pp->dev,
+		(pp->detect_checker == DETECT_CHECKER_ON)? "yes" : "no",
+		origin);
+	return 0;
+}
+
 int select_deferred_remove(struct config *conf, struct multipath *mp)
 {
 	char *origin;
diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
index e5b6f93..58a32f3 100644
--- a/libmultipath/propsel.h
+++ b/libmultipath/propsel.h
@@ -19,6 +19,7 @@ int select_dev_loss(struct config *conf, struct multipath *mp);
 int select_reservation_key(struct config *conf, struct multipath *mp);
 int select_retain_hwhandler (struct config *conf, struct multipath * mp);
 int select_detect_prio(struct config *conf, struct path * pp);
+int select_detect_checker(struct config *conf, struct path * pp);
 int select_deferred_remove(struct config *conf, struct multipath *mp);
 int select_delay_watch_checks (struct config *conf, struct multipath * mp);
 int select_delay_wait_checks (struct config *conf, struct multipath * mp);
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index ba9edf9..f36a055 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -17,6 +17,7 @@
 #include "structs_vec.h"
 #include "blacklist.h"
 #include "prio.h"
+#include "prioritizers/alua_spc3.h"
 
 struct adapter_group *
 alloc_adaptergroup(void)
@@ -96,6 +97,7 @@ alloc_path (void)
 		pp->sg_id.lun = -1;
 		pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
 		pp->fd = -1;
+		pp->tpgs = TPGS_UNDEF;
 		pp->priority = PRIO_UNDEF;
 	}
 	return pp;
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index dfd65ae..fdcfc85 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -122,6 +122,12 @@ enum detect_prio_states {
 	DETECT_PRIO_ON = YNU_YES,
 };
 
+enum detect_checker_states {
+	DETECT_CHECKER_UNDEF = YNU_UNDEF,
+	DETECT_CHECKER_OFF = YNU_NO,
+	DETECT_CHECKER_ON = YNU_YES,
+};
+
 enum deferred_remove_states {
 	DEFERRED_REMOVE_UNDEF = YNU_UNDEF,
 	DEFERRED_REMOVE_OFF = YNU_NO,
@@ -211,6 +217,7 @@ struct path {
 	int priority;
 	int pgindex;
 	int detect_prio;
+	int detect_checker;
 	int watch_checks;
 	int wait_checks;
 	int tpgs;
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 179013c..b08bda3 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -681,6 +681,20 @@ The default is: \fByes\fR
 .
 .
 .TP
+.B detect_checker
+if set to
+.I yes
+, multipath will try to detect if the device supports SCSI-3 ALUA. If so, the
+device will automatically use the \fItur\fR checker. If set to
+.I no
+, the checker will be selected as usual.
+.RS
+.TP
+The default is: \fByes\fR
+.RE
+.
+.
+.TP
 .B force_sync
 If set to
 .I yes
@@ -1169,6 +1183,8 @@ section:
 .TP
 .B detect_prio
 .TP
+.B detect_checker
+.TP
 .B deferred_remove
 .TP
 .B san_path_err_threshold
@@ -1239,6 +1255,8 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
 .TP
 .B detect_prio
 .TP
+.B detect_checker
+.TP
 .B deferred_remove
 .TP
 .B san_path_err_threshold
-- 
1.8.3.1

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

* [PATCH v4 2/6] libmultipath: cleanup orphan device states
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 3/6] multipathd: don't update priority of failed paths Benjamin Marzinski
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

After a path device is orphaned, multipathd stops checking its state.
However, multipathd show state still shows its old state. It should
display "undef unknown" instead.

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

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 5b03383..00a3626 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -392,7 +392,7 @@ snprint_dev_t (char * buff, size_t len, struct path * pp)
 static int
 snprint_offline (char * buff, size_t len, struct path * pp)
 {
-	if (!pp)
+	if (!pp || !pp->mpp)
 		return snprintf(buff, len, "unknown");
 	else if (pp->offline)
 		return snprintf(buff, len, "offline");
@@ -403,7 +403,7 @@ snprint_offline (char * buff, size_t len, struct path * pp)
 static int
 snprint_chk_state (char * buff, size_t len, struct path * pp)
 {
-	if (!pp)
+	if (!pp || !pp->mpp)
 		return snprintf(buff, len, "undef");
 
 	switch (pp->state) {
-- 
1.8.3.1

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

* [PATCH v4 3/6] multipathd: don't update priority of failed paths
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 2/6] libmultipath: cleanup orphan device states Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 4/6] multipathd: add messages on delayed path addition Benjamin Marzinski
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

Multipathd shouldn't be updating the priority of failed paths in the
checkerloop. The current avoids this in almost all cases, but not all.
Close the loophole.

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

diff --git a/multipathd/main.c b/multipathd/main.c
index 0f32b2c..553d1d1 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1467,7 +1467,8 @@ int update_prio(struct path *pp, int refresh_all)
 	}
 	oldpriority = pp->priority;
 	conf = get_multipath_config();
-	pathinfo(pp, conf, DI_PRIO);
+	if (pp->state != PATH_DOWN)
+		pathinfo(pp, conf, DI_PRIO);
 	put_multipath_config(conf);
 
 	if (pp->priority == oldpriority)
-- 
1.8.3.1

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

* [PATCH v4 4/6] multipathd: add messages on delayed path addition
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
                   ` (2 preceding siblings ...)
  2017-02-27 18:26 ` [PATCH v4 3/6] multipathd: don't update priority of failed paths Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 5/6] multipathd: allow resetting stats Benjamin Marzinski
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

When multipath delays adding a path because the device is waiting for
udev to finish initialization, it now logs a message, so the users
know what happened to the path.

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

diff --git a/multipathd/main.c b/multipathd/main.c
index 553d1d1..1d73f9d 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -486,6 +486,8 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
 
 	if (mpp) {
 		if (mpp->wait_for_udev > 1) {
+			condlog(2, "%s: performing delayed actions",
+				mpp->alias);
 			if (update_map(mpp, vecs))
 				/* setup multipathd removed the map */
 				return 1;
@@ -720,6 +722,7 @@ ev_add_path (struct path * pp, struct vectors * vecs)
 	    (pathcount(mpp, PATH_UP) > 0 ||
 	     (pathcount(mpp, PATH_GHOST) > 0 && pp->tpgs != TPGS_IMPLICIT))) {
 		/* if wait_for_udev is set and valid paths exist */
+		condlog(2, "%s: delaying path addition until %s is fully initialized", pp->dev, mpp->alias);
 		mpp->wait_for_udev = 2;
 		orphan_path(pp, "waiting for create to complete");
 		return 0;
-- 
1.8.3.1

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

* [PATCH v4 5/6] multipathd: allow resetting stats
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
                   ` (3 preceding siblings ...)
  2017-02-27 18:26 ` [PATCH v4 4/6] multipathd: add messages on delayed path addition Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-02-27 18:26 ` [PATCH v4 6/6] fix udev rules for failed multipath devices Benjamin Marzinski
  2017-02-27 20:46 ` [PATCH v4 0/6] misc patches Christophe Varoqui
  6 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

This patch adds two multipathd interactive commands:

multipathd reset maps stats

and

multipathd reset map <map> stats

to reset the statistics that are shown with the "show stats" commands.

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

diff --git a/multipathd/cli.c b/multipathd/cli.c
index 50161be..32d4976 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -547,6 +547,8 @@ cli_init (void) {
 	add_handler(LIST+BLACKLIST, NULL);
 	add_handler(LIST+DEVICES, NULL);
 	add_handler(LIST+WILDCARDS, NULL);
+	add_handler(RESET+MAPS+STATS, NULL);
+	add_handler(RESET+MAP+STATS, NULL);
 	add_handler(ADD+PATH, NULL);
 	add_handler(DEL+PATH, NULL);
 	add_handler(ADD+MAP, NULL);
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index b0eeca6..b20b054 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -277,6 +277,17 @@ show_config (char ** r, int * len)
 	return 0;
 }
 
+void
+reset_stats(struct multipath * mpp)
+{
+	mpp->stat_switchgroup = 0;
+	mpp->stat_path_failures = 0;
+	mpp->stat_map_loads = 0;
+	mpp->stat_total_queueing_time = 0;
+	mpp->stat_queueing_timeouts = 0;
+	mpp->stat_map_failures = 0;
+}
+
 int
 cli_list_config (void * v, char ** reply, int * len, void * data)
 {
@@ -627,6 +638,39 @@ cli_list_daemon (void * v, char ** reply, int * len, void * data)
 }
 
 int
+cli_reset_maps_stats (void * v, char ** reply, int * len, void * data)
+{
+	struct vectors * vecs = (struct vectors *)data;
+	int i;
+	struct multipath * mpp;
+
+	condlog(3, "reset multipaths stats (operator)");
+
+	vector_foreach_slot(vecs->mpvec, mpp, i) {
+		reset_stats(mpp);
+	}
+	return 0;
+}
+
+int
+cli_reset_map_stats (void * v, char ** reply, int * len, void * data)
+{
+	struct vectors * vecs = (struct vectors *)data;
+	struct multipath * mpp;
+	char * param = get_keyparam(v, MAP);
+
+	param = convert_dev(param, 0);
+	mpp = find_mp_by_str(vecs->mpvec, param);
+
+	if (!mpp)
+		return 1;
+
+	condlog(3, "reset multipath %s stats (operator)", param);
+	reset_stats(mpp);
+	return 0;
+}
+
+int
 cli_add_path (void * v, char ** reply, int * len, void * data)
 {
 	struct vectors * vecs = (struct vectors *)data;
diff --git a/multipathd/cli_handlers.h b/multipathd/cli_handlers.h
index 19e003d..f4d02cc 100644
--- a/multipathd/cli_handlers.h
+++ b/multipathd/cli_handlers.h
@@ -19,6 +19,8 @@ int cli_list_config (void * v, char ** reply, int * len, void * data);
 int cli_list_blacklist (void * v, char ** reply, int * len, void * data);
 int cli_list_devices (void * v, char ** reply, int * len, void * data);
 int cli_list_wildcards (void * v, char ** reply, int * len, void * data);
+int cli_reset_maps_stats (void * v, char ** reply, int * len, void * data);
+int cli_reset_map_stats (void * v, char ** reply, int * len, void * data);
 int cli_add_path (void * v, char ** reply, int * len, void * data);
 int cli_del_path (void * v, char ** reply, int * len, void * data);
 int cli_add_map (void * v, char ** reply, int * len, void * data);
diff --git a/multipathd/main.c b/multipathd/main.c
index 1d73f9d..e317a4c 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1238,6 +1238,8 @@ uxlsnrloop (void * ap)
 	set_handler_callback(LIST+BLACKLIST, cli_list_blacklist);
 	set_handler_callback(LIST+DEVICES, cli_list_devices);
 	set_handler_callback(LIST+WILDCARDS, cli_list_wildcards);
+	set_handler_callback(RESET+MAPS+STATS, cli_reset_maps_stats);
+	set_handler_callback(RESET+MAP+STATS, cli_reset_map_stats);
 	set_handler_callback(ADD+PATH, cli_add_path);
 	set_handler_callback(DEL+PATH, cli_del_path);
 	set_handler_callback(ADD+MAP, cli_add_map);
-- 
1.8.3.1

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

* [PATCH v4 6/6] fix udev rules for failed multipath devices
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
                   ` (4 preceding siblings ...)
  2017-02-27 18:26 ` [PATCH v4 5/6] multipathd: allow resetting stats Benjamin Marzinski
@ 2017-02-27 18:26 ` Benjamin Marzinski
  2017-02-27 20:46 ` [PATCH v4 0/6] misc patches Christophe Varoqui
  6 siblings, 0 replies; 16+ messages in thread
From: Benjamin Marzinski @ 2017-02-27 18:26 UTC (permalink / raw)
  To: device-mapper development

11-dm-mpath.rules was only correctly dealing with the case where the
multipath device was unusable because the last path had failed.  If
instead, the last working path was removed from the device on a table
reload, it was not correctly marking the device as unusable. One problem
with fixing this is that when the device table is reloaded,
device-mapper doesn't know if the path devices are usable or not.  To
deal with this, multipath now flags reloads with no usable paths with
DM_SUBSYSTEM_UDEV_FLAG2.

11-dm-mpath.rules now checks for both PATH_FAILED events and reloads
with no valid paths. and disables the other rules.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c    |  3 +-
 libmultipath/devmapper.h    |  6 ++++
 multipath/11-dm-mpath.rules | 67 +++++++++++++++++++++++++++++++--------------
 3 files changed, 55 insertions(+), 21 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 9c0b240..9b6b053 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -374,7 +374,8 @@ int dm_addmap_reload(struct multipath *mpp, char *params, int flush)
 	int r;
 	uint16_t udev_flags = (flush ? 0 : MPATH_UDEV_RELOAD_FLAG) |
 			      ((mpp->skip_kpartx == SKIP_KPARTX_ON)?
-			       MPATH_UDEV_NO_KPARTX_FLAG : 0);
+			       MPATH_UDEV_NO_KPARTX_FLAG : 0) |
+			      ((mpp->nr_active)? 0 : MPATH_UDEV_NO_PATHS_FLAG);
 
 	/*
 	 * DM_DEVICE_RELOAD cannot wait on a cookie, as
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 411177d..3ea4329 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -18,6 +18,12 @@
 #define MPATH_UDEV_NO_KPARTX_FLAG 0
 #endif
 
+#ifdef DM_SUBSYSTEM_UDEV_FLAG2
+#define MPATH_UDEV_NO_PATHS_FLAG DM_SUBSYSTEM_UDEV_FLAG2
+#else
+#define MPATH_UDEV_NO_PATHS_FLAG 0
+#endif
+
 void dm_init(int verbosity);
 int dm_prereq (void);
 int dm_drv_version (unsigned int * version, char * str);
diff --git a/multipath/11-dm-mpath.rules b/multipath/11-dm-mpath.rules
index 5559af3..b131a10 100644
--- a/multipath/11-dm-mpath.rules
+++ b/multipath/11-dm-mpath.rules
@@ -2,39 +2,66 @@ ACTION!="add|change", GOTO="mpath_end"
 ENV{DM_UDEV_RULES_VSN}!="?*", GOTO="mpath_end"
 ENV{DM_UUID}!="mpath-?*", GOTO="mpath_end"
 
+IMPORT{db}="DM_DISABLE_OTHER_RULES_FLAG_OLD"
+IMPORT{db}="MPATH_DEVICE_READY"
+
+# If this uevent didn't come from dm, don't try to update the
+# device state
+ENV{DM_COOKIE}!="?*", ENV{DM_ACTION}!="PATH_*", IMPORT{db}="DM_UDEV_DISABLE_OTHER_RULES_FLAG", IMPORT{db}="DM_NOSCAN", GOTO="scan_import"
+
+ENV{.MPATH_DEVICE_READY_OLD}="$env{MPATH_DEVICE_READY}"
+
+# multipath sets DM_SUBSYSTEM_UDEV_FLAG2 when it reloads a
+# table with no active devices. If this happens, mark the
+# device not ready
+ENV{DM_SUBSYSTEM_UDEV_FLAG2}=="1", ENV{MPATH_DEVICE_READY}="0",\
+	GOTO="mpath_action"
+
+# If the last path has failed mark the device not ready
+ENV{DM_ACTION}=="PATH_FAILED", ENV{DM_NR_VALID_PATHS}=="0",\
+	ENV{MPATH_DEVICE_READY}="0", GOTO="mpath_action"
+
+# Don't mark a device ready on a PATH_FAILED event. even if
+# DM_NR_VALID_PATHS is greater than 0. Just keep the existing
+# value
+ENV{DM_ACTION}=="PATH_FAILED", GOTO="mpath_action"
+
+# This event is either a PATH_REINSTATED or a table reload where
+# there are active paths. Mark the device ready
+ENV{MPATH_DEVICE_READY}=""
+
+LABEL="mpath_action"
+# DM_SUBSYSTEM_UDEV_FLAG0 is the "RELOAD" flag for multipath subsystem.
+# Drop the DM_ACTIVATION flag here as mpath reloads tables if any of its
+# paths are lost/recovered. For any stack above the mpath device, this is not
+# something that should be reacted upon since it would be useless extra work.
+# It's exactly mpath's job to provide *seamless* device access to any of the
+# paths that are available underneath.
+ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{DM_ACTIVATION}="0"
+
 # Do not initiate scanning if no path is available,
 # otherwise there would be a hang or IO error on access.
 # We'd like to avoid this, especially within udev processing.
-ENV{DM_NR_VALID_PATHS}!="?*", IMPORT{db}="DM_NR_VALID_PATHS"
-ENV{DM_NR_VALID_PATHS}!="0", GOTO="mpath_blkid_end"
-ENV{ID_FS_TYPE}!="?*", IMPORT{db}="ID_FS_TYPE"
-ENV{ID_FS_USAGE}!="?*", IMPORT{db}="ID_FS_USAGE"
-ENV{ID_FS_UUID}!="?*", IMPORT{db}="ID_FS_UUID"
-ENV{ID_FS_ENC}!="?*", IMPORT{db}="ID_FS_UUID_ENC"
-ENV{ID_FS_VERSION}!="?*", IMPORT{db}="ID_FS_VERSION"
-LABEL="mpath_blkid_end"
+ENV{MPATH_DEVICE_READY}=="0", ENV{DM_NOSCAN}="1"
 
 # Also skip all foreign rules if no path is available.
 # Remember the original value of DM_DISABLE_OTHER_RULES_FLAG
 # and restore it back once we have at least one path available.
-IMPORT{db}="DM_DISABLE_OTHER_RULES_FLAG_OLD"
-ENV{DM_ACTION}=="PATH_FAILED",\
-	ENV{DM_NR_VALID_PATHS}=="0",\
+ENV{MPATH_DEVICE_READY}=="0", ENV{.MPATH_DEVICE_READY_OLD}!="0",\
 	ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}=="",\
 	ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}="$env{DM_UDEV_DISABLE_OTHER_RULES_FLAG}",\
 	ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="1"
-ENV{DM_ACTION}=="PATH_REINSTATED",\
-	ENV{DM_NR_VALID_PATHS}=="1",\
+ENV{MPATH_DEVICE_READY}!="0", ENV{.MPATH_DEVICE_READY_OLD}=="0",\
 	ENV{DM_UDEV_DISABLE_OTHER_RULES_FLAG}="$env{DM_DISABLE_OTHER_RULES_FLAG_OLD}",\
 	ENV{DM_DISABLE_OTHER_RULES_FLAG_OLD}="",\
 	ENV{DM_ACTIVATION}="1"
 
-# DM_SUBSYSTEM_UDEV_FLAG0 is the "RELOAD" flag for multipath subsystem.
-# Drop the DM_ACTIVATION flag here as mpath reloads tables if any of its
-# paths are lost/recovered. For any stack above the mpath device, this is not
-# something that should be reacted upon since it would be useless extra work.
-# It's exactly mpath's job to provide *seamless* device access to any of the
-# paths that are available underneath.
-ENV{DM_SUBSYSTEM_UDEV_FLAG0}=="1", ENV{DM_ACTIVATION}="0"
+LABEL="scan_import"
+ENV{DM_NOSCAN}!="1", GOTO="mpath_end"
+ENV{ID_FS_TYPE}!="?*", IMPORT{db}="ID_FS_TYPE"
+ENV{ID_FS_USAGE}!="?*", IMPORT{db}="ID_FS_USAGE"
+ENV{ID_FS_UUID}!="?*", IMPORT{db}="ID_FS_UUID"
+ENV{ID_FS_ENC}!="?*", IMPORT{db}="ID_FS_UUID_ENC"
+ENV{ID_FS_VERSION}!="?*", IMPORT{db}="ID_FS_VERSION"
 
 LABEL="mpath_end"
-- 
1.8.3.1

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

* Re: [PATCH v4 0/6] misc patches
  2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
                   ` (5 preceding siblings ...)
  2017-02-27 18:26 ` [PATCH v4 6/6] fix udev rules for failed multipath devices Benjamin Marzinski
@ 2017-02-27 20:46 ` Christophe Varoqui
  6 siblings, 0 replies; 16+ messages in thread
From: Christophe Varoqui @ 2017-02-27 20:46 UTC (permalink / raw)
  To: Benjamin Marzinski; +Cc: device-mapper development


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

Applied.
Thanks.

On Mon, Feb 27, 2017 at 7:26 PM, Benjamin Marzinski <bmarzins@redhat.com>
wrote:

> Here's a couple of minor features bugfixes and cleanups. The biggest
> change is that the detect_checker option now sets the checker for
> devices with detected ALUA to TUR. Also, udev wasn't correctly
> disabling rules for multipath devices that lost their last usable
> path through a table reload. This was causing lvmetad some problems
> on failing multipath devices.
>
> Differences from v3:
> Rebased patches
>
> Benjamin Marzinski (6):
>   libmultipath: add detect_checker option
>   libmultipath: cleanup orphan device states
>   multipathd: don't update priority of failed paths
>   multipathd: add messages on delayed path addition
>   multipathd: allow resetting stats
>   fix udev rules for failed multipath devices
>
>  libmultipath/config.c                 |  3 ++
>  libmultipath/config.h                 |  2 ++
>  libmultipath/defaults.h               |  1 +
>  libmultipath/devmapper.c              |  3 +-
>  libmultipath/devmapper.h              |  6 ++++
>  libmultipath/dict.c                   | 10 ++++++
>  libmultipath/discovery.c              | 28 +++++++++++++--
>  libmultipath/hwtable.c                |  1 +
>  libmultipath/print.c                  |  4 +--
>  libmultipath/prioritizers/alua_spc3.h |  1 +
>  libmultipath/propsel.c                | 31 ++++++++++------
>  libmultipath/propsel.h                |  1 +
>  libmultipath/structs.c                |  2 ++
>  libmultipath/structs.h                |  7 ++++
>  multipath/11-dm-mpath.rules           | 67 ++++++++++++++++++++++++------
> -----
>  multipath/multipath.conf.5            | 18 ++++++++++
>  multipathd/cli.c                      |  2 ++
>  multipathd/cli_handlers.c             | 44 +++++++++++++++++++++++
>  multipathd/cli_handlers.h             |  2 ++
>  multipathd/main.c                     |  8 ++++-
>  20 files changed, 205 insertions(+), 36 deletions(-)
>
> --
> 1.8.3.1
>
>

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

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



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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-02-27 18:26 ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
@ 2017-09-25 16:15   ` Xose Vazquez Perez
  2017-09-25 16:49     ` Benjamin Marzinski
  2017-09-25 20:12     ` Stewart, Sean
  0 siblings, 2 replies; 16+ messages in thread
From: Xose Vazquez Perez @ 2017-09-25 16:15 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development, Stewart, Sean

On 02/27/2017 07:26 PM, Benjamin Marzinski wrote:

> This patch adds a detect_checker option that works just like the
> detect_prio option.  It currently only detects ALUA devices, and
> if it finds ALUA support, it sets the priortizier to TUR. This is
> useful for devices like the VNX2, where it should be using the
> TUR checker when in ALUA mode (or so I have been told). It is set on by
> default just like detect_prio and retain_attached_hw_handler.

RDAC devices are going also to be affected with this change.
Is it desired?

> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/config.c                 |  3 +++
>  libmultipath/config.h                 |  2 ++
>  libmultipath/defaults.h               |  1 +
>  libmultipath/dict.c                   | 10 ++++++++++
>  libmultipath/discovery.c              | 28 ++++++++++++++++++++++++++--
>  libmultipath/hwtable.c                |  1 +
>  libmultipath/prioritizers/alua_spc3.h |  1 +
>  libmultipath/propsel.c                | 31 +++++++++++++++++++++----------
>  libmultipath/propsel.h                |  1 +
>  libmultipath/structs.c                |  2 ++
>  libmultipath/structs.h                |  7 +++++++
>  multipath/multipath.conf.5            | 18 ++++++++++++++++++
>  12 files changed, 93 insertions(+), 12 deletions(-)
> 
> diff --git a/libmultipath/config.c b/libmultipath/config.c
> index 5837dc6..9d3f3e1 100644
> --- a/libmultipath/config.c
> +++ b/libmultipath/config.c
> @@ -343,6 +343,7 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
>  	merge_num(user_friendly_names);
>  	merge_num(retain_hwhandler);
>  	merge_num(detect_prio);
> +	merge_num(detect_checker);
>  	merge_num(deferred_remove);
>  	merge_num(delay_watch_checks);
>  	merge_num(delay_wait_checks);
> @@ -423,6 +424,7 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
>  	hwe->user_friendly_names = dhwe->user_friendly_names;
>  	hwe->retain_hwhandler = dhwe->retain_hwhandler;
>  	hwe->detect_prio = dhwe->detect_prio;
> +	hwe->detect_checker = dhwe->detect_checker;
>  
>  	if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
>  		goto out;
> @@ -610,6 +612,7 @@ load_config (char * file)
>  	conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
>  	conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
>  	conf->detect_prio = DEFAULT_DETECT_PRIO;
> +	conf->detect_checker = DEFAULT_DETECT_CHECKER;
>  	conf->force_sync = DEFAULT_FORCE_SYNC;
>  	conf->partition_delim = DEFAULT_PARTITION_DELIM;
>  	conf->processed_main_config = 0;
> diff --git a/libmultipath/config.h b/libmultipath/config.h
> index 9e47894..9a90745 100644
> --- a/libmultipath/config.h
> +++ b/libmultipath/config.h
> @@ -62,6 +62,7 @@ struct hwentry {
>  	int user_friendly_names;
>  	int retain_hwhandler;
>  	int detect_prio;
> +	int detect_checker;
>  	int deferred_remove;
>  	int delay_watch_checks;
>  	int delay_wait_checks;
> @@ -139,6 +140,7 @@ struct config {
>  	int reassign_maps;
>  	int retain_hwhandler;
>  	int detect_prio;
> +	int detect_checker;
>  	int force_sync;
>  	int deferred_remove;
>  	int processed_main_config;
> diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> index 3ef1579..db2b756 100644
> --- a/libmultipath/defaults.h
> +++ b/libmultipath/defaults.h
> @@ -22,6 +22,7 @@
>  #define DEFAULT_DEV_LOSS_TMO	600
>  #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
>  #define DEFAULT_DETECT_PRIO	DETECT_PRIO_ON
> +#define DEFAULT_DETECT_CHECKER	DETECT_CHECKER_ON
>  #define DEFAULT_DEFERRED_REMOVE	DEFERRED_REMOVE_OFF
>  #define DEFAULT_DELAY_CHECKS	NU_NO
>  #define DEFAULT_ERR_CHECKS	NU_NO
> diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> index 3521c78..bababdb 100644
> --- a/libmultipath/dict.c
> +++ b/libmultipath/dict.c
> @@ -379,6 +379,13 @@ declare_ovr_snprint(detect_prio, print_yes_no_undef)
>  declare_hw_handler(detect_prio, set_yes_no_undef)
>  declare_hw_snprint(detect_prio, print_yes_no_undef)
>  
> +declare_def_handler(detect_checker, set_yes_no_undef)
> +declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO)
> +declare_ovr_handler(detect_checker, set_yes_no_undef)
> +declare_ovr_snprint(detect_checker, print_yes_no_undef)
> +declare_hw_handler(detect_checker, set_yes_no_undef)
> +declare_hw_snprint(detect_checker, print_yes_no_undef)
> +
>  declare_def_handler(force_sync, set_yes_no)
>  declare_def_snprint(force_sync, print_yes_no)
>  
> @@ -1419,6 +1426,7 @@ init_keywords(vector keywords)
>  	install_keyword("reservation_key", &def_reservation_key_handler, &snprint_def_reservation_key);
>  	install_keyword("retain_attached_hw_handler", &def_retain_hwhandler_handler, &snprint_def_retain_hwhandler);
>  	install_keyword("detect_prio", &def_detect_prio_handler, &snprint_def_detect_prio);
> +	install_keyword("detect_checker", &def_detect_checker_handler, &snprint_def_detect_checker);
>  	install_keyword("force_sync", &def_force_sync_handler, &snprint_def_force_sync);
>  	install_keyword("strict_timing", &def_strict_timing_handler, &snprint_def_strict_timing);
>  	install_keyword("deferred_remove", &def_deferred_remove_handler, &snprint_def_deferred_remove);
> @@ -1509,6 +1517,7 @@ init_keywords(vector keywords)
>  	install_keyword("user_friendly_names", &hw_user_friendly_names_handler, &snprint_hw_user_friendly_names);
>  	install_keyword("retain_attached_hw_handler", &hw_retain_hwhandler_handler, &snprint_hw_retain_hwhandler);
>  	install_keyword("detect_prio", &hw_detect_prio_handler, &snprint_hw_detect_prio);
> +	install_keyword("detect_checker", &hw_detect_checker_handler, &snprint_hw_detect_checker);
>  	install_keyword("deferred_remove", &hw_deferred_remove_handler, &snprint_hw_deferred_remove);
>  	install_keyword("delay_watch_checks", &hw_delay_watch_checks_handler, &snprint_hw_delay_watch_checks);
>  	install_keyword("delay_wait_checks", &hw_delay_wait_checks_handler, &snprint_hw_delay_wait_checks);
> @@ -1541,6 +1550,7 @@ init_keywords(vector keywords)
>  	install_keyword("user_friendly_names", &ovr_user_friendly_names_handler, &snprint_ovr_user_friendly_names);
>  	install_keyword("retain_attached_hw_handler", &ovr_retain_hwhandler_handler, &snprint_ovr_retain_hwhandler);
>  	install_keyword("detect_prio", &ovr_detect_prio_handler, &snprint_ovr_detect_prio);
> +	install_keyword("detect_checker", &ovr_detect_checker_handler, &snprint_ovr_detect_checker);
>  	install_keyword("deferred_remove", &ovr_deferred_remove_handler, &snprint_ovr_deferred_remove);
>  	install_keyword("delay_watch_checks", &ovr_delay_watch_checks_handler, &snprint_ovr_delay_watch_checks);
>  	install_keyword("delay_wait_checks", &ovr_delay_wait_checks_handler, &snprint_ovr_delay_wait_checks);
> diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> index 39dd92a..4e99845 100644
> --- a/libmultipath/discovery.c
> +++ b/libmultipath/discovery.c
> @@ -30,6 +30,7 @@
>  #include "discovery.h"
>  #include "prio.h"
>  #include "defaults.h"
> +#include "prioritizers/alua_rtpg.h"
>  
>  int
>  alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
> @@ -829,6 +830,25 @@ get_serial (char * str, int maxlen, int fd)
>  	return 1;
>  }
>  
> +static void
> +detect_alua(struct path * pp, struct config *conf)
> +{
> +	int ret;
> +	int tpgs;
> +	unsigned int timeout = conf->checker_timeout;
> +
> +	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0) {
> +		pp->tpgs = TPGS_NONE;
> +		return;
> +	}
> +	ret = get_target_port_group(pp, timeout);
> +	if (ret < 0 || get_asymmetric_access_state(pp->fd, ret, timeout) < 0) {
> +		pp->tpgs = TPGS_NONE;
> +		return;
> +	}
> +	pp->tpgs = tpgs;
> +}
> +
>  #define DEFAULT_SGIO_LEN 254
>  
>  static int
> @@ -1460,11 +1480,14 @@ sysfs_pathinfo(struct path * pp, vector hwtable)
>  }
>  
>  static int
> -scsi_ioctl_pathinfo (struct path * pp, int mask)
> +scsi_ioctl_pathinfo (struct path * pp, struct config *conf, int mask)
>  {
>  	struct udev_device *parent;
>  	const char *attr_path = NULL;
>  
> +	if (pp->tpgs == TPGS_UNDEF)
> +		detect_alua(pp, conf);
> +
>  	if (!(mask & DI_SERIAL))
>  		return 0;
>  
> @@ -1524,6 +1547,7 @@ get_state (struct path * pp, struct config *conf, int daemon)
>  				return PATH_UNCHECKED;
>  			}
>  		}
> +		select_detect_checker(conf, pp);
>  		select_checker(conf, pp);
>  		if (!checker_selected(c)) {
>  			condlog(3, "%s: No checker selected", pp->dev);
> @@ -1832,7 +1856,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
>  		get_geometry(pp);
>  
>  	if (path_state == PATH_UP && pp->bus == SYSFS_BUS_SCSI &&
> -	    scsi_ioctl_pathinfo(pp, mask))
> +	    scsi_ioctl_pathinfo(pp, conf, mask))
>  		goto blank;
>  
>  	if (pp->bus == SYSFS_BUS_CCISS &&
> diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> index 8409261..c944015 100644
> --- a/libmultipath/hwtable.c
> +++ b/libmultipath/hwtable.c
> @@ -1122,6 +1122,7 @@ static struct hwentry default_hw[] = {
>  		.dev_loss      = 600,
>  		.retain_hwhandler = RETAIN_HWHANDLER_ON,
>  		.detect_prio   = DETECT_PRIO_ON,
> +		.detect_checker = DETECT_CHECKER_ON,
>  		.deferred_remove = DEFERRED_REMOVE_OFF,
>  		.delay_watch_checks = DELAY_CHECKS_OFF,
>  		.delay_wait_checks = DELAY_CHECKS_OFF,
> diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
> index 4d4969b..13a0924 100644
> --- a/libmultipath/prioritizers/alua_spc3.h
> +++ b/libmultipath/prioritizers/alua_spc3.h
> @@ -109,6 +109,7 @@ inquiry_command_set_evpd(struct inquiry_command *ic)
>  #define VERSION_SPC3					0x05
>  
>  /* Defined TPGS field values. */
> +#define TPGS_UNDEF					 -1
>  #define TPGS_NONE					0x0
>  #define TPGS_IMPLICIT					0x1
>  #define TPGS_EXPLICIT					0x2
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index 1b27476..bba8194 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -312,6 +312,11 @@ int select_checker(struct config *conf, struct path *pp)
>  	char *origin, *checker_name;
>  	struct checker * c = &pp->checker;
>  
> +	if (pp->detect_checker == DETECT_CHECKER_ON && pp->tpgs > 0) {
> +		checker_name = TUR;
> +		origin = "(setting: array autodetected)";
> +		goto out;
> +	}
>  	do_set(checker_name, conf->overrides, checker_name, "(setting: multipath.conf overrides section)");
>  	do_set(checker_name, pp->hwe, checker_name, "(setting: array configuration)");
>  	do_set(checker_name, conf, checker_name, "(setting: multipath.conf defaults/devices section)");
> @@ -359,20 +364,11 @@ out:
>  void
>  detect_prio(struct config *conf, struct path * pp)
>  {
> -	int ret;
>  	struct prio *p = &pp->prio;
> -	int tpgs = 0;
> -	unsigned int timeout = conf->checker_timeout;
>  	char buff[512];
>  	char *default_prio = PRIO_ALUA;
>  
> -	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0)
> -		return;
> -	pp->tpgs = tpgs;
> -	ret = get_target_port_group(pp, timeout);
> -	if (ret < 0)
> -		return;
> -	if (get_asymmetric_access_state(pp->fd, ret, timeout) < 0)
> +	if (pp->tpgs <= 0)
>  		return;
>  	if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
>  		default_prio = PRIO_SYSFS;
> @@ -588,6 +584,21 @@ out:
>  	return 0;
>  }
>  
> +int select_detect_checker(struct config *conf, struct path *pp)
> +{
> +	char *origin;
> +
> +	pp_set_ovr(detect_checker);
> +	pp_set_hwe(detect_checker);
> +	pp_set_conf(detect_checker);
> +	pp_set_default(detect_checker, DEFAULT_DETECT_CHECKER);
> +out:
> +	condlog(3, "%s: detect_checker = %s %s", pp->dev,
> +		(pp->detect_checker == DETECT_CHECKER_ON)? "yes" : "no",
> +		origin);
> +	return 0;
> +}
> +
>  int select_deferred_remove(struct config *conf, struct multipath *mp)
>  {
>  	char *origin;
> diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
> index e5b6f93..58a32f3 100644
> --- a/libmultipath/propsel.h
> +++ b/libmultipath/propsel.h
> @@ -19,6 +19,7 @@ int select_dev_loss(struct config *conf, struct multipath *mp);
>  int select_reservation_key(struct config *conf, struct multipath *mp);
>  int select_retain_hwhandler (struct config *conf, struct multipath * mp);
>  int select_detect_prio(struct config *conf, struct path * pp);
> +int select_detect_checker(struct config *conf, struct path * pp);
>  int select_deferred_remove(struct config *conf, struct multipath *mp);
>  int select_delay_watch_checks (struct config *conf, struct multipath * mp);
>  int select_delay_wait_checks (struct config *conf, struct multipath * mp);
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index ba9edf9..f36a055 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -17,6 +17,7 @@
>  #include "structs_vec.h"
>  #include "blacklist.h"
>  #include "prio.h"
> +#include "prioritizers/alua_spc3.h"
>  
>  struct adapter_group *
>  alloc_adaptergroup(void)
> @@ -96,6 +97,7 @@ alloc_path (void)
>  		pp->sg_id.lun = -1;
>  		pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
>  		pp->fd = -1;
> +		pp->tpgs = TPGS_UNDEF;
>  		pp->priority = PRIO_UNDEF;
>  	}
>  	return pp;
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index dfd65ae..fdcfc85 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -122,6 +122,12 @@ enum detect_prio_states {
>  	DETECT_PRIO_ON = YNU_YES,
>  };
>  
> +enum detect_checker_states {
> +	DETECT_CHECKER_UNDEF = YNU_UNDEF,
> +	DETECT_CHECKER_OFF = YNU_NO,
> +	DETECT_CHECKER_ON = YNU_YES,
> +};
> +
>  enum deferred_remove_states {
>  	DEFERRED_REMOVE_UNDEF = YNU_UNDEF,
>  	DEFERRED_REMOVE_OFF = YNU_NO,
> @@ -211,6 +217,7 @@ struct path {
>  	int priority;
>  	int pgindex;
>  	int detect_prio;
> +	int detect_checker;
>  	int watch_checks;
>  	int wait_checks;
>  	int tpgs;
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index 179013c..b08bda3 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -681,6 +681,20 @@ The default is: \fByes\fR
>  .
>  .
>  .TP
> +.B detect_checker
> +if set to
> +.I yes
> +, multipath will try to detect if the device supports SCSI-3 ALUA. If so, the
> +device will automatically use the \fItur\fR checker. If set to
> +.I no
> +, the checker will be selected as usual.
> +.RS
> +.TP
> +The default is: \fByes\fR
> +.RE
> +.
> +.
> +.TP
>  .B force_sync
>  If set to
>  .I yes
> @@ -1169,6 +1183,8 @@ section:
>  .TP
>  .B detect_prio
>  .TP
> +.B detect_checker
> +.TP
>  .B deferred_remove
>  .TP
>  .B san_path_err_threshold
> @@ -1239,6 +1255,8 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
>  .TP
>  .B detect_prio
>  .TP
> +.B detect_checker
> +.TP
>  .B deferred_remove
>  .TP
>  .B san_path_err_threshold
> 

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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-09-25 16:15   ` Xose Vazquez Perez
@ 2017-09-25 16:49     ` Benjamin Marzinski
  2017-10-05 20:25       ` Martin Wilck
  2017-09-25 20:12     ` Stewart, Sean
  1 sibling, 1 reply; 16+ messages in thread
From: Benjamin Marzinski @ 2017-09-25 16:49 UTC (permalink / raw)
  To: Xose Vazquez Perez; +Cc: device-mapper development, Stewart, Sean

On Mon, Sep 25, 2017 at 06:15:20PM +0200, Xose Vazquez Perez wrote:
> On 02/27/2017 07:26 PM, Benjamin Marzinski wrote:
> 
> > This patch adds a detect_checker option that works just like the
> > detect_prio option.  It currently only detects ALUA devices, and
> > if it finds ALUA support, it sets the priortizier to TUR. This is
> > useful for devices like the VNX2, where it should be using the
> > TUR checker when in ALUA mode (or so I have been told). It is set on by
> > default just like detect_prio and retain_attached_hw_handler.
> 
> RDAC devices are going also to be affected with this change.
> Is it desired?

No. I'll send a patch that adds a check_rdac() call before setting the
checker type to TUR.

Good catch
-Ben

> 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> > ---
> >  libmultipath/config.c                 |  3 +++
> >  libmultipath/config.h                 |  2 ++
> >  libmultipath/defaults.h               |  1 +
> >  libmultipath/dict.c                   | 10 ++++++++++
> >  libmultipath/discovery.c              | 28 ++++++++++++++++++++++++++--
> >  libmultipath/hwtable.c                |  1 +
> >  libmultipath/prioritizers/alua_spc3.h |  1 +
> >  libmultipath/propsel.c                | 31 +++++++++++++++++++++----------
> >  libmultipath/propsel.h                |  1 +
> >  libmultipath/structs.c                |  2 ++
> >  libmultipath/structs.h                |  7 +++++++
> >  multipath/multipath.conf.5            | 18 ++++++++++++++++++
> >  12 files changed, 93 insertions(+), 12 deletions(-)
> > 
> > diff --git a/libmultipath/config.c b/libmultipath/config.c
> > index 5837dc6..9d3f3e1 100644
> > --- a/libmultipath/config.c
> > +++ b/libmultipath/config.c
> > @@ -343,6 +343,7 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
> >  	merge_num(user_friendly_names);
> >  	merge_num(retain_hwhandler);
> >  	merge_num(detect_prio);
> > +	merge_num(detect_checker);
> >  	merge_num(deferred_remove);
> >  	merge_num(delay_watch_checks);
> >  	merge_num(delay_wait_checks);
> > @@ -423,6 +424,7 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
> >  	hwe->user_friendly_names = dhwe->user_friendly_names;
> >  	hwe->retain_hwhandler = dhwe->retain_hwhandler;
> >  	hwe->detect_prio = dhwe->detect_prio;
> > +	hwe->detect_checker = dhwe->detect_checker;
> >  
> >  	if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
> >  		goto out;
> > @@ -610,6 +612,7 @@ load_config (char * file)
> >  	conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
> >  	conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
> >  	conf->detect_prio = DEFAULT_DETECT_PRIO;
> > +	conf->detect_checker = DEFAULT_DETECT_CHECKER;
> >  	conf->force_sync = DEFAULT_FORCE_SYNC;
> >  	conf->partition_delim = DEFAULT_PARTITION_DELIM;
> >  	conf->processed_main_config = 0;
> > diff --git a/libmultipath/config.h b/libmultipath/config.h
> > index 9e47894..9a90745 100644
> > --- a/libmultipath/config.h
> > +++ b/libmultipath/config.h
> > @@ -62,6 +62,7 @@ struct hwentry {
> >  	int user_friendly_names;
> >  	int retain_hwhandler;
> >  	int detect_prio;
> > +	int detect_checker;
> >  	int deferred_remove;
> >  	int delay_watch_checks;
> >  	int delay_wait_checks;
> > @@ -139,6 +140,7 @@ struct config {
> >  	int reassign_maps;
> >  	int retain_hwhandler;
> >  	int detect_prio;
> > +	int detect_checker;
> >  	int force_sync;
> >  	int deferred_remove;
> >  	int processed_main_config;
> > diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
> > index 3ef1579..db2b756 100644
> > --- a/libmultipath/defaults.h
> > +++ b/libmultipath/defaults.h
> > @@ -22,6 +22,7 @@
> >  #define DEFAULT_DEV_LOSS_TMO	600
> >  #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
> >  #define DEFAULT_DETECT_PRIO	DETECT_PRIO_ON
> > +#define DEFAULT_DETECT_CHECKER	DETECT_CHECKER_ON
> >  #define DEFAULT_DEFERRED_REMOVE	DEFERRED_REMOVE_OFF
> >  #define DEFAULT_DELAY_CHECKS	NU_NO
> >  #define DEFAULT_ERR_CHECKS	NU_NO
> > diff --git a/libmultipath/dict.c b/libmultipath/dict.c
> > index 3521c78..bababdb 100644
> > --- a/libmultipath/dict.c
> > +++ b/libmultipath/dict.c
> > @@ -379,6 +379,13 @@ declare_ovr_snprint(detect_prio, print_yes_no_undef)
> >  declare_hw_handler(detect_prio, set_yes_no_undef)
> >  declare_hw_snprint(detect_prio, print_yes_no_undef)
> >  
> > +declare_def_handler(detect_checker, set_yes_no_undef)
> > +declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO)
> > +declare_ovr_handler(detect_checker, set_yes_no_undef)
> > +declare_ovr_snprint(detect_checker, print_yes_no_undef)
> > +declare_hw_handler(detect_checker, set_yes_no_undef)
> > +declare_hw_snprint(detect_checker, print_yes_no_undef)
> > +
> >  declare_def_handler(force_sync, set_yes_no)
> >  declare_def_snprint(force_sync, print_yes_no)
> >  
> > @@ -1419,6 +1426,7 @@ init_keywords(vector keywords)
> >  	install_keyword("reservation_key", &def_reservation_key_handler, &snprint_def_reservation_key);
> >  	install_keyword("retain_attached_hw_handler", &def_retain_hwhandler_handler, &snprint_def_retain_hwhandler);
> >  	install_keyword("detect_prio", &def_detect_prio_handler, &snprint_def_detect_prio);
> > +	install_keyword("detect_checker", &def_detect_checker_handler, &snprint_def_detect_checker);
> >  	install_keyword("force_sync", &def_force_sync_handler, &snprint_def_force_sync);
> >  	install_keyword("strict_timing", &def_strict_timing_handler, &snprint_def_strict_timing);
> >  	install_keyword("deferred_remove", &def_deferred_remove_handler, &snprint_def_deferred_remove);
> > @@ -1509,6 +1517,7 @@ init_keywords(vector keywords)
> >  	install_keyword("user_friendly_names", &hw_user_friendly_names_handler, &snprint_hw_user_friendly_names);
> >  	install_keyword("retain_attached_hw_handler", &hw_retain_hwhandler_handler, &snprint_hw_retain_hwhandler);
> >  	install_keyword("detect_prio", &hw_detect_prio_handler, &snprint_hw_detect_prio);
> > +	install_keyword("detect_checker", &hw_detect_checker_handler, &snprint_hw_detect_checker);
> >  	install_keyword("deferred_remove", &hw_deferred_remove_handler, &snprint_hw_deferred_remove);
> >  	install_keyword("delay_watch_checks", &hw_delay_watch_checks_handler, &snprint_hw_delay_watch_checks);
> >  	install_keyword("delay_wait_checks", &hw_delay_wait_checks_handler, &snprint_hw_delay_wait_checks);
> > @@ -1541,6 +1550,7 @@ init_keywords(vector keywords)
> >  	install_keyword("user_friendly_names", &ovr_user_friendly_names_handler, &snprint_ovr_user_friendly_names);
> >  	install_keyword("retain_attached_hw_handler", &ovr_retain_hwhandler_handler, &snprint_ovr_retain_hwhandler);
> >  	install_keyword("detect_prio", &ovr_detect_prio_handler, &snprint_ovr_detect_prio);
> > +	install_keyword("detect_checker", &ovr_detect_checker_handler, &snprint_ovr_detect_checker);
> >  	install_keyword("deferred_remove", &ovr_deferred_remove_handler, &snprint_ovr_deferred_remove);
> >  	install_keyword("delay_watch_checks", &ovr_delay_watch_checks_handler, &snprint_ovr_delay_watch_checks);
> >  	install_keyword("delay_wait_checks", &ovr_delay_wait_checks_handler, &snprint_ovr_delay_wait_checks);
> > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
> > index 39dd92a..4e99845 100644
> > --- a/libmultipath/discovery.c
> > +++ b/libmultipath/discovery.c
> > @@ -30,6 +30,7 @@
> >  #include "discovery.h"
> >  #include "prio.h"
> >  #include "defaults.h"
> > +#include "prioritizers/alua_rtpg.h"
> >  
> >  int
> >  alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
> > @@ -829,6 +830,25 @@ get_serial (char * str, int maxlen, int fd)
> >  	return 1;
> >  }
> >  
> > +static void
> > +detect_alua(struct path * pp, struct config *conf)
> > +{
> > +	int ret;
> > +	int tpgs;
> > +	unsigned int timeout = conf->checker_timeout;
> > +
> > +	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0) {
> > +		pp->tpgs = TPGS_NONE;
> > +		return;
> > +	}
> > +	ret = get_target_port_group(pp, timeout);
> > +	if (ret < 0 || get_asymmetric_access_state(pp->fd, ret, timeout) < 0) {
> > +		pp->tpgs = TPGS_NONE;
> > +		return;
> > +	}
> > +	pp->tpgs = tpgs;
> > +}
> > +
> >  #define DEFAULT_SGIO_LEN 254
> >  
> >  static int
> > @@ -1460,11 +1480,14 @@ sysfs_pathinfo(struct path * pp, vector hwtable)
> >  }
> >  
> >  static int
> > -scsi_ioctl_pathinfo (struct path * pp, int mask)
> > +scsi_ioctl_pathinfo (struct path * pp, struct config *conf, int mask)
> >  {
> >  	struct udev_device *parent;
> >  	const char *attr_path = NULL;
> >  
> > +	if (pp->tpgs == TPGS_UNDEF)
> > +		detect_alua(pp, conf);
> > +
> >  	if (!(mask & DI_SERIAL))
> >  		return 0;
> >  
> > @@ -1524,6 +1547,7 @@ get_state (struct path * pp, struct config *conf, int daemon)
> >  				return PATH_UNCHECKED;
> >  			}
> >  		}
> > +		select_detect_checker(conf, pp);
> >  		select_checker(conf, pp);
> >  		if (!checker_selected(c)) {
> >  			condlog(3, "%s: No checker selected", pp->dev);
> > @@ -1832,7 +1856,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
> >  		get_geometry(pp);
> >  
> >  	if (path_state == PATH_UP && pp->bus == SYSFS_BUS_SCSI &&
> > -	    scsi_ioctl_pathinfo(pp, mask))
> > +	    scsi_ioctl_pathinfo(pp, conf, mask))
> >  		goto blank;
> >  
> >  	if (pp->bus == SYSFS_BUS_CCISS &&
> > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
> > index 8409261..c944015 100644
> > --- a/libmultipath/hwtable.c
> > +++ b/libmultipath/hwtable.c
> > @@ -1122,6 +1122,7 @@ static struct hwentry default_hw[] = {
> >  		.dev_loss      = 600,
> >  		.retain_hwhandler = RETAIN_HWHANDLER_ON,
> >  		.detect_prio   = DETECT_PRIO_ON,
> > +		.detect_checker = DETECT_CHECKER_ON,
> >  		.deferred_remove = DEFERRED_REMOVE_OFF,
> >  		.delay_watch_checks = DELAY_CHECKS_OFF,
> >  		.delay_wait_checks = DELAY_CHECKS_OFF,
> > diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
> > index 4d4969b..13a0924 100644
> > --- a/libmultipath/prioritizers/alua_spc3.h
> > +++ b/libmultipath/prioritizers/alua_spc3.h
> > @@ -109,6 +109,7 @@ inquiry_command_set_evpd(struct inquiry_command *ic)
> >  #define VERSION_SPC3					0x05
> >  
> >  /* Defined TPGS field values. */
> > +#define TPGS_UNDEF					 -1
> >  #define TPGS_NONE					0x0
> >  #define TPGS_IMPLICIT					0x1
> >  #define TPGS_EXPLICIT					0x2
> > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> > index 1b27476..bba8194 100644
> > --- a/libmultipath/propsel.c
> > +++ b/libmultipath/propsel.c
> > @@ -312,6 +312,11 @@ int select_checker(struct config *conf, struct path *pp)
> >  	char *origin, *checker_name;
> >  	struct checker * c = &pp->checker;
> >  
> > +	if (pp->detect_checker == DETECT_CHECKER_ON && pp->tpgs > 0) {
> > +		checker_name = TUR;
> > +		origin = "(setting: array autodetected)";
> > +		goto out;
> > +	}
> >  	do_set(checker_name, conf->overrides, checker_name, "(setting: multipath.conf overrides section)");
> >  	do_set(checker_name, pp->hwe, checker_name, "(setting: array configuration)");
> >  	do_set(checker_name, conf, checker_name, "(setting: multipath.conf defaults/devices section)");
> > @@ -359,20 +364,11 @@ out:
> >  void
> >  detect_prio(struct config *conf, struct path * pp)
> >  {
> > -	int ret;
> >  	struct prio *p = &pp->prio;
> > -	int tpgs = 0;
> > -	unsigned int timeout = conf->checker_timeout;
> >  	char buff[512];
> >  	char *default_prio = PRIO_ALUA;
> >  
> > -	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0)
> > -		return;
> > -	pp->tpgs = tpgs;
> > -	ret = get_target_port_group(pp, timeout);
> > -	if (ret < 0)
> > -		return;
> > -	if (get_asymmetric_access_state(pp->fd, ret, timeout) < 0)
> > +	if (pp->tpgs <= 0)
> >  		return;
> >  	if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
> >  		default_prio = PRIO_SYSFS;
> > @@ -588,6 +584,21 @@ out:
> >  	return 0;
> >  }
> >  
> > +int select_detect_checker(struct config *conf, struct path *pp)
> > +{
> > +	char *origin;
> > +
> > +	pp_set_ovr(detect_checker);
> > +	pp_set_hwe(detect_checker);
> > +	pp_set_conf(detect_checker);
> > +	pp_set_default(detect_checker, DEFAULT_DETECT_CHECKER);
> > +out:
> > +	condlog(3, "%s: detect_checker = %s %s", pp->dev,
> > +		(pp->detect_checker == DETECT_CHECKER_ON)? "yes" : "no",
> > +		origin);
> > +	return 0;
> > +}
> > +
> >  int select_deferred_remove(struct config *conf, struct multipath *mp)
> >  {
> >  	char *origin;
> > diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
> > index e5b6f93..58a32f3 100644
> > --- a/libmultipath/propsel.h
> > +++ b/libmultipath/propsel.h
> > @@ -19,6 +19,7 @@ int select_dev_loss(struct config *conf, struct multipath *mp);
> >  int select_reservation_key(struct config *conf, struct multipath *mp);
> >  int select_retain_hwhandler (struct config *conf, struct multipath * mp);
> >  int select_detect_prio(struct config *conf, struct path * pp);
> > +int select_detect_checker(struct config *conf, struct path * pp);
> >  int select_deferred_remove(struct config *conf, struct multipath *mp);
> >  int select_delay_watch_checks (struct config *conf, struct multipath * mp);
> >  int select_delay_wait_checks (struct config *conf, struct multipath * mp);
> > diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> > index ba9edf9..f36a055 100644
> > --- a/libmultipath/structs.c
> > +++ b/libmultipath/structs.c
> > @@ -17,6 +17,7 @@
> >  #include "structs_vec.h"
> >  #include "blacklist.h"
> >  #include "prio.h"
> > +#include "prioritizers/alua_spc3.h"
> >  
> >  struct adapter_group *
> >  alloc_adaptergroup(void)
> > @@ -96,6 +97,7 @@ alloc_path (void)
> >  		pp->sg_id.lun = -1;
> >  		pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
> >  		pp->fd = -1;
> > +		pp->tpgs = TPGS_UNDEF;
> >  		pp->priority = PRIO_UNDEF;
> >  	}
> >  	return pp;
> > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> > index dfd65ae..fdcfc85 100644
> > --- a/libmultipath/structs.h
> > +++ b/libmultipath/structs.h
> > @@ -122,6 +122,12 @@ enum detect_prio_states {
> >  	DETECT_PRIO_ON = YNU_YES,
> >  };
> >  
> > +enum detect_checker_states {
> > +	DETECT_CHECKER_UNDEF = YNU_UNDEF,
> > +	DETECT_CHECKER_OFF = YNU_NO,
> > +	DETECT_CHECKER_ON = YNU_YES,
> > +};
> > +
> >  enum deferred_remove_states {
> >  	DEFERRED_REMOVE_UNDEF = YNU_UNDEF,
> >  	DEFERRED_REMOVE_OFF = YNU_NO,
> > @@ -211,6 +217,7 @@ struct path {
> >  	int priority;
> >  	int pgindex;
> >  	int detect_prio;
> > +	int detect_checker;
> >  	int watch_checks;
> >  	int wait_checks;
> >  	int tpgs;
> > diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> > index 179013c..b08bda3 100644
> > --- a/multipath/multipath.conf.5
> > +++ b/multipath/multipath.conf.5
> > @@ -681,6 +681,20 @@ The default is: \fByes\fR
> >  .
> >  .
> >  .TP
> > +.B detect_checker
> > +if set to
> > +.I yes
> > +, multipath will try to detect if the device supports SCSI-3 ALUA. If so, the
> > +device will automatically use the \fItur\fR checker. If set to
> > +.I no
> > +, the checker will be selected as usual.
> > +.RS
> > +.TP
> > +The default is: \fByes\fR
> > +.RE
> > +.
> > +.
> > +.TP
> >  .B force_sync
> >  If set to
> >  .I yes
> > @@ -1169,6 +1183,8 @@ section:
> >  .TP
> >  .B detect_prio
> >  .TP
> > +.B detect_checker
> > +.TP
> >  .B deferred_remove
> >  .TP
> >  .B san_path_err_threshold
> > @@ -1239,6 +1255,8 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
> >  .TP
> >  .B detect_prio
> >  .TP
> > +.B detect_checker
> > +.TP
> >  .B deferred_remove
> >  .TP
> >  .B san_path_err_threshold
> > 

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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-09-25 16:15   ` Xose Vazquez Perez
  2017-09-25 16:49     ` Benjamin Marzinski
@ 2017-09-25 20:12     ` Stewart, Sean
  1 sibling, 0 replies; 16+ messages in thread
From: Stewart, Sean @ 2017-09-25 20:12 UTC (permalink / raw)
  To: Xose Vazquez Perez, Benjamin Marzinski,
	device-mapper development, Schremmer, Steven

Adding Steve Schremmer to the thread.

On 9/25/17, 11:15 AM, "Xose Vazquez Perez" <xose.vazquez@gmail.com> wrote:

    On 02/27/2017 07:26 PM, Benjamin Marzinski wrote:
    
    > This patch adds a detect_checker option that works just like the
    > detect_prio option.  It currently only detects ALUA devices, and
    > if it finds ALUA support, it sets the priortizier to TUR. This is
    > useful for devices like the VNX2, where it should be using the
    > TUR checker when in ALUA mode (or so I have been told). It is set on by
    > default just like detect_prio and retain_attached_hw_handler.
    
    RDAC devices are going also to be affected with this change.
    Is it desired?

Ideally, I think we would rather stick with the RDAC checker for RDAC devices for the time being, regardless of TPGS support. From our standpoint, when our array has TPGS enabled, it still responds to our vendor-specific inquiry in the same way and handles the path status just as well as in non-TPGS mode.

Plus, I think some testing has been done with this option enabled for us, and it has run into some momentary additional path losses when the TUR checker is in use. I am not sure I understand why ALUA and the TUR checker should necessarily be coupled like this, and what benefit it is supposed to provide? For us, the net effect is it seems to introduce a slight risk compared to the current configuration.

Thanks,
Sean Stewart
    
    > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
    > ---
    >  libmultipath/config.c                 |  3 +++
    >  libmultipath/config.h                 |  2 ++
    >  libmultipath/defaults.h               |  1 +
    >  libmultipath/dict.c                   | 10 ++++++++++
    >  libmultipath/discovery.c              | 28 ++++++++++++++++++++++++++--
    >  libmultipath/hwtable.c                |  1 +
    >  libmultipath/prioritizers/alua_spc3.h |  1 +
    >  libmultipath/propsel.c                | 31 +++++++++++++++++++++----------
    >  libmultipath/propsel.h                |  1 +
    >  libmultipath/structs.c                |  2 ++
    >  libmultipath/structs.h                |  7 +++++++
    >  multipath/multipath.conf.5            | 18 ++++++++++++++++++
    >  12 files changed, 93 insertions(+), 12 deletions(-)
    > 
    > diff --git a/libmultipath/config.c b/libmultipath/config.c
    > index 5837dc6..9d3f3e1 100644
    > --- a/libmultipath/config.c
    > +++ b/libmultipath/config.c
    > @@ -343,6 +343,7 @@ merge_hwe (struct hwentry * dst, struct hwentry * src)
    >  	merge_num(user_friendly_names);
    >  	merge_num(retain_hwhandler);
    >  	merge_num(detect_prio);
    > +	merge_num(detect_checker);
    >  	merge_num(deferred_remove);
    >  	merge_num(delay_watch_checks);
    >  	merge_num(delay_wait_checks);
    > @@ -423,6 +424,7 @@ store_hwe (vector hwtable, struct hwentry * dhwe)
    >  	hwe->user_friendly_names = dhwe->user_friendly_names;
    >  	hwe->retain_hwhandler = dhwe->retain_hwhandler;
    >  	hwe->detect_prio = dhwe->detect_prio;
    > +	hwe->detect_checker = dhwe->detect_checker;
    >  
    >  	if (dhwe->bl_product && !(hwe->bl_product = set_param_str(dhwe->bl_product)))
    >  		goto out;
    > @@ -610,6 +612,7 @@ load_config (char * file)
    >  	conf->fast_io_fail = DEFAULT_FAST_IO_FAIL;
    >  	conf->retain_hwhandler = DEFAULT_RETAIN_HWHANDLER;
    >  	conf->detect_prio = DEFAULT_DETECT_PRIO;
    > +	conf->detect_checker = DEFAULT_DETECT_CHECKER;
    >  	conf->force_sync = DEFAULT_FORCE_SYNC;
    >  	conf->partition_delim = DEFAULT_PARTITION_DELIM;
    >  	conf->processed_main_config = 0;
    > diff --git a/libmultipath/config.h b/libmultipath/config.h
    > index 9e47894..9a90745 100644
    > --- a/libmultipath/config.h
    > +++ b/libmultipath/config.h
    > @@ -62,6 +62,7 @@ struct hwentry {
    >  	int user_friendly_names;
    >  	int retain_hwhandler;
    >  	int detect_prio;
    > +	int detect_checker;
    >  	int deferred_remove;
    >  	int delay_watch_checks;
    >  	int delay_wait_checks;
    > @@ -139,6 +140,7 @@ struct config {
    >  	int reassign_maps;
    >  	int retain_hwhandler;
    >  	int detect_prio;
    > +	int detect_checker;
    >  	int force_sync;
    >  	int deferred_remove;
    >  	int processed_main_config;
    > diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
    > index 3ef1579..db2b756 100644
    > --- a/libmultipath/defaults.h
    > +++ b/libmultipath/defaults.h
    > @@ -22,6 +22,7 @@
    >  #define DEFAULT_DEV_LOSS_TMO	600
    >  #define DEFAULT_RETAIN_HWHANDLER RETAIN_HWHANDLER_ON
    >  #define DEFAULT_DETECT_PRIO	DETECT_PRIO_ON
    > +#define DEFAULT_DETECT_CHECKER	DETECT_CHECKER_ON
    >  #define DEFAULT_DEFERRED_REMOVE	DEFERRED_REMOVE_OFF
    >  #define DEFAULT_DELAY_CHECKS	NU_NO
    >  #define DEFAULT_ERR_CHECKS	NU_NO
    > diff --git a/libmultipath/dict.c b/libmultipath/dict.c
    > index 3521c78..bababdb 100644
    > --- a/libmultipath/dict.c
    > +++ b/libmultipath/dict.c
    > @@ -379,6 +379,13 @@ declare_ovr_snprint(detect_prio, print_yes_no_undef)
    >  declare_hw_handler(detect_prio, set_yes_no_undef)
    >  declare_hw_snprint(detect_prio, print_yes_no_undef)
    >  
    > +declare_def_handler(detect_checker, set_yes_no_undef)
    > +declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO)
    > +declare_ovr_handler(detect_checker, set_yes_no_undef)
    > +declare_ovr_snprint(detect_checker, print_yes_no_undef)
    > +declare_hw_handler(detect_checker, set_yes_no_undef)
    > +declare_hw_snprint(detect_checker, print_yes_no_undef)
    > +
    >  declare_def_handler(force_sync, set_yes_no)
    >  declare_def_snprint(force_sync, print_yes_no)
    >  
    > @@ -1419,6 +1426,7 @@ init_keywords(vector keywords)
    >  	install_keyword("reservation_key", &def_reservation_key_handler, &snprint_def_reservation_key);
    >  	install_keyword("retain_attached_hw_handler", &def_retain_hwhandler_handler, &snprint_def_retain_hwhandler);
    >  	install_keyword("detect_prio", &def_detect_prio_handler, &snprint_def_detect_prio);
    > +	install_keyword("detect_checker", &def_detect_checker_handler, &snprint_def_detect_checker);
    >  	install_keyword("force_sync", &def_force_sync_handler, &snprint_def_force_sync);
    >  	install_keyword("strict_timing", &def_strict_timing_handler, &snprint_def_strict_timing);
    >  	install_keyword("deferred_remove", &def_deferred_remove_handler, &snprint_def_deferred_remove);
    > @@ -1509,6 +1517,7 @@ init_keywords(vector keywords)
    >  	install_keyword("user_friendly_names", &hw_user_friendly_names_handler, &snprint_hw_user_friendly_names);
    >  	install_keyword("retain_attached_hw_handler", &hw_retain_hwhandler_handler, &snprint_hw_retain_hwhandler);
    >  	install_keyword("detect_prio", &hw_detect_prio_handler, &snprint_hw_detect_prio);
    > +	install_keyword("detect_checker", &hw_detect_checker_handler, &snprint_hw_detect_checker);
    >  	install_keyword("deferred_remove", &hw_deferred_remove_handler, &snprint_hw_deferred_remove);
    >  	install_keyword("delay_watch_checks", &hw_delay_watch_checks_handler, &snprint_hw_delay_watch_checks);
    >  	install_keyword("delay_wait_checks", &hw_delay_wait_checks_handler, &snprint_hw_delay_wait_checks);
    > @@ -1541,6 +1550,7 @@ init_keywords(vector keywords)
    >  	install_keyword("user_friendly_names", &ovr_user_friendly_names_handler, &snprint_ovr_user_friendly_names);
    >  	install_keyword("retain_attached_hw_handler", &ovr_retain_hwhandler_handler, &snprint_ovr_retain_hwhandler);
    >  	install_keyword("detect_prio", &ovr_detect_prio_handler, &snprint_ovr_detect_prio);
    > +	install_keyword("detect_checker", &ovr_detect_checker_handler, &snprint_ovr_detect_checker);
    >  	install_keyword("deferred_remove", &ovr_deferred_remove_handler, &snprint_ovr_deferred_remove);
    >  	install_keyword("delay_watch_checks", &ovr_delay_watch_checks_handler, &snprint_ovr_delay_watch_checks);
    >  	install_keyword("delay_wait_checks", &ovr_delay_wait_checks_handler, &snprint_ovr_delay_wait_checks);
    > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
    > index 39dd92a..4e99845 100644
    > --- a/libmultipath/discovery.c
    > +++ b/libmultipath/discovery.c
    > @@ -30,6 +30,7 @@
    >  #include "discovery.h"
    >  #include "prio.h"
    >  #include "defaults.h"
    > +#include "prioritizers/alua_rtpg.h"
    >  
    >  int
    >  alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
    > @@ -829,6 +830,25 @@ get_serial (char * str, int maxlen, int fd)
    >  	return 1;
    >  }
    >  
    > +static void
    > +detect_alua(struct path * pp, struct config *conf)
    > +{
    > +	int ret;
    > +	int tpgs;
    > +	unsigned int timeout = conf->checker_timeout;
    > +
    > +	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0) {
    > +		pp->tpgs = TPGS_NONE;
    > +		return;
    > +	}
    > +	ret = get_target_port_group(pp, timeout);
    > +	if (ret < 0 || get_asymmetric_access_state(pp->fd, ret, timeout) < 0) {
    > +		pp->tpgs = TPGS_NONE;
    > +		return;
    > +	}
    > +	pp->tpgs = tpgs;
    > +}
    > +
    >  #define DEFAULT_SGIO_LEN 254
    >  
    >  static int
    > @@ -1460,11 +1480,14 @@ sysfs_pathinfo(struct path * pp, vector hwtable)
    >  }
    >  
    >  static int
    > -scsi_ioctl_pathinfo (struct path * pp, int mask)
    > +scsi_ioctl_pathinfo (struct path * pp, struct config *conf, int mask)
    >  {
    >  	struct udev_device *parent;
    >  	const char *attr_path = NULL;
    >  
    > +	if (pp->tpgs == TPGS_UNDEF)
    > +		detect_alua(pp, conf);
    > +
    >  	if (!(mask & DI_SERIAL))
    >  		return 0;
    >  
    > @@ -1524,6 +1547,7 @@ get_state (struct path * pp, struct config *conf, int daemon)
    >  				return PATH_UNCHECKED;
    >  			}
    >  		}
    > +		select_detect_checker(conf, pp);
    >  		select_checker(conf, pp);
    >  		if (!checker_selected(c)) {
    >  			condlog(3, "%s: No checker selected", pp->dev);
    > @@ -1832,7 +1856,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
    >  		get_geometry(pp);
    >  
    >  	if (path_state == PATH_UP && pp->bus == SYSFS_BUS_SCSI &&
    > -	    scsi_ioctl_pathinfo(pp, mask))
    > +	    scsi_ioctl_pathinfo(pp, conf, mask))
    >  		goto blank;
    >  
    >  	if (pp->bus == SYSFS_BUS_CCISS &&
    > diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
    > index 8409261..c944015 100644
    > --- a/libmultipath/hwtable.c
    > +++ b/libmultipath/hwtable.c
    > @@ -1122,6 +1122,7 @@ static struct hwentry default_hw[] = {
    >  		.dev_loss      = 600,
    >  		.retain_hwhandler = RETAIN_HWHANDLER_ON,
    >  		.detect_prio   = DETECT_PRIO_ON,
    > +		.detect_checker = DETECT_CHECKER_ON,
    >  		.deferred_remove = DEFERRED_REMOVE_OFF,
    >  		.delay_watch_checks = DELAY_CHECKS_OFF,
    >  		.delay_wait_checks = DELAY_CHECKS_OFF,
    > diff --git a/libmultipath/prioritizers/alua_spc3.h b/libmultipath/prioritizers/alua_spc3.h
    > index 4d4969b..13a0924 100644
    > --- a/libmultipath/prioritizers/alua_spc3.h
    > +++ b/libmultipath/prioritizers/alua_spc3.h
    > @@ -109,6 +109,7 @@ inquiry_command_set_evpd(struct inquiry_command *ic)
    >  #define VERSION_SPC3					0x05
    >  
    >  /* Defined TPGS field values. */
    > +#define TPGS_UNDEF					 -1
    >  #define TPGS_NONE					0x0
    >  #define TPGS_IMPLICIT					0x1
    >  #define TPGS_EXPLICIT					0x2
    > diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
    > index 1b27476..bba8194 100644
    > --- a/libmultipath/propsel.c
    > +++ b/libmultipath/propsel.c
    > @@ -312,6 +312,11 @@ int select_checker(struct config *conf, struct path *pp)
    >  	char *origin, *checker_name;
    >  	struct checker * c = &pp->checker;
    >  
    > +	if (pp->detect_checker == DETECT_CHECKER_ON && pp->tpgs > 0) {
    > +		checker_name = TUR;
    > +		origin = "(setting: array autodetected)";
    > +		goto out;
    > +	}
    >  	do_set(checker_name, conf->overrides, checker_name, "(setting: multipath.conf overrides section)");
    >  	do_set(checker_name, pp->hwe, checker_name, "(setting: array configuration)");
    >  	do_set(checker_name, conf, checker_name, "(setting: multipath.conf defaults/devices section)");
    > @@ -359,20 +364,11 @@ out:
    >  void
    >  detect_prio(struct config *conf, struct path * pp)
    >  {
    > -	int ret;
    >  	struct prio *p = &pp->prio;
    > -	int tpgs = 0;
    > -	unsigned int timeout = conf->checker_timeout;
    >  	char buff[512];
    >  	char *default_prio = PRIO_ALUA;
    >  
    > -	if ((tpgs = get_target_port_group_support(pp->fd, timeout)) <= 0)
    > -		return;
    > -	pp->tpgs = tpgs;
    > -	ret = get_target_port_group(pp, timeout);
    > -	if (ret < 0)
    > -		return;
    > -	if (get_asymmetric_access_state(pp->fd, ret, timeout) < 0)
    > +	if (pp->tpgs <= 0)
    >  		return;
    >  	if (sysfs_get_asymmetric_access_state(pp, buff, 512) >= 0)
    >  		default_prio = PRIO_SYSFS;
    > @@ -588,6 +584,21 @@ out:
    >  	return 0;
    >  }
    >  
    > +int select_detect_checker(struct config *conf, struct path *pp)
    > +{
    > +	char *origin;
    > +
    > +	pp_set_ovr(detect_checker);
    > +	pp_set_hwe(detect_checker);
    > +	pp_set_conf(detect_checker);
    > +	pp_set_default(detect_checker, DEFAULT_DETECT_CHECKER);
    > +out:
    > +	condlog(3, "%s: detect_checker = %s %s", pp->dev,
    > +		(pp->detect_checker == DETECT_CHECKER_ON)? "yes" : "no",
    > +		origin);
    > +	return 0;
    > +}
    > +
    >  int select_deferred_remove(struct config *conf, struct multipath *mp)
    >  {
    >  	char *origin;
    > diff --git a/libmultipath/propsel.h b/libmultipath/propsel.h
    > index e5b6f93..58a32f3 100644
    > --- a/libmultipath/propsel.h
    > +++ b/libmultipath/propsel.h
    > @@ -19,6 +19,7 @@ int select_dev_loss(struct config *conf, struct multipath *mp);
    >  int select_reservation_key(struct config *conf, struct multipath *mp);
    >  int select_retain_hwhandler (struct config *conf, struct multipath * mp);
    >  int select_detect_prio(struct config *conf, struct path * pp);
    > +int select_detect_checker(struct config *conf, struct path * pp);
    >  int select_deferred_remove(struct config *conf, struct multipath *mp);
    >  int select_delay_watch_checks (struct config *conf, struct multipath * mp);
    >  int select_delay_wait_checks (struct config *conf, struct multipath * mp);
    > diff --git a/libmultipath/structs.c b/libmultipath/structs.c
    > index ba9edf9..f36a055 100644
    > --- a/libmultipath/structs.c
    > +++ b/libmultipath/structs.c
    > @@ -17,6 +17,7 @@
    >  #include "structs_vec.h"
    >  #include "blacklist.h"
    >  #include "prio.h"
    > +#include "prioritizers/alua_spc3.h"
    >  
    >  struct adapter_group *
    >  alloc_adaptergroup(void)
    > @@ -96,6 +97,7 @@ alloc_path (void)
    >  		pp->sg_id.lun = -1;
    >  		pp->sg_id.proto_id = SCSI_PROTOCOL_UNSPEC;
    >  		pp->fd = -1;
    > +		pp->tpgs = TPGS_UNDEF;
    >  		pp->priority = PRIO_UNDEF;
    >  	}
    >  	return pp;
    > diff --git a/libmultipath/structs.h b/libmultipath/structs.h
    > index dfd65ae..fdcfc85 100644
    > --- a/libmultipath/structs.h
    > +++ b/libmultipath/structs.h
    > @@ -122,6 +122,12 @@ enum detect_prio_states {
    >  	DETECT_PRIO_ON = YNU_YES,
    >  };
    >  
    > +enum detect_checker_states {
    > +	DETECT_CHECKER_UNDEF = YNU_UNDEF,
    > +	DETECT_CHECKER_OFF = YNU_NO,
    > +	DETECT_CHECKER_ON = YNU_YES,
    > +};
    > +
    >  enum deferred_remove_states {
    >  	DEFERRED_REMOVE_UNDEF = YNU_UNDEF,
    >  	DEFERRED_REMOVE_OFF = YNU_NO,
    > @@ -211,6 +217,7 @@ struct path {
    >  	int priority;
    >  	int pgindex;
    >  	int detect_prio;
    > +	int detect_checker;
    >  	int watch_checks;
    >  	int wait_checks;
    >  	int tpgs;
    > diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
    > index 179013c..b08bda3 100644
    > --- a/multipath/multipath.conf.5
    > +++ b/multipath/multipath.conf.5
    > @@ -681,6 +681,20 @@ The default is: \fByes\fR
    >  .
    >  .
    >  .TP
    > +.B detect_checker
    > +if set to
    > +.I yes
    > +, multipath will try to detect if the device supports SCSI-3 ALUA. If so, the
    > +device will automatically use the \fItur\fR checker. If set to
    > +.I no
    > +, the checker will be selected as usual.
    > +.RS
    > +.TP
    > +The default is: \fByes\fR
    > +.RE
    > +.
    > +.
    > +.TP
    >  .B force_sync
    >  If set to
    >  .I yes
    > @@ -1169,6 +1183,8 @@ section:
    >  .TP
    >  .B detect_prio
    >  .TP
    > +.B detect_checker
    > +.TP
    >  .B deferred_remove
    >  .TP
    >  .B san_path_err_threshold
    > @@ -1239,6 +1255,8 @@ the values are taken from the \fIdevices\fR or \fIdefaults\fR sections:
    >  .TP
    >  .B detect_prio
    >  .TP
    > +.B detect_checker
    > +.TP
    >  .B deferred_remove
    >  .TP
    >  .B san_path_err_threshold
    > 
    
    

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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-09-25 16:49     ` Benjamin Marzinski
@ 2017-10-05 20:25       ` Martin Wilck
  2017-10-05 20:25         ` [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker Martin Wilck
  2017-10-09 19:09         ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
  0 siblings, 2 replies; 16+ messages in thread
From: Martin Wilck @ 2017-10-05 20:25 UTC (permalink / raw)
  To: Benjamin Marzinski; +Cc: Martin Wilck, dm-devel, Xose Vazquez Perez

On Mon, 2017-09-25 at 11:49 -0500, Benjamin Marzinski wrote:
> On Mon, Sep 25, 2017 at 06:15:20PM +0200, Xose Vazquez Perez wrote:
> > On 02/27/2017 07:26 PM, Benjamin Marzinski wrote:
> > 
> > > This patch adds a detect_checker option that works just like the
> > > detect_prio option.  It currently only detects ALUA devices, and
> > > if it finds ALUA support, it sets the priortizier to TUR. This is
> > > useful for devices like the VNX2, where it should be using the
> > > TUR checker when in ALUA mode (or so I have been told). It is set
> > > on by
> > > default just like detect_prio and retain_attached_hw_handler.
> > 
> > RDAC devices are going also to be affected with this change.
> > Is it desired?
> 
> No. I'll send a patch that adds a check_rdac() call before setting
> the
> checker type to TUR.
> 

Like this, perhaps?

Martin Wilck (1):
  limbultipath: prefer RDAC checker with detect_checker

 libmultipath/propsel.c | 52 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

-- 
2.14.2

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

* [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker
  2017-10-05 20:25       ` Martin Wilck
@ 2017-10-05 20:25         ` Martin Wilck
  2017-10-06 15:35           ` Schremmer, Steven
  2017-10-09 19:09         ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
  1 sibling, 1 reply; 16+ messages in thread
From: Martin Wilck @ 2017-10-05 20:25 UTC (permalink / raw)
  To: Benjamin Marzinski; +Cc: Martin Wilck, dm-devel, Xose Vazquez Perez

With "detect_checker yes", ALUA is used for all storage devices
that support ALUA. But currently RDAC is still preferred for RDAC
devices
(https://www.redhat.com/archives/dm-devel/2017-September/msg00326.html)

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/propsel.c | 52 +++++++++++++++++++++++++++++---------------------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 00adc0da4cb1..4a7e3811d21c 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -367,16 +367,42 @@ out:
 	return 0;
 }
 
+/*
+ * Current RDAC (NetApp E-Series) firmware relies
+ * on periodic REPORT TARGET PORT GROUPS for
+ * internal load balancing.
+ * Using the sysfs priority checker defeats this purpose.
+ *
+ * Moreover, NetApp would also prefer the RDAC checker over ALUA.
+ * (https://www.redhat.com/archives/dm-devel/2017-September/msg00326.html)
+ */
+static int
+check_rdac(struct path * pp)
+{
+	int len;
+	char buff[44];
+
+	len = get_vpd_sgio(pp->fd, 0xC9, buff, 44);
+	if (len <= 0)
+		return 0;
+	return !(memcmp(buff + 4, "vac1", 4));
+}
+
 int select_checker(struct config *conf, struct path *pp)
 {
 	char *origin, *checker_name;
 	struct checker * c = &pp->checker;
 
-	if (pp->detect_checker == DETECT_CHECKER_ON && pp->tpgs > 0) {
-		checker_name = TUR;
+	if (pp->detect_checker == DETECT_CHECKER_ON) {
 		origin = "(setting: storage device autodetected)";
-		goto out;
-	}
+		if (check_rdac(pp)) {
+			checker_name = RDAC;
+			goto out;
+		} else if (pp->tpgs > 0) {
+			checker_name = TUR;
+			goto out;
+		}
+ 	}
 	do_set(checker_name, conf->overrides, checker_name, "(setting: multipath.conf overrides section)");
 	do_set(checker_name, pp->hwe, checker_name, "(setting: storage device configuration)");
 	do_set(checker_name, conf, checker_name, "(setting: multipath.conf defaults/devices section)");
@@ -427,24 +453,6 @@ out:
 	return 0;
 }
 
-/*
- * Current RDAC (NetApp E-Series) firmware relies
- * on periodic REPORT TARGET PORT GROUPS for
- * internal load balancing.
- * Using the sysfs priority checker defeats this purpose.
- */
-static int
-check_rdac(struct path * pp)
-{
-	int len;
-	char buff[44];
-
-	len = get_vpd_sgio(pp->fd, 0xC9, buff, 44);
-	if (len <= 0)
-		return 0;
-	return !(memcmp(buff + 4, "vac1", 4));
-}
-
 void
 detect_prio(struct config *conf, struct path * pp)
 {
-- 
2.14.2

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

* Re: [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker
  2017-10-05 20:25         ` [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker Martin Wilck
@ 2017-10-06 15:35           ` Schremmer, Steven
  0 siblings, 0 replies; 16+ messages in thread
From: Schremmer, Steven @ 2017-10-06 15:35 UTC (permalink / raw)
  To: Martin Wilck, Benjamin Marzinski
  Cc: Martin Wilck, dm-devel, Xose Vazquez Perez

> From: dm-devel-bounces@redhat.com [mailto:dm-devel-bounces@redhat.com] On Behalf Of Martin Wilck
> Sent: Thursday, October 05, 2017 3:26 PM
> To: Benjamin Marzinski <bmarzins@redhat.com>
> Cc: Martin Wilck <mwilck@suse.de>; dm-devel@redhat.com; Xose Vazquez Perez <xose.vazquez@gmail.com>
> Subject: [dm-devel] [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker
> 
> With "detect_checker yes", ALUA is used for all storage devices
> that support ALUA. But currently RDAC is still preferred for RDAC
> devices
> (https://www.redhat.com/archives/dm-devel/2017-September/msg00326.html)
> 
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>  libmultipath/propsel.c | 52 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 

Reviewed-by: Steve Schremmer <steve.schremmer@netapp.com>

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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-10-05 20:25       ` Martin Wilck
  2017-10-05 20:25         ` [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker Martin Wilck
@ 2017-10-09 19:09         ` Benjamin Marzinski
  2017-10-09 19:25           ` Martin Wilck
  1 sibling, 1 reply; 16+ messages in thread
From: Benjamin Marzinski @ 2017-10-09 19:09 UTC (permalink / raw)
  To: Martin Wilck; +Cc: Martin Wilck, dm-devel, Xose Vazquez Perez

On Thu, Oct 05, 2017 at 10:25:50PM +0200, Martin Wilck wrote:
> On Mon, 2017-09-25 at 11:49 -0500, Benjamin Marzinski wrote:
> > On Mon, Sep 25, 2017 at 06:15:20PM +0200, Xose Vazquez Perez wrote:
> > > On 02/27/2017 07:26 PM, Benjamin Marzinski wrote:
> > > 
> > > > This patch adds a detect_checker option that works just like the
> > > > detect_prio option.  It currently only detects ALUA devices, and
> > > > if it finds ALUA support, it sets the priortizier to TUR. This is
> > > > useful for devices like the VNX2, where it should be using the
> > > > TUR checker when in ALUA mode (or so I have been told). It is set
> > > > on by
> > > > default just like detect_prio and retain_attached_hw_handler.
> > > 
> > > RDAC devices are going also to be affected with this change.
> > > Is it desired?
> > 
> > No. I'll send a patch that adds a check_rdac() call before setting
> > the
> > checker type to TUR.
> > 
> 
> Like this, perhaps?
> 
> Martin Wilck (1):
>   limbultipath: prefer RDAC checker with detect_checker

Oops. I totally dropped this off my plate. But, yes, just like that.

ACK

-Ben

> 
>  libmultipath/propsel.c | 52 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 30 insertions(+), 22 deletions(-)
> 
> -- 
> 2.14.2

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

* Re: [PATCH v4 1/6] libmultipath: add detect_checker option
  2017-10-09 19:09         ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
@ 2017-10-09 19:25           ` Martin Wilck
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Wilck @ 2017-10-09 19:25 UTC (permalink / raw)
  To: Benjamin Marzinski, Christophe Varoqui
  Cc: dm-devel, Xose Vazquez Perez, Hannes

On Mon, 2017-10-09 at 14:09 -0500, Benjamin Marzinski wrote:
> On Thu, Oct 05, 2017 at 10:25:50PM +0200, Martin Wilck wrote:
> > 
> > Like this, perhaps?
> > 
> > Martin Wilck (1):
> >   limbultipath: prefer RDAC checker with detect_checker
> 
> Oops. I totally dropped this off my plate. But, yes, just like that.
> 
> ACK
> 
> -Ben

Thanks a lot. Sorry Christophe, I forgot to send the patch to your
email address. Please consider it for inclusion.

Regards
Martin

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

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

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

end of thread, other threads:[~2017-10-09 19:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-27 18:26 [PATCH v4 0/6] misc patches Benjamin Marzinski
2017-02-27 18:26 ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
2017-09-25 16:15   ` Xose Vazquez Perez
2017-09-25 16:49     ` Benjamin Marzinski
2017-10-05 20:25       ` Martin Wilck
2017-10-05 20:25         ` [PATCH 1/1] limbultipath: prefer RDAC checker with detect_checker Martin Wilck
2017-10-06 15:35           ` Schremmer, Steven
2017-10-09 19:09         ` [PATCH v4 1/6] libmultipath: add detect_checker option Benjamin Marzinski
2017-10-09 19:25           ` Martin Wilck
2017-09-25 20:12     ` Stewart, Sean
2017-02-27 18:26 ` [PATCH v4 2/6] libmultipath: cleanup orphan device states Benjamin Marzinski
2017-02-27 18:26 ` [PATCH v4 3/6] multipathd: don't update priority of failed paths Benjamin Marzinski
2017-02-27 18:26 ` [PATCH v4 4/6] multipathd: add messages on delayed path addition Benjamin Marzinski
2017-02-27 18:26 ` [PATCH v4 5/6] multipathd: allow resetting stats Benjamin Marzinski
2017-02-27 18:26 ` [PATCH v4 6/6] fix udev rules for failed multipath devices Benjamin Marzinski
2017-02-27 20:46 ` [PATCH v4 0/6] misc patches Christophe Varoqui

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