[PATCH v2 0/3] multipath: change default devnode blacklist

[PATCH v2 0/3] multipath: change default devnode blacklist

[Benjamin Marzinski]

I recently got a request to add the Oracle ASM filer driver devices to
multipath's builtin devnode blacklist.  However, instead of having to do
always this for each device type individually, I decided to make
multipath blacklist all non scsi, nvme, and dasd devnodes by default.
This is what the multipath udev rules already do.

Changes from v1:
Everything. Martin suggested an alternative method for doing this, which
is to treat an exclamation mark at the start of the blacklist/exceptions
regexes as inverting the matching on the rest of the regex.

Benjamin Marzinski (3):
  libmultipath: remove _blacklist_exceptions functions
  libmultipath: fix parser issue with comments in strings
  libmultipath: invert regexes that start with exclamation point

 libmultipath/blacklist.c   | 103 +++++++++++++++-------------------
 libmultipath/blacklist.h   |   3 +
 libmultipath/parser.c      |   4 +-
 multipath/multipath.conf.5 |  17 ++++--
 tests/blacklist.c          | 110 +++++++++++++++++++++++++++++++++++++
 tests/parser.c             |  42 ++++++++++++++
 tests/test-lib.c           |   2 +-
 7 files changed, 215 insertions(+), 66 deletions(-)

-- 
2.17.2

[PATCH v2 1/3] libmultipath: remove _blacklist_exceptions functions

[Benjamin Marzinski]

_blacklist_exceptions() and _blacklist_exceptions_device() are exactly
the same as _blacklist() and _blacklist_device(), so remove them, and
give the remaining functions to a more general name.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c | 62 ++++++++++------------------------------
 1 file changed, 15 insertions(+), 47 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 00e8dbdb..c21a0e27 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -101,21 +101,8 @@ int set_ble_device(vector blist, char * vendor, char * product, int origin)
 	return 0;
 }
 
-int
-_blacklist_exceptions (vector elist, const char * str)
-{
-	int i;
-	struct blentry * ele;
-
-	vector_foreach_slot (elist, ele, i) {
-		if (!regexec(&ele->regex, str, 0, NULL, 0))
-			return 1;
-	}
-	return 0;
-}
-
-int
-_blacklist (vector blist, const char * str)
+static int
+match_reglist (vector blist, const char * str)
 {
 	int i;
 	struct blentry * ble;
@@ -127,28 +114,9 @@ _blacklist (vector blist, const char * str)
 	return 0;
 }
 
