All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/10] Misc multipath patches
@ 2018-07-13 19:39 Benjamin Marzinski
  2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

- patches 0001 and 0005 patch is some small unused code cleanup
- patches 0002 & 0003 are just some small bugfixes
- patch 0004 adds --param-alltgpt support to mpathpersist, since it
  appears to work correctly on devices that accept ALL_TG_PT
- patches 0006 & 0007 add a new method for blacklisting devices,
  "protocol", along with path wildcards to see the value of this for
  paths. I added this to deal with requests for being able to set
  multipath to only look at, for instancei, FC and iSCSI devices,
  without knowing beforehand what devices are going to be present on a
  system. There wasn't a set of udev properties that always worked to
  distinguish this for all devices, even if we could blacklist based on
  the values (which multipath currently can't).
- patch 0008 cleans up the blacklist filtering code, and fixes some
  printing inconsistencies.  It does mean that when filter_path is
  called, all of the whitelist messages are printed, even if a path is
  eventually blacklisted. Of course the blacklist message is printed
  last, and there can already be prior whitelist messages in the logs
  from calls to the other filter_* functions.
- patch 0009 is a small change to the how the tests Makefile works, to
  make the unit tests compile on an older machine I tested on.
- patch 0010 is a new set of unit tests that validate the return codes
  and the messages printed by the blacklist filter_* functions.

Benjamin Marzinski (10):
  libmultipath: remove last of rbd code
  libmultipath: fix detect alua corner case
  multipath: fix setting conf->version
  mpathpersist: add --param-alltgpt option
  libmutipath: remove unused IDE bus type
  multipathd: add new protocol path wildcard
  libmultipath: add "protocol" blacklist option.
  libmultipath: remove _filter_* blacklist functions
  multipath tests: change to work with old make versions
  multipath tests: add blacklist tests

 kpartx/del-part-nodes.rules     |   2 +-
 libmpathpersist/mpath_persist.c |  10 +-
 libmultipath/blacklist.c        | 177 +++++++-------
 libmultipath/blacklist.h        |   5 +-
 libmultipath/config.c           |  15 ++
 libmultipath/config.h           |   2 +
 libmultipath/configure.c        |   2 +-
 libmultipath/devmapper.c        |   6 +-
 libmultipath/dict.c             |  14 +-
 libmultipath/discovery.c        |   5 +-
 libmultipath/print.c            |  74 ++++++
 libmultipath/print.h            |   2 +
 libmultipath/propsel.c          |   4 +-
 libmultipath/structs.h          |   2 -
 mpathpersist/main.c             |  11 +-
 mpathpersist/main.h             |   1 +
 mpathpersist/mpathpersist.8     |   4 +
 multipath/multipath.conf.5      |  24 +-
 tests/Makefile                  |   6 +-
 tests/blacklist.c               | 512 ++++++++++++++++++++++++++++++++++++++++
 20 files changed, 762 insertions(+), 116 deletions(-)
 create mode 100644 tests/blacklist.c

-- 
2.7.4

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

* [PATCH 01/10] libmultipath: remove last of rbd code
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16  9:25   ` Martin Wilck
  2018-07-16  9:52   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 02/10] libmultipath: fix detect alua corner case Benjamin Marzinski
                   ` (8 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

My previous patch to remove the rbd code missed a little bit.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 kpartx/del-part-nodes.rules | 2 +-
 libmultipath/structs.h      | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/kpartx/del-part-nodes.rules b/kpartx/del-part-nodes.rules
index 17bc505..0ceecf5 100644
--- a/kpartx/del-part-nodes.rules
+++ b/kpartx/del-part-nodes.rules
@@ -10,7 +10,7 @@
 # or create an udev rule file that sets ENV{DONT_DEL_PART_NODES}="1".
 
 SUBSYSTEM!="block", GOTO="end_del_part_nodes"
-KERNEL!="sd*|dasd*|rbd*", GOTO="end_del_part_nodes"
+KERNEL!="sd*|dasd*", GOTO="end_del_part_nodes"
 ACTION!="add|change", GOTO="end_del_part_nodes"
 ENV{DEVTYPE}=="partition", GOTO="end_del_part_nodes"
 
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index e5b698b..ca14315 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -61,7 +61,6 @@ enum sysfs_buses {
 	SYSFS_BUS_IDE,
 	SYSFS_BUS_CCW,
 	SYSFS_BUS_CCISS,
-	SYSFS_BUS_RBD,
 	SYSFS_BUS_NVME,
 };
 
-- 
2.7.4

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

* [PATCH 02/10] libmultipath: fix detect alua corner case
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
  2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16  9:32   ` Martin Wilck
  2018-07-16  9:52   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 03/10] multipath: fix setting conf->version Benjamin Marzinski
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

If retain_attach_hw_handler = no, then the paths tpgs state will never
be checked, and the multipath device will always select the alua
handler, if no other handler is selected. The paths tpgs state
should be checked, regardless of the retain_hwhandler value.

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

diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index af3ed62..fdb5953 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -420,9 +420,11 @@ int select_hwhandler(struct config *conf, struct multipath *mp)
 	bool all_tpgs = true;
 
 	dh_state = &handler[2];
+
+	vector_foreach_slot(mp->paths, pp, i)
+		all_tpgs = all_tpgs && (pp->tpgs > 0);
 	if (mp->retain_hwhandler != RETAIN_HWHANDLER_OFF) {
 		vector_foreach_slot(mp->paths, pp, i) {
-			all_tpgs = all_tpgs && (pp->tpgs > 0);
 			if (get_dh_state(pp, dh_state, sizeof(handler) - 2) > 0
 			    && strcmp(dh_state, "detached")) {
 				memcpy(handler, "1 ", 2);
-- 
2.7.4

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

* [PATCH 03/10] multipath: fix setting conf->version
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
  2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
  2018-07-13 19:39 ` [PATCH 02/10] libmultipath: fix detect alua corner case Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16  9:51   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 04/10] mpathpersist: add --param-alltgpt option Benjamin Marzinski
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

Commit d3b71498 stopped multipath from setting conf->version. Instead,
it was always being set to 0.0.0. Multipathd was still setting this
correctly.

Fixes: d3b71498 "multipath: fix rcu thread cancellation hang"
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/devmapper.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index f2befad..8136d15 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -245,13 +245,13 @@ void libmp_dm_init(void)
 	int verbosity;
 	unsigned int version[3];
 
+	if (dm_prereq(version))
+		exit(1);
 	conf = get_multipath_config();
 	verbosity = conf->verbosity;
-	memcpy(version, conf->version, sizeof(version));
+	memcpy(conf->version, version, sizeof(version));
 	put_multipath_config(conf);
 	dm_init(verbosity);
-	if (dm_prereq(version))
-		exit(1);
 	dm_udev_set_sync_support(libmp_dm_udev_sync);
 }
 
-- 
2.7.4

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

* [PATCH 04/10] mpathpersist: add --param-alltgpt option
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (2 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 03/10] multipath: fix setting conf->version Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16  9:59   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 05/10] libmutipath: remove unused IDE bus type Benjamin Marzinski
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

From the limited testing I've been able to do, commit 5b54e772
"mpathpersist: add all_tg_pt option", does appear to enable
--param-alltgpt to work correctly on devices that accept the ALL_TG_PT
flag, so I've added the option to mpathpersist.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathpersist/mpath_persist.c | 10 ++++------
 mpathpersist/main.c             | 11 ++++++++---
 mpathpersist/main.h             |  1 +
 mpathpersist/mpathpersist.8     |  4 ++++
 multipath/multipath.conf.5      |  8 +++++---
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 6e9e67f..61818e0 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -466,11 +466,14 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 	int rc;
 	int count=0;
 	int status = MPATH_PR_SUCCESS;
+	int all_tg_pt;
 	uint64_t sa_key = 0;
 
 	if (!mpp)
 		return MPATH_PR_DMMP_ERROR;
 
+	all_tg_pt = (mpp->all_tg_pt == ALL_TG_PT_ON ||
+		     paramp->sa_flags & MPATH_F_ALL_TG_PT_MASK);
 	active_pathcount = pathcount(mpp, PATH_UP) + pathcount(mpp, PATH_GHOST);
 
 	if (active_pathcount == 0) {
@@ -478,10 +481,6 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 		return MPATH_PR_DMMP_ERROR;
 	}
 
-	if ( paramp->sa_flags & MPATH_F_ALL_TG_PT_MASK ) {
-		condlog (1, "Warning: ALL_TG_PT is set. Configuration not supported");
-	}
-
 	struct threadinfo thread[active_pathcount];
 	int hosts[active_pathcount];
 
@@ -518,8 +517,7 @@ int mpath_prout_reg(struct multipath *mpp,int rq_servact, int rq_scope,
 				condlog (1, "%s: %s path not up. Skip.", mpp->wwid, pp->dev);
 				continue;
 			}
