[PATCH 0/5] misc multipath patches

[PATCH 0/5] misc multipath patches

[Benjamin Marzinski]

This is another collection of miscellaneous patches. The first one is
just a resend. The second is a change to how the rados checker is enable
that makes it easier to build packages for multiple architectures (since
librados isn't supported on all architectures yet). If people have
objections, I can continue to use the existing build option. The third
is a change that's necessary to get warning messages for bad config
files.  Another option would be to just change all the config warning
messages to the highest priority. The last two are bugfixes.

Benjamin Marzinski (5):
  libmultipath: add comment about resuming
  libmultipath: change how RADOS checker is enabled
  multipath: set verbosity to default during config
  mpath: skip device configs without vendor/product
  multipathd: fix "show maps json" crash

 Makefile.inc                   | 13 +++++++++++++
 libmultipath/checkers/Makefile |  3 ++-
 libmultipath/config.c          |  7 +++++++
 libmultipath/debug.c           |  3 ++-
 libmultipath/devmapper.c       |  3 +++
 multipathd/cli_handlers.c      |  6 ++++--
 6 files changed, 31 insertions(+), 4 deletions(-)

-- 
2.7.4

[PATCH 1/5] libmultipath: add comment about resuming

[Benjamin Marzinski]

The reason for the second resume in my commit "libmultipath: fix
suspended devs from failed reloads" is not obvious from the multipath
code, so add a comment.

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

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 69b634b..ee83e0f 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -399,6 +399,9 @@ int dm_addmap_reload(struct multipath *mpp, char *params, int flush)
 	if (r)
 		return r;
 
+	/* If the resume failed, dm will leave the device suspended, and
+ 	 * drop the new table, so doing a second resume will try using
+ 	 * the original table */
 	if (dm_is_suspended(mpp->alias))
 		dm_simplecmd(DM_DEVICE_RESUME, mpp->alias, !flush, 1,
 			     udev_flags, 0);
-- 
2.7.4

[PATCH 2/5] libmultipath: change how RADOS checker is enabled

[Benjamin Marzinski]

Instead of making the user call "make", "make install" and "make clean"
with ENABLE_RADOS set correctly, have the makefile check if
/usr/include/rados/librados.h exists, just like it checks if specific
functions exist in a file.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile.inc                   | 13 +++++++++++++
 libmultipath/checkers/Makefile |  3 ++-
 2 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/Makefile.inc b/Makefile.inc
index 1815f9a..ec41dbf 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -109,5 +109,18 @@ check_func =								       \
 	echo "$$found"							       \
     )
 
+# Checker whether a file with name $1 exists
+check_file = $(shell	\
+	if [ -f "$1" ]; then \
+		found=1; \
+		status="yes"; \
+	else \
+		found=0; \
+		status="no"; \
+	fi; \
+	echo 1>&2 "Checking if $1 exists ... $$status"; \
+	echo "$$found" \
+	)
+
 %.o:	%.c
 	$(CC) $(CFLAGS) -c -o $@ $<
diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile
index 732ca9d..bce6b8b 100644
--- a/libmultipath/checkers/Makefile
+++ b/libmultipath/checkers/Makefile
@@ -14,7 +14,8 @@ LIBS= \
 	libcheckemc_clariion.so \
 	libcheckhp_sw.so \
 	libcheckrdac.so
-ifneq ($(ENABLE_RADOS),0)
+
+ifneq ($(call check_file,/usr/include/rados/librados.h),0)
 LIBS += libcheckrbd.so
 endif
 
-- 
2.7.4

[PATCH 3/5] multipath: set verbosity to default during config

[Benjamin Marzinski]

condlog was setting the verbosity to 0 if there was no configuration.
This keeps multipath from printing warning messages about config file
problems that are found while loading the configuration. Instead, it
should use the default config level until it loads the configuration
to find the current value.

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

diff --git a/libmultipath/debug.c b/libmultipath/debug.c
index fbe171a..f89b264 100644
--- a/libmultipath/debug.c
+++ b/libmultipath/debug.c
@@ -11,6 +11,7 @@
 #include "../third-party/valgrind/drd.h"
 #include "vector.h"
 #include "config.h"