-int
-_blacklist_exceptions_device(const struct _vector *elist, const char * vendor,
-			     const char * product)
-{
-	int i;
-	struct blentry_device * ble;
-
-	vector_foreach_slot (elist, ble, i) {
-		if (!ble->vendor && !ble->product)
-			continue;
-		if ((!ble->vendor ||
-		     !regexec(&ble->vendor_reg, vendor, 0, NULL, 0)) &&
-		    (!ble->product ||
-		     !regexec(&ble->product_reg, product, 0, NULL, 0)))
-			return 1;
-	}
-	return 0;
-}
-
-int
-_blacklist_device (const struct _vector *blist, const char * vendor,
-		   const char * product)
+static int
+match_reglist_device (const struct _vector *blist, const char * vendor,
+		    const char * product)
 {
 	int i;
 	struct blentry_device * ble;
@@ -300,9 +268,9 @@ filter_device (vector blist, vector elist, char * vendor, char * product,
 	int r = MATCH_NOTHING;
 
 	if (vendor && product) {
-		if (_blacklist_exceptions_device(elist, vendor, product))
+		if (match_reglist_device(elist, vendor, product))
 			r = MATCH_DEVICE_BLIST_EXCEPT;
-		else if (_blacklist_device(blist, vendor, product))
+		else if (match_reglist_device(blist, vendor, product))
 			r = MATCH_DEVICE_BLIST;
 	}
 
@@ -316,9 +284,9 @@ filter_devnode (vector blist, vector elist, char * dev)
 	int r = MATCH_NOTHING;
 
 	if (dev) {
-		if (_blacklist_exceptions(elist, dev))
+		if (match_reglist(elist, dev))
 			r = MATCH_DEVNODE_BLIST_EXCEPT;
-		else if (_blacklist(blist, dev))
+		else if (match_reglist(blist, dev))
 			r = MATCH_DEVNODE_BLIST;
 	}
 
@@ -332,9 +300,9 @@ filter_wwid (vector blist, vector elist, char * wwid, char * dev)
 	int r = MATCH_NOTHING;
 
 	if (wwid) {
-		if (_blacklist_exceptions(elist, wwid))
+		if (match_reglist(elist, wwid))
 			r = MATCH_WWID_BLIST_EXCEPT;
-		else if (_blacklist(blist, wwid))
+		else if (match_reglist(blist, wwid))
 			r = MATCH_WWID_BLIST;
 	}
 
@@ -351,9 +319,9 @@ filter_protocol(vector blist, vector elist, struct path * pp)
 	if (pp) {
 		snprint_path_protocol(buf, sizeof(buf), pp);
 
-		if (_blacklist_exceptions(elist, buf))
+		if (match_reglist(elist, buf))
 			r = MATCH_PROTOCOL_BLIST_EXCEPT;
-		else if (_blacklist(blist, buf))
+		else if (match_reglist(blist, buf))
 			r = MATCH_PROTOCOL_BLIST;
 	}
 
@@ -422,11 +390,11 @@ filter_property(struct config *conf, struct udev_device *udev, int lvl,
 			if (check_missing_prop && !strcmp(env, uid_attribute))
 				uid_attr_seen = true;
 
-			if (_blacklist_exceptions(conf->elist_property, env)) {
+			if (match_reglist(conf->elist_property, env)) {
 				r = MATCH_PROPERTY_BLIST_EXCEPT;
 				break;
 			}
-			if (_blacklist(conf->blist_property, env)) {
+			if (match_reglist(conf->blist_property, env)) {
 				r = MATCH_PROPERTY_BLIST;
 				break;
 			}
-- 
2.17.2

[PATCH v2 2/3] libmultipath: fix parser issue with comments in strings

[Benjamin Marzinski]

If a quoted string starts with '#' or '!', the parser will stop
parsing the line, thinking that it's a comment.  It should only
be checking for comments outside of quoted strings. Fixed this and
added unit tests to verify it.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/parser.c |  4 +++-
 tests/parser.c        | 42 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index d478b177..11a6168c 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -300,8 +300,10 @@ alloc_strvec(char *string)
 			(isspace((int) *cp) || !isascii((int) *cp)))
 		       && *cp != '\0')
 			cp++;
-		if (*cp == '\0' || *cp == '!' || *cp == '#')
+		if (*cp == '\0' ||
+		    (!in_string && (*cp == '!' || *cp == '#'))) {
 			return strvec;
+		}
 	}
 out:
 	vector_free(strvec);
diff --git a/tests/parser.c b/tests/parser.c
index 29859dac..5772391e 100644
--- a/tests/parser.c
+++ b/tests/parser.c
@@ -440,6 +440,46 @@ static void test18(void **state)
 	free_strvec(v);
 }
 
+static void test19(void **state)
+{
+#define QUOTED19 "!value"
+	vector v = alloc_strvec("key \"" QUOTED19 "\"");
+	char *val;
+
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "key");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED19);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED19);
+
+	free(val);
+	free_strvec(v);
+}
+
+static void test20(void **state)
+{
+#define QUOTED20 "#value"
+	vector v = alloc_strvec("key \"" QUOTED20 "\"");
+	char *val;
+
+	assert_int_equal(VECTOR_SIZE(v), 4);
+	assert_string_equal(VECTOR_SLOT(v, 0), "key");
+	assert_true(is_quote(VECTOR_SLOT(v, 1)));
+	assert_string_equal(VECTOR_SLOT(v, 2), QUOTED20);
+	assert_true(is_quote(VECTOR_SLOT(v, 3)));
+	assert_int_equal(validate_config_strvec(v, test_file), 0);
+
+	val = set_value(v);
+	assert_string_equal(val, QUOTED20);
+
+	free(val);
+	free_strvec(v);
+}
+
 int test_config_parser(void)
 {
 	const struct CMUnitTest tests[] = {
@@ -461,6 +501,8 @@ int test_config_parser(void)
 		cmocka_unit_test(test16),
 		cmocka_unit_test(test17),
 		cmocka_unit_test(test18),
+		cmocka_unit_test(test19),
+		cmocka_unit_test(test20),
 	};
 	return cmocka_run_group_tests(tests, setup, teardown);
 }
