[PATCH 0/6] multipath cleanup patches

[PATCH 0/6] multipath cleanup patches

[Benjamin Marzinski]

The first four of these patches are minor fixes from Martin Wilck's
review of my recently merged patchset. The fifth patch is a license
clarification for libmpathcmd, and the sixth is is minor bugfix for
multipathd config value printing

Benjamin Marzinski (6):
  multipathd: remove incorrect pthread_testcancel
  multipath: add comments
  multipathd: minor dmevents polling code cleanups
  multipathd: remove unneeded function parameter
  mpathcmd: fix libmpathcmd license
  libmultipath: don't print undefined values

 libmpathcmd/mpath_cmd.c | 19 +++++++++++++
 libmpathcmd/mpath_cmd.h |  2 +-
 libmultipath/dict.c     |  8 ++++++
 multipathd/dmevents.c   | 10 ++++---
 multipathd/main.c       | 15 +++++------
 multipathd/waiter.c     |  1 -
 tests/dmevents.c        | 72 ++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/util.c            |  2 ++
 8 files changed, 114 insertions(+), 15 deletions(-)

-- 
2.7.4

[PATCH 1/6] multipathd: remove incorrect pthread_testcancel

[Benjamin Marzinski]

As Martin Wilck pointed out, a thread that's trying to stop the waiter
thread should not cancel itself before it gets a chance to do so

Cc: Martin Wilck <mwilck@suse.com>
Fixes: c7625f92 "multipathd: fix waiter thread cancelling"
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/waiter.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/multipathd/waiter.c b/multipathd/waiter.c
index 595c69a..0ef8b25 100644
--- a/multipathd/waiter.c
+++ b/multipathd/waiter.c
@@ -68,7 +68,6 @@ void stop_waiter_thread (struct multipath *mpp, struct vectors *vecs)
 	mpp->waiter = (pthread_t)0;
 	pthread_cleanup_push(cleanup_lock, &waiter_lock);
 	lock(&waiter_lock);
-	pthread_testcancel();
 	pthread_kill(thread, SIGUSR2);
 	pthread_cancel(thread);
 	lock_cleanup_pop(&waiter_lock);
-- 
2.7.4

[PATCH 2/6] multipath: add comments

[Benjamin Marzinski]

This commit simply adds a number of comments based on suggestions by
Martin Wilck.

Cc: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/dmevents.c |  4 ++-
 tests/dmevents.c      | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 tests/util.c          |  2 ++
 3 files changed, 76 insertions(+), 2 deletions(-)

diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
index 2281a10..0b0d0ce 100644
--- a/multipathd/dmevents.c
+++ b/multipathd/dmevents.c
@@ -122,6 +122,8 @@ static int arm_dm_event_poll(int fd)
 	dmi.version[0] = DM_VERSION_MAJOR;
 	dmi.version[1] = DM_VERSION_MINOR;
 	dmi.version[2] = DM_VERSION_PATCHLEVEL;
+	/* This flag currently does nothing. It simply exists to
+	 * duplicate the behavior of libdevmapper */
 	dmi.flags = 0x4;
 	dmi.data_start = offsetof(struct dm_ioctl, data);
 	dmi.data_size = sizeof(dmi);
@@ -189,7 +191,7 @@ fail:
 	return -1;
 }
 