+#include "defaults.h"
 
 void dlog (int sink, int prio, const char * fmt, ...)
 {
@@ -21,7 +22,7 @@ void dlog (int sink, int prio, const char * fmt, ...)
 	va_start(ap, fmt);
 	conf = get_multipath_config();
 	ANNOTATE_IGNORE_READS_BEGIN();
-	thres = (conf) ? conf->verbosity : 0;
+	thres = (conf) ? conf->verbosity : DEFAULT_VERBOSITY;
 	ANNOTATE_IGNORE_READS_END();
 	put_multipath_config(conf);
 
-- 
2.7.4

[PATCH 4/5] mpath: skip device configs without vendor/product

[Benjamin Marzinski]

Right now if multipath.conf includes a device configuration without a
vendor or product string, it will automatically be applied to all
devices, skipping all other configs entirely.  This is clearly wrong.
This patch makes sure that user added configs include vendor and
product strings

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

diff --git a/libmultipath/config.c b/libmultipath/config.c
index bb6619b..6b23601 100644
--- a/libmultipath/config.c
+++ b/libmultipath/config.c
@@ -451,6 +451,13 @@ restart:
 			break;
 		j = n;
 		vector_foreach_slot_after(hw, hwe2, j) {
+			/* drop invalid device configs */
+			if (!hwe2->vendor || !hwe2->product) {
+				condlog(0, "device config missing vendor or product parameter");
+				vector_del_slot(hw, j--);
+				free_hwe(hwe2);
+				continue;
+			}
 			if (hwe_regmatch(hwe1, hwe2))
 				continue;
 			/* dup */
-- 
2.7.4

[PATCH 5/5] multipathd: fix "show maps json" crash

[Benjamin Marzinski]

If there are no multipath devices, show_maps_json sets the maximum size
of the reply buffer to 0. Having a size of 0 causes the calls to calloc
and realloc to behave in ways that the code isn't designed to handle,
leading to a double-free crash. Instead, show_maps_json should just
use the INITIAL_REPLY_LEN if there are no multipath devices.

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

diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index 04c7386..7b0d00c 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -162,10 +162,12 @@ show_maps_json (char ** r, int * len, struct vectors * vecs)
 	struct multipath * mpp;
 	char * c;
 	char * reply;
-	unsigned int maxlen = INITIAL_REPLY_LEN *
-			PRINT_JSON_MULTIPLIER * VECTOR_SIZE(vecs->mpvec);
+	unsigned int maxlen = INITIAL_REPLY_LEN;
 	int again = 1;
 
+	if (VECTOR_SIZE(vecs->mpvec) > 0)
+		maxlen *= PRINT_JSON_MULTIPLIER * VECTOR_SIZE(vecs->mpvec);
+
 	vector_foreach_slot(vecs->mpvec, mpp, i) {
 		if (update_multipath(vecs, mpp->alias, 0)) {
 			return 1;
-- 
2.7.4

Re: [PATCH 0/5] misc multipath patches

[Martin Wilck]

On Thu, 2017-06-01 at 22:09 -0500, Benjamin Marzinski wrote:
> This is another collection of miscellaneous patches. The first one is
> just a resend. The second is a change to how the rados checker is
> enable
> that makes it easier to build packages for multiple architectures
> (since
> librados isn't supported on all architectures yet). If people have
> objections, I can continue to use the existing build option.

Well, we're getting close to having to write configure.ac,
or something of the kind :-)

But it's fine with me. ACK from me for this series.

Martin

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

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

Re: [PATCH 0/5] misc multipath patches

[Christophe Varoqui]

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

The set is merged.
Thanks.

On Tue, Jun 6, 2017 at 10:40 PM, Martin Wilck <mwilck@suse.com> wrote:

> On Thu, 2017-06-01 at 22:09 -0500, Benjamin Marzinski wrote:
> > This is another collection of miscellaneous patches. The first one is
> > just a resend. The second is a change to how the rados checker is
> > enable
> > that makes it easier to build packages for multiple architectures
> > (since
> > librados isn't supported on all architectures yet). If people have
> > objections, I can continue to use the existing build option.
>
> Well, we're getting close to having to write configure.ac,
> or something of the kind :-)
>
> But it's fine with me. ACK from me for this series.
>
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton
> HRB 21284 (AG Nürnberg)
>
>

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

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