-- 
2.17.2

[PATCH v2 3/3] libmultipath: invert regexes that start with exclamation point

[Benjamin Marzinski]

The number of devices that multipath needs to blacklist keeps growing,
and the udev rules already have

KERNEL!="sd*|dasd*|nvme*", GOTO="end_mpath"

so they only work correctly with these device types.  Instead of
individually blacklisting every type of device that can't be
multipathed, multipath's default blacklist should work like the udev
rule, and blacklist all devices that aren't scsi, dasd, or nvme.
Unfortunately, the c regex library doesn't support negative lookahead.
Instead, multipath should treat "!" at the beginning of
blacklist/exceptions regexes as inverse matching the rest of the regex.
If users need to match a literal '!' as the first character of their
regex, they can use "\!" instead. This allows multipath to change the
default devnode blacklist regex to "!^(sd[a-z]|dasd[a-z]|nvme[0-9])".

Extra tests have been added to the blacklist unit tests to verify the
inverse matching code and the new default blacklist.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/blacklist.c   |  41 +++++++++-----
 libmultipath/blacklist.h   |   3 +
 multipath/multipath.conf.5 |  17 ++++--
 tests/blacklist.c          | 110 +++++++++++++++++++++++++++++++++++++
 tests/test-lib.c           |   2 +-
 5 files changed, 155 insertions(+), 18 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index c21a0e27..db58ccca 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -15,9 +15,24 @@
 #include "structs_vec.h"
 #include "print.h"
 