-			if (mpp->all_tg_pt == ALL_TG_PT_ON &&
-			    pp->sg_id.host_no != -1) {
+			if (all_tg_pt && pp->sg_id.host_no != -1) {
 				for (k = 0; k < count; k++) {
 					if (pp->sg_id.host_no == hosts[k]) {
 						condlog(3, "%s: %s host %d matches skip.", pp->wwid, pp->dev, pp->sg_id.host_no);
diff --git a/mpathpersist/main.c b/mpathpersist/main.c
index 5b37f3a..99151fe 100644
--- a/mpathpersist/main.c
+++ b/mpathpersist/main.c
@@ -118,7 +118,7 @@ int main (int argc, char * argv[])
 	{
 		int option_index = 0;
 
-		c = getopt_long (argc, argv, "v:Cd:hHioZK:S:PAT:skrGILcRX:l:",
+		c = getopt_long (argc, argv, "v:Cd:hHioYZK:S:PAT:skrGILcRX:l:",
 				long_options, &option_index);
 		if (c == -1)
 			break;
@@ -158,6 +158,10 @@ int main (int argc, char * argv[])
 				prout_flag = 1;
 				break;
 
+			case 'Y':
+				param_alltgpt = 1;
+				++num_prout_param;
+				break;
 			case 'Z':
 				param_aptpl = 1;
 				++num_prout_param;
@@ -443,9 +447,9 @@ int main (int argc, char * argv[])
 		}
 
 		if (param_alltgpt)
-			paramp->sa_flags |= 0x4;
+			paramp->sa_flags |= MPATH_F_ALL_TG_PT_MASK;
 		if (param_aptpl)
-			paramp->sa_flags |= 0x1;
+			paramp->sa_flags |= MPATH_F_APTPL_MASK;
 
 		if (num_transport)
 		{
@@ -698,6 +702,7 @@ static void usage(void)
 			"    --hex|-H                   output response in hex\n"
 			"    --in|-i                    request PR In command \n"
 			"    --out|-o                   request PR Out command\n"
+			"    --param-alltgpt|-Y         PR Out parameter 'ALL_TG_PT\n"
 			"    --param-aptpl|-Z           PR Out parameter 'APTPL'\n"
 			"    --read-keys|-k             PR In: Read Keys\n"
 			"    --param-sark=SARK|-S SARK  PR Out parameter service "
diff --git a/mpathpersist/main.h b/mpathpersist/main.h
index 5c0e089..beb8a21 100644
--- a/mpathpersist/main.h
+++ b/mpathpersist/main.h
@@ -6,6 +6,7 @@ static struct option long_options[] = {
 	{"hex", 0, NULL, 'H'},
 	{"in", 0, NULL, 'i'},
 	{"out", 0, NULL, 'o'},
+	{"param-alltgpt", 0, NULL, 'Y'},
 	{"param-aptpl", 0, NULL, 'Z'},
 	{"param-rk", 1, NULL, 'K'},
 	{"param-sark", 1, NULL, 'S'},
diff --git a/mpathpersist/mpathpersist.8 b/mpathpersist/mpathpersist.8
index a8982e6..885491d 100644
--- a/mpathpersist/mpathpersist.8
+++ b/mpathpersist/mpathpersist.8
@@ -87,6 +87,10 @@ Request PR In command.
 Request PR Out command.
 .
 .TP
+.B \--param-alltgpt|\-Y
+PR Out parameter 'ALL_TG_PT'.
+.
+.TP
 .B \--param-aptpl|\-Z
 PR Out parameter 'APTPL'.
 .
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index e4b25a0..fb863fd 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -756,9 +756,11 @@ The default is: \fB<unset>\fR
 .
 .TP
 .B all_tg_pt
-This must be set to \fByes\fR to successfully use mpathpersist on arrays that
-automatically set and clear registration keys on all target ports from a
-host, instead of per target port per host.
+Set the 'all targets ports' flag when registering keys with mpathpersist. Some
+arrays automatically set and clear registration keys on all target ports from a
+host, instead of per target port per host. The ALL_TG_PT flag must be set to
+successfully use mpathpersist on these arrays. Setting this option is identical
+to calling mpathpersist with \fI--param-alltgpt\fR
 .RS
 .TP
 The default is: \fBno\fR
-- 
2.7.4

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

* [PATCH 05/10] libmutipath: remove unused IDE bus type
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (3 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 04/10] mpathpersist: add --param-alltgpt option Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16 10:01   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 06/10] multipathd: add new protocol path wildcard Benjamin Marzinski
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

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

diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index ca14315..0a2623a 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -58,7 +58,6 @@ enum failback_mode {
 enum sysfs_buses {
 	SYSFS_BUS_UNDEF,
 	SYSFS_BUS_SCSI,
-	SYSFS_BUS_IDE,
 	SYSFS_BUS_CCW,
 	SYSFS_BUS_CCISS,
 	SYSFS_BUS_NVME,
-- 
2.7.4

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

* [PATCH 06/10] multipathd: add new protocol path wildcard
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (4 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 05/10] libmutipath: remove unused IDE bus type Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-16 10:09   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 07/10] libmultipath: add "protocol" blacklist option Benjamin Marzinski
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

This patch adds a new path wildcard 'P', that will print the path's
protocol.  For scsi devices, it will additionally print the transport
protocol being used.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/print.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 libmultipath/print.h |  2 ++
 2 files changed, 45 insertions(+)

diff --git a/libmultipath/print.c b/libmultipath/print.c
index 222d270..ecfcb48 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -638,6 +638,48 @@ snprint_path_failures(char * buff, size_t len, const struct path * pp)
 	return snprint_int(buff, len, pp->failcount);
 }
 
+/* if you add a protocol string bigger than "scsi:unspec" you must
+ * also change PROTOCOL_BUF_SIZE */
+int
+snprint_path_protocol(char * buff, size_t len, const struct path * pp)
+{
+	switch (pp->bus) {
+	case SYSFS_BUS_SCSI:
+		switch (pp->sg_id.proto_id) {
+		case SCSI_PROTOCOL_FCP:
+			return snprintf(buff, len, "scsi:fcp");
+		case SCSI_PROTOCOL_SPI:
+			return snprintf(buff, len, "scsi:spi");
+		case SCSI_PROTOCOL_SSA:
+			return snprintf(buff, len, "scsi:ssa");
+		case SCSI_PROTOCOL_SBP:
+			return snprintf(buff, len, "scsi:sbp");
+		case SCSI_PROTOCOL_SRP:
+			return snprintf(buff, len, "scsi:srp");
+		case SCSI_PROTOCOL_ISCSI:
+			return snprintf(buff, len, "scsi:iscsi");
+		case SCSI_PROTOCOL_SAS:
+			return snprintf(buff, len, "scsi:sas");
+		case SCSI_PROTOCOL_ADT:
+			return snprintf(buff, len, "scsi:adt");
+		case SCSI_PROTOCOL_ATA:
+			return snprintf(buff, len, "scsi:ata");
+		case SCSI_PROTOCOL_UNSPEC:
+		default:
+			return snprintf(buff, len, "scsi:unspec");
+		}
+	case SYSFS_BUS_CCW:
+		return snprintf(buff, len, "ccw");
+	case SYSFS_BUS_CCISS:
+		return snprintf(buff, len, "cciss");
+	case SYSFS_BUS_NVME:
+		return snprintf(buff, len, "nvme");
+	case SYSFS_BUS_UNDEF:
+	default:
+		return snprintf(buff, len, "undef");
+	}
+}
+
 struct multipath_data mpd[] = {
 	{'n', "name",          0, snprint_name},
 	{'w', "uuid",          0, snprint_multipath_uuid},
@@ -687,6 +729,7 @@ struct path_data pd[] = {
 	{'a', "host adapter",  0, snprint_host_adapter},
 	{'G', "foreign",       0, snprint_path_foreign},
 	{'0', "failures",      0, snprint_path_failures},
+	{'P', "protocol",      0, snprint_path_protocol},
 	{0, NULL, 0 , NULL}
 };
 
diff --git a/libmultipath/print.h b/libmultipath/print.h
index 608b7d5..e2fb865 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -133,6 +133,8 @@ int snprint_host_wwnn (char *, size_t, const struct path *);
 int snprint_host_wwpn (char *, size_t, const struct path *);
 int snprint_tgt_wwnn (char *, size_t, const struct path *);
 int snprint_tgt_wwpn (char *, size_t, const struct path *);
+#define PROTOCOL_BUF_SIZE sizeof("scsi:unspec")
+int snprint_path_protocol(char *, size_t, const struct path *);
 
 void _print_multipath_topology (const struct gen_multipath * gmp,
 				int verbosity);
-- 
2.7.4

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

* [PATCH 07/10] libmultipath: add "protocol" blacklist option.
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (5 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 06/10] multipathd: add new protocol path wildcard Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-20 18:49   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 08/10] libmultipath: remove _filter_* blacklist functions Benjamin Marzinski
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

Multiple users have requested an easy way to setup blacklists that do
things such as blacklisting all non FC and iSCSI devices. Currently
there is no easy way to do this, without knowing in advance what the
devices are.  Looking into the udev property values, I didn't see a
consistent set of values that would worked for all the different types
of requests like this (which would have allowed us to solve this by
extending the "property" blacklist option to allow comparing values,
instead of just keywords).

Instead I've opted to allow multipath to blacklist/whitelist devices
by the protocol strings printed by "multipathd: add new protocol path
wildcard". This check happens after multipath checks the "device"
keyword, and before it checks wwid. This gives users an easily
understandible method to set up these types of blacklists, without
needing to know the exact arrays being used.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c   | 51 ++++++++++++++++++++++++++++++++++++++--------
 libmultipath/blacklist.h   |  3 +++
 libmultipath/config.c      | 15 ++++++++++++++
 libmultipath/config.h      |  2 ++
 libmultipath/dict.c        | 14 +++++++++++--
 libmultipath/discovery.c   |  5 +++--
 libmultipath/print.c       | 31 ++++++++++++++++++++++++++++
 multipath/multipath.conf.5 | 16 +++++++++++++--
 8 files changed, 123 insertions(+), 14 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 361c603..fdd36f7 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -12,6 +12,8 @@
 #include "structs.h"
 #include "config.h"
 #include "blacklist.h"
+#include "structs_vec.h"
+#include "print.h"
 
 int store_ble(vector blist, char * str, int origin)
 {
@@ -240,12 +242,14 @@ setup_default_blist (struct config * conf)
 		condlog(3, "%s: %s %s %s", dev, (M), wwid, (S));	\
 	else if (env)							\
 		condlog(3, "%s: %s %s %s", dev, (M), env, (S));		\
+	else if (protocol)						\
+		condlog(3, "%s: %s %s %s", dev, (M), protocol, (S));	\
 	else								\
 		condlog(3, "%s: %s %s", dev, (M), (S))
 
 void
 log_filter (const char *dev, char *vendor, char *product, char *wwid,
-	    const char *env, int r)
+	    const char *env, const char *protocol, int r)
 {
 	/*
 	 * Try to sort from most likely to least.
@@ -265,6 +269,9 @@ log_filter (const char *dev, char *vendor, char *product, char *wwid,
 	case MATCH_PROPERTY_BLIST:
 		LOG_BLIST("udev property", "blacklisted");
 		break;
+	case MATCH_PROTOCOL_BLIST:
+		LOG_BLIST("protocol", "blacklisted");
+		break;
 	case MATCH_DEVICE_BLIST_EXCEPT:
 		LOG_BLIST("vendor/product", "whitelisted");
 		break;
@@ -280,6 +287,9 @@ log_filter (const char *dev, char *vendor, char *product, char *wwid,
 	case MATCH_PROPERTY_BLIST_MISSING:
 		LOG_BLIST("blacklisted,", "udev property missing");
 		break;
+	case MATCH_PROTOCOL_BLIST_EXCEPT:
+		LOG_BLIST("protocol", "whitelisted");
+		break;
 	}
 }
 
@@ -299,7 +309,7 @@ int
 filter_device (vector blist, vector elist, char * vendor, char * product)
 {
 	int r = _filter_device(blist, elist, vendor, product);
-	log_filter(NULL, vendor, product, NULL, NULL, r);
+	log_filter(NULL, vendor, product, NULL, NULL, NULL, r);
 	return r;
 }
 
@@ -319,7 +329,7 @@ int
 filter_devnode (vector blist, vector elist, char * dev)
 {
 	int r = _filter_devnode(blist, elist, dev);
-	log_filter(dev, NULL, NULL, NULL, NULL, r);
+	log_filter(dev, NULL, NULL, NULL, NULL, NULL, r);
 	return r;
 }
 
@@ -339,7 +349,29 @@ int
 filter_wwid (vector blist, vector elist, char * wwid, char * dev)
 {
 	int r = _filter_wwid(blist, elist, wwid);
-	log_filter(dev, NULL, NULL, wwid, NULL, r);
+	log_filter(dev, NULL, NULL, wwid, NULL, NULL, r);
+	return r;
+}
+
+static int
+_filter_protocol (vector blist, vector elist, const char * protocol_str)
+{
+	if (_blacklist_exceptions(elist, protocol_str))
+		return MATCH_PROTOCOL_BLIST_EXCEPT;
+	if (_blacklist(blist, protocol_str))
+		return MATCH_PROTOCOL_BLIST;
+	return 0;
+}
+
+int
+filter_protocol(vector blist, vector elist, struct path * pp)
+{
+	char buf[PROTOCOL_BUF_SIZE];
+	int r;
+
+	snprint_path_protocol(buf, sizeof(buf), pp);
+	r = _filter_protocol(blist, elist, buf);
+	log_filter(pp->dev, NULL, NULL, NULL, NULL, buf, r);
 	return r;
 }
 
@@ -351,7 +383,6 @@ _filter_path (struct config * conf, struct path * pp)
 	r = filter_property(conf, pp->udev);
 	if (r > 0)
 		return r;
-
 	r = _filter_devnode(conf->blist_devnode, conf->elist_devnode,pp->dev);
 	if (r > 0)
 		return r;
@@ -359,6 +390,9 @@ _filter_path (struct config * conf, struct path * pp)
 			   pp->vendor_id, pp->product_id);
 	if (r > 0)
 		return r;
+	r = filter_protocol(conf->blist_protocol, conf->elist_protocol, pp);
+	if (r > 0)
+		return r;
 	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
 	return r;
 }
@@ -367,7 +401,8 @@ int
 filter_path (struct config * conf, struct path * pp)
 {
 	int r=_filter_path(conf, pp);
-	log_filter(pp->dev, pp->vendor_id, pp->product_id, pp->wwid, NULL, r);
+	log_filter(pp->dev, pp->vendor_id, pp->product_id, pp->wwid, NULL,
+		   NULL, r);
 	return r;
 }
 
@@ -402,7 +437,7 @@ filter_property(struct config * conf, struct udev_device * udev)
 
 		r = _filter_property(conf, env);
 		if (r) {
-			log_filter(devname, NULL, NULL, NULL, env, r);
+			log_filter(devname, NULL, NULL, NULL, env, NULL, r);
 			return r;
 		}
 	}
@@ -411,7 +446,7 @@ filter_property(struct config * conf, struct udev_device * udev)
 	 * This is the inverse of the 'normal' matching;
 	 * the environment variable _has_ to match.
 	 */
-	log_filter(devname, NULL, NULL, NULL, NULL,
+	log_filter(devname, NULL, NULL, NULL, NULL, NULL,
 		   MATCH_PROPERTY_BLIST_MISSING);
 	return MATCH_PROPERTY_BLIST_MISSING;
 }
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index 0b028d4..f7beef2 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -10,10 +10,12 @@
 #define MATCH_DEVNODE_BLIST  3
 #define MATCH_PROPERTY_BLIST 4
 #define MATCH_PROPERTY_BLIST_MISSING 5
+#define MATCH_PROTOCOL_BLIST 6
 #define MATCH_WWID_BLIST_EXCEPT     -MATCH_WWID_BLIST
 #define MATCH_DEVICE_BLIST_EXCEPT   -MATCH_DEVICE_BLIST
 #define MATCH_DEVNODE_BLIST_EXCEPT  -MATCH_DEVNODE_BLIST
 #define MATCH_PROPERTY_BLIST_EXCEPT -MATCH_PROPERTY_BLIST
+#define MATCH_PROTOCOL_BLIST_EXCEPT -MATCH_PROTOCOL_BLIST
 
 struct blentry {
 	char * str;
@@ -36,6 +38,7 @@ int filter_wwid (vector, vector, char *, char *);
 int filter_device (vector, vector, char *, char *);
 int filter_path (struct config *, struct path *);
 int filter_property(struct config *, struct udev_device *);
+int filter_protocol(vector, vector, struct path *);
 int store_ble (vector, char *, int);
 int set_ble_device (vector, char *, char *, int);
 void free_blacklist (vector);
diff --git a/libmultipath/config.c b/libmultipath/config.c
index afa309d..0aef186 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -623,11 +623,13 @@ free_config (struct config * conf)
 	free_blacklist(conf->blist_devnode);
 	free_blacklist(conf->blist_wwid);
 	free_blacklist(conf->blist_property);
+	free_blacklist(conf->blist_protocol);
 	free_blacklist_device(conf->blist_device);
 
 	free_blacklist(conf->elist_devnode);
 	free_blacklist(conf->elist_wwid);
 	free_blacklist(conf->elist_property);
+	free_blacklist(conf->elist_protocol);
 	free_blacklist_device(conf->elist_device);
 
 	free_mptable(conf->mptable);
@@ -780,6 +782,12 @@ load_config (char * file)
 		if (!conf->blist_property)
 			goto out;
 	}
+	if (conf->blist_protocol == NULL) {
+		conf->blist_protocol = vector_alloc();
+
+		if (!conf->blist_protocol)
+			goto out;
+	}
 
 	if (conf->elist_devnode == NULL) {
 		conf->elist_devnode = vector_alloc();
@@ -807,6 +815,13 @@ load_config (char * file)
 		if (!conf->elist_property)
 			goto out;
 	}
+	if (conf->elist_protocol == NULL) {
+		conf->elist_protocol = vector_alloc();
+
+		if (!conf->elist_protocol)
+			goto out;
+	}
+
 	if (setup_default_blist(conf))
 		goto out;
 
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 6bd42f0..7d0cd9a 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -210,10 +210,12 @@ struct config {
 	vector blist_wwid;
 	vector blist_device;
 	vector blist_property;
+	vector blist_protocol;
 	vector elist_devnode;
 	vector elist_wwid;
 	vector elist_device;
 	vector elist_property;
+	vector elist_protocol;
 };
 
 extern struct udev * udev;
diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index 15e7582..32524d5 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -1291,9 +1291,12 @@ blacklist_handler(struct config *conf, vector strvec)
 		conf->blist_device = vector_alloc();
 	if (!conf->blist_property)
 		conf->blist_property = vector_alloc();
+	if (!conf->blist_protocol)
+		conf->blist_protocol = vector_alloc();
 
 	if (!conf->blist_devnode || !conf->blist_wwid ||
-	    !conf->blist_device || !conf->blist_property)
+	    !conf->blist_device || !conf->blist_property ||
+	    !conf->blist_protocol)
 		return 1;
 
 	return 0;
@@ -1310,9 +1313,12 @@ blacklist_exceptions_handler(struct config *conf, vector strvec)
 		conf->elist_device = vector_alloc();
 	if (!conf->elist_property)
 		conf->elist_property = vector_alloc();
+	if (!conf->elist_protocol)
+		conf->elist_protocol = vector_alloc();
 
 	if (!conf->elist_devnode || !conf->elist_wwid ||
-	    !conf->elist_device || !conf->elist_property)
+	    !conf->elist_device || !conf->elist_property ||
+	    !conf->elist_protocol)
 		return 1;
 
 	return 0;
@@ -1356,6 +1362,8 @@ declare_ble_handler(blist_wwid)
 declare_ble_handler(elist_wwid)
 declare_ble_handler(blist_property)
 declare_ble_handler(elist_property)
+declare_ble_handler(blist_protocol)
+declare_ble_handler(elist_protocol)
 
 static int
 snprint_def_uxsock_timeout(struct config *conf, char * buff, int len,
@@ -1627,6 +1635,7 @@ init_keywords(vector keywords)
 	install_keyword_multi("devnode", &ble_blist_devnode_handler, &snprint_ble_simple);
 	install_keyword_multi("wwid", &ble_blist_wwid_handler, &snprint_ble_simple);
 	install_keyword_multi("property", &ble_blist_property_handler, &snprint_ble_simple);
+	install_keyword_multi("protocol", &ble_blist_protocol_handler, &snprint_ble_simple);
 	install_keyword_multi("device", &ble_device_handler, NULL);
 	install_sublevel();
 	install_keyword("vendor", &ble_blist_device_vendor_handler, &snprint_bled_vendor);
@@ -1636,6 +1645,7 @@ init_keywords(vector keywords)
 	install_keyword_multi("devnode", &ble_elist_devnode_handler, &snprint_ble_simple);
 	install_keyword_multi("wwid", &ble_elist_wwid_handler, &snprint_ble_simple);
 	install_keyword_multi("property", &ble_elist_property_handler, &snprint_ble_simple);
+	install_keyword_multi("protocol", &ble_elist_protocol_handler, &snprint_ble_simple);
 	install_keyword_multi("device", &ble_except_device_handler, NULL);
 	install_sublevel();
 	install_keyword("vendor", &ble_elist_device_vendor_handler, &snprint_bled_vendor);
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 573d98b..e58a3fa 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1887,9 +1887,10 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 
 	if (mask & DI_BLACKLIST && mask & DI_SYSFS) {
 		if (filter_device(conf->blist_device, conf->elist_device,
-				  pp->vendor_id, pp->product_id) > 0) {
+				  pp->vendor_id, pp->product_id) > 0 ||
+		    filter_protocol(conf->blist_protocol, conf->elist_protocol,
+				    pp) > 0)
 			return PATHINFO_SKIPPED;
-		}
 	}
 
 	path_state = path_offline(pp);
diff --git a/libmultipath/print.c b/libmultipath/print.c
index ecfcb48..9da6a77 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1688,6 +1688,19 @@ int snprint_blacklist_report(struct config *conf, char *buff, int len)
 
 	if ((len - fwd - threshold) <= 0)
 		return len;
+	fwd += snprintf(buff + fwd, len - fwd, "protocol rules:\n"
+					       "- blacklist:\n");
+	if (!snprint_blacklist_group(buff, len, &fwd, &conf->blist_protocol))
+		return len;
+
+	if ((len - fwd - threshold) <= 0)
+		return len;
+	fwd += snprintf(buff + fwd, len - fwd, "- exceptions:\n");
+	if (snprint_blacklist_group(buff, len, &fwd, &conf->elist_protocol) == 0)
+		return len;
+
+	if ((len - fwd - threshold) <= 0)
+		return len;
 	fwd += snprintf(buff + fwd, len - fwd, "wwid rules:\n"
 					       "- blacklist:\n");
 	if (snprint_blacklist_group(buff, len, &fwd, &conf->blist_wwid) == 0)
@@ -1761,6 +1774,15 @@ static int snprint_blacklist(const struct config *conf, char *buff, int len)
 		if (fwd >= len)
 			return len;
 	}
+	vector_foreach_slot (conf->blist_protocol, ble, i) {
+		kw = find_keyword(conf->keywords, rootkw->sub, "protocol");
+		if (!kw)
+			return 0;
+		fwd += snprint_keyword(buff + fwd, len - fwd, "\t%k %v\n",
+				       kw, ble);
+		if (fwd >= len)
+			return len;
+	}
 	rootkw = find_keyword(conf->keywords, rootkw->sub, "device");
 	if (!rootkw)
 		return 0;
@@ -1838,6 +1860,15 @@ static int snprint_blacklist_except(const struct config *conf,
 		if (fwd >= len)
 			return len;
 	}
+	vector_foreach_slot (conf->elist_protocol, ele, i) {
+		kw = find_keyword(conf->keywords, rootkw->sub, "protocol");
+		if (!kw)
+			return 0;
+		fwd += snprint_keyword(buff + fwd, len - fwd, "\t%k %v\n",
+				       kw, ele);
+		if (fwd >= len)
+			return len;
+	}
 	rootkw = find_keyword(conf->keywords, rootkw->sub, "device");
 	if (!rootkw)
 		return 0;
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index fb863fd..6333366 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1205,9 +1205,21 @@ The default \fIblacklist exception\fR is: \fB(SCSI_IDENT_|ID_WWN)\fR, causing
 well-behaved SCSI devices and devices that provide a WWN (World Wide Number)
 to be included, and all others to be excluded.
 .RE
+.TP
+.B protocol
+Regular expression for the protocol of a device to be excluded/included.
+.RS
+.PP
+The protocol strings that multipath recognizes are \fIscsi:fcp\fR,
+\fIscsi:spi\fR, \fIscsi:ssa\fR, \fIscsi:sbp\fR, \fIscsi:srp\fR,
+\fIscsi:iscsi\fR, \fIscsi:sas\fR, \fIscsi:adt\fR, \fIscsi:ata\fR,
+\fIscsi:unspec\fR, \fIccw\fR, \fIcciss\fR, \fInvme\fR, and \fIundef\fR.
+The protocol that a path is using can be viewed by running
+\fBmultipathd show paths format "%d %P"\fR
+.RE
 .LP
-For every device, these 4 blacklist criteria are evaluated in the the order
-"property, dev\%node, device, wwid". If a device turns out to be
+For every device, these 5 blacklist criteria are evaluated in the the order
+"property, dev\%node, device, protocol, wwid". If a device turns out to be
 blacklisted by any criterion, it's excluded from handling by multipathd, and
 the later criteria aren't evaluated any more. For each
 criterion, the whitelist takes precedence over the blacklist if a device
-- 
2.7.4

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

* [PATCH 08/10] libmultipath: remove _filter_* blacklist functions
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (6 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 07/10] libmultipath: add "protocol" blacklist option Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-20 19:05   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 09/10] multipath tests: change to work with old make versions Benjamin Marzinski
  2018-07-13 19:39 ` [PATCH 10/10] multipath tests: add blacklist tests Benjamin Marzinski
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

The one point of these functions was for _filter_path(), and that wasn't
improved by using them. Since filter_path() only printed one message at
the end, you could have situations where the wwid was blacklisted, but
the blacklist message included the vendor/product instead. Also, the
protocol and property messages were printed twice, and if the device was
on multiple whitelists, only the last one is printed.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c | 168 +++++++++++++++++++----------------------------
 libmultipath/blacklist.h |   2 +-
 libmultipath/configure.c |   2 +-
 libmultipath/discovery.c |   2 +-
 4 files changed, 71 insertions(+), 103 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index fdd36f7..318ec03 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -294,161 +294,129 @@ log_filter (const char *dev, char *vendor, char *product, char *wwid,
 }
 
 int
-_filter_device (vector blist, vector elist, char * vendor, char * product)
+filter_device (vector blist, vector elist, char * vendor, char * product,
+	       char * dev)
 {
-	if (!vendor || !product)
-		return 0;
-	if (_blacklist_exceptions_device(elist, vendor, product))
-		return MATCH_DEVICE_BLIST_EXCEPT;
-	if (_blacklist_device(blist, vendor, product))
-		return MATCH_DEVICE_BLIST;
-	return 0;
-}
+	int r = MATCH_NOTHING;
 
-int
-filter_device (vector blist, vector elist, char * vendor, char * product)
-{
-	int r = _filter_device(blist, elist, vendor, product);
-	log_filter(NULL, vendor, product, NULL, NULL, NULL, r);
-	return r;
-}
+	if (vendor && product) {
+		if (_blacklist_exceptions_device(elist, vendor, product))
+			r = MATCH_DEVICE_BLIST_EXCEPT;
+		else if (_blacklist_device(blist, vendor, product))
+			r = MATCH_DEVICE_BLIST;
+	}
 
-int
-_filter_devnode (vector blist, vector elist, char * dev)
-{
-	if (!dev)
-		return 0;
-	if (_blacklist_exceptions(elist, dev))
-		return MATCH_DEVNODE_BLIST_EXCEPT;
-	if (_blacklist(blist, dev))
-		return MATCH_DEVNODE_BLIST;
-	return 0;
+	log_filter(dev, vendor, product, NULL, NULL, NULL, r);
+	return r;
 }
 
 int
 filter_devnode (vector blist, vector elist, char * dev)
 {
-	int r = _filter_devnode(blist, elist, dev);
+	int r = MATCH_NOTHING;
+
+	if (dev) {
+		if (_blacklist_exceptions(elist, dev))
+			r = MATCH_DEVNODE_BLIST_EXCEPT;
+		else if (_blacklist(blist, dev))
+			r = MATCH_DEVNODE_BLIST;
+	}
+
 	log_filter(dev, NULL, NULL, NULL, NULL, NULL, r);
 	return r;
 }
 
 int
-_filter_wwid (vector blist, vector elist, char * wwid)
-{
-	if (!wwid)
-		return 0;
-	if (_blacklist_exceptions(elist, wwid))
-		return MATCH_WWID_BLIST_EXCEPT;
-	if (_blacklist(blist, wwid))
-		return MATCH_WWID_BLIST;
-	return 0;
-}
-
-int
 filter_wwid (vector blist, vector elist, char * wwid, char * dev)
 {
-	int r = _filter_wwid(blist, elist, wwid);
+	int r = MATCH_NOTHING;
+
+	if (wwid) {
+		if (_blacklist_exceptions(elist, wwid))
+			r = MATCH_WWID_BLIST_EXCEPT;
+		else if (_blacklist(blist, wwid))
+			r = MATCH_WWID_BLIST;
+	}
+
 	log_filter(dev, NULL, NULL, wwid, NULL, NULL, r);
 	return r;
 }
 
-static int
-_filter_protocol (vector blist, vector elist, const char * protocol_str)
-{
-	if (_blacklist_exceptions(elist, protocol_str))
-		return MATCH_PROTOCOL_BLIST_EXCEPT;
-	if (_blacklist(blist, protocol_str))
-		return MATCH_PROTOCOL_BLIST;
-	return 0;
-}
-
 int
 filter_protocol(vector blist, vector elist, struct path * pp)
 {
 	char buf[PROTOCOL_BUF_SIZE];
-	int r;
+	int r = MATCH_NOTHING;
+
+	if (pp) {
+		snprint_path_protocol(buf, sizeof(buf), pp);
+
+		if (_blacklist_exceptions(elist, buf))
+			r = MATCH_PROTOCOL_BLIST_EXCEPT;
+		else if (_blacklist(blist, buf))
+			r = MATCH_PROTOCOL_BLIST;
+	}
 
-	snprint_path_protocol(buf, sizeof(buf), pp);
-	r = _filter_protocol(blist, elist, buf);
 	log_filter(pp->dev, NULL, NULL, NULL, NULL, buf, r);
 	return r;
 }
 
 int
-_filter_path (struct config * conf, struct path * pp)
+filter_path (struct config * conf, struct path * pp)
 {
 	int r;
 
 	r = filter_property(conf, pp->udev);
 	if (r > 0)
 		return r;
-	r = _filter_devnode(conf->blist_devnode, conf->elist_devnode,pp->dev);
+	r = filter_devnode(conf->blist_devnode, conf->elist_devnode, pp->dev);
 	if (r > 0)
 		return r;
-	r = _filter_device(conf->blist_device, conf->elist_device,
-			   pp->vendor_id, pp->product_id);
+	r = filter_device(conf->blist_device, conf->elist_device,
+			   pp->vendor_id, pp->product_id, pp->dev);
 	if (r > 0)
 		return r;
 	r = filter_protocol(conf->blist_protocol, conf->elist_protocol, pp);
 	if (r > 0)
 		return r;
-	r = _filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid);
+	r = filter_wwid(conf->blist_wwid, conf->elist_wwid, pp->wwid, pp->dev);
 	return r;
 }
 
 int
-filter_path (struct config * conf, struct path * pp)
-{
-	int r=_filter_path(conf, pp);
-	log_filter(pp->dev, pp->vendor_id, pp->product_id, pp->wwid, NULL,
-		   NULL, r);
-	return r;
-}
-
-int
-_filter_property (struct config *conf, const char *env)
-{
-	if (_blacklist_exceptions(conf->elist_property, env))
-		return MATCH_PROPERTY_BLIST_EXCEPT;
-	if (_blacklist(conf->blist_property, env))
-		return MATCH_PROPERTY_BLIST;
-
-	return 0;
-}
-
-int
 filter_property(struct config * conf, struct udev_device * udev)
 {
 	const char *devname = udev_device_get_sysname(udev);
 	struct udev_list_entry *list_entry;
-	int r;
-
-	if (!udev)
-		return 0;
-
-	udev_list_entry_foreach(list_entry,
+	const char *env = NULL;
+	int r = MATCH_NOTHING;
+
+	if (udev) {
+		/*
+		 * This is the inverse of the 'normal' matching;
+		 * the environment variable _has_ to match.
+		 */
+		r = MATCH_PROPERTY_BLIST_MISSING;
+		udev_list_entry_foreach(list_entry,
 				udev_device_get_properties_list_entry(udev)) {
-		const char *env;
-
-		env = udev_list_entry_get_name(list_entry);
-		if (!env)
-			continue;
 
-		r = _filter_property(conf, env);
-		if (r) {
-			log_filter(devname, NULL, NULL, NULL, env, NULL, r);
-			return r;
+			env = udev_list_entry_get_name(list_entry);
+			if (!env)
+				continue;
+			if (_blacklist_exceptions(conf->elist_property, env)) {
+				r = MATCH_PROPERTY_BLIST_EXCEPT;
+				break;
+			}
+			if (_blacklist(conf->blist_property, env)) {
+				r = MATCH_PROPERTY_BLIST;
+				break;
+			}
+			env = NULL;
 		}
 	}
 
-	/*
-	 * This is the inverse of the 'normal' matching;
-	 * the environment variable _has_ to match.
-	 */
-	log_filter(devname, NULL, NULL, NULL, NULL, NULL,
-		   MATCH_PROPERTY_BLIST_MISSING);
-	return MATCH_PROPERTY_BLIST_MISSING;
+	log_filter(devname, NULL, NULL, NULL, env, NULL, r);
+	return r;
 }
 
 static void free_ble(struct blentry *ble)
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index f7beef2..18903b6 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -35,7 +35,7 @@ int setup_default_blist (struct config *);
 int alloc_ble_device (vector);
 int filter_devnode (vector, vector, char *);
 int filter_wwid (vector, vector, char *, char *);
-int filter_device (vector, vector, char *, char *);
+int filter_device (vector, vector, char *, char *, char *);
 int filter_path (struct config *, struct path *);
 int filter_property(struct config *, struct udev_device *);
 int filter_protocol(vector, vector, struct path *);
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 5c54f9b..09c3dcf 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1030,7 +1030,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 			invalid = 1;
 		pthread_cleanup_pop(1);
 		if (invalid) {
-			orphan_path(pp1, "wwid blacklisted");
+			orphan_path(pp1, "blacklisted");
 			continue;
 		}
 
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index e58a3fa..0b1855d 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1887,7 +1887,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 
 	if (mask & DI_BLACKLIST && mask & DI_SYSFS) {
 		if (filter_device(conf->blist_device, conf->elist_device,
-				  pp->vendor_id, pp->product_id) > 0 ||
+				  pp->vendor_id, pp->product_id, pp->dev) > 0 ||
 		    filter_protocol(conf->blist_protocol, conf->elist_protocol,
 				    pp) > 0)
 			return PATHINFO_SKIPPED;
-- 
2.7.4

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

* [PATCH 09/10] multipath tests: change to work with old make versions
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (7 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 08/10] libmultipath: remove _filter_* blacklist functions Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-20 19:08   ` Martin Wilck
  2018-07-13 19:39 ` [PATCH 10/10] multipath tests: add blacklist tests Benjamin Marzinski
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

the $(file <) operation only works with make 4.2 and above. I tried
running the tests on an old machine and it failed. The $shell function
can do the same thing and multipath has been using that in its
Makefiles for a while.

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

diff --git a/tests/Makefile b/tests/Makefile
index 78755ed..d293c87 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -51,4 +51,4 @@ COLON:=:
 		$(multipathdir)/libmultipath.so Makefile
 	$(CC) $(CFLAGS) -o $@ $(LDFLAGS) $< $($@_TESTDEPS) $($@_OBJDEPS) \
 		$(LIBDEPS) $($@_LIBDEPS) \
-		$(file <$<.wrap) $(foreach dep,$($@_TESTDEPS),$(file <$(dep).wrap))
+		$(shell cat $<.wrap) $(foreach dep,$($@_TESTDEPS),$(shell cat $(dep).wrap))
-- 
2.7.4

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

* [PATCH 10/10] multipath tests: add blacklist tests
  2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
                   ` (8 preceding siblings ...)
  2018-07-13 19:39 ` [PATCH 09/10] multipath tests: change to work with old make versions Benjamin Marzinski
@ 2018-07-13 19:39 ` Benjamin Marzinski
  2018-07-20 19:16   ` Martin Wilck
  9 siblings, 1 reply; 23+ messages in thread
From: Benjamin Marzinski @ 2018-07-13 19:39 UTC (permalink / raw)
  To: device-mapper development; +Cc: Martin Wilck

These are tests to validate the filter_* blacklist functions. They not
only verify that the device is correctly blacklisted/whitelisted, but
they also verify the log messages that are printed out.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 tests/Makefile    |   4 +-
 tests/blacklist.c | 512 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 515 insertions(+), 1 deletion(-)
 create mode 100644 tests/blacklist.c

diff --git a/tests/Makefile b/tests/Makefile
index d293c87..98b5c93 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
 CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
 LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
 
-TESTS := uevent parser util dmevents hwtable
+TESTS := uevent parser util dmevents hwtable blacklist
 
 .SILENT: $(TESTS:%=%.o)
 .PRECIOUS: $(TESTS:%=%-test)
@@ -23,6 +23,8 @@ hwtable-test_TESTDEPS := test-lib.o
 hwtable-test_OBJDEPS := ../libmultipath/discovery.o ../libmultipath/blacklist.o \
 	../libmultipath/prio.o ../libmultipath/callout.o ../libmultipath/structs.o
 hwtable-test_LIBDEPS := -ludev -lpthread -ldl
+blacklist-test_OBJDEPS := ../libmultipath/blacklist.o
+blacklist-test_LIBDEPS := -ludev
 
 %.out:	%-test
 	@echo == running $< ==
diff --git a/tests/blacklist.c b/tests/blacklist.c
new file mode 100644
index 0000000..a55c1c0
--- /dev/null
+++ b/tests/blacklist.c
@@ -0,0 +1,512 @@
+/*
+ * Copyright (c) 2018 Benjamin Marzinski, Redhat
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ *
+ */
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <cmocka.h>
+#include "globals.c"
+#include "blacklist.h"
+#include "log.h"
+
+struct udev_device {
+	const char *sysname;
+	char *property_list[];
+};
+
+const char *
+__wrap_udev_device_get_sysname(struct udev_device *udev_device)
+{
+	assert_non_null(udev_device);
+	assert_non_null(udev_device->sysname);
+	return udev_device->sysname;
+}
+
+struct udev_list_entry *
+__wrap_udev_device_get_properties_list_entry(struct udev_device *udev_device)
+{
+	assert_non_null(udev_device);
+	if (!udev_device->property_list)
+		return NULL;
+	if (!*udev_device->property_list)
+		return NULL;
+	return (struct udev_list_entry *)udev_device->property_list;
+}
+
+struct udev_list_entry *
+__wrap_udev_list_entry_get_next(struct udev_list_entry *list_entry)
+{
+	assert_non_null(list_entry);
+	if (!*((char **)list_entry + 1))
+		return NULL;
+	return (struct udev_list_entry *)(((char **)list_entry) + 1);
+}
+
+const char *
+__wrap_udev_list_entry_get_name(struct udev_list_entry *list_entry)
+{
+	return *(const char **)list_entry;
+}
+
+void __wrap_dlog (int sink, int prio, const char * fmt, ...)
+{
+	char buff[MAX_MSG_SIZE];
+	va_list ap;
+
+	assert_int_equal(prio, mock_type(int));
+	va_start(ap, fmt);
+	vsnprintf(buff, MAX_MSG_SIZE, fmt, ap);
+	va_end(ap);
+	assert_string_equal(buff, mock_ptr_type(char *));
+}
+
+void expect_condlog(int prio, char *string)
+{
+	will_return(__wrap_dlog, prio);
+	will_return(__wrap_dlog, string);
+}
+
+vector blist_devnode_sdb;
+vector blist_all;
+vector blist_device_foo_bar;
+vector blist_device_all;
+vector blist_wwid_xyzzy;
+vector blist_protocol_fcp;
+vector blist_property_wwn;
+
+static int setup(void **state)
+{
+	blist_devnode_sdb = vector_alloc();
+	if (!blist_devnode_sdb ||
+	    store_ble(blist_devnode_sdb, strdup("sdb"), ORIGIN_CONFIG))
+		return -1;
+
+	blist_all = vector_alloc();
+	if (!blist_all || store_ble(blist_all, strdup(".*"), ORIGIN_CONFIG))
+		return -1;
+
+	blist_device_foo_bar = vector_alloc();
+	if (!blist_device_foo_bar || alloc_ble_device(blist_device_foo_bar) ||
+	    set_ble_device(blist_device_foo_bar, strdup("foo"), strdup("bar"),
+			   ORIGIN_CONFIG))
+		return -1;
+
+	blist_device_all = vector_alloc();
+	if (!blist_device_all || alloc_ble_device(blist_device_all) ||
+	    set_ble_device(blist_device_all, strdup(".*"), strdup(".*"),
+			   ORIGIN_CONFIG))
+		return -1;
+
+	blist_wwid_xyzzy = vector_alloc();
+	if (!blist_wwid_xyzzy ||
+	    store_ble(blist_wwid_xyzzy, strdup("xyzzy"), ORIGIN_CONFIG))
+		return -1;
+
+	blist_protocol_fcp = vector_alloc();
+	if (!blist_protocol_fcp ||
+	    store_ble(blist_protocol_fcp, strdup("scsi:fcp"), ORIGIN_CONFIG))
+		return -1;
+
+	blist_property_wwn = vector_alloc();
+	if (!blist_property_wwn ||
+	    store_ble(blist_property_wwn, strdup("ID_WWN"), ORIGIN_CONFIG))
+		return -1;
+
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	free_blacklist(blist_devnode_sdb);
+	free_blacklist(blist_all);
+	free_blacklist_device(blist_device_foo_bar);
+	free_blacklist_device(blist_device_all);
+	free_blacklist(blist_wwid_xyzzy);
+	free_blacklist(blist_protocol_fcp);
+	free_blacklist(blist_property_wwn);
+	return 0;
+}
+
+static int reset_blists(void **state)
+{
+	conf.blist_devnode = NULL;
+	conf.blist_wwid = NULL;
+	conf.blist_property = NULL;
+	conf.blist_protocol = NULL;
+	conf.blist_device = NULL;
+	conf.elist_devnode = NULL;
+	conf.elist_wwid = NULL;
+	conf.elist_property = NULL;
+	conf.elist_protocol = NULL;
+	conf.elist_device = NULL;
+	return 0;
+}
+
+static void test_devnode_blacklist(void **state)
+{
+	expect_condlog(3, "sdb: device node name blacklisted\n");
+	assert_int_equal(filter_devnode(blist_devnode_sdb, NULL, "sdb"),
+			 MATCH_DEVNODE_BLIST);
+}
+
+static void test_devnode_whitelist(void **state)
+{
+	expect_condlog(3, "sdb: device node name whitelisted\n");
+	assert_int_equal(filter_devnode(blist_all, blist_devnode_sdb, "sdb"),
+			 MATCH_DEVNODE_BLIST_EXCEPT);
+	expect_condlog(3, "sdc: device node name blacklisted\n");
+	assert_int_equal(filter_devnode(blist_all, blist_devnode_sdb, "sdc"),
+			 MATCH_DEVNODE_BLIST);
+}
+
+static void test_devnode_missing(void **state)
+{
+	assert_int_equal(filter_devnode(blist_devnode_sdb, NULL, "sdc"),
+			 MATCH_NOTHING);
+}
+
+static void test_device_blacklist(void **state)
+{
+	expect_condlog(3, "sdb: (foo:bar) vendor/product blacklisted\n");
+	assert_int_equal(filter_device(blist_device_foo_bar, NULL, "foo",
+				       "bar", "sdb"),
+			 MATCH_DEVICE_BLIST);
+}
+
+static void test_device_whitelist(void **state)
+{
+	expect_condlog(3, "sdb: (foo:bar) vendor/product whitelisted\n");
+	assert_int_equal(filter_device(blist_device_all, blist_device_foo_bar,
+				       "foo", "bar", "sdb"),
+			 MATCH_DEVICE_BLIST_EXCEPT);
+	expect_condlog(3, "sdb: (foo:baz) vendor/product blacklisted\n");
+	assert_int_equal(filter_device(blist_device_all, blist_device_foo_bar,
+				       "foo", "baz", "sdb"),
+			 MATCH_DEVICE_BLIST);
+}
+
+static void test_device_missing(void **state)
+{
+	assert_int_equal(filter_device(blist_device_foo_bar, NULL, "foo",
+				       "baz", "sdb"),
+			 MATCH_NOTHING);
+}
+
+static void test_wwid_blacklist(void **state)
+{
+	expect_condlog(3, "sdb: wwid xyzzy blacklisted\n");
+	assert_int_equal(filter_wwid(blist_wwid_xyzzy, NULL, "xyzzy", "sdb"),
+			 MATCH_WWID_BLIST);
+}
+
+static void test_wwid_whitelist(void **state)
+{
+	expect_condlog(3, "sdb: wwid xyzzy whitelisted\n");
+	assert_int_equal(filter_wwid(blist_all, blist_wwid_xyzzy,
+				     "xyzzy", "sdb"),
+			 MATCH_WWID_BLIST_EXCEPT);
+	expect_condlog(3, "sdb: wwid plugh blacklisted\n");
+	assert_int_equal(filter_wwid(blist_all, blist_wwid_xyzzy,
+				     "plugh", "sdb"),
+			 MATCH_WWID_BLIST);
+}
+
+static void test_wwid_missing(void **state)
+{
+	assert_int_equal(filter_wwid(blist_wwid_xyzzy, NULL, "plugh", "sdb"),
+			 MATCH_NOTHING);
+}
+
+static void test_protocol_blacklist(void **state)
+{
+	struct path pp = { .dev = "sdb", .bus = SYSFS_BUS_SCSI,
+			   .sg_id.proto_id = SCSI_PROTOCOL_FCP };
+	expect_condlog(3, "sdb: protocol scsi:fcp blacklisted\n");
+	assert_int_equal(filter_protocol(blist_protocol_fcp, NULL, &pp),
+			 MATCH_PROTOCOL_BLIST);
+}
+
+static void test_protocol_whitelist(void **state)
+{
+	struct path pp1 = { .dev = "sdb", .bus = SYSFS_BUS_SCSI,
+			   .sg_id.proto_id = SCSI_PROTOCOL_FCP };
+	struct path pp2 = { .dev = "sdb", .bus = SYSFS_BUS_SCSI,
+			   .sg_id.proto_id = SCSI_PROTOCOL_ISCSI };
+	expect_condlog(3, "sdb: protocol scsi:fcp whitelisted\n");
+	assert_int_equal(filter_protocol(blist_all, blist_protocol_fcp, &pp1),
+			 MATCH_PROTOCOL_BLIST_EXCEPT);
+	expect_condlog(3, "sdb: protocol scsi:iscsi blacklisted\n");
+	assert_int_equal(filter_protocol(blist_all, blist_protocol_fcp, &pp2),
+			 MATCH_PROTOCOL_BLIST);
+}
+
+static void test_protocol_missing(void **state)
+{
+	struct path pp = { .dev = "sdb", .bus = SYSFS_BUS_SCSI,
+			   .sg_id.proto_id = SCSI_PROTOCOL_ISCSI };
+	assert_int_equal(filter_protocol(blist_protocol_fcp, NULL, &pp),
+			 MATCH_NOTHING);
+}
+
+static void test_property_blacklist(void **state)
+{
+	static struct udev_device udev = { "sdb", { "ID_FOO", "ID_WWN", "ID_BAR", NULL } };
+	conf.blist_property = blist_property_wwn;
+	expect_condlog(3, "sdb: udev property ID_WWN blacklisted\n");
+	assert_int_equal(filter_property(&conf, &udev), MATCH_PROPERTY_BLIST);
+}
+
+/* the property check works different in that you check all the property
+ * names, so setting a blacklist value will blacklist the device if any
+ * of the property on the blacklist are found before the property names
+ * in the whitelist.  This might be worth changing. although it would
+ * force multipath to go through the properties twice */
+static void test_property_whitelist(void **state)
+{
+	static struct udev_device udev = { "sdb", { "ID_FOO", "ID_WWN", "ID_BAR", NULL } };
+	conf.elist_property = blist_property_wwn;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	assert_int_equal(filter_property(&conf, &udev),
+			 MATCH_PROPERTY_BLIST_EXCEPT);
+}
+
+static void test_property_missing(void **state)
+{
+	static struct udev_device udev = { "sdb", { "ID_FOO", "ID_BAZ", "ID_BAR", NULL } };
+	conf.blist_property = blist_property_wwn;
+	expect_condlog(3, "sdb: blacklisted, udev property missing\n");
+	assert_int_equal(filter_property(&conf, &udev),
+			 MATCH_PROPERTY_BLIST_MISSING);
+}
+
+struct udev_device test_udev = { "sdb", { "ID_FOO", "ID_WWN", "ID_BAR", NULL } };
+
+struct path test_pp = { .dev = "sdb", .bus = SYSFS_BUS_SCSI, .udev = &test_udev,
+			.sg_id.proto_id = SCSI_PROTOCOL_FCP, .vendor_id = "foo",
+			.product_id = "bar", .wwid = "xyzzy" };
+
+static void test_filter_path_property(void **state)
+{
+	conf.blist_property = blist_property_wwn;
+	expect_condlog(3, "sdb: udev property ID_WWN blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_PROPERTY_BLIST);
+}
+
+static void test_filter_path_devnode(void **state)
+{
+	/* always must include property elist, to avoid "missing property"
+	 * blacklisting */
+	conf.elist_property = blist_property_wwn;
+	conf.blist_devnode = blist_devnode_sdb;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_DEVNODE_BLIST);
+}
+
+static void test_filter_path_device(void **state)
+{
+	/* always must include property elist, to avoid "missing property"
+	 * blacklisting */
+	conf.elist_property = blist_property_wwn;
+	conf.blist_device = blist_device_foo_bar;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: (foo:bar) vendor/product blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_DEVICE_BLIST);
+}
+
+static void test_filter_path_protocol(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.blist_protocol = blist_protocol_fcp;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: protocol scsi:fcp blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_PROTOCOL_BLIST);
+}
+
+static void test_filter_path_wwid(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.blist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: wwid xyzzy blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_WWID_BLIST);
+}
+
+struct udev_device miss_udev = { "sdb", { "ID_FOO", "ID_BAZ", "ID_BAR", NULL } };
+
+struct path miss1_pp = { .dev = "sdc", .bus = SYSFS_BUS_SCSI,
+			.udev = &miss_udev,
+			.sg_id.proto_id = SCSI_PROTOCOL_ISCSI,
+			.vendor_id = "foo", .product_id = "baz",
+			.wwid = "plugh" };
+
+struct path miss2_pp = { .dev = "sdc", .bus = SYSFS_BUS_SCSI,
+			.udev = &test_udev,
+			.sg_id.proto_id = SCSI_PROTOCOL_ISCSI,
+			.vendor_id = "foo", .product_id = "baz",
+			.wwid = "plugh" };
+
+static void test_filter_path_missing1(void **state)
+{
+	conf.blist_property = blist_property_wwn;
+	conf.blist_devnode = blist_devnode_sdb;
+	conf.blist_device = blist_device_foo_bar;
+	conf.blist_protocol = blist_protocol_fcp;
+	conf.blist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: blacklisted, udev property missing\n");
+	assert_int_equal(filter_path(&conf, &miss1_pp),
+			 MATCH_PROPERTY_BLIST_MISSING);
+}
+
+/* This one matches the property whitelist, to test the other missing
+ * functions */
+static void test_filter_path_missing2(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.blist_devnode = blist_devnode_sdb;
+	conf.blist_device = blist_device_foo_bar;
+	conf.blist_protocol = blist_protocol_fcp;
+	conf.blist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	assert_int_equal(filter_path(&conf, &miss2_pp),
+			 MATCH_NOTHING);
+}
+
+static void test_filter_path_whitelist(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.elist_devnode = blist_devnode_sdb;
+	conf.elist_device = blist_device_foo_bar;
+	conf.elist_protocol = blist_protocol_fcp;
+	conf.elist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name whitelisted\n");
+	expect_condlog(3, "sdb: (foo:bar) vendor/product whitelisted\n");
+	expect_condlog(3, "sdb: protocol scsi:fcp whitelisted\n");
+	expect_condlog(3, "sdb: wwid xyzzy whitelisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp),
+			 MATCH_WWID_BLIST_EXCEPT);
+}
+
+static void test_filter_path_whitelist_property(void **state)
+{
+	conf.blist_property = blist_property_wwn;
+	conf.elist_devnode = blist_devnode_sdb;
+	conf.elist_device = blist_device_foo_bar;
+	conf.elist_protocol = blist_protocol_fcp;
+	conf.elist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_PROPERTY_BLIST);
+}
+
+static void test_filter_path_whitelist_devnode(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.blist_devnode = blist_devnode_sdb;
+	conf.elist_device = blist_device_foo_bar;
+	conf.elist_protocol = blist_protocol_fcp;
+	conf.elist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_DEVNODE_BLIST);
+}
+
+static void test_filter_path_whitelist_device(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.elist_devnode = blist_devnode_sdb;
+	conf.blist_device = blist_device_foo_bar;
+	conf.elist_protocol = blist_protocol_fcp;
+	conf.elist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name whitelisted\n");
+	expect_condlog(3, "sdb: (foo:bar) vendor/product blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_DEVICE_BLIST);
+}
+
+static void test_filter_path_whitelist_protocol(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.elist_devnode = blist_devnode_sdb;
+	conf.elist_device = blist_device_foo_bar;
+	conf.blist_protocol = blist_protocol_fcp;
+	conf.elist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name whitelisted\n");
+	expect_condlog(3, "sdb: (foo:bar) vendor/product whitelisted\n");
+	expect_condlog(3, "sdb: protocol scsi:fcp blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_PROTOCOL_BLIST);
+}
+
+static void test_filter_path_whitelist_wwid(void **state)
+{
+	conf.elist_property = blist_property_wwn;
+	conf.elist_devnode = blist_devnode_sdb;
+	conf.elist_device = blist_device_foo_bar;
+	conf.elist_protocol = blist_protocol_fcp;
+	conf.blist_wwid = blist_wwid_xyzzy;
+	expect_condlog(3, "sdb: udev property ID_WWN whitelisted\n");
+	expect_condlog(3, "sdb: device node name whitelisted\n");
+	expect_condlog(3, "sdb: (foo:bar) vendor/product whitelisted\n");
+	expect_condlog(3, "sdb: protocol scsi:fcp whitelisted\n");
+	expect_condlog(3, "sdb: wwid xyzzy blacklisted\n");
+	assert_int_equal(filter_path(&conf, &test_pp), MATCH_WWID_BLIST);
+}
+
+#define test_and_reset(x) cmocka_unit_test_teardown((x), reset_blists)
+
+int test_blacklist(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_devnode_blacklist),
+		cmocka_unit_test(test_devnode_whitelist),
+		cmocka_unit_test(test_devnode_missing),
+		cmocka_unit_test(test_device_blacklist),
+		cmocka_unit_test(test_device_whitelist),
+		cmocka_unit_test(test_device_missing),
+		cmocka_unit_test(test_wwid_blacklist),
+		cmocka_unit_test(test_wwid_whitelist),
+		cmocka_unit_test(test_wwid_missing),
+		cmocka_unit_test(test_protocol_blacklist),
+		cmocka_unit_test(test_protocol_whitelist),
+		cmocka_unit_test(test_protocol_missing),
+		test_and_reset(test_property_blacklist),
+		test_and_reset(test_property_whitelist),
+		test_and_reset(test_property_missing),
+		test_and_reset(test_filter_path_property),
+		test_and_reset(test_filter_path_devnode),
+		test_and_reset(test_filter_path_device),
+		test_and_reset(test_filter_path_protocol),
+		test_and_reset(test_filter_path_wwid),
+		test_and_reset(test_filter_path_missing1),
+		test_and_reset(test_filter_path_missing2),
+		test_and_reset(test_filter_path_whitelist),
+		test_and_reset(test_filter_path_whitelist_property),
+		test_and_reset(test_filter_path_whitelist_devnode),
+		test_and_reset(test_filter_path_whitelist_device),
+		test_and_reset(test_filter_path_whitelist_protocol),
+		test_and_reset(test_filter_path_whitelist_wwid),
+	};
+	return cmocka_run_group_tests(tests, setup, teardown);
+}
+
+int main(void)
+{
+	int ret = 0;
+	ret += test_blacklist();
+	return ret;
+}
-- 
2.7.4

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

* Re: [PATCH 01/10] libmultipath: remove last of rbd code
  2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
@ 2018-07-16  9:25   ` Martin Wilck
  2018-07-16  9:52   ` Martin Wilck
  1 sibling, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:25 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> My previous patch to remove the rbd code missed a little bit.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/del-part-nodes.rules | 2 +-
>  libmultipath/structs.h      | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>


> diff --git a/kpartx/del-part-nodes.rules b/kpartx/del-part-
> nodes.rules
> index 17bc505..0ceecf5 100644
> --- a/kpartx/del-part-nodes.rules
> +++ b/kpartx/del-part-nodes.rules
> @@ -10,7 +10,7 @@
>  # or create an udev rule file that sets
> ENV{DONT_DEL_PART_NODES}="1".
>  
>  SUBSYSTEM!="block", GOTO="end_del_part_nodes"
> -KERNEL!="sd*|dasd*|rbd*", GOTO="end_del_part_nodes"
> +KERNEL!="sd*|dasd*", GOTO="end_del_part_nodes"
>  ACTION!="add|change", GOTO="end_del_part_nodes"
>  ENV{DEVTYPE}=="partition", GOTO="end_del_part_nodes"
>  
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index e5b698b..ca14315 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -61,7 +61,6 @@ enum sysfs_buses {
>  	SYSFS_BUS_IDE,
>  	SYSFS_BUS_CCW,
>  	SYSFS_BUS_CCISS,
> -	SYSFS_BUS_RBD,
>  	SYSFS_BUS_NVME,
>  };
>  

-- 
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] 23+ messages in thread

* Re: [PATCH 02/10] libmultipath: fix detect alua corner case
  2018-07-13 19:39 ` [PATCH 02/10] libmultipath: fix detect alua corner case Benjamin Marzinski
@ 2018-07-16  9:32   ` Martin Wilck
  2018-07-16  9:52   ` Martin Wilck
  1 sibling, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:32 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> If retain_attach_hw_handler = no, then the paths tpgs state will
> never
> be checked, and the multipath device will always select the alua
> handler, if no other handler is selected. The paths tpgs state
> should be checked, regardless of the retain_hwhandler value.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/propsel.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Martin Wilck <mwilck@suse.com>

> 
> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index af3ed62..fdb5953 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -420,9 +420,11 @@ int select_hwhandler(struct config *conf, struct
> multipath *mp)
>  	bool all_tpgs = true;
>  
>  	dh_state = &handler[2];
> +
> +	vector_foreach_slot(mp->paths, pp, i)
> +		all_tpgs = all_tpgs && (pp->tpgs > 0);
>  	if (mp->retain_hwhandler != RETAIN_HWHANDLER_OFF) {
>  		vector_foreach_slot(mp->paths, pp, i) {
> -			all_tpgs = all_tpgs && (pp->tpgs > 0);
>  			if (get_dh_state(pp, dh_state,
> sizeof(handler) - 2) > 0
>  			    && strcmp(dh_state, "detached")) {
>  				memcpy(handler, "1 ", 2);

-- 
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] 23+ messages in thread

* Re: [PATCH 03/10] multipath: fix setting conf->version
  2018-07-13 19:39 ` [PATCH 03/10] multipath: fix setting conf->version Benjamin Marzinski
@ 2018-07-16  9:51   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:51 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> Commit d3b71498 stopped multipath from setting conf->version.
> Instead,
> it was always being set to 0.0.0. Multipathd was still setting this
> correctly.
> 
> Fixes: d3b71498 "multipath: fix rcu thread cancellation hang"
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/devmapper.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
Reviewed-by: Martin Wilck <mwilck@suse.com>


> diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
> index f2befad..8136d15 100644
> --- a/libmultipath/devmapper.c
> +++ b/libmultipath/devmapper.c
> @@ -245,13 +245,13 @@ void libmp_dm_init(void)
>  	int verbosity;
>  	unsigned int version[3];
>  
> +	if (dm_prereq(version))
> +		exit(1);
>  	conf = get_multipath_config();
>  	verbosity = conf->verbosity;
> -	memcpy(version, conf->version, sizeof(version));
> +	memcpy(conf->version, version, sizeof(version));
>  	put_multipath_config(conf);
>  	dm_init(verbosity);
> -	if (dm_prereq(version))
> -		exit(1);
>  	dm_udev_set_sync_support(libmp_dm_udev_sync);
>  }
>  

-- 
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] 23+ messages in thread

* Re: [PATCH 02/10] libmultipath: fix detect alua corner case
  2018-07-13 19:39 ` [PATCH 02/10] libmultipath: fix detect alua corner case Benjamin Marzinski
  2018-07-16  9:32   ` Martin Wilck
@ 2018-07-16  9:52   ` Martin Wilck
  1 sibling, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:52 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> If retain_attach_hw_handler = no, then the paths tpgs state will
> never
> be checked, and the multipath device will always select the alua
> handler, if no other handler is selected. The paths tpgs state
> should be checked, regardless of the retain_hwhandler value.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/propsel.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
Reviewed-by: Martin Wilck <mwilck@suse.com>
> 


> diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
> index af3ed62..fdb5953 100644
> --- a/libmultipath/propsel.c
> +++ b/libmultipath/propsel.c
> @@ -420,9 +420,11 @@ int select_hwhandler(struct config *conf, struct
> multipath *mp)
>  	bool all_tpgs = true;
>  
>  	dh_state = &handler[2];
> +
> +	vector_foreach_slot(mp->paths, pp, i)
> +		all_tpgs = all_tpgs && (pp->tpgs > 0);
>  	if (mp->retain_hwhandler != RETAIN_HWHANDLER_OFF) {
>  		vector_foreach_slot(mp->paths, pp, i) {
> -			all_tpgs = all_tpgs && (pp->tpgs > 0);
>  			if (get_dh_state(pp, dh_state,
> sizeof(handler) - 2) > 0
>  			    && strcmp(dh_state, "detached")) {
>  				memcpy(handler, "1 ", 2);

-- 
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] 23+ messages in thread

* Re: [PATCH 01/10] libmultipath: remove last of rbd code
  2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
  2018-07-16  9:25   ` Martin Wilck
@ 2018-07-16  9:52   ` Martin Wilck
  1 sibling, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:52 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> My previous patch to remove the rbd code missed a little bit.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  kpartx/del-part-nodes.rules | 2 +-
>  libmultipath/structs.h      | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>

> diff --git a/kpartx/del-part-nodes.rules b/kpartx/del-part-
> nodes.rules
> index 17bc505..0ceecf5 100644
> --- a/kpartx/del-part-nodes.rules
> +++ b/kpartx/del-part-nodes.rules
> @@ -10,7 +10,7 @@
>  # or create an udev rule file that sets
> ENV{DONT_DEL_PART_NODES}="1".
>  
>  SUBSYSTEM!="block", GOTO="end_del_part_nodes"
> -KERNEL!="sd*|dasd*|rbd*", GOTO="end_del_part_nodes"
> +KERNEL!="sd*|dasd*", GOTO="end_del_part_nodes"
>  ACTION!="add|change", GOTO="end_del_part_nodes"
>  ENV{DEVTYPE}=="partition", GOTO="end_del_part_nodes"
>  
> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index e5b698b..ca14315 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -61,7 +61,6 @@ enum sysfs_buses {
>  	SYSFS_BUS_IDE,
>  	SYSFS_BUS_CCW,
>  	SYSFS_BUS_CCISS,
> -	SYSFS_BUS_RBD,
>  	SYSFS_BUS_NVME,
>  };
>  

-- 
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] 23+ messages in thread

* Re: [PATCH 04/10] mpathpersist: add --param-alltgpt option
  2018-07-13 19:39 ` [PATCH 04/10] mpathpersist: add --param-alltgpt option Benjamin Marzinski
@ 2018-07-16  9:59   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16  9:59 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> From the limited testing I've been able to do, commit 5b54e772
> "mpathpersist: add all_tg_pt option", does appear to enable
> --param-alltgpt to work correctly on devices that accept the
> ALL_TG_PT
> flag, so I've added the option to mpathpersist.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmpathpersist/mpath_persist.c | 10 ++++------
>  mpathpersist/main.c             | 11 ++++++++---
>  mpathpersist/main.h             |  1 +
>  mpathpersist/mpathpersist.8     |  4 ++++
>  multipath/multipath.conf.5      |  8 +++++---
>  5 files changed, 22 insertions(+), 12 deletions(-)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>


> diff --git a/libmpathpersist/mpath_persist.c
> b/libmpathpersist/mpath_persist.c
> index 6e9e67f..61818e0 100644
> --- a/libmpathpersist/mpath_persist.c
> +++ b/libmpathpersist/mpath_persist.c
> @@ -466,11 +466,14 @@ int mpath_prout_reg(struct multipath *mpp,int
> rq_servact, int rq_scope,
>  	int rc;
>  	int count=0;
>  	int status = MPATH_PR_SUCCESS;
> +	int all_tg_pt;
>  	uint64_t sa_key = 0;
>  
>  	if (!mpp)
>  		return MPATH_PR_DMMP_ERROR;
>  
> +	all_tg_pt = (mpp->all_tg_pt == ALL_TG_PT_ON ||
> +		     paramp->sa_flags & MPATH_F_ALL_TG_PT_MASK);
>  	active_pathcount = pathcount(mpp, PATH_UP) + pathcount(mpp,
> PATH_GHOST);
>  
>  	if (active_pathcount == 0) {
> @@ -478,10 +481,6 @@ int mpath_prout_reg(struct multipath *mpp,int
> rq_servact, int rq_scope,
>  		return MPATH_PR_DMMP_ERROR;
>  	}
>  
> -	if ( paramp->sa_flags & MPATH_F_ALL_TG_PT_MASK ) {
> -		condlog (1, "Warning: ALL_TG_PT is set.
> Configuration not supported");
> -	}
> -
>  	struct threadinfo thread[active_pathcount];
>  	int hosts[active_pathcount];
>  
> @@ -518,8 +517,7 @@ int mpath_prout_reg(struct multipath *mpp,int
> rq_servact, int rq_scope,
>  				condlog (1, "%s: %s path not up.
> Skip.", mpp->wwid, pp->dev);
>  				continue;
>  			}
> -			if (mpp->all_tg_pt == ALL_TG_PT_ON &&
> -			    pp->sg_id.host_no != -1) {
> +			if (all_tg_pt && pp->sg_id.host_no != -1) {
>  				for (k = 0; k < count; k++) {
>  					if (pp->sg_id.host_no ==
> hosts[k]) {
>  						condlog(3, "%s: %s
> host %d matches skip.", pp->wwid, pp->dev, pp->sg_id.host_no);
> diff --git a/mpathpersist/main.c b/mpathpersist/main.c
> index 5b37f3a..99151fe 100644
> --- a/mpathpersist/main.c
> +++ b/mpathpersist/main.c
> @@ -118,7 +118,7 @@ int main (int argc, char * argv[])
>  	{
>  		int option_index = 0;
>  
> -		c = getopt_long (argc, argv,
> "v:Cd:hHioZK:S:PAT:skrGILcRX:l:",
> +		c = getopt_long (argc, argv,
> "v:Cd:hHioYZK:S:PAT:skrGILcRX:l:",
>  				long_options, &option_index);
>  		if (c == -1)
>  			break;
> @@ -158,6 +158,10 @@ int main (int argc, char * argv[])
>  				prout_flag = 1;
>  				break;
>  
> +			case 'Y':
> +				param_alltgpt = 1;
> +				++num_prout_param;
> +				break;
>  			case 'Z':
>  				param_aptpl = 1;
>  				++num_prout_param;
> @@ -443,9 +447,9 @@ int main (int argc, char * argv[])
>  		}
>  
>  		if (param_alltgpt)
> -			paramp->sa_flags |= 0x4;
> +			paramp->sa_flags |= MPATH_F_ALL_TG_PT_MASK;
>  		if (param_aptpl)
> -			paramp->sa_flags |= 0x1;
> +			paramp->sa_flags |= MPATH_F_APTPL_MASK;
>  
>  		if (num_transport)
>  		{
> @@ -698,6 +702,7 @@ static void usage(void)
>  			"    --hex|-H                   output
> response in hex\n"
>  			"    --in|-i                    request PR
> In command \n"
>  			"    --out|-o                   request PR
> Out command\n"
> +			"    --param-alltgpt|-Y         PR Out
> parameter 'ALL_TG_PT\n"
>  			"    --param-aptpl|-Z           PR Out
> parameter 'APTPL'\n"
>  			"    --read-keys|-k             PR In: Read
> Keys\n"
>  			"    --param-sark=SARK|-S SARK  PR Out
> parameter service "
> diff --git a/mpathpersist/main.h b/mpathpersist/main.h
> index 5c0e089..beb8a21 100644
> --- a/mpathpersist/main.h
> +++ b/mpathpersist/main.h
> @@ -6,6 +6,7 @@ static struct option long_options[] = {
>  	{"hex", 0, NULL, 'H'},
>  	{"in", 0, NULL, 'i'},
>  	{"out", 0, NULL, 'o'},
> +	{"param-alltgpt", 0, NULL, 'Y'},
>  	{"param-aptpl", 0, NULL, 'Z'},
>  	{"param-rk", 1, NULL, 'K'},
>  	{"param-sark", 1, NULL, 'S'},
> diff --git a/mpathpersist/mpathpersist.8
> b/mpathpersist/mpathpersist.8
> index a8982e6..885491d 100644
> --- a/mpathpersist/mpathpersist.8
> +++ b/mpathpersist/mpathpersist.8
> @@ -87,6 +87,10 @@ Request PR In command.
>  Request PR Out command.
>  .
>  .TP
> +.B \--param-alltgpt|\-Y
> +PR Out parameter 'ALL_TG_PT'.
> +.
> +.TP
>  .B \--param-aptpl|\-Z
>  PR Out parameter 'APTPL'.
>  .
> diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
> index e4b25a0..fb863fd 100644
> --- a/multipath/multipath.conf.5
> +++ b/multipath/multipath.conf.5
> @@ -756,9 +756,11 @@ The default is: \fB<unset>\fR
>  .
>  .TP
>  .B all_tg_pt
> -This must be set to \fByes\fR to successfully use mpathpersist on
> arrays that
> -automatically set and clear registration keys on all target ports
> from a
> -host, instead of per target port per host.
> +Set the 'all targets ports' flag when registering keys with
> mpathpersist. Some
> +arrays automatically set and clear registration keys on all target
> ports from a
> +host, instead of per target port per host. The ALL_TG_PT flag must
> be set to
> +successfully use mpathpersist on these arrays. Setting this option
> is identical
> +to calling mpathpersist with \fI--param-alltgpt\fR
>  .RS
>  .TP
>  The default is: \fBno\fR

-- 
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] 23+ messages in thread

* Re: [PATCH 05/10] libmutipath: remove unused IDE bus type
  2018-07-13 19:39 ` [PATCH 05/10] libmutipath: remove unused IDE bus type Benjamin Marzinski
@ 2018-07-16 10:01   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16 10:01 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/structs.h | 1 -
>  1 file changed, 1 deletion(-)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>

> diff --git a/libmultipath/structs.h b/libmultipath/structs.h
> index ca14315..0a2623a 100644
> --- a/libmultipath/structs.h
> +++ b/libmultipath/structs.h
> @@ -58,7 +58,6 @@ enum failback_mode {
>  enum sysfs_buses {
>  	SYSFS_BUS_UNDEF,
>  	SYSFS_BUS_SCSI,
> -	SYSFS_BUS_IDE,
>  	SYSFS_BUS_CCW,
>  	SYSFS_BUS_CCISS,
>  	SYSFS_BUS_NVME,

-- 
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] 23+ messages in thread

* Re: [PATCH 06/10] multipathd: add new protocol path wildcard
  2018-07-13 19:39 ` [PATCH 06/10] multipathd: add new protocol path wildcard Benjamin Marzinski
@ 2018-07-16 10:09   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-16 10:09 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> This patch adds a new path wildcard 'P', that will print the path's
> protocol.  For scsi devices, it will additionally print the transport
> protocol being used.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/print.c | 43
> +++++++++++++++++++++++++++++++++++++++++++
>  libmultipath/print.h |  2 ++
>  2 files changed, 45 insertions(+)

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
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] 23+ messages in thread

* Re: [PATCH 07/10] libmultipath: add "protocol" blacklist option.
  2018-07-13 19:39 ` [PATCH 07/10] libmultipath: add "protocol" blacklist option Benjamin Marzinski
@ 2018-07-20 18:49   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-20 18:49 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> Multiple users have requested an easy way to setup blacklists that do
> things such as blacklisting all non FC and iSCSI devices. Currently
> there is no easy way to do this, without knowing in advance what the
> devices are.  Looking into the udev property values, I didn't see a
> consistent set of values that would worked for all the different
> types
> of requests like this (which would have allowed us to solve this by
> extending the "property" blacklist option to allow comparing values,
> instead of just keywords).
> 
> Instead I've opted to allow multipath to blacklist/whitelist devices
> by the protocol strings printed by "multipathd: add new protocol path
> wildcard". This check happens after multipath checks the "device"
> keyword, and before it checks wwid. This gives users an easily
> understandible method to set up these types of blacklists, without
> needing to know the exact arrays being used.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/blacklist.c   | 51
> ++++++++++++++++++++++++++++++++++++++--------
>  libmultipath/blacklist.h   |  3 +++
>  libmultipath/config.c      | 15 ++++++++++++++
>  libmultipath/config.h      |  2 ++
>  libmultipath/dict.c        | 14 +++++++++++--
>  libmultipath/discovery.c   |  5 +++--
>  libmultipath/print.c       | 31 ++++++++++++++++++++++++++++
>  multipath/multipath.conf.5 | 16 +++++++++++++--
>  8 files changed, 123 insertions(+), 14 deletions(-)

Reviewed-by: Martin Wilck <mwilck@suse.com>

-- 
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] 23+ messages in thread

* Re: [PATCH 08/10] libmultipath: remove _filter_* blacklist functions
  2018-07-13 19:39 ` [PATCH 08/10] libmultipath: remove _filter_* blacklist functions Benjamin Marzinski
@ 2018-07-20 19:05   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-20 19:05 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> The one point of these functions was for _filter_path(), and that
> wasn't
> improved by using them. Since filter_path() only printed one message
> at
> the end, you could have situations where the wwid was blacklisted,
> but
> the blacklist message included the vendor/product instead. Also, the
> protocol and property messages were printed twice, and if the device
> was
> on multiple whitelists, only the last one is printed.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  libmultipath/blacklist.c | 168 +++++++++++++++++++----------------
> ------------
>  libmultipath/blacklist.h |   2 +-
>  libmultipath/configure.c |   2 +-
>  libmultipath/discovery.c |   2 +-
>  4 files changed, 71 insertions(+), 103 deletions(-)

Reviewed-by: Martin Wilck <mwilck@suse.com>

One remark:

> +filter_device (vector blist, vector elist, char * vendor, char *
> product,
> +	       char * dev)

While we are at it, could we further simplify these to take a "conf"
argument instead of elist and blist?

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] 23+ messages in thread

* Re: [PATCH 09/10] multipath tests: change to work with old make versions
  2018-07-13 19:39 ` [PATCH 09/10] multipath tests: change to work with old make versions Benjamin Marzinski
@ 2018-07-20 19:08   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-20 19:08 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> the $(file <) operation only works with make 4.2 and above. I tried
> running the tests on an old machine and it failed. The $shell
> function
> can do the same thing and multipath has been using that in its
> Makefiles for a while.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  tests/Makefile | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 

Reviewed-by: Martin Wilck <mwilck@suse.com>

I guess I'm lucky that your old make grok'd the magic in 0820bd24df ;-)

-- 
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] 23+ messages in thread

* Re: [PATCH 10/10] multipath tests: add blacklist tests
  2018-07-13 19:39 ` [PATCH 10/10] multipath tests: add blacklist tests Benjamin Marzinski
@ 2018-07-20 19:16   ` Martin Wilck
  0 siblings, 0 replies; 23+ messages in thread
From: Martin Wilck @ 2018-07-20 19:16 UTC (permalink / raw)
  To: Benjamin Marzinski, device-mapper development

On Fri, 2018-07-13 at 14:39 -0500, Benjamin Marzinski wrote:
> These are tests to validate the filter_* blacklist functions. They
> not
> only verify that the device is correctly blacklisted/whitelisted, but
> they also verify the log messages that are printed out.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> ---
>  tests/Makefile    |   4 +-
>  tests/blacklist.c | 512
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 515 insertions(+), 1 deletion(-)
>  create mode 100644 tests/blacklist.c

Reviewed-by: Martin Wilck <mwilck@suse.com>

__wrap_dlog() / expect_condlog() are brilliant, maybe we should
put them in test-lib.c.


-- 
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] 23+ messages in thread

end of thread, other threads:[~2018-07-20 19:16 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-13 19:39 [PATCH 00/10] Misc multipath patches Benjamin Marzinski
2018-07-13 19:39 ` [PATCH 01/10] libmultipath: remove last of rbd code Benjamin Marzinski
2018-07-16  9:25   ` Martin Wilck
2018-07-16  9:52   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 02/10] libmultipath: fix detect alua corner case Benjamin Marzinski
2018-07-16  9:32   ` Martin Wilck
2018-07-16  9:52   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 03/10] multipath: fix setting conf->version Benjamin Marzinski
2018-07-16  9:51   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 04/10] mpathpersist: add --param-alltgpt option Benjamin Marzinski
2018-07-16  9:59   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 05/10] libmutipath: remove unused IDE bus type Benjamin Marzinski
2018-07-16 10:01   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 06/10] multipathd: add new protocol path wildcard Benjamin Marzinski
2018-07-16 10:09   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 07/10] libmultipath: add "protocol" blacklist option Benjamin Marzinski
2018-07-20 18:49   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 08/10] libmultipath: remove _filter_* blacklist functions Benjamin Marzinski
2018-07-20 19:05   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 09/10] multipath tests: change to work with old make versions Benjamin Marzinski
2018-07-20 19:08   ` Martin Wilck
2018-07-13 19:39 ` [PATCH 10/10] multipath tests: add blacklist tests Benjamin Marzinski
2018-07-20 19:16   ` Martin Wilck

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.