-/* You must call update_multipath() after calling this function, to
+/* You must call __setup_multipath() after calling this function, to
  * deal with any events that came in before the device was added */
 int watch_dmevents(char *name)
 {
diff --git a/tests/dmevents.c b/tests/dmevents.c
index 4442fc2..bba51dc 100644
--- a/tests/dmevents.c
+++ b/tests/dmevents.c
@@ -33,10 +33,13 @@
 /* I have to do this to get at the static variables */
 #include "../multipathd/dmevents.c"
 
+/* pretend dm device */
 struct dm_device {
 	char name[WWID_SIZE];
+	/* is this a mpath device, or other dm device */
 	int is_mpath;
 	uint32_t evt_nr;
+	/* tracks the event number when the multipath device was updated */
 	uint32_t update_nr;
 };
 
@@ -48,6 +51,9 @@ struct test_data {
 
 struct test_data data;
 
+/* Add a pretend dm device, or update its event number. This is used to build
+ * up the dm devices that the dmevents code queries with dm_task_get_names,
+ * dm_geteventnr, and dm_is_mpath */
 int add_dm_device_event(char *name, int is_mpath, uint32_t evt_nr)
 {
 	struct dm_device *dev;
@@ -77,6 +83,7 @@ int add_dm_device_event(char *name, int is_mpath, uint32_t evt_nr)
 	return 0;
 }
 
+/* helper function for pretend dm devices */
 struct dm_device *find_dm_device(const char *name)
 {
 	struct dm_device *dev;
@@ -88,6 +95,7 @@ struct dm_device *find_dm_device(const char *name)
 	return NULL;
 }
 
+/* helper function for pretend dm devices */
 int remove_dm_device_event(const char *name)
 {
 	struct dm_device *dev;
@@ -103,6 +111,7 @@ int remove_dm_device_event(const char *name)
 	return -1;
 }
 
+/* helper function for pretend dm devices */
 void remove_all_dm_device_events(void)
 {
 	struct dm_device *dev;
@@ -122,7 +131,9 @@ static inline void *align_ptr(void *ptr)
 	return (void *)align_val((size_t)ptr);
 }
 
-/* copied off of list_devices in dm-ioctl.c */
+/* copied off of list_devices in dm-ioctl.c except that it uses
+ * the pretend dm devices, and saves the output to the test_data
+ * structure */
 struct dm_names *build_dm_names(void)
 {
 	struct dm_names *names, *np, *old_np = NULL;
@@ -199,12 +210,16 @@ int __wrap_open(const char *pathname, int flags)
 	return mock_type(int);
 }
 
+/* We never check the result of the close(), so there's no need to
+ * to mock a return value */
 int __wrap_close(int fd)
 {
 	assert_int_equal(fd, waiter->fd);
 	return 0;
 }
 
+/* the pretend dm device code checks the input and supplies the
+ * return value, so there's no need to do that here */
 int __wrap_dm_is_mpath(const char *name)
 {
 	struct dm_device *dev;
@@ -216,6 +231,8 @@ int __wrap_dm_is_mpath(const char *name)
 	return 0;
 }
 
+/* either get return info from the pretend dm device, or
+ * override it to test -1 return */
 int __wrap_dm_geteventnr(const char *name)
 {
 	struct dm_device *dev;
@@ -257,6 +274,8 @@ int __wrap_dm_task_run(struct dm_task *dmt)
 	return mock_type(int);
 }
 
+/* either get return info from the pretend dm device, or
+ * override it to test NULL return */
 struct dm_names * __wrap_dm_task_get_names(struct dm_task *dmt)
 {
 	int good = mock_type(int);
@@ -299,6 +318,9 @@ void __wrap_remove_map_by_alias(const char *alias, struct vectors * vecs,
 	assert_int_equal(purge_vec, 1);
 }
 
+/* pretend update the pretend dm devices. If fail is set, it
+ * simulates having the dm device removed. Otherwise it just sets
+ * update_nr to record when the update happened */
 int __wrap_update_multipath(struct vectors *vecs, char *mapname, int reset)
 {
 	int fail;
@@ -325,6 +347,9 @@ int __wrap_update_multipath(struct vectors *vecs, char *mapname, int reset)
 	return fail;
 }
 
+/* helper function used to check if the dmevents list of devices
+ * includes a specific device. To make sure that dmevents is
+ * in the correct state after running a function */
 struct dev_event *find_dmevents(const char *name)
 {
 	struct dev_event *dev_evt;
@@ -336,14 +361,19 @@ struct dev_event *find_dmevents(const char *name)
 	return NULL;
 }
 
+/* null vecs pointer when initialized dmevents */
 static void test_init_waiter_bad0(void **state)
 {
+	/* this boilerplate code just skips the test if
+	 * dmevents polling is not supported */
 	struct test_data *datap = (struct test_data *)(*state);
 	if (datap == NULL)
 		skip();
+
 	assert_int_equal(init_dmevent_waiter(NULL), -1);
 }
 
+/* fail to open /dev/mapper/control */
 static void test_init_waiter_bad1(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -354,6 +384,7 @@ static void test_init_waiter_bad1(void **state)
 	assert_ptr_equal(waiter, NULL);
 }
 
+/* waiter remains initialized after this test */
 static void test_init_waiter_good0(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -364,6 +395,7 @@ static void test_init_waiter_good0(void **state)
 	assert_ptr_not_equal(waiter, NULL);
 }
 
+/* No dm device named foo */
 static void test_watch_dmevents_bad0(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -373,6 +405,7 @@ static void test_watch_dmevents_bad0(void **state)
 	assert_ptr_equal(find_dmevents("foo"), NULL);
 }
 
+/* foo is not a multipath device */
 static void test_watch_dmevents_bad1(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -384,6 +417,7 @@ static void test_watch_dmevents_bad1(void **state)
 	assert_ptr_equal(find_dmevents("foo"), NULL);
 }
 
+/* failed getting the dmevent number */
 static void test_watch_dmevents_bad2(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -396,6 +430,8 @@ static void test_watch_dmevents_bad2(void **state)
 	assert_int_equal(watch_dmevents("foo"), -1);
 	assert_ptr_equal(find_dmevents("foo"), NULL);
 }
+
+/* verify that you can watch and unwatch dm multipath device "foo" */
 static void test_watch_dmevents_good0(void **state)
 {
 	struct dev_event *dev_evt;
@@ -407,16 +443,20 @@ static void test_watch_dmevents_good0(void **state)
 	assert_int_equal(add_dm_device_event("foo", 1, 5), 0);
 	will_return(__wrap_dm_geteventnr, 0);
 	assert_int_equal(watch_dmevents("foo"), 0);
+	/* verify foo is being watched */
 	dev_evt = find_dmevents("foo");
 	assert_ptr_not_equal(dev_evt, NULL);
 	assert_int_equal(dev_evt->evt_nr, 5);
 	assert_int_equal(dev_evt->action, EVENT_NOTHING);
 	assert_int_equal(VECTOR_SIZE(waiter->events), 1);
 	unwatch_dmevents("foo");
+	/* verify foo is no longer being watched */
 	assert_int_equal(VECTOR_SIZE(waiter->events), 0);
 	assert_ptr_equal(find_dmevents("foo"), NULL);
 }
 
+/* verify that if you try to watch foo multiple times, it only
+ * is placed on the waiter list once */
 static void test_watch_dmevents_good1(void **state)
 {
 	struct dev_event *dev_evt;
@@ -445,6 +485,7 @@ static void test_watch_dmevents_good1(void **state)
 	assert_ptr_equal(find_dmevents("foo"), NULL);
 }
 
+/* watch and then unwatch multiple devices */
 static void test_watch_dmevents_good2(void **state)
 {
 	struct dev_event *dev_evt;
@@ -480,6 +521,7 @@ static void test_watch_dmevents_good2(void **state)
 	assert_ptr_equal(find_dmevents("bar"), NULL);
 }
 
+/* dm_task_create fails */
 static void test_get_events_bad0(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -493,6 +535,7 @@ static void test_get_events_bad0(void **state)
 	assert_int_equal(dm_get_events(), -1);
 }
 
+/* dm_task_run fails */
 static void test_get_events_bad1(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -505,6 +548,7 @@ static void test_get_events_bad1(void **state)
 	assert_int_equal(dm_get_events(), -1);
 }
 
+/* dm_task_get_names fails */
 static void test_get_events_bad2(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -518,6 +562,7 @@ static void test_get_events_bad2(void **state)
 	assert_int_equal(dm_get_events(), -1);
 }
 
+/* If the device isn't being watched, dm_get_events returns NULL */
 static void test_get_events_good0(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -534,6 +579,11 @@ static void test_get_events_good0(void **state)
 	assert_int_equal(VECTOR_SIZE(waiter->events), 0);
 }
 
+/* There are 5 dm devices. 4 of them are multipath devices.
+ * Only 3 of them are being watched. "foo" has a new event
+ * "xyzzy" gets removed. Nothing happens to bar. Verify
+ * that all the events are properly set, and that nothing
+ * happens with the two devices that aren't being watched */
 static void test_get_events_good1(void **state)
 {
 	struct dev_event *dev_evt;
@@ -577,6 +627,8 @@ static void test_get_events_good1(void **state)
 	assert_int_equal(VECTOR_SIZE(waiter->events), 3);
 }
 
+/* poll does not return an event. nothing happens. The
+ * devices remain after this test */
 static void test_dmevent_loop_bad0(void **state)
 {
 	struct dm_device *dev;
@@ -603,6 +655,7 @@ static void test_dmevent_loop_bad0(void **state)
 	assert_int_equal(dev->update_nr, 5);
 }
 
+/* arm_dm_event_poll's ioctl fails. Nothing happens */
 static void test_dmevent_loop_bad1(void **state)
 {
 	struct dm_device *dev;
@@ -624,6 +677,7 @@ static void test_dmevent_loop_bad1(void **state)
 	assert_int_equal(dev->update_nr, 5);
 }
 
+/* dm_get_events fails. Nothing happens */
 static void test_dmevent_loop_bad2(void **state)
 {
 	struct dm_device *dev;
@@ -646,6 +700,8 @@ static void test_dmevent_loop_bad2(void **state)
 	assert_int_equal(dev->update_nr, 5);
 }
 
+/* verify dmevent_loop runs successfully when no devices are being
+ * watched */
 static void test_dmevent_loop_good0(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -663,6 +719,11 @@ static void test_dmevent_loop_good0(void **state)
 	assert_int_equal(dmevent_loop(), 1);
 }
 
+/* Watch 3 devices, where one device has an event (foo), one device is
+ * removed (xyzzy), and one device does nothing (bar). Verify that
+ * the device with the event gets updated, the device that is removed
+ * gets unwatched, and the device with no events stays the same.
+ * The devices remain after this test */
 static void test_dmevent_loop_good1(void **state)
 {
 	struct dm_device *dev;
@@ -717,6 +778,10 @@ static void test_dmevent_loop_good1(void **state)
 	assert_ptr_equal(find_dm_device("xyzzy"), NULL);
 }
 
+/* watch another dm device and add events to two of them, so bar and
+ * baz have new events, and foo doesn't. Set update_multipath to
+ * fail for baz. Verify that baz is unwatched, bar is updated, and
+ * foo stays the same. */
 static void test_dmevent_loop_good2(void **state)
 {
 	struct dm_device *dev;
@@ -762,6 +827,8 @@ static void test_dmevent_loop_good2(void **state)
 	assert_ptr_equal(find_dm_device("baz"), NULL);
 }
 
+/* remove dm device foo, and unwatch events on bar. Verify that
+ * foo is cleaned up and unwatched, and bar is no longer updated */
 static void test_dmevent_loop_good3(void **state)
 {
 	struct dm_device *dev;
@@ -790,6 +857,8 @@ static void test_dmevent_loop_good3(void **state)
 	assert_ptr_equal(find_dm_device("foo"), NULL);
 }
 
+
+/* verify that rearming the dmevents polling works */
 static void test_arm_poll(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
@@ -799,6 +868,7 @@ static void test_arm_poll(void **state)
 	assert_int_equal(arm_dm_event_poll(waiter->fd), 0);
 }
 
+/* verify that the waiter is cleaned up */
 static void test_cleanup_waiter(void **state)
 {
 	struct test_data *datap = (struct test_data *)(*state);
diff --git a/tests/util.c b/tests/util.c
index e9a004f..113b134 100644
--- a/tests/util.c
+++ b/tests/util.c
@@ -74,6 +74,8 @@ static void test_basenamecpy_good5(void **state)
 	assert_string_equal(dst, "bar");
 }
 
+/* multipath expects any trailing whitespace to be stripped off the basename,
+ * so that it will match pp->dev */
 static void test_basenamecpy_good6(void **state)
 {
         char dst[6];
-- 
2.7.4

[PATCH 3/6] multipathd: minor dmevents polling code cleanups

[Benjamin Marzinski]

Change strncpy to strlcpy and lock_cleanup_pop to pthread_cleanup_pop,
based on suggestions by Martin Wilck

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

diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
index 0b0d0ce..e98a974 100644
--- a/multipathd/dmevents.c
+++ b/multipathd/dmevents.c
@@ -24,6 +24,7 @@
 #include "debug.h"
 #include "main.h"
 #include "dmevents.h"
+#include "util.h"
 
 #ifndef DM_DEV_ARM_POLL
 #define DM_DEV_ARM_POLL _IOWR(DM_IOCTL, DM_DEV_SET_GEOMETRY_CMD + 1, struct dm_ioctl)
@@ -214,8 +215,7 @@ int watch_dmevents(char *name)
 		return -1;
 	}
 
-	strncpy(dev_evt->name, name, WWID_SIZE);
-	dev_evt->name[WWID_SIZE - 1] = 0;
+	strlcpy(dev_evt->name, name, WWID_SIZE);
 	dev_evt->evt_nr = event_nr;
 	dev_evt->action = EVENT_NOTHING;
 
@@ -350,7 +350,7 @@ static int dmevent_loop (void)
 			remove_map_by_alias(curr_dev.name, waiter->vecs, 1);
 		else
 			r = update_multipath(waiter->vecs, curr_dev.name, 1);
-		lock_cleanup_pop(&waiter->vecs->lock);
+		pthread_cleanup_pop(1);
 
 		if (r) {
 			condlog(2, "%s: stopped watching dmevents",
-- 
2.7.4

[PATCH 4/6] multipathd: remove unneeded function parameter

[Benjamin Marzinski]

remove_map_and_stop_waiter was always called with purge_vecs = 1, so
it can simply be removed, as suggested by Martin Wilck

Cc: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 multipathd/main.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/multipathd/main.c b/multipathd/main.c
index 9a4f671..841d3e9 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -311,14 +311,13 @@ wait_for_events(struct multipath *mpp, struct vectors *vecs)
 }
 
 static void
-remove_map_and_stop_waiter(struct multipath *mpp, struct vectors *vecs,
-			   int purge_vec)
+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 */
 	if (!poll_dmevents)
 		stop_waiter_thread(mpp, vecs);
-	remove_map(mpp, vecs, purge_vec);
+	remove_map(mpp, vecs, PURGE_VEC);
 }
 
 static void
@@ -400,7 +399,7 @@ int __setup_multipath(struct vectors *vecs, struct multipath *mpp,
 
 	return 0;
 out:
-	remove_map_and_stop_waiter(mpp, vecs, PURGE_VEC);
+	remove_map_and_stop_waiter(mpp, vecs);
 	return 1;
 }
 
@@ -637,7 +636,7 @@ flush_map(struct multipath * mpp, struct vectors * vecs, int nopaths)
 	}
 
 	orphan_paths(vecs->pathvec, mpp);
-	remove_map_and_stop_waiter(mpp, vecs, 1);
+	remove_map_and_stop_waiter(mpp, vecs);
 
 	return 0;
 }
@@ -769,7 +768,7 @@ uev_remove_map (struct uevent * uev, struct vectors * vecs)
 	}
 
 	orphan_paths(vecs->pathvec, mpp);
-	remove_map_and_stop_waiter(mpp, vecs, 1);
+	remove_map_and_stop_waiter(mpp, vecs);
 out:
 	lock_cleanup_pop(vecs->lock);
 	FREE(alias);
@@ -1154,7 +1153,7 @@ out:
 	return retval;
 
 fail:
-	remove_map_and_stop_waiter(mpp, vecs, 1);
+	remove_map_and_stop_waiter(mpp, vecs);
 	return 1;
 }
 
@@ -1612,7 +1611,7 @@ mpvec_garbage_collector (struct vectors * vecs)
 	vector_foreach_slot (vecs->mpvec, mpp, i) {
 		if (mpp && mpp->alias && !dm_map_present(mpp->alias)) {
 			condlog(2, "%s: remove dead map", mpp->alias);
-			remove_map_and_stop_waiter(mpp, vecs, 1);
+			remove_map_and_stop_waiter(mpp, vecs);
 			i--;
 		}
 	}
-- 
2.7.4

[PATCH 5/6] mpathcmd: fix libmpathcmd license

[Benjamin Marzinski]

There is no version 2 of the GNU Lesser General Public License, so
change the license header to version 2.1, which does exist. Also copy
the license header to mpath_cmd.c. I've CC'ed everyone who has
contributed code to libmpathcmd.

Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
Cc: Hannes Reinecke <hare@suse.de>
Cc: Bart Van Assche <bart.vanassche@wdc.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: tang.junhui <tang.junhui@zte.com.cn>
Cc: Gris Ge <fge@redhat.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmpathcmd/mpath_cmd.c | 19 +++++++++++++++++++
 libmpathcmd/mpath_cmd.h |  2 +-
 2 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/libmpathcmd/mpath_cmd.c b/libmpathcmd/mpath_cmd.c
index 29d148c..61e6a98 100644
--- a/libmpathcmd/mpath_cmd.c
+++ b/libmpathcmd/mpath_cmd.c
@@ -1,3 +1,22 @@
+/*
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This file is part of the device-mapper multipath userspace tools.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2.1
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
 #include <stdlib.h>
 #include <unistd.h>
 #include <stdio.h>
diff --git a/libmpathcmd/mpath_cmd.h b/libmpathcmd/mpath_cmd.h
index b57b708..aaa8da9 100644
--- a/libmpathcmd/mpath_cmd.h
+++ b/libmpathcmd/mpath_cmd.h
@@ -5,7 +5,7 @@
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
- * as published by the Free Software Foundation; either version 2
+ * as published by the Free Software Foundation; either version 2.1
  * of the License, or (at your option) any later version.
  *
  * This program is distributed in the hope that it will be useful,
-- 
2.7.4

[PATCH 6/6] libmultipath: don't print undefined values

[Benjamin Marzinski]

commit 48e9fd9f ("libmultipath: parser: use call-by-value for "snprint"
methods") removed some of the code that checked for undefined values to
avoid printing them.  This lead to device configurations that printed
out values for parameters that they hadn't configured. This patch adds
that code back in.

Fixes: 48e9fd9f ("libmultipath: parser: use call-by-value for "snprint" methods")
Cc: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/dict.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/libmultipath/dict.c b/libmultipath/dict.c
index ac9216c..1a18337 100644
--- a/libmultipath/dict.c
+++ b/libmultipath/dict.c
@@ -101,12 +101,16 @@ print_int (char *buff, int len, long v)
 static int
 print_nonzero (char *buff, int len, long v)
 {
+	if (!v)
+		return 0;
 	return snprintf(buff, len, "%li", v);
 }
 
 static int
 print_str (char *buff, int len, const char *ptr)
 {
+	if (!ptr)
+		return 0;
 	return snprintf(buff, len, "\"%s\"", ptr);
 }
 
@@ -120,6 +124,8 @@ print_yes_no (char *buff, int len, long v)
 static int
 print_yes_no_undef (char *buff, int len, long v)
 {
+	if (!v)
+		return 0;
 	return snprintf(buff, len, "\"%s\"",
 			(v == YNU_NO)? "no" : "yes");
 }
@@ -665,6 +671,8 @@ set_dev_loss(vector strvec, void *ptr)
 int
 print_dev_loss(char * buff, int len, unsigned long v)
 {
+	if (!v)
+		return 0;
 	if (v >= MAX_DEV_LOSS_TMO)
 		return snprintf(buff, len, "\"infinity\"");
 	return snprintf(buff, len, "%lu", v);
-- 
2.7.4

Re: [PATCH 1/6] multipathd: remove incorrect pthread_testcancel

[Martin Wilck]

On Thu, 2018-03-29 at 22:36 -0500, Benjamin Marzinski wrote:
> As Martin Wilck pointed out, a thread that's trying to stop the
> waiter
> thread should not cancel itself before it gets a chance to do so
> 
> Cc: Martin Wilck <mwilck@suse.com>
> Fixes: c7625f92 "multipathd: fix waiter thread cancelling"
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

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

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

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

Re: [PATCH 2/6] multipath: add comments

[Martin Wilck]

On Thu, 2018-03-29 at 22:36 -0500, Benjamin Marzinski wrote:
> This commit simply adds a number of comments based on suggestions by
> Martin Wilck.
> 
> Cc: Martin Wilck <mwilck@suse.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

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

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

Re: [PATCH 3/6] multipathd: minor dmevents polling code cleanups

[Martin Wilck]

On Thu, 2018-03-29 at 22:37 -0500, Benjamin Marzinski wrote:
> Change strncpy to strlcpy and lock_cleanup_pop to
> pthread_cleanup_pop,
> based on suggestions by Martin Wilck
> 
> Cc: Martin Wilck <mwilck@suse.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

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

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

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

Re: [PATCH 4/6] multipathd: remove unneeded function parameter

[Martin Wilck]

On Thu, 2018-03-29 at 22:37 -0500, Benjamin Marzinski wrote:
> remove_map_and_stop_waiter was always called with purge_vecs = 1, so
> it can simply be removed, as suggested by Martin Wilck
> 
> Cc: Martin Wilck <mwilck@suse.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

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

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

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

Re: [PATCH 5/6] mpathcmd: fix libmpathcmd license

[Martin Wilck]

On Thu, 2018-03-29 at 22:37 -0500, Benjamin Marzinski wrote:
> There is no version 2 of the GNU Lesser General Public License, so
> change the license header to version 2.1, which does exist. Also copy
> the license header to mpath_cmd.c. I've CC'ed everyone who has
> contributed code to libmpathcmd.
> 
> Cc: Xose Vazquez Perez <xose.vazquez@gmail.com>
> Cc: Hannes Reinecke <hare@suse.de>
> Cc: Bart Van Assche <bart.vanassche@wdc.com>
> Cc: Martin Wilck <mwilck@suse.com>
> Cc: tang.junhui <tang.junhui@zte.com.cn>
> Cc: Gris Ge <fge@redhat.com>
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>

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

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

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