+char *check_invert(char *str, bool *invert)
+{
+	if (str[0] == '!') {
+		*invert = true;
+		return str + 1;
+	}
+	if (str[0] == '\\' && str[1] == '!') {
+		*invert = false;
+		return str + 1;
+	}
+	*invert = false;
+	return str;
+}
+
 int store_ble(vector blist, char * str, int origin)
 {
 	struct blentry * ble;
+	char *regex_str;
 
 	if (!str)
 		return 0;
@@ -30,7 +45,8 @@ int store_ble(vector blist, char * str, int origin)
 	if (!ble)
 		goto out;
 
-	if (regcomp(&ble->regex, str, REG_EXTENDED|REG_NOSUB))
+	regex_str = check_invert(str, &ble->invert);
+	if (regcomp(&ble->regex, regex_str, REG_EXTENDED|REG_NOSUB))
 		goto out1;
 
 	if (!vector_alloc_slot(blist))
@@ -66,6 +82,7 @@ int alloc_ble_device(vector blist)
 int set_ble_device(vector blist, char * vendor, char * product, int origin)
 {
 	struct blentry_device * ble;
+	char *regex_str;
 
 	if (!blist)
 		return 1;
@@ -76,7 +93,8 @@ int set_ble_device(vector blist, char * vendor, char * product, int origin)
 		return 1;
 
 	if (vendor) {
-		if (regcomp(&ble->vendor_reg, vendor,
+		regex_str = check_invert(vendor, &ble->vendor_invert);
+		if (regcomp(&ble->vendor_reg, regex_str,
 			    REG_EXTENDED|REG_NOSUB)) {
 			FREE(vendor);
 			if (product)
@@ -86,7 +104,8 @@ int set_ble_device(vector blist, char * vendor, char * product, int origin)
 		ble->vendor = vendor;
 	}
 	if (product) {
-		if (regcomp(&ble->product_reg, product,
+		regex_str = check_invert(product, &ble->product_invert);
+		if (regcomp(&ble->product_reg, regex_str,
 			    REG_EXTENDED|REG_NOSUB)) {
 			FREE(product);
 			if (vendor) {
@@ -108,7 +127,7 @@ match_reglist (vector blist, const char * str)
 	struct blentry * ble;
 
 	vector_foreach_slot (blist, ble, i) {
-		if (!regexec(&ble->regex, str, 0, NULL, 0))
+		if (!!regexec(&ble->regex, str, 0, NULL, 0) == ble->invert)
 			return 1;
 	}
 	return 0;
@@ -125,9 +144,11 @@ match_reglist_device (const struct _vector *blist, const char * vendor,
 		if (!ble->vendor && !ble->product)
 			continue;
 		if ((!ble->vendor ||
-		     !regexec(&ble->vendor_reg, vendor, 0, NULL, 0)) &&
+		     !!regexec(&ble->vendor_reg, vendor, 0, NULL, 0) ==
+		     ble->vendor_invert) &&
 		    (!ble->product ||
-		     !regexec(&ble->product_reg, product, 0, NULL, 0)))
+		     !!regexec(&ble->product_reg, product, 0, NULL, 0) ==
+		     ble->product_invert))
 			return 1;
 	}
 	return 0;
@@ -160,13 +181,7 @@ setup_default_blist (struct config * conf)
 	char * str;
 	int i;
 
-	str = STRDUP("^(ram|zram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]");
-	if (!str)
-		return 1;
-	if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
-		return 1;
-
-	str = STRDUP("^(td|hd|vd)[a-z]");
+	str = STRDUP("!^(sd[a-z]|dasd[a-z]|nvme[0-9])");
 	if (!str)
 		return 1;
 	if (store_ble(conf->blist_devnode, str, ORIGIN_DEFAULT))
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index 2d721f60..4305857d 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -20,6 +20,7 @@
 struct blentry {
 	char * str;
 	regex_t regex;
+	bool invert;
 	int origin;
 };
 
@@ -28,6 +29,8 @@ struct blentry_device {
 	char * product;
 	regex_t vendor_reg;
 	regex_t product_reg;
+	bool vendor_invert;
+	bool product_invert;
 	int origin;
 };
 
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index 05a5e8ff..28cb6057 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -1248,6 +1248,16 @@ being handled by multipath-tools.
 .LP
 .
 .
+In the \fIblacklist\fR and \fIblacklist_exceptions\fR sections, starting a
+quoted value with an exclamation mark \fB"!"\fR will invert the matching
+of the rest of the regular expression. For instance, \fB"!^sd[a-z]"\fR will
+match all values that do not start with \fB"sd[a-z]"\fR. The exclamation mark
+can be escaped \fB"\\!"\fR to match a literal \fB!\fR at the start of a
+regular expression. \fBNote:\fR The exclamation mark must be inside quotes,
+otherwise it will be treated as starting a comment.
+.LP
+.
+.
 The \fIblacklist_exceptions\fR section is used to revert the actions of the
 \fIblacklist\fR section. This allows one to selectively include ("whitelist") devices which
 would normally be excluded via the \fIblacklist\fR section. A common usage is
@@ -1264,10 +1274,9 @@ unless explicitly stated.
 Regular expression matching the device nodes to be excluded/included.
 .RS
 .PP
-The default \fIblacklist\fR consists of the regular expressions
-"^(ram|zram|raw|loop|fd|md|dm-|sr|scd|st|dcssblk)[0-9]" and
-"^(td|hd|vd)[a-z]". This causes virtual devices, non-disk devices, and some other
-device types to be excluded from multipath handling by default.
+The default \fIblacklist\fR consists of the regular expression
+\fB"!^(sd[a-z]|dasd[a-z]|nvme[0-9])"\fR. This causes all device types other
+than scsi, dasd, and nvme to be excluded from multipath handling by default.
 .RE
 .TP
 .B wwid
diff --git a/tests/blacklist.c b/tests/blacklist.c
index 6e7c1864..d5c40898 100644
--- a/tests/blacklist.c
+++ b/tests/blacklist.c
@@ -60,20 +60,46 @@ __wrap_udev_list_entry_get_name(struct udev_list_entry *list_entry)
 	return *(const char **)list_entry;
 }
 
+vector elist_property_default;
+vector blist_devnode_default;
 vector blist_devnode_sdb;
+vector blist_devnode_sdb_inv;
 vector blist_all;
 vector blist_device_foo_bar;
+vector blist_device_foo_inv_bar;
+vector blist_device_foo_bar_inv;
 vector blist_device_all;
 vector blist_wwid_xyzzy;
+vector blist_wwid_xyzzy_inv;
 vector blist_protocol_fcp;
+vector blist_protocol_fcp_inv;
 vector blist_property_wwn;
+vector blist_property_wwn_inv;
 
 static int setup(void **state)
 {
+	struct config conf;
+
+	memset(&conf, 0, sizeof(conf));
+	conf.blist_devnode = vector_alloc();
+	if (!conf.blist_devnode)
+		return -1;
+	conf.elist_property = vector_alloc();
+	if (!conf.elist_property)
+		return -1;
+	if (setup_default_blist(&conf) != 0)
+		return -1;
+	elist_property_default = conf.elist_property;
+	blist_devnode_default = conf.blist_devnode;
+
 	blist_devnode_sdb = vector_alloc();
 	if (!blist_devnode_sdb ||
 	    store_ble(blist_devnode_sdb, strdup("sdb"), ORIGIN_CONFIG))
 		return -1;
+	blist_devnode_sdb_inv = vector_alloc();
+	if (!blist_devnode_sdb_inv ||
+	    store_ble(blist_devnode_sdb_inv, strdup("!sdb"), ORIGIN_CONFIG))
+		return -1;
 
 	blist_all = vector_alloc();
 	if (!blist_all || store_ble(blist_all, strdup(".*"), ORIGIN_CONFIG))
@@ -84,6 +110,18 @@ static int setup(void **state)
 	    set_ble_device(blist_device_foo_bar, strdup("foo"), strdup("bar"),
 			   ORIGIN_CONFIG))
 		return -1;
+	blist_device_foo_inv_bar = vector_alloc();
+	if (!blist_device_foo_inv_bar ||
+	    alloc_ble_device(blist_device_foo_inv_bar) ||
+	    set_ble_device(blist_device_foo_inv_bar, strdup("!foo"),
+			   strdup("bar"), ORIGIN_CONFIG))
+		return -1;
+	blist_device_foo_bar_inv = vector_alloc();
+	if (!blist_device_foo_bar_inv ||
+	    alloc_ble_device(blist_device_foo_bar_inv) ||
+	    set_ble_device(blist_device_foo_bar_inv, strdup("foo"),
+			   strdup("!bar"), ORIGIN_CONFIG))
+		return -1;
 
 	blist_device_all = vector_alloc();
 	if (!blist_device_all || alloc_ble_device(blist_device_all) ||
@@ -95,29 +133,50 @@ static int setup(void **state)
 	if (!blist_wwid_xyzzy ||
 	    store_ble(blist_wwid_xyzzy, strdup("xyzzy"), ORIGIN_CONFIG))
 		return -1;
+	blist_wwid_xyzzy_inv = vector_alloc();
+	if (!blist_wwid_xyzzy_inv ||
+	    store_ble(blist_wwid_xyzzy_inv, 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_protocol_fcp_inv = vector_alloc();
+	if (!blist_protocol_fcp_inv ||
+	    store_ble(blist_protocol_fcp_inv, 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;
+	blist_property_wwn_inv = vector_alloc();
+	if (!blist_property_wwn_inv ||
+	    store_ble(blist_property_wwn_inv, strdup("!ID_WWN"), ORIGIN_CONFIG))
+		return -1;
 
 	return 0;
 }
 
 static int teardown(void **state)
 {
+	free_blacklist(elist_property_default);
+	free_blacklist(blist_devnode_default);
 	free_blacklist(blist_devnode_sdb);
+	free_blacklist(blist_devnode_sdb_inv);
 	free_blacklist(blist_all);
 	free_blacklist_device(blist_device_foo_bar);
+	free_blacklist_device(blist_device_foo_inv_bar);
+	free_blacklist_device(blist_device_foo_bar_inv);
 	free_blacklist_device(blist_device_all);
 	free_blacklist(blist_wwid_xyzzy);
+	free_blacklist(blist_wwid_xyzzy_inv);
 	free_blacklist(blist_protocol_fcp);
+	free_blacklist(blist_protocol_fcp_inv);
 	free_blacklist(blist_property_wwn);
+	free_blacklist(blist_property_wwn_inv);
 	return 0;
 }
 
@@ -141,6 +200,11 @@ 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);
+	assert_int_equal(filter_devnode(blist_devnode_sdb_inv, NULL, "sdb"),
+			 MATCH_NOTHING);
+	expect_condlog(3, "sdc: device node name blacklisted\n");
+	assert_int_equal(filter_devnode(blist_devnode_sdb_inv, NULL, "sdc"),
+			 MATCH_DEVNODE_BLIST);
 }
 
 static void test_devnode_whitelist(void **state)
@@ -159,12 +223,39 @@ static void test_devnode_missing(void **state)
 			 MATCH_NOTHING);
 }
 
+static void test_devnode_default(void **state)
+{
+	assert_int_equal(filter_devnode(blist_devnode_default, NULL, "sdaa"),
+			 MATCH_NOTHING);
+	assert_int_equal(filter_devnode(blist_devnode_default, NULL, "nvme0n1"),
+			 MATCH_NOTHING);
+	assert_int_equal(filter_devnode(blist_devnode_default, NULL, "dasda"),
+			 MATCH_NOTHING);
+	expect_condlog(3, "hda: device node name blacklisted\n");
+	assert_int_equal(filter_devnode(blist_devnode_default, NULL, "hda"),
+			 MATCH_DEVNODE_BLIST);
+}
+
 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);
+	assert_int_equal(filter_device(blist_device_foo_inv_bar, NULL, "foo",
+				        "bar", "sdb"),
+			 MATCH_NOTHING);
+	assert_int_equal(filter_device(blist_device_foo_bar_inv, NULL, "foo",
+				        "bar", "sdb"),
+			 MATCH_NOTHING);
+	expect_condlog(3, "sdb: (baz:bar) vendor/product blacklisted\n");
+	assert_int_equal(filter_device(blist_device_foo_inv_bar, NULL, "baz",
+				        "bar", "sdb"),
+			 MATCH_DEVICE_BLIST);
+	expect_condlog(3, "sdb: (foo:baz) vendor/product blacklisted\n");
+	assert_int_equal(filter_device(blist_device_foo_bar_inv, NULL, "foo",
+				        "baz", "sdb"),
+			 MATCH_DEVICE_BLIST);
 }
 
 static void test_device_whitelist(void **state)
@@ -191,6 +282,11 @@ 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);
+	assert_int_equal(filter_wwid(blist_wwid_xyzzy_inv, NULL, "xyzzy",
+				     "sdb"), MATCH_NOTHING);
+	expect_condlog(3, "sdb: wwid plugh blacklisted\n");
+	assert_int_equal(filter_wwid(blist_wwid_xyzzy_inv, NULL, "plugh",
+				     "sdb"), MATCH_WWID_BLIST);
 }
 
 static void test_wwid_whitelist(void **state)
@@ -218,6 +314,12 @@ static void test_protocol_blacklist(void **state)
 	expect_condlog(3, "sdb: protocol scsi:fcp blacklisted\n");
 	assert_int_equal(filter_protocol(blist_protocol_fcp, NULL, &pp),
 			 MATCH_PROTOCOL_BLIST);
+	assert_int_equal(filter_protocol(blist_protocol_fcp_inv, NULL, &pp),
+			 MATCH_NOTHING);
+	pp.sg_id.proto_id = SCSI_PROTOCOL_ATA;
+	expect_condlog(3, "sdb: protocol scsi:ata blacklisted\n");
+	assert_int_equal(filter_protocol(blist_protocol_fcp_inv, NULL, &pp),
+			 MATCH_PROTOCOL_BLIST);
 }
 
 static void test_protocol_whitelist(void **state)
@@ -245,10 +347,17 @@ static void test_protocol_missing(void **state)
 static void test_property_blacklist(void **state)
 {
 	static struct udev_device udev = { "sdb", { "ID_FOO", "ID_WWN", "ID_BAR", NULL } };
+	static struct udev_device udev_inv = { "sdb", { "ID_WWN", NULL } };
 	conf.blist_property = blist_property_wwn;
 	expect_condlog(3, "sdb: udev property ID_WWN blacklisted\n");
 	assert_int_equal(filter_property(&conf, &udev, 3, "ID_SERIAL"),
 			 MATCH_PROPERTY_BLIST);
+	conf.blist_property = blist_property_wwn_inv;
+	expect_condlog(3, "sdb: udev property ID_FOO blacklisted\n");
+	assert_int_equal(filter_property(&conf, &udev, 3, "ID_SERIAL"),
+			 MATCH_PROPERTY_BLIST);
+	assert_int_equal(filter_property(&conf, &udev_inv, 3, "ID_SERIAL"),
+			 MATCH_NOTHING);
 }
 
 /* the property check works different in that you check all the property
@@ -484,6 +593,7 @@ int test_blacklist(void)
 		cmocka_unit_test(test_devnode_blacklist),
 		cmocka_unit_test(test_devnode_whitelist),
 		cmocka_unit_test(test_devnode_missing),
+		cmocka_unit_test(test_devnode_default),
 		cmocka_unit_test(test_device_blacklist),
 		cmocka_unit_test(test_device_whitelist),
 		cmocka_unit_test(test_device_missing),
diff --git a/tests/test-lib.c b/tests/test-lib.c
index 00bae58e..b7c09cc2 100644
--- a/tests/test-lib.c
+++ b/tests/test-lib.c
@@ -15,7 +15,7 @@
 #include "test-lib.h"
 
 const int default_mask = (DI_SYSFS|DI_BLACKLIST|DI_WWID|DI_CHECKER|DI_PRIO);
-const char default_devnode[] = "sdTEST";
+const char default_devnode[] = "sdxTEST";
 const char default_wwid[] = "TEST-WWID";
 /* default_wwid should be a substring of default_wwid_1! */
 const char default_wwid_1[] = "TEST-WWID-1";
-- 
2.17.2

Re: [PATCH v2 0/3] multipath: change default devnode blacklist

[Martin Wilck]

On Tue, 2020-06-09 at 16:35 -0500, Benjamin Marzinski wrote:
> I recently got a request to add the Oracle ASM filer driver devices
> to
> multipath's builtin devnode blacklist.  However, instead of having to
> do
> always this for each device type individually, I decided to make
> multipath blacklist all non scsi, nvme, and dasd devnodes by default.
> This is what the multipath udev rules already do.
> 
> Changes from v1:
> Everything. Martin suggested an alternative method for doing this,
> which
> is to treat an exclamation mark at the start of the
> blacklist/exceptions
> regexes as inverting the matching on the rest of the regex.
> 
> Benjamin Marzinski (3):
>   libmultipath: remove _blacklist_exceptions functions
>   libmultipath: fix parser issue with comments in strings
>   libmultipath: invert regexes that start with exclamation point
> 
>  libmultipath/blacklist.c   | 103 +++++++++++++++-------------------
>  libmultipath/blacklist.h   |   3 +
>  libmultipath/parser.c      |   4 +-
>  multipath/multipath.conf.5 |  17 ++++--
>  tests/blacklist.c          | 110
> +++++++++++++++++++++++++++++++++++++
>  tests/parser.c             |  42 ++++++++++++++
>  tests/test-lib.c           |   2 +-
>  7 files changed, 215 insertions(+), 66 deletions(-)
> 

Very nice work, thank you!

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

-- 
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE  Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer