[PATCH v3 00/24] multipath-tools: improve logging at -v3

[PATCH v3 00/24] multipath-tools: improve logging at -v3

[Martin Wilck]

Hi Christophe,

most of the patches in this series reduce log levels of frequently
printed messages at verbosity level 3. My goal was to limit the
output of multipathd to one line per path per checker invocation,
which is sufficient to track multipathd's view of path health in
the logs.

The standard setting of -v2 is not enough for post-mortem analysis of many
failures. With this series, running multipathd with verbosity 3 becomes a
realistic option even in production environments. So far the amount of output
from multipathd with -v3 pretty much made this impossible, at least over
longer time periods, and also made reading these logs very cumbersome due to
the amount of redundant partly superfluos verbosity. I've taken care not
to loose important information in the logs.

Apart from that, the series fixes errors in the unit tests introduced by my
last "checker overhaul" patch series (proving that I forgot to run the
tests before submitting :-( ), and fixes a problem that I found while testing
handling of a bad configuration (paths with size mismatch).

Regards,
Martin

Changes in v3:

Resent full series on Christophe's request. All patches except 22/24, 23/24 are
the same as before. Added Ben's "Reviewed-by:" where appropriate.

 - 22/24: added one fix for a non-symbolic reference to a domap() return value,
which I'd overlooked before.
 - 23/24: removed on Ben's review. The new 23/24 is just a code cleanup
without functional change.

Changes in v2:

The first 19 patches are identical to v1 as ACK'd by Ben, except 16/24
"libmultipath: coalesce_paths: fix size mismatch handling".
No. 8/24 "libmultipath: decrease log level of word splitting"
(not yet ACKd by Ben) also stays the same; the issue Ben raised
in his review is addressed in a separate patch, 20/24.
21/24 addresses implements Ben's suggestion to use named constants
as return values in coalesce_paths(). 22, 23, 24 do the same for
other important, related functions, as I found it strange to make
this change just for coalesce_paths() alone.  

Martin Wilck (24):
  tests/hwtable: set multipath_dir in local configuration
  tests/hwtable: adjust to new checker API
  multipath-tools: decrease verbosity of state messages
  libmultipath: decrease verbosity of pathinfo messages
  libmultipath: decrease verbosity of TUR checker messages
  libmultipath: avoid frequent messages from filter_property()
  libmultipath: decrease log level of "disassembled" messages
  libmultipath: decrease log level of word splitting
  libmultipath: increase log level of map removal
  multipathd: decrease log level of checker timing
  libmultipath: decrease log level of "prioritizer refcount" message
  libmpathpersist/update_map_pr: decrease log level for nop
  libmultipath: simplify devt2devname()
  libmultipath: decrease log level for failed VPD c9
  libmultipath: adopt_paths: check for size match
  libmultipath: coalesce_paths: fix size mismatch handling
  tests: add unit tests for bitmask functions
  multipathd: uev_remove_path: remove redundant orphan_paths call
  libmultipath: improve logging from orphan_paths
  libmultipath: avoid syslog loglevel > LOG_DEBUG
  coalesce_paths(): use symbolic return value
  domap(): use symbolic return value
  multipathd: simplify retry logic in ev_add_path()
  multipath: use symbolic return value and exit code

 libmpathpersist/mpath_persist.c |   3 +-
 libmultipath/blacklist.c        |  54 +++++++-------
 libmultipath/blacklist.h        |   2 +-
 libmultipath/checkers/tur.c     |   6 +-
 libmultipath/configure.c        |  48 +++++++------
 libmultipath/configure.h        |  23 ++++++
 libmultipath/discovery.c        |  20 +++---
 libmultipath/dmparser.c         |   6 +-
 libmultipath/log_pthread.c      |   3 +
 libmultipath/prio.c             |   2 +-
 libmultipath/structs_vec.c      |  18 +++--
 libmultipath/structs_vec.h      |   3 +-
 libmultipath/util.c             |   7 +-
 libmultipath/util.h             |  16 +++++
 multipath/main.c                | 121 ++++++++++++++++++--------------
 multipathd/cli_handlers.c       |   5 +-
 multipathd/main.c               |  45 ++++++------
 tests/Makefile                  |   7 +-
 tests/blacklist.c               |   7 +-
 tests/hwtable.c                 |  89 ++++++++++++-----------
 tests/util.c                    |  98 ++++++++++++++++++++++++++
 21 files changed, 383 insertions(+), 200 deletions(-)

-- 
2.19.2

[PATCH v3 01/24] tests/hwtable: set multipath_dir in local configuration

[Martin Wilck]

If internal libmultipath APIs change (such as, recently, the
checker API), test programs will fail because they'll link
with the standard system prioritizer / checker / foreign APIs.

Make sure we always link with our own shared libraries.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/Makefile  | 7 ++++++-
 tests/hwtable.c | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/tests/Makefile b/tests/Makefile
index b37b5027..ef900866 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -26,7 +26,11 @@ hwtable-test_LIBDEPS := -ludev -lpthread -ldl
 blacklist-test_OBJDEPS := ../libmultipath/blacklist.o
 blacklist-test_LIBDEPS := -ludev
 
-%.out:	%-test
+lib/libchecktur.so:
+	mkdir lib
+	ln -t lib ../libmultipath/{checkers,prioritizers,foreign}/*.so
+
+%.out:	%-test lib/libchecktur.so
 	@echo == running $< ==
 	@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
 
@@ -34,6 +38,7 @@ OBJS = $(TESTS:%=%.o) test-lib.o
 
 clean: dep_clean
 	$(RM) $(TESTS:%=%-test) $(TESTS:%=%.out) $(OBJS)
+	$(RM) -rf lib
 
 .SECONDARY: $(OBJS)
 
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 9146ecc3..1cd788ac 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -250,14 +250,19 @@ static void write_defaults(const struct hwt_state *hwt)
 	static struct key_value defaults[] = {
 		{ "config_dir", NULL },
 		{ "bindings_file", NULL },
+		{ "multipath_dir", NULL },
 		{ "detect_prio", "no" },
 		{ "detect_checker", "no" },
 	};
 	char buf[sizeof(tmplate) + sizeof(bindings_name)];
+	char dirbuf[PATH_MAX];
 
 	snprintf(buf, sizeof(buf), "%s/%s", hwt->tmpname, bindings_name);
 	defaults[0].value = hwt->dirname;
 	defaults[1].value = buf;
+	assert_ptr_not_equal(getcwd(dirbuf, sizeof(dirbuf)), NULL);
+	strncat(dirbuf, "/lib", sizeof(dirbuf));
+	defaults[2].value = dirbuf;
 	write_section(hwt->config_file, "defaults",
 		      ARRAY_SIZE(defaults), defaults);
 }
-- 
2.19.2

[PATCH v3 02/24] tests/hwtable: adjust to new checker API

[Martin Wilck]

checker.name doesn't exist any more.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/hwtable.c | 84 ++++++++++++++++++++++++-------------------------
 1 file changed, 42 insertions(+), 42 deletions(-)

diff --git a/tests/hwtable.c b/tests/hwtable.c
index 1cd788ac..789481ff 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -570,7 +570,7 @@ static void test_internal_nvme(const struct hwt_state *hwt)
 	pp = mock_path("NVME", "NoName");
 	mp = mock_multipath(pp);
 	assert_ptr_not_equal(mp, NULL);
-	TEST_PROP(pp->checker.name, NONE);
+	TEST_PROP(checker_name(&pp->checker), NONE);
 	TEST_PROP(pp->uid_attribute, "ID_WWN");
 	assert_int_equal(mp->pgpolicy, DEFAULT_PGPOLICY);
 	assert_int_equal(mp->no_path_retry, DEFAULT_NO_PATH_RETRY);
@@ -583,7 +583,7 @@ static void test_internal_nvme(const struct hwt_state *hwt)
 			    default_wwid_1);
 	mp = mock_multipath(pp);
 	assert_ptr_not_equal(mp, NULL);
-	TEST_PROP(pp->checker.name, NONE);
+	TEST_PROP(checker_name(&pp->checker), NONE);
 	TEST_PROP(pp->uid_attribute, "ID_WWN");
 	assert_int_equal(mp->pgpolicy, MULTIBUS);
 	assert_int_equal(mp->no_path_retry, NO_PATH_RETRY_QUEUE);
@@ -755,31 +755,31 @@ static void test_regex_string_hwe(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* boo:baz matches kv1 */
 	pp = mock_path(vnd_boo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .oo:ba. matches kv1 */
 	pp = mock_path(vnd__oo.value, prd_ba_.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .foo:(bar|baz|ba\.) doesn't match */
 	pp = mock_path(vnd__oo.value, prd_ba_s.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches kv2 and kv1 */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_regex_string_hwe(void **state)
@@ -812,32 +812,32 @@ static void test_regex_string_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* boo:baz matches kv1 */
 	pp = mock_path(vnd_boo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .oo:ba. matches kv1 */
 	pp = mock_path(vnd__oo.value, prd_ba_.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .oo:(bar|baz|ba\.)$ doesn't match */
 	pp = mock_path(vnd__oo.value, prd_ba_s.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches kv2 */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	/* Later match takes prio */
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_regex_string_hwe_dir(void **state)
@@ -868,28 +868,28 @@ static void test_regex_2_strings_hwe_dir(const struct hwt_state *hwt)
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
 	TEST_PROP(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* boo:baz doesn't match */
 	pp = mock_path(vnd_boo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
 	TEST_PROP(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches kv2 and kv1 */
 	pp = mock_path(vnd_foo.value, prd_bar.value);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, NULL);
 	TEST_PROP(pp->uid_attribute, uid_baz.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* foo:barz matches kv3 and kv2 and kv1 */
 	pp = mock_path_flags(vnd_foo.value, prd_barz.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_rdac.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
 	TEST_PROP(pp->uid_attribute, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_regex_2_strings_hwe_dir(void **state)
@@ -926,31 +926,31 @@ static void test_string_regex_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* foo:baz matches kv1 */
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* boo:baz matches kv1 */
 	pp = mock_path(vnd_boo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .oo:ba. matches kv1 */
 	pp = mock_path(vnd__oo.value, prd_ba_.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* .oo:(bar|baz|ba\.)$ doesn't match */
 	pp = mock_path(vnd__oo.value, prd_ba_s.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 }
 
 static int setup_string_regex_hwe_dir(void **state)
@@ -980,13 +980,13 @@ static void test_2_ident_strings_hwe(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_strings_hwe(void **state)
@@ -1015,13 +1015,13 @@ static void test_2_ident_strings_both_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_strings_both_dir(void **state)
@@ -1055,13 +1055,13 @@ static void test_2_ident_strings_both_dir_w_prev(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_strings_both_dir_w_prev(void **state)
@@ -1100,13 +1100,13 @@ static void test_2_ident_strings_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_strings_hwe_dir(void **state)
@@ -1134,13 +1134,13 @@ static void test_3_ident_strings_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_3_ident_strings_hwe_dir(void **state)
@@ -1178,13 +1178,13 @@ static void test_2_ident_self_matching_re_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_self_matching_re_hwe_dir(void **state)
@@ -1213,13 +1213,13 @@ static void test_2_ident_self_matching_re_hwe(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_self_matching_re_hwe(void **state)
@@ -1250,13 +1250,13 @@ test_2_ident_not_self_matching_re_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_baz.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	/* foo:bar matches both */
 	pp = mock_path_flags(vnd_foo.value, prd_bar.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_ident_not_self_matching_re_hwe_dir(void **state)
@@ -1287,19 +1287,19 @@ static void test_2_matching_res_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_bar.value);
 	TEST_PROP(prio_name(&pp->prio), prio_emc.value);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* foo:bay matches k1 and k2 */
 	pp = mock_path_flags(vnd_foo.value, "bay", USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 
 	/* foo:baz matches k2 only. */
 	pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 }
 
 static int setup_2_matching_res_hwe_dir(void **state)
@@ -1328,12 +1328,12 @@ static void test_2_nonmatching_res_hwe_dir(const struct hwt_state *hwt)
 	pp = mock_path(vnd_foo.value, prd_bar.value);
 	TEST_PROP(prio_name(&pp->prio), DEFAULT_PRIO);
 	TEST_PROP(pp->getuid, NULL);
-	TEST_PROP(pp->checker.name, DEFAULT_CHECKER);
+	TEST_PROP(checker_name(&pp->checker), DEFAULT_CHECKER);
 
 	pp = mock_path_flags(vnd_foo.value, prd_baz.value, USE_GETUID);
 	TEST_PROP(prio_name(&pp->prio), prio_hds.value);
 	TEST_PROP(pp->getuid, gui_foo.value);
-	TEST_PROP(pp->checker.name, chk_hp.value);
+	TEST_PROP(checker_name(&pp->checker), chk_hp.value);
 }
 
 static int setup_2_nonmatching_res_hwe_dir(void **state)
-- 
2.19.2

[PATCH v3 03/24] multipath-tools: decrease verbosity of state messages

[Martin Wilck]

With verbosity level 3, thousands of "path state = running" messages
are logged, which are pretty much irrelevant, as the checker state
takes precedence. Also the "get_state" message is totally useless.

With this patch, the path state is reported exactly once per path
and check.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c |  4 +---
 multipathd/main.c        | 18 ++++++++++--------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 63558ad8..f9a59011 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1433,7 +1433,7 @@ path_offline (struct path * pp)
 	}
 
 
-	condlog(3, "%s: path state = %s", pp->dev, buff);
+	condlog(4, "%s: path state = %s", pp->dev, buff);
 
 	if (pp->bus == SYSFS_BUS_SCSI) {
 		if (!strncmp(buff, "offline", 7)) {
@@ -1552,8 +1552,6 @@ get_state (struct path * pp, struct config *conf, int daemon, int oldstate)
 	struct checker * c = &pp->checker;
 	int state;
 
-	condlog(3, "%s: get_state", pp->dev);
-
 	if (!checker_selected(c)) {
 		if (daemon) {
 			if (pathinfo(pp, conf, DI_SYSFS) != PATHINFO_OK) {
diff --git a/multipathd/main.c b/multipathd/main.c
index c781115c..e07ef502 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1904,6 +1904,16 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 	pp->tick = checkint;
 
 	newstate = path_offline(pp);
+	if (newstate == PATH_UP) {
+		conf = get_multipath_config();
+		pthread_cleanup_push(put_multipath_config, conf);
+		newstate = get_state(pp, conf, 1, newstate);
+		pthread_cleanup_pop(1);
+	} else {
+		checker_clear_message(&pp->checker);
+		condlog(3, "%s: state %s, checker not called",
+			pp->dev, checker_state_name(newstate));
+	}
 	/*
 	 * Wait for uevent for removed paths;
 	 * some LLDDs like zfcp keep paths unavailable
@@ -1912,14 +1922,6 @@ check_path (struct vectors * vecs, struct path * pp, int ticks)
 	if (newstate == PATH_REMOVED)
 		newstate = PATH_DOWN;
 
-	if (newstate == PATH_UP) {
-		conf = get_multipath_config();
-		pthread_cleanup_push(put_multipath_config, conf);
-		newstate = get_state(pp, conf, 1, newstate);
-		pthread_cleanup_pop(1);
-	} else
-		checker_clear_message(&pp->checker);
-
 	if (pp->wwid_changed) {
 		condlog(2, "%s: path wwid has changed. Refusing to use",
 			pp->dev);
-- 
2.19.2

[PATCH v3 04/24] libmultipath: decrease verbosity of pathinfo messages

[Martin Wilck]

Reduce the verbosity of pointless messages from pathinfo(), in
particular those that are printed on every checker invocation.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index f9a59011..3ad33492 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1382,7 +1382,7 @@ common_sysfs_pathinfo (struct path * pp)
 	devt = udev_device_get_devnum(pp->udev);
 	snprintf(pp->dev_t, BLK_DEV_SIZE, "%d:%d", major(devt), minor(devt));
 
-	condlog(3, "%s: dev_t = %s", pp->dev, pp->dev_t);
+	condlog(4, "%s: dev_t = %s", pp->dev, pp->dev_t);
 
 	if (sysfs_get_size(pp, &pp->size))
 		return PATHINFO_FAILED;
@@ -1599,6 +1599,7 @@ get_prio (struct path * pp)
 	struct prio * p;
 	struct config *conf;
 	int checker_timeout;
+	int old_prio;
 
 	if (!pp)
 		return 0;
@@ -1619,13 +1620,14 @@ get_prio (struct path * pp)
 	conf = get_multipath_config();
 	checker_timeout = conf->checker_timeout;
 	put_multipath_config(conf);
+	old_prio = pp->priority;
 	pp->priority = prio_getprio(p, pp, checker_timeout);
 	if (pp->priority < 0) {
 		condlog(3, "%s: %s prio error", pp->dev, prio_name(p));
 		pp->priority = PRIO_UNDEF;
 		return 1;
 	}
-	condlog(3, "%s: %s prio = %u",
+	condlog((old_prio == pp->priority ? 4 : 3), "%s: %s prio = %u",
 		pp->dev, prio_name(p), pp->priority);
 	return 0;
 }
@@ -1863,7 +1865,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 			udev_device_get_sysattr_value(pp->udev, "hidden");
 
 		if (hidden && !strcmp(hidden, "1")) {
-			condlog(3, "%s: hidden", pp->dev);
+			condlog(4, "%s: hidden", pp->dev);
 			return PATHINFO_SKIPPED;
 		}
 		if (is_claimed_by_foreign(pp->udev) ||
@@ -1876,7 +1878,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 			   pp->dev) > 0)
 		return PATHINFO_SKIPPED;
 
-	condlog(3, "%s: mask = 0x%x", pp->dev, mask);
+	condlog(4, "%s: mask = 0x%x", pp->dev, mask);
 
 	/*
 	 * Sanity check: we need the device number to
-- 
2.19.2

[PATCH v3 05/24] libmultipath: decrease verbosity of TUR checker messages

[Martin Wilck]

The thread startup/stop messages aren't very interesting.
Decrease their verbosity level to v4.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/checkers/tur.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/checkers/tur.c b/libmultipath/checkers/tur.c
index 63b19624..6b08dbbb 100644
--- a/libmultipath/checkers/tur.c
+++ b/libmultipath/checkers/tur.c
@@ -261,7 +261,7 @@ static void *tur_thread(void *ctx)
 	tur_thread_cleanup_push(ct);
 	rcu_register_thread();
 
-	condlog(3, "%d:%d : tur checker starting up", major(ct->devt),
+	condlog(4, "%d:%d : tur checker starting up", major(ct->devt),
 		minor(ct->devt));
 
 	tur_deep_sleep(ct);
@@ -275,7 +275,7 @@ static void *tur_thread(void *ctx)
 	pthread_cond_signal(&ct->active);
 	pthread_mutex_unlock(&ct->lock);
 
-	condlog(3, "%d:%d : tur checker finished, state %s", major(ct->devt),
+	condlog(4, "%d:%d : tur checker finished, state %s", major(ct->devt),
 		minor(ct->devt), checker_state_name(state));
 
 	running = uatomic_xchg(&ct->running, 0);
@@ -415,7 +415,7 @@ int libcheck_check(struct checker * c)
 		}
 		pthread_mutex_unlock(&ct->lock);
 		if (tur_status == PATH_PENDING) {
-			condlog(3, "%d:%d : tur checker still running",
+			condlog(4, "%d:%d : tur checker still running",
 				major(ct->devt), minor(ct->devt));
 		} else {
 			int running = uatomic_xchg(&ct->running, 0);
-- 
2.19.2

[PATCH v3 06/24] libmultipath: avoid frequent messages from filter_property()

[Martin Wilck]

Unlike the other filter functions, filter_property() is called
on every pathinfo() call, and prints a message with -v3 every
time. Avoid that by using a lower log priority for the call
from pathinfo().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/blacklist.c | 54 ++++++++++++++++++++--------------------
 libmultipath/blacklist.h |  2 +-
 libmultipath/configure.c |  6 ++---
 libmultipath/discovery.c |  2 +-
 tests/blacklist.c        |  7 +++---
 5 files changed, 36 insertions(+), 35 deletions(-)

diff --git a/libmultipath/blacklist.c b/libmultipath/blacklist.c
index 318ec03f..709895e2 100644
--- a/libmultipath/blacklist.c
+++ b/libmultipath/blacklist.c
@@ -232,24 +232,24 @@ setup_default_blist (struct config * conf)
 	return 0;
 }
 
-#define LOG_BLIST(M,S)							\
+#define LOG_BLIST(M, S, lvl)						\
 	if (vendor && product)						\
-		condlog(3, "%s: (%s:%s) %s %s",				\
+		condlog(lvl, "%s: (%s:%s) %s %s",			\
 			dev, vendor, product, (M), (S));		\
 	else if (wwid && !dev)						\
-		condlog(3, "%s: %s %s", wwid, (M), (S));		\
+		condlog(lvl, "%s: %s %s", wwid, (M), (S));		\
 	else if (wwid)							\
-		condlog(3, "%s: %s %s %s", dev, (M), wwid, (S));	\
+		condlog(lvl, "%s: %s %s %s", dev, (M), wwid, (S));	\
 	else if (env)							\
-		condlog(3, "%s: %s %s %s", dev, (M), env, (S));		\
+		condlog(lvl, "%s: %s %s %s", dev, (M), env, (S));	\
 	else if (protocol)						\
-		condlog(3, "%s: %s %s %s", dev, (M), protocol, (S));	\
+		condlog(lvl, "%s: %s %s %s", dev, (M), protocol, (S));	\
 	else								\
-		condlog(3, "%s: %s %s", dev, (M), (S))
+		condlog(lvl, "%s: %s %s", dev, (M), (S))
 
-void
+static void
 log_filter (const char *dev, char *vendor, char *product, char *wwid,
-	    const char *env, const char *protocol, int r)
+	    const char *env, const char *protocol, int r, int lvl)
 {
 	/*
 	 * Try to sort from most likely to least.
@@ -258,37 +258,37 @@ log_filter (const char *dev, char *vendor, char *product, char *wwid,
 	case MATCH_NOTHING:
 		break;
 	case MATCH_DEVICE_BLIST:
-		LOG_BLIST("vendor/product", "blacklisted");
+		LOG_BLIST("vendor/product", "blacklisted", lvl);
 		break;
 	case MATCH_WWID_BLIST:
-		LOG_BLIST("wwid", "blacklisted");
+		LOG_BLIST("wwid", "blacklisted", lvl);
 		break;
 	case MATCH_DEVNODE_BLIST:
-		LOG_BLIST("device node name", "blacklisted");
+		LOG_BLIST("device node name", "blacklisted", lvl);
 		break;
 	case MATCH_PROPERTY_BLIST:
-		LOG_BLIST("udev property", "blacklisted");
+		LOG_BLIST("udev property", "blacklisted", lvl);
 		break;
 	case MATCH_PROTOCOL_BLIST:
-		LOG_BLIST("protocol", "blacklisted");
+		LOG_BLIST("protocol", "blacklisted", lvl);
 		break;
 	case MATCH_DEVICE_BLIST_EXCEPT:
-		LOG_BLIST("vendor/product", "whitelisted");
+		LOG_BLIST("vendor/product", "whitelisted", lvl);
 		break;
 	case MATCH_WWID_BLIST_EXCEPT:
-		LOG_BLIST("wwid", "whitelisted");
+		LOG_BLIST("wwid", "whitelisted", lvl);
 		break;
 	case MATCH_DEVNODE_BLIST_EXCEPT:
-		LOG_BLIST("device node name", "whitelisted");
+		LOG_BLIST("device node name", "whitelisted", lvl);
 		break;
 	case MATCH_PROPERTY_BLIST_EXCEPT:
-		LOG_BLIST("udev property", "whitelisted");
+		LOG_BLIST("udev property", "whitelisted", lvl);
 		break;
 	case MATCH_PROPERTY_BLIST_MISSING:
-		LOG_BLIST("blacklisted,", "udev property missing");
+		LOG_BLIST("blacklisted,", "udev property missing", lvl);
 		break;
 	case MATCH_PROTOCOL_BLIST_EXCEPT:
-		LOG_BLIST("protocol", "whitelisted");
+		LOG_BLIST("protocol", "whitelisted", lvl);
 		break;
 	}
 }
@@ -306,7 +306,7 @@ filter_device (vector blist, vector elist, char * vendor, char * product,
 			r = MATCH_DEVICE_BLIST;
 	}
 
-	log_filter(dev, vendor, product, NULL, NULL, NULL, r);
+	log_filter(dev, vendor, product, NULL, NULL, NULL, r, 3);
 	return r;
 }
 
@@ -322,7 +322,7 @@ filter_devnode (vector blist, vector elist, char * dev)
 			r = MATCH_DEVNODE_BLIST;
 	}
 
-	log_filter(dev, NULL, NULL, NULL, NULL, NULL, r);
+	log_filter(dev, NULL, NULL, NULL, NULL, NULL, r, 3);
 	return r;
 }
 
@@ -338,7 +338,7 @@ filter_wwid (vector blist, vector elist, char * wwid, char * dev)
 			r = MATCH_WWID_BLIST;
 	}
 
-	log_filter(dev, NULL, NULL, wwid, NULL, NULL, r);
+	log_filter(dev, NULL, NULL, wwid, NULL, NULL, r, 3);
 	return r;
 }
 
@@ -357,7 +357,7 @@ filter_protocol(vector blist, vector elist, struct path * pp)
 			r = MATCH_PROTOCOL_BLIST;
 	}
 
-	log_filter(pp->dev, NULL, NULL, NULL, NULL, buf, r);
+	log_filter(pp->dev, NULL, NULL, NULL, NULL, buf, r, 3);
 	return r;
 }
 
@@ -366,7 +366,7 @@ filter_path (struct config * conf, struct path * pp)
 {
 	int r;
 
-	r = filter_property(conf, pp->udev);
+	r = filter_property(conf, pp->udev, 3);
 	if (r > 0)
 		return r;
 	r = filter_devnode(conf->blist_devnode, conf->elist_devnode, pp->dev);
@@ -384,7 +384,7 @@ filter_path (struct config * conf, struct path * pp)
 }
 
 int
-filter_property(struct config * conf, struct udev_device * udev)
+filter_property(struct config *conf, struct udev_device *udev, int lvl)
 {
 	const char *devname = udev_device_get_sysname(udev);
 	struct udev_list_entry *list_entry;
@@ -415,7 +415,7 @@ filter_property(struct config * conf, struct udev_device * udev)
 		}
 	}
 
-	log_filter(devname, NULL, NULL, NULL, env, NULL, r);
+	log_filter(devname, NULL, NULL, NULL, env, NULL, r, lvl);
 	return r;
 }
 
diff --git a/libmultipath/blacklist.h b/libmultipath/blacklist.h
index 18903b6b..4c8ec99e 100644
--- a/libmultipath/blacklist.h
+++ b/libmultipath/blacklist.h
@@ -37,7 +37,7 @@ int filter_devnode (vector, vector, char *);
 int filter_wwid (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_property(struct config *, struct udev_device *, int);
 int filter_protocol(vector, vector, struct path *);
 int store_ble (vector, char *, int);
 int set_ble_device (vector, char *, char *, int);
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index ed3e30f5..406cd4c9 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1289,7 +1289,7 @@ int get_refwwid(enum mpath_cmds cmd, char *dev, enum devtypes dev_type,
 		conf = get_multipath_config();
 		pthread_cleanup_push(put_multipath_config, conf);
 		if (pp->udev && pp->uid_attribute &&
-		    filter_property(conf, pp->udev) > 0)
+		    filter_property(conf, pp->udev, 3) > 0)
 			invalid = 1;
 		pthread_cleanup_pop(1);
 		if (invalid)
@@ -1329,7 +1329,7 @@ int get_refwwid(enum mpath_cmds cmd, char *dev, enum devtypes dev_type,
 		conf = get_multipath_config();
 		pthread_cleanup_push(put_multipath_config, conf);
 		if (pp->udev && pp->uid_attribute &&
-		    filter_property(conf, pp->udev) > 0)
+		    filter_property(conf, pp->udev, 3) > 0)
 			invalid = 1;
 		pthread_cleanup_pop(1);
 		if (invalid)
@@ -1358,7 +1358,7 @@ int get_refwwid(enum mpath_cmds cmd, char *dev, enum devtypes dev_type,
 		conf = get_multipath_config();
 		pthread_cleanup_push(put_multipath_config, conf);
 		if (pp->udev && pp->uid_attribute &&
-		    filter_property(conf, pp->udev) > 0)
+		    filter_property(conf, pp->udev, 3) > 0)
 			invalid = 1;
 		pthread_cleanup_pop(1);
 		if (invalid)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 3ad33492..1c87277f 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1869,7 +1869,7 @@ int pathinfo(struct path *pp, struct config *conf, int mask)
 			return PATHINFO_SKIPPED;
 		}
 		if (is_claimed_by_foreign(pp->udev) ||
-			 filter_property(conf, pp->udev) > 0)
+		    filter_property(conf, pp->udev, 4) > 0)
 			return PATHINFO_SKIPPED;
 	}
 
diff --git a/tests/blacklist.c b/tests/blacklist.c
index a55c1c07..54d568f5 100644
--- a/tests/blacklist.c
+++ b/tests/blacklist.c
@@ -267,7 +267,8 @@ 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);
+	assert_int_equal(filter_property(&conf, &udev, 3),
+			 MATCH_PROPERTY_BLIST);
 }
 
 /* the property check works different in that you check all the property
@@ -280,7 +281,7 @@ 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),
+	assert_int_equal(filter_property(&conf, &udev, 3),
 			 MATCH_PROPERTY_BLIST_EXCEPT);
 }
 
@@ -289,7 +290,7 @@ 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),
+	assert_int_equal(filter_property(&conf, &udev, 3),
 			 MATCH_PROPERTY_BLIST_MISSING);
 }
 
-- 
2.19.2

[PATCH v3 07/24] libmultipath: decrease log level of "disassembled" messages

[Martin Wilck]

These messages are printed from update_multipath_strings, and
thus from check_path(), which is too much for -v3.

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

diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 620f507d..ac13ec06 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -117,7 +117,7 @@ assemble_map (struct multipath * mp, char * params, int len)
 	}
 
 	FREE(f);
-	condlog(3, "%s: assembled map [%s]", mp->alias, params);
+	condlog(4, "%s: assembled map [%s]", mp->alias, params);
 	return 0;
 
 err:
@@ -145,7 +145,7 @@ int disassemble_map(vector pathvec, char *params, struct multipath *mpp,
 
 	p = params;
 
-	condlog(3, "%s: disassemble map [%s]", mpp->alias, params);
+	condlog(4, "%s: disassemble map [%s]", mpp->alias, params);
 
 	/*
 	 * features
@@ -410,7 +410,7 @@ int disassemble_status(char *params, struct multipath *mpp)
 
 	p = params;
 
-	condlog(3, "%s: disassemble status [%s]", mpp->alias, params);
+	condlog(4, "%s: disassemble status [%s]", mpp->alias, params);
 
 	/*
 	 * features
-- 
2.19.2

[PATCH v3 08/24] libmultipath: decrease log level of word splitting

[Martin Wilck]

This will make log level 4 actually usable.

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

diff --git a/libmultipath/util.c b/libmultipath/util.c
index 66c47611..b5d2e706 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -104,7 +104,7 @@ get_word (char * sentence, char ** word)
 	}
 	strncpy(*word, sentence, len);
 	strchop(*word);
-	condlog(4, "*word = %s, len = %i", *word, len);
+	condlog(5, "*word = %s, len = %i", *word, len);
 
 	if (*p == '\0')
 		return 0;
-- 
2.19.2

[PATCH v3 09/24] libmultipath: increase log level of map removal

[Martin Wilck]

This is an important event which we want to see with -v3.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/structs_vec.c | 6 +++---
 multipathd/main.c          | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index e28a88ac..01391d77 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -113,8 +113,6 @@ remove_map(struct multipath * mpp, struct vectors * vecs, int purge_vec)
 {
 	int i;
 
-	condlog(4, "%s: remove multipath map", mpp->alias);
-
 	/*
 	 * clear references to this map
 	 */
@@ -134,8 +132,10 @@ void
 remove_map_by_alias(const char *alias, struct vectors * vecs, int purge_vec)
 {
 	struct multipath * mpp = find_mp_by_alias(vecs->mpvec, alias);
-	if (mpp)
+	if (mpp) {
+		condlog(2, "%s: removing map by alias", alias);
 		remove_map(mpp, vecs, purge_vec);
+	}
 }
 
 void
diff --git a/multipathd/main.c b/multipathd/main.c
index e07ef502..85055993 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -334,6 +334,7 @@ remove_map_and_stop_waiter(struct multipath *mpp, struct vectors *vecs)
 {
 	/* devices are automatically removed by the dmevent polling code,
 	 * so they don't need to be manually removed here */
+	condlog(3, "%s: removing map from internal tables", mpp->alias);
 	if (!poll_dmevents)
 		stop_waiter_thread(mpp, vecs);
 	remove_map(mpp, vecs, PURGE_VEC);
-- 
2.19.2

[PATCH v3 10/24] multipathd: decrease log level of checker timing

[Martin Wilck]

These messages, printed for every checker invocation, were helpful
in the past but they don't make much sense under normal operations.
The same information can be derived from journal time stamps on
recent Linux distributions.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 85055993..4d7c2598 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2262,7 +2262,7 @@ checkerloop (void *ap)
 			if (num_paths) {
 				unsigned int max_checkint;
 
-				condlog(3, "checked %d path%s in %lu.%06lu secs",
+				condlog(4, "checked %d path%s in %lu.%06lu secs",
 					num_paths, num_paths > 1 ? "s" : "",
 					diff_time.tv_sec,
 					diff_time.tv_nsec / 1000);
-- 
2.19.2

[PATCH v3 11/24] libmultipath: decrease log level of "prioritizer refcount" message

[Martin Wilck]

This was important during development of the refcounting code, but
now it isn't any more.

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

diff --git a/libmultipath/prio.c b/libmultipath/prio.c
index 17acfd05..0590218d 100644
--- a/libmultipath/prio.c
+++ b/libmultipath/prio.c
@@ -42,7 +42,7 @@ void free_prio (struct prio * p)
 		return;
 	p->refcount--;
 	if (p->refcount) {
-		condlog(3, "%s prioritizer refcount %d",
+		condlog(4, "%s prioritizer refcount %d",
 			p->name, p->refcount);
 		return;
 	}
-- 
2.19.2

[PATCH v3 12/24] libmpathpersist/update_map_pr: decrease log level for nop

[Martin Wilck]

No need to log at -v3 if nothing to do.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmpathpersist/mpath_persist.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libmpathpersist/mpath_persist.c b/libmpathpersist/mpath_persist.c
index 7e8a676c..6505774f 100644
--- a/libmpathpersist/mpath_persist.c
+++ b/libmpathpersist/mpath_persist.c
@@ -889,7 +889,8 @@ int update_map_pr(struct multipath *mpp)
 	if (!get_be64(mpp->reservation_key))
 	{
 		/* Nothing to do. Assuming pr mgmt feature is disabled*/
-		condlog(3, "%s: reservation_key not set in multipath.conf", mpp->alias);
+		condlog(4, "%s: reservation_key not set in multipath.conf",
+			mpp->alias);
 		return MPATH_PR_SUCCESS;
 	}
 
-- 
2.19.2

[PATCH v3 13/24] libmultipath: simplify devt2devname()

[Martin Wilck]

Since 1e79548, we don't fallback from /sys/dev/block to
/proc/partitions anyway. The strncmp() at "skip_proc"
is doomed to fail. So we might as well return immediately.

Also, decrease the log level; whether this failure is
noteworthy is rather up to the callers to decide.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/util.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libmultipath/util.c b/libmultipath/util.c
index b5d2e706..28eb7577 100644
--- a/libmultipath/util.c
+++ b/libmultipath/util.c
@@ -191,7 +191,8 @@ int devt2devname(char *devname, int devname_len, char *devt)
 				return 0;
 			}
 		}
-		goto skip_proc;
+		condlog(4, "%s is invalid", block_path);
+		return 1;
 	}
 	memset(block_path, 0, sizeof(block_path));
 
@@ -220,7 +221,7 @@ int devt2devname(char *devname, int devname_len, char *devt)
 		}
 	}
 	fclose(fd);
-skip_proc:
+
 	if (strncmp(block_path,"/sys/block", 10)) {
 		condlog(3, "No device found for %u:%u", major, minor);
 		return 1;
-- 
2.19.2

[PATCH v3 14/24] libmultipath: decrease log level for failed VPD c9

[Martin Wilck]

It's normal that SCSI devices don't support this page. Only RDAC
devices need it.

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

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 1c87277f..7f983a63 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1106,7 +1106,9 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
 
 	memset(buff, 0x0, 4096);
 	if (sgio_get_vpd(buff, 4096, fd, pg) < 0) {
-		condlog(3, "failed to issue vpd inquiry for pg%02x",
+		int lvl = pg == 0x80 || pg == 0x83 ? 3 : 4;
+
+		condlog(lvl, "failed to issue vpd inquiry for pg%02x",
 			pg);
 		return -errno;
 	}
-- 
2.19.2

[PATCH v3 15/24] libmultipath: adopt_paths: check for size match

[Martin Wilck]

In coalesce_paths() and ev_add_path(), we check for size mismatch.
We should do it here, too.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/structs_vec.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 01391d77..5305bd31 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -60,6 +60,12 @@ int adopt_paths(vector pathvec, struct multipath *mpp)
 
 	vector_foreach_slot (pathvec, pp, i) {
 		if (!strncmp(mpp->wwid, pp->wwid, WWID_SIZE)) {
+			if (pp->size != 0 && mpp->size != 0 &&
+			    pp->size != mpp->size) {
+				condlog(3, "%s: size mismatch for %s, not adding path",
+					pp->dev, mpp->alias);
+				continue;
+			}
 			condlog(3, "%s: ownership set to %s",
 				pp->dev, mpp->alias);
 			pp->mpp = mpp;
-- 
2.19.2

[PATCH v3 16/24] libmultipath: coalesce_paths: fix size mismatch handling

[Martin Wilck]

When there are paths with the same WWID but different sizes, and
coalesce_paths() walks the pathvec, it checks paths _after_
the current one for size mismatch and sets ACT_REJECT. However,
these paths will be reached in the main loop later, and this time
the already handled paths will not be checked for size mismatch;
thus a map could be created, possibly even with mismatching
devices.

Fix that by tracking which paths were already discarded, and
skipping them in the main loop later.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c | 32 ++++++++++++++++++++++++--------
 libmultipath/util.h      | 16 ++++++++++++++++
 2 files changed, 40 insertions(+), 8 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 406cd4c9..f48664a0 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -1009,6 +1009,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 	vector pathvec = vecs->pathvec;
 	struct config *conf;
 	int allow_queueing;
+	uint64_t *size_mismatch_seen;
 
 	/* ignore refwwid if it's empty */
 	if (refwwid && !strlen(refwwid))
@@ -1019,6 +1020,14 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 			pp1->mpp = NULL;
 		}
 	}
+
+	if (VECTOR_SIZE(pathvec) == 0)
+		return 0;
+	size_mismatch_seen = calloc((VECTOR_SIZE(pathvec) - 1) / 64 + 1,
+				    sizeof(uint64_t));
+	if (size_mismatch_seen == NULL)
+		return 1;
+
 	vector_foreach_slot (pathvec, pp1, k) {
 		int invalid;
 		/* skip this path for some reason */
@@ -1038,8 +1047,8 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 			continue;
 		}
 
-		/* 2. if path already coalesced */
-		if (pp1->mpp)
+		/* 2. if path already coalesced, or seen and discarded */
+		if (pp1->mpp || is_bit_set_in_array(k, size_mismatch_seen))
 			continue;
 
 		/* 3. if path has disappeared */
@@ -1088,9 +1097,10 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 				 * ouch, avoid feeding that to the DM
 				 */
 				condlog(0, "%s: size %llu, expected %llu. "
-					"Discard", pp2->dev_t, pp2->size,
+					"Discard", pp2->dev, pp2->size,
 					mpp->size);
 				mpp->action = ACT_REJECT;
+				set_bit_in_array(i, size_mismatch_seen);
 			}
 		}
 		verify_paths(mpp, vecs);
@@ -1119,8 +1129,9 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 					"ignoring" : "removing");
 				remove_map(mpp, vecs, 0);
 				continue;
-			} else /* if (r == DOMAP_RETRY) */
-				return r;
+			} else /* if (r == DOMAP_RETRY && !is_daemon) */ {
+				goto out;
+			}
 		}
 		if (r == DOMAP_DRY)
 			continue;
@@ -1161,8 +1172,10 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 
 		if (newmp) {
 			if (mpp->action != ACT_REJECT) {
-				if (!vector_alloc_slot(newmp))
-					return 1;
+				if (!vector_alloc_slot(newmp)) {
+					r = 1;
+					goto out;
+				}
 				vector_set_slot(newmp, mpp);
 			}
 			else
@@ -1193,7 +1206,10 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 				condlog(2, "%s: remove (dead)", alias);
 		}
 	}
-	return 0;
+	r = 0;
+out:
+	free(size_mismatch_seen);
+	return r;
 }
 
 struct udev_device *get_udev_device(const char *dev, enum devtypes dev_type)
diff --git a/libmultipath/util.h b/libmultipath/util.h
index a818e29a..1f13c913 100644
--- a/libmultipath/util.h
+++ b/libmultipath/util.h
@@ -3,6 +3,7 @@
 
 #include <sys/types.h>
 #include <inttypes.h>
+#include <stdbool.h>
 
 size_t strchop(char *);
 int basenamecpy (const char *src, char *dst, size_t size);
@@ -39,4 +40,19 @@ struct scandir_result {
 };
 void free_scandir_result(struct scandir_result *);
 
+static inline bool is_bit_set_in_array(unsigned int bit, const uint64_t *arr)
+{
+	return arr[bit / 64] & (1ULL << (bit % 64)) ? 1 : 0;
+}
+
+static inline void set_bit_in_array(unsigned int bit, uint64_t *arr)
+{
+	arr[bit / 64] |= (1ULL << (bit % 64));
+}
+
+static inline void clear_bit_in_array(unsigned int bit, uint64_t *arr)
+{
+	arr[bit / 64] &= ~(1ULL << (bit % 64));
+}
+
 #endif /* _UTIL_H */
-- 
2.19.2

[PATCH v3 17/24] tests: add unit tests for bitmask functions

[Martin Wilck]

Add unit tests for the bitmask management functions the previous
patch added in util.h.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 tests/util.c | 98 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 98 insertions(+)

diff --git a/tests/util.c b/tests/util.c
index 839effd2..e6d4b9ab 100644
--- a/tests/util.c
+++ b/tests/util.c
@@ -26,6 +26,8 @@
 
 #include "globals.c"
 
+#define BITARR_SZ 4
+
 static void test_basenamecpy_good0(void **state)
 {
 	char dst[10];
@@ -139,6 +141,100 @@ static void test_basenamecpy_bad5(void **state)
         assert_int_equal(basenamecpy("baz/qux", NULL, sizeof(dst)), 0);
 }
 
+static void test_bitmask_1(void **state)
+{
+	uint64_t arr[BITARR_SZ];
+	int i, j, k, m, b;
+
+	memset(arr, 0, sizeof(arr));
+
+	for (j = 0; j < BITARR_SZ; j++) {
+		for (i = 0; i < 64; i++) {
+			b = 64 * j + i;
+			assert(!is_bit_set_in_array(b, arr));
+			set_bit_in_array(b, arr);
+			for (k = 0; k < BITARR_SZ; k++) {
+				printf("b = %d j = %d k = %d a = %"PRIx64"\n",
+				       b, j, k, arr[k]);
+				if (k == j)
+					assert_int_equal(arr[j], 1ULL << i);
+				else
+					assert_int_equal(arr[k], 0ULL);
+			}
+			for (m = 0; m < 64; m++)
+				if (i == m)
+					assert(is_bit_set_in_array(64 * j + m,
+								   arr));
+				else
+					assert(!is_bit_set_in_array(64 * j + m,
+								    arr));
+			clear_bit_in_array(b, arr);
+			assert(!is_bit_set_in_array(b, arr));
+			for (k = 0; k < BITARR_SZ; k++)
+				assert_int_equal(arr[k], 0ULL);
+		}
+	}
+}
+
+static void test_bitmask_2(void **state)
+{
+	uint64_t arr[BITARR_SZ];
+	int i, j, k, m, b;
+
+	memset(arr, 0, sizeof(arr));
+
+	for (j = 0; j < BITARR_SZ; j++) {
+		for (i = 0; i < 64; i++) {
+			b = 64 * j + i;
+			assert(!is_bit_set_in_array(b, arr));
+			set_bit_in_array(b, arr);
+			for (m = 0; m < 64; m++)
+				if (m <= i)
+					assert(is_bit_set_in_array(64 * j + m,
+								   arr));
+				else
+					assert(!is_bit_set_in_array(64 * j + m,
+								    arr));
+			assert(is_bit_set_in_array(b, arr));
+			for (k = 0; k < BITARR_SZ; k++) {
+				if (k < j || (k == j && i == 63))
+					assert_int_equal(arr[k], ~0ULL);
+				else if (k > j)
+					assert_int_equal(arr[k], 0ULL);
+				else
+					assert_int_equal(
+						arr[k],
+						(1ULL << (i + 1)) - 1);
+			}
+		}
+	}
+	for (j = 0; j < BITARR_SZ; j++) {
+		for (i = 0; i < 64; i++) {
+			b = 64 * j + i;
+			assert(is_bit_set_in_array(b, arr));
+			clear_bit_in_array(b, arr);
+			for (m = 0; m < 64; m++)
+				if (m <= i)
+					assert(!is_bit_set_in_array(64 * j + m,
+								    arr));
+				else
+					assert(is_bit_set_in_array(64 * j + m,
+								   arr));
+			assert(!is_bit_set_in_array(b, arr));
+			for (k = 0; k < BITARR_SZ; k++) {
+				if (k < j || (k == j && i == 63))
+					assert_int_equal(arr[k], 0ULL);
+				else if (k > j)
+					assert_int_equal(arr[k], ~0ULL);
+				else
+					assert_int_equal(
+						arr[k],
+						~((1ULL << (i + 1)) - 1));
+			}
+		}
+	}
+}
+
 int test_basenamecpy(void)
 {
 	const struct CMUnitTest tests[] = {
@@ -156,6 +252,8 @@ int test_basenamecpy(void)
 		cmocka_unit_test(test_basenamecpy_bad3),
 		cmocka_unit_test(test_basenamecpy_bad4),
 		cmocka_unit_test(test_basenamecpy_bad5),
+		cmocka_unit_test(test_bitmask_1),
+		cmocka_unit_test(test_bitmask_2),
 	};
 	return cmocka_run_group_tests(tests, NULL, NULL);
 }
-- 
2.19.2

[PATCH v3 18/24] multipathd: uev_remove_path: remove redundant orphan_paths call

[Martin Wilck]

The remove_map_and_stop_waiter() call which follows immediately
will do the same thing.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 4d7c2598..5e6c35d4 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -786,7 +786,6 @@ uev_remove_map (struct uevent * uev, struct vectors * vecs)
 		goto out;
 	}
 
-	orphan_paths(vecs->pathvec, mpp);
 	remove_map_and_stop_waiter(mpp, vecs);
 out:
 	lock_cleanup_pop(vecs->lock);
-- 
2.19.2

[PATCH v3 19/24] libmultipath: improve logging from orphan_paths

[Martin Wilck]

It's a big difference if a map is flushed from DM (changing kernel
state) or just removed from internal multipathd tables. Convey
this information in log messages.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/structs_vec.c | 6 +++---
 libmultipath/structs_vec.h | 3 ++-
 multipathd/main.c          | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 5305bd31..03e2b978 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -102,14 +102,14 @@ void orphan_path(struct path *pp, const char *reason)
 	pp->fd = -1;
 }
 
-void orphan_paths(vector pathvec, struct multipath *mpp)
+void orphan_paths(vector pathvec, struct multipath *mpp, const char *reason)
 {
 	int i;
 	struct path * pp;
 
 	vector_foreach_slot (pathvec, pp, i) {
 		if (pp->mpp == mpp) {
-			orphan_path(pp, "map flushed");
+			orphan_path(pp, reason);
 		}
 	}
 }
@@ -122,7 +122,7 @@ remove_map(struct multipath * mpp, struct vectors * vecs, int purge_vec)
 	/*
 	 * clear references to this map
 	 */
-	orphan_paths(vecs->pathvec, mpp);
+	orphan_paths(vecs->pathvec, mpp, "map removed internally");
 
 	if (purge_vec &&
 	    (i = find_slot(vecs->mpvec, (void *)mpp)) != -1)
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index f7777aaf..f8b9f63e 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -14,7 +14,8 @@ struct vectors {
 void enter_recovery_mode(struct multipath *mpp);
 
 int adopt_paths (vector pathvec, struct multipath * mpp);
-void orphan_paths (vector pathvec, struct multipath * mpp);
+void orphan_paths(vector pathvec, struct multipath *mpp,
+		  const char *reason);
 void orphan_path (struct path * pp, const char *reason);
 
 int verify_paths(struct multipath * mpp, struct vectors * vecs);
diff --git a/multipathd/main.c b/multipathd/main.c
index 5e6c35d4..04f02d99 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -654,7 +654,7 @@ flush_map(struct multipath * mpp, struct vectors * vecs, int nopaths)
 		condlog(2, "%s: map flushed", mpp->alias);
 	}
 
-	orphan_paths(vecs->pathvec, mpp);
+	orphan_paths(vecs->pathvec, mpp, "map flushed");
 	remove_map_and_stop_waiter(mpp, vecs);
 
 	return 0;
-- 
2.19.2

[PATCH v3 20/24] libmultipath: avoid syslog loglevel > LOG_DEBUG

[Martin Wilck]

We use condlog(5, ...) in some places, which doesn't work well
with syslog.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/log_pthread.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/libmultipath/log_pthread.c b/libmultipath/log_pthread.c
index bb35dfc7..be57bb1a 100644
--- a/libmultipath/log_pthread.c
+++ b/libmultipath/log_pthread.c
@@ -25,6 +25,9 @@ static int log_messages_pending;
 
 void log_safe (int prio, const char * fmt, va_list ap)
 {
+	if (prio > LOG_DEBUG)
+		prio = LOG_DEBUG;
+
 	if (log_thr == (pthread_t)0) {
 		vsyslog(prio, fmt, ap);
 		return;
-- 
2.19.2

[PATCH v3 21/24] coalesce_paths(): use symbolic return value

[Martin Wilck]

Use an enum to represent the return value of coalesce_paths() to
improve readability of the code.
This patch doesn't introduce changes in behavior.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c  | 17 ++++++++---------
 libmultipath/configure.h  | 10 ++++++++++
 multipath/main.c          |  5 +++--
 multipathd/cli_handlers.c |  3 ++-
 multipathd/main.c         |  2 +-
 5 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index f48664a0..5daf0c13 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -998,8 +998,8 @@ out:
 int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 		    int force_reload, enum mpath_cmds cmd)
 {
-	int r = 1;
-	int k, i;
+	int ret = CP_FAIL;
+	int k, i, r;
 	int is_daemon = (cmd == CMD_NONE) ? 1 : 0;
 	char params[PARAMS_SIZE];
 	struct multipath * mpp;
@@ -1022,11 +1022,11 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 	}
 
 	if (VECTOR_SIZE(pathvec) == 0)
-		return 0;
+		return CP_OK;
 	size_mismatch_seen = calloc((VECTOR_SIZE(pathvec) - 1) / 64 + 1,
 				    sizeof(uint64_t));
 	if (size_mismatch_seen == NULL)
-		return 1;
+		return CP_FAIL;
 
 	vector_foreach_slot (pathvec, pp1, k) {
 		int invalid;
@@ -1130,6 +1130,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 				remove_map(mpp, vecs, 0);
 				continue;
 			} else /* if (r == DOMAP_RETRY && !is_daemon) */ {
+				ret = CP_RETRY;
 				goto out;
 			}
 		}
@@ -1172,10 +1173,8 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 
 		if (newmp) {
 			if (mpp->action != ACT_REJECT) {
-				if (!vector_alloc_slot(newmp)) {
-					r = 1;
+				if (!vector_alloc_slot(newmp))
 					goto out;
-				}
 				vector_set_slot(newmp, mpp);
 			}
 			else
@@ -1206,10 +1205,10 @@ int coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid,
 				condlog(2, "%s: remove (dead)", alias);
 		}
 	}
-	r = 0;
+	ret = CP_OK;
 out:
 	free(size_mismatch_seen);
-	return r;
+	return ret;
 }
 
 struct udev_device *get_udev_device(const char *dev, enum devtypes dev_type)
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 8b56d33a..64520c57 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -23,6 +23,16 @@ enum actions {
 	ACT_IMPOSSIBLE,
 };
 
+/*
+ * Return value of coalesce_paths()
+ * CP_RETRY is only used in non-daemon case (multipath).
+ */
+enum {
+	CP_OK = 0,
+	CP_FAIL,
+	CP_RETRY,
+};
+
 #define FLUSH_ONE 1
 #define FLUSH_ALL 2
 
diff --git a/multipath/main.c b/multipath/main.c
index 98fee1c7..1d34b87b 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -537,7 +537,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	vector curmp = NULL;
 	vector pathvec = NULL;
 	struct vectors vecs;
-	int r = 1;
+	int r = 1, rc;
 	int di_flag = 0;
 	char * refwwid = NULL;
 	char * dev = NULL;
@@ -752,8 +752,9 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	/*
 	 * core logic entry point
 	 */
-	r = coalesce_paths(&vecs, NULL, refwwid,
+	rc = coalesce_paths(&vecs, NULL, refwwid,
 			   conf->force_reload, cmd);
+	r = rc == CP_RETRY ? -1 : rc == CP_OK ? 0 : 1;
 
 print_valid:
 	if (cmd == CMD_VALID_PATH)
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index a0d57a53..4fbd8841 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -803,7 +803,8 @@ cli_add_map (void * v, char ** reply, int * len, void * data)
 				    vecs->pathvec, &refwwid);
 			if (refwwid) {
 				if (coalesce_paths(vecs, NULL, refwwid,
-						   FORCE_RELOAD_NONE, CMD_NONE))
+						   FORCE_RELOAD_NONE, CMD_NONE)
+				    != CP_OK)
 					condlog(2, "%s: coalesce_paths failed",
 									param);
 				dm_lib_release();
diff --git a/multipathd/main.c b/multipathd/main.c
index 04f02d99..fd1ac8fe 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2366,7 +2366,7 @@ configure (struct vectors * vecs)
 	ret = coalesce_paths(vecs, mpvec, NULL, force_reload, CMD_NONE);
 	if (force_reload == FORCE_RELOAD_WEAK)
 		force_reload = FORCE_RELOAD_YES;
-	if (ret) {
+	if (ret != CP_OK) {
 		condlog(0, "configure failed while coalescing paths");
 		goto fail;
 	}
-- 
2.19.2

[PATCH v3 22/24] domap(): use symbolic return value

[Martin Wilck]

Use an enum for the already-symbolic return value of domap(), and
avoid the use of "<= 0" for the return value, which is against the
spirit of symbolic values. A return value less or equal than 0 means
DOMAP_FAIL or DOMAP_RETRY. Use the fact that DOMAP_RETRY is only
returned in the ACT_CREATE case for simplification of the logic
in those cases where ACT_CREATE is not used.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/configure.c  |  9 ---------
 libmultipath/configure.h  | 13 +++++++++++++
 multipathd/cli_handlers.c |  2 +-
 multipathd/main.c         |  8 ++++----
 4 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 5daf0c13..84ae5f56 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -788,15 +788,6 @@ fail:
 	return 1;
 }
 
-/*
- * Return value:
- */
-#define DOMAP_RETRY	-1
-#define DOMAP_FAIL	0
-#define DOMAP_OK	1
-#define DOMAP_EXIST	2
-#define DOMAP_DRY	3
-
 int domap(struct multipath *mpp, char *params, int is_daemon)
 {
 	int r = DOMAP_FAIL;
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 64520c57..1b73bf42 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -23,6 +23,19 @@ enum actions {
 	ACT_IMPOSSIBLE,
 };
 
+/*
+ * Return value of domap()
+ * DAEMON_RETRY is only used in non-daemon case (multipath),
+ * and only for ACT_CREATE (see domap()).
+ */
+enum {
+	DOMAP_RETRY	= -1,
+	DOMAP_FAIL	= 0,
+	DOMAP_OK	= 1,
+	DOMAP_EXIST	= 2,
+	DOMAP_DRY	= 3
+};
+
 /*
  * Return value of coalesce_paths()
  * CP_RETRY is only used in non-daemon case (multipath).
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 4fbd8841..14aec17b 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -893,7 +893,7 @@ int resize_map(struct multipath *mpp, unsigned long long size,
 	}
 	mpp->action = ACT_RESIZE;
 	mpp->force_udev_reload = 1;
-	if (domap(mpp, params, 1) <= 0) {
+	if (domap(mpp, params, 1) == DOMAP_FAIL) {
 		condlog(0, "%s: failed to resize map : %s", mpp->alias,
 			strerror(errno));
 		mpp->size = orig_size;
diff --git a/multipathd/main.c b/multipathd/main.c
index fd1ac8fe..fd3459f7 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -497,7 +497,7 @@ retry:
 		retries = -1;
 		goto fail;
 	}
-	if (domap(mpp, params, 1) <= 0 && retries-- > 0) {
+	if (domap(mpp, params, 1) == DOMAP_FAIL && retries-- > 0) {
 		condlog(0, "%s: map_udate sleep", mpp->alias);
 		sleep(1);
 		goto retry;
@@ -995,8 +995,8 @@ rescan:
 	 */
 retry:
 	ret = domap(mpp, params, 1);
-	if (ret <= 0) {
-		if (ret < 0 && retries-- > 0) {
+	if (ret == DOMAP_FAIL || ret == DOMAP_RETRY) {
+		if (ret == DOMAP_RETRY && retries-- > 0) {
 			condlog(0, "%s: retry domap for addition of new "
 				"path %s", mpp->alias, pp->dev);
 			sleep(1);
@@ -1152,7 +1152,7 @@ ev_remove_path (struct path *pp, struct vectors * vecs, int need_do_map)
 		 * reload the map
 		 */
 		mpp->action = ACT_RELOAD;
-		if (domap(mpp, params, 1) <= 0) {
+		if (domap(mpp, params, 1) == DOMAP_FAIL) {
 			condlog(0, "%s: failed in domap for "
 				"removal of path %s",
 				mpp->alias, pp->dev);
-- 
2.19.2

[PATCH v3 23/24] multipathd: simplify retry logic in ev_add_path()

[Martin Wilck]

Move the retry loop for DOMAP_RETRY out of the redundant outer
if conditional, and replace the goto construct with a while loop.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipathd/main.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index fd3459f7..99145293 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -993,15 +993,14 @@ rescan:
 	/*
 	 * reload the map for the multipath mapped device
 	 */
-retry:
 	ret = domap(mpp, params, 1);
+	while (ret == DOMAP_RETRY && retries-- > 0) {
+		condlog(0, "%s: retry domap for addition of new "
+			"path %s", mpp->alias, pp->dev);
+		sleep(1);
+		ret = domap(mpp, params, 1);
+	}
 	if (ret == DOMAP_FAIL || ret == DOMAP_RETRY) {
-		if (ret == DOMAP_RETRY && retries-- > 0) {
-			condlog(0, "%s: retry domap for addition of new "
-				"path %s", mpp->alias, pp->dev);
-			sleep(1);
-			goto retry;
-		}
 		condlog(0, "%s: failed in domap for addition of new "
 			"path %s", mpp->alias, pp->dev);
 		/*
-- 
2.19.2

[PATCH v3 24/24] multipath: use symbolic return value and exit code

[Martin Wilck]

Use an enum to represent the return code and exit status of
multipath and its most important subroutine, configure().
This clarifies the confusing use of booleans and status
codes a bit, hopefully.

This patch doesn't introduce a change in behavior.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 multipath/main.c | 120 ++++++++++++++++++++++++++---------------------
 1 file changed, 66 insertions(+), 54 deletions(-)

diff --git a/multipath/main.c b/multipath/main.c
index 1d34b87b..f40c179b 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -68,6 +68,19 @@ int logsink;
 struct udev *udev;
 struct config *multipath_conf;
 
+/*
+ * Return values of configure(), print_cmd_valid(), and main().
+ * RTVL_{YES,NO} are synonyms for RTVL_{OK,FAIL} for the CMD_VALID_PATH case.
+ */
+enum {
+	RTVL_OK = 0,
+	RTVL_YES = RTVL_OK,
+	RTVL_FAIL = 1,
+	RTVL_NO = RTVL_FAIL,
+	RTVL_MAYBE, /* only used internally, never returned */
+	RTVL_RETRY, /* returned by configure(), not by main() */
+};
+
 struct config *get_multipath_config(void)
 {
 	return multipath_conf;
@@ -475,15 +488,14 @@ retry:
 static int print_cmd_valid(int k, const vector pathvec,
 			   struct config *conf)
 {
-	static const int vals[] = { 1, 0, 2 };
 	int wait = FIND_MULTIPATHS_NEVER;
 	struct timespec until;
 	struct path *pp;
 
-	if (k < 0 || k >= (sizeof(vals) / sizeof(int)))
-		return 1;
+	if (k != RTVL_YES && k != RTVL_NO && k != RTVL_MAYBE)
+		return RTVL_NO;
 
-	if (k == 2) {
+	if (k == RTVL_MAYBE) {
 		/*
 		 * Caller ensures that pathvec[0] is the path to
 		 * examine.
@@ -493,7 +505,7 @@ static int print_cmd_valid(int k, const vector pathvec,
 		wait = find_multipaths_check_timeout(
 			pp, pp->find_multipaths_timeout, &until);
 		if (wait != FIND_MULTIPATHS_WAITING)
-			k = 1;
+			k = RTVL_NO;
 	} else if (pathvec != NULL && (pp = VECTOR_SLOT(pathvec, 0)))
 		wait = find_multipaths_check_timeout(pp, 0, &until);
 	if (wait == FIND_MULTIPATHS_WAITING)
@@ -501,8 +513,10 @@ static int print_cmd_valid(int k, const vector pathvec,
 			       until.tv_sec, until.tv_nsec/1000);
 	else if (wait == FIND_MULTIPATHS_WAIT_DONE)
 		printf("FIND_MULTIPATHS_WAIT_UNTIL=\"0\"\n");
-	printf("DM_MULTIPATH_DEVICE_PATH=\"%d\"\n", vals[k]);
-	return k == 1;
+	printf("DM_MULTIPATH_DEVICE_PATH=\"%d\"\n",
+	       k == RTVL_MAYBE ? 2 : k == RTVL_YES ? 1 : 0);
+	/* Never return RTVL_MAYBE */
+	return k == RTVL_NO ? RTVL_NO : RTVL_YES;
 }
 
 /*
@@ -524,12 +538,6 @@ static bool released_to_systemd(void)
 	return ret;
 }
 
-/*
- * Return value:
- *  -1: Retry
- *   0: Success
- *   1: Failure
- */
 static int
 configure (struct config *conf, enum mpath_cmds cmd,
 	   enum devtypes dev_type, char *devpath)
@@ -537,7 +545,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	vector curmp = NULL;
 	vector pathvec = NULL;
 	struct vectors vecs;
-	int r = 1, rc;
+	int r = RTVL_FAIL, rc;
 	int di_flag = 0;
 	char * refwwid = NULL;
 	char * dev = NULL;
@@ -585,21 +593,23 @@ configure (struct config *conf, enum mpath_cmds cmd,
 			goto out;
 		}
 		if (cmd == CMD_REMOVE_WWID) {
-			r = remove_wwid(refwwid);
-			if (r == 0)
+			rc = remove_wwid(refwwid);
+			if (rc == 0) {
 				printf("wwid '%s' removed\n", refwwid);
-			else if (r == 1) {
+				r = RTVL_OK;
+			} else if (rc == 1) {
 				printf("wwid '%s' not in wwids file\n",
 					refwwid);
-				r = 0;
+				r = RTVL_OK;
 			}
 			goto out;
 		}
 		if (cmd == CMD_ADD_WWID) {
-			r = remember_wwid(refwwid);
-			if (r >= 0)
+			rc = remember_wwid(refwwid);
+			if (rc >= 0) {
 				printf("wwid '%s' added\n", refwwid);
-			else
+				r = RTVL_OK;
+			} else
 				printf("failed adding '%s' to wwids file\n",
 				       refwwid);
 			goto out;
@@ -614,13 +624,13 @@ configure (struct config *conf, enum mpath_cmds cmd,
 		 */
 		if (cmd == CMD_VALID_PATH) {
 			if (is_failed_wwid(refwwid) == WWID_IS_FAILED) {
-				r = 1;
+				r = RTVL_NO;
 				goto print_valid;
 			}
 			if ((!find_multipaths_on(conf) &&
 				    ignore_wwids_on(conf)) ||
 				   check_wwids_file(refwwid, 0) == 0)
-				r = 0;
+				r = RTVL_YES;
 			if (!ignore_wwids_on(conf))
 				goto print_valid;
 			/* At this point, either r==0 or find_multipaths_on. */
@@ -630,7 +640,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
 			 * Quick check if path is already multipathed.
 			 */
 			if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0))) {
-				r = 0;
+				r = RTVL_YES;
 				goto print_valid;
 			}
 
@@ -644,10 +654,10 @@ configure (struct config *conf, enum mpath_cmds cmd,
 			 * Leave DM_MULTIPATH_DEVICE_PATH="0".
 			 */
 			if (released) {
-				r = 1;
+				r = RTVL_NO;
 				goto print_valid;
 			}
-			if (r == 0)
+			if (r == RTVL_YES)
 				goto print_valid;
 			/* find_multipaths_on: Fall through to path detection */
 		}
@@ -703,13 +713,12 @@ configure (struct config *conf, enum mpath_cmds cmd,
 		 * the refwwid, or there is more than one path matching
 		 * the refwwid, then the path is valid */
 		if (VECTOR_SIZE(curmp) != 0) {
-			r = 0;
+			r = RTVL_YES;
 			goto print_valid;
 		} else if (VECTOR_SIZE(pathvec) > 1)
-			r = 0;
+			r = RTVL_YES;
 		else
-			/* Use r=2 as an indication for "maybe" */
-			r = 2;
+			r = RTVL_MAYBE;
 
 		/*
 		 * If opening the path with O_EXCL fails, the path
@@ -739,13 +748,14 @@ configure (struct config *conf, enum mpath_cmds cmd,
 			/*
 			 * Check if we raced with multipathd
 			 */
-			r = !sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0));
+			r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0)) ?
+				RTVL_YES : RTVL_NO;
 		}
 		goto print_valid;
 	}
 
 	if (cmd != CMD_CREATE && cmd != CMD_DRY_RUN) {
-		r = 0;
+		r = RTVL_OK;
 		goto out;
 	}
 
@@ -754,7 +764,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
 	 */
 	rc = coalesce_paths(&vecs, NULL, refwwid,
 			   conf->force_reload, cmd);
-	r = rc == CP_RETRY ? -1 : rc == CP_OK ? 0 : 1;
+	r = rc == CP_RETRY ? RTVL_RETRY : rc == CP_OK ? RTVL_OK : RTVL_FAIL;
 
 print_valid:
 	if (cmd == CMD_VALID_PATH)
@@ -855,7 +865,7 @@ main (int argc, char *argv[])
 	int arg;
 	extern char *optarg;
 	extern int optind;
-	int r = 1;
+	int r = RTVL_FAIL;
 	enum mpath_cmds cmd = CMD_CREATE;
 	enum devtypes dev_type = DEV_NONE;
 	char *dev = NULL;
@@ -866,7 +876,7 @@ main (int argc, char *argv[])
 	logsink = 0;
 	conf = load_config(DEFAULT_CONFIGFILE);
 	if (!conf)
-		exit(1);
+		exit(RTVL_FAIL);
 	multipath_conf = conf;
 	conf->retrigger_tries = 0;
 	while ((arg = getopt(argc, argv, ":adcChl::FfM:v:p:b:BrR:itTquUwW")) != EOF ) {
@@ -877,7 +887,7 @@ main (int argc, char *argv[])
 			if (sizeof(optarg) > sizeof(char *) ||
 			    !isdigit(optarg[0])) {
 				usage (argv[0]);
-				exit(1);
+				exit(RTVL_FAIL);
 			}
 
 			conf->verbosity = atoi(optarg);
@@ -924,7 +934,7 @@ main (int argc, char *argv[])
 			if (conf->pgpolicy_flag == IOPOLICY_UNDEF) {
 				printf("'%s' is not a valid policy\n", optarg);
 				usage(argv[0]);
-				exit(1);
+				exit(RTVL_FAIL);
 			}
 			break;
 		case 'r':
@@ -934,14 +944,14 @@ main (int argc, char *argv[])
 			conf->find_multipaths |= _FIND_MULTIPATHS_I;
 			break;
 		case 't':
-			r = dump_config(conf, NULL, NULL);
+			r = dump_config(conf, NULL, NULL) ? RTVL_FAIL : RTVL_OK;
 			goto out_free_config;
 		case 'T':
 			cmd = CMD_DUMP_CONFIG;
 			break;
 		case 'h':
 			usage(argv[0]);
-			exit(0);
+			exit(RTVL_OK);
 		case 'u':
 			cmd = CMD_VALID_PATH;
 			dev_type = DEV_UEVENT;
@@ -965,20 +975,20 @@ main (int argc, char *argv[])
 		case ':':
 			fprintf(stderr, "Missing option argument\n");
 			usage(argv[0]);
-			exit(1);
+			exit(RTVL_FAIL);
 		case '?':
 			fprintf(stderr, "Unknown switch: %s\n", optarg);
 			usage(argv[0]);
-			exit(1);
+			exit(RTVL_FAIL);
 		default:
 			usage(argv[0]);
-			exit(1);
+			exit(RTVL_FAIL);
 		}
 	}
 
 	if (getuid() != 0) {
 		fprintf(stderr, "need to be root\n");
-		exit(1);
+		exit(RTVL_FAIL);
 	}
 
 	if (optind < argc) {
@@ -1016,7 +1026,8 @@ main (int argc, char *argv[])
 	/* Failing here is non-fatal */
 	init_foreign(conf->multipath_dir);
 	if (cmd == CMD_USABLE_PATHS) {
-		r = check_usable_paths(conf, dev, dev_type);
+		r = check_usable_paths(conf, dev, dev_type) ?
+			RTVL_FAIL : RTVL_OK;
 		goto out;
 	}
 	if (cmd == CMD_VALID_PATH &&
@@ -1032,7 +1043,7 @@ main (int argc, char *argv[])
 		if (fd == -1) {
 			condlog(3, "%s: daemon is not running", dev);
 			if (!systemd_service_enabled(dev)) {
-				r = print_cmd_valid(1, NULL, conf);
+				r = print_cmd_valid(RTVL_NO, NULL, conf);
 				goto out;
 			}
 		} else
@@ -1046,9 +1057,9 @@ main (int argc, char *argv[])
 
 	switch(delegate_to_multipathd(cmd, dev, dev_type, conf)) {
 	case DELEGATE_OK:
-		exit(0);
+		exit(RTVL_OK);
 	case DELEGATE_ERROR:
-		exit(1);
+		exit(RTVL_FAIL);
 	case NOT_DELEGATED:
 		break;
 	}
@@ -1064,8 +1075,8 @@ main (int argc, char *argv[])
 			goto out;
 		}
 		if (dm_get_maps(curmp) == 0)
-			r = replace_wwids(curmp);
-		if (r == 0)
+			r = replace_wwids(curmp) ? RTVL_FAIL : RTVL_OK;
+		if (r == RTVL_OK)
 			printf("successfully reset wwids\n");
 		vector_foreach_slot_backwards(curmp, mpp, i) {
 			vector_del_slot(curmp, i);
@@ -1078,17 +1089,18 @@ main (int argc, char *argv[])
 		retries = conf->remove_retries;
 	if (conf->remove == FLUSH_ONE) {
 		if (dev_type == DEV_DEVMAP) {
-			r = dm_suspend_and_flush_map(dev, retries);
+			r = dm_suspend_and_flush_map(dev, retries) ?
+				RTVL_FAIL : RTVL_OK;
 		} else
 			condlog(0, "must provide a map name to remove");
 
 		goto out;
 	}
 	else if (conf->remove == FLUSH_ALL) {
-		r = dm_flush_maps(retries);
+		r = dm_flush_maps(retries) ? RTVL_FAIL : RTVL_OK;
 		goto out;
 	}
-	while ((r = configure(conf, cmd, dev_type, dev)) < 0)
+	while ((r = configure(conf, cmd, dev_type, dev)) == RTVL_RETRY)
 		condlog(3, "restart multipath configuration process");
 
 out:
@@ -1103,8 +1115,8 @@ out:
 	 * multipath -u must exit with status 0, otherwise udev won't
 	 * import its output.
 	 */
-	if (cmd == CMD_VALID_PATH && dev_type == DEV_UEVENT && r == 1)
-		r = 0;
+	if (cmd == CMD_VALID_PATH && dev_type == DEV_UEVENT && r == RTVL_NO)
+		r = RTVL_OK;
 
 	if (dev_type == DEV_UEVENT)
 		closelog();
-- 
2.19.2

Re: [PATCH v3 00/24] multipath-tools: improve logging at -v3

[Benjamin Marzinski]

On Mon, Dec 10, 2018 at 10:49:35AM +0100, Martin Wilck wrote:
> Changes in v3:
> 
> Resent full series on Christophe's request. All patches except 22/24, 23/24 are
> the same as before. Added Ben's "Reviewed-by:" where appropriate.

Belated ACKs on 23/24 and v4 of 22/24

-Ben

> 
>  - 22/24: added one fix for a non-symbolic reference to a domap() return value,
> which I'd overlooked before.
>  - 23/24: removed on Ben's review. The new 23/24 is just a code cleanup
> without functional change.
> 
> Changes in v2:
> 
> The first 19 patches are identical to v1 as ACK'd by Ben, except 16/24
> "libmultipath: coalesce_paths: fix size mismatch handling".
> No. 8/24 "libmultipath: decrease log level of word splitting"
> (not yet ACKd by Ben) also stays the same; the issue Ben raised
> in his review is addressed in a separate patch, 20/24.
> 21/24 addresses implements Ben's suggestion to use named constants
> as return values in coalesce_paths(). 22, 23, 24 do the same for
> other important, related functions, as I found it strange to make
> this change just for coalesce_paths() alone.  
> 
> Martin Wilck (24):
>   tests/hwtable: set multipath_dir in local configuration
>   tests/hwtable: adjust to new checker API
>   multipath-tools: decrease verbosity of state messages
>   libmultipath: decrease verbosity of pathinfo messages
>   libmultipath: decrease verbosity of TUR checker messages
>   libmultipath: avoid frequent messages from filter_property()
>   libmultipath: decrease log level of "disassembled" messages
>   libmultipath: decrease log level of word splitting
>   libmultipath: increase log level of map removal
>   multipathd: decrease log level of checker timing
>   libmultipath: decrease log level of "prioritizer refcount" message
>   libmpathpersist/update_map_pr: decrease log level for nop
>   libmultipath: simplify devt2devname()
>   libmultipath: decrease log level for failed VPD c9
>   libmultipath: adopt_paths: check for size match
>   libmultipath: coalesce_paths: fix size mismatch handling
>   tests: add unit tests for bitmask functions
>   multipathd: uev_remove_path: remove redundant orphan_paths call
>   libmultipath: improve logging from orphan_paths
>   libmultipath: avoid syslog loglevel > LOG_DEBUG
>   coalesce_paths(): use symbolic return value
>   domap(): use symbolic return value
>   multipathd: simplify retry logic in ev_add_path()
>   multipath: use symbolic return value and exit code
> 
>  libmpathpersist/mpath_persist.c |   3 +-
>  libmultipath/blacklist.c        |  54 +++++++-------
>  libmultipath/blacklist.h        |   2 +-
>  libmultipath/checkers/tur.c     |   6 +-
>  libmultipath/configure.c        |  48 +++++++------
>  libmultipath/configure.h        |  23 ++++++
>  libmultipath/discovery.c        |  20 +++---
>  libmultipath/dmparser.c         |   6 +-
>  libmultipath/log_pthread.c      |   3 +
>  libmultipath/prio.c             |   2 +-
>  libmultipath/structs_vec.c      |  18 +++--
>  libmultipath/structs_vec.h      |   3 +-
>  libmultipath/util.c             |   7 +-
>  libmultipath/util.h             |  16 +++++
>  multipath/main.c                | 121 ++++++++++++++++++--------------
>  multipathd/cli_handlers.c       |   5 +-
>  multipathd/main.c               |  45 ++++++------
>  tests/Makefile                  |   7 +-
>  tests/blacklist.c               |   7 +-
>  tests/hwtable.c                 |  89 ++++++++++++-----------
>  tests/util.c                    |  98 ++++++++++++++++++++++++++
>  21 files changed, 383 insertions(+), 200 deletions(-)
> 
> -- 
> 2.19.2