[PATCH 0/7] multipath-tools: uevent processing fixes and unit tests

[PATCH 0/7] multipath-tools: uevent processing fixes and unit tests

[Martin Wilck]

Hi Christophe,

The incentive for creating this series was Ritika's patch "multipath-tools:
Skip CHANGE uevent for non-mpath devices" from October 2017, plus the
observation that uevent.c contains a lot of code repetition.
I've refactored and simplified the uevent_get_XXX code, and started to use
"const" qualifiers in the part of the code which I touched in the process.

In order to avoid regressions in the process, I took this as a starting point
for implementing a simple unit test framework. For now, it covers only the
stuff that I changed in this series.

The series is based on my previous patch series ("Various 
multipath-tools fixes"), v2. The first patch doesn't belong in the series
logically, but I thought it may help you track the ordering if I include it
here rather then posting separately or spamming the list with the big series
again.

Regards,
Martin

Martin Wilck (7):
  assemble_map: no newline at end of params string
  tests: cmocka-based unit test for uevent_get_XXX
  libmultipath: refactor uevent_get_XXX
  libmultipath: const qualifier for wwid and alias
  libmultipath: move UUID_PREFIX to devmapper.h
  libmultipath: add uevent_is_mpath
  multipathd: ignore uevents for non-mpath devices

 .gitignore                 |   3 +
 Makefile                   |   4 +
 libmultipath/devmapper.c   |   3 -
 libmultipath/devmapper.h   |   3 +
 libmultipath/discovery.c   |   2 +-
 libmultipath/discovery.h   |   2 +-
 libmultipath/dmparser.c    |   1 -
 libmultipath/memory.h      |   2 +-
 libmultipath/structs.c     |   4 +-
 libmultipath/structs.h     |   4 +-
 libmultipath/structs_vec.c |   2 +-
 libmultipath/structs_vec.h |   2 +-
 libmultipath/uevent.c      | 183 +++++++++++++--------------
 libmultipath/uevent.h      |  15 +--
 multipathd/main.c          |  10 +-
 multipathd/main.h          |   2 +-
 tests/Makefile             |  23 ++++
 tests/globals.c            |  17 +++
 tests/uevent.c             | 304 +++++++++++++++++++++++++++++++++++++++++++++
 19 files changed, 462 insertions(+), 124 deletions(-)
 create mode 100644 tests/Makefile
 create mode 100644 tests/globals.c
 create mode 100644 tests/uevent.c

-- 
2.15.1

[PATCH 1/7] assemble_map: no newline at end of params string

[Martin Wilck]

This newline is superfluous for device-mapper, and causes ugly
debug output.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/dmparser.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index 642f44fb0439..027ae989781e 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -114,7 +114,6 @@ assemble_map (struct multipath * mp, char * params, int len)
 			APPEND(p, end, " %s %d", pp->dev_t, tmp_minio);
 		}
 	}
-	APPEND(p, end, "\n");
 
 	FREE(f);
 	condlog(3, "%s: assembled map [%s]", mp->alias, params);
-- 
2.15.1

[PATCH 2/7] tests: cmocka-based unit test for uevent_get_XXX

[Martin Wilck]

This patch starts a simple unit test framework for multipath-tools
based on the cmocka framework (https://cmocka.org/). As a start,
it adds unit tests for the uevent_get_XXX set of functions.

Note that some tests currently fail. This will be fixed by the
following patches.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 .gitignore      |   3 +
 Makefile        |   4 +
 tests/Makefile  |  23 ++++++
 tests/globals.c |  17 ++++
 tests/uevent.c  | 241 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 5 files changed, 288 insertions(+)
 create mode 100644 tests/Makefile
 create mode 100644 tests/globals.c
 create mode 100644 tests/uevent.c

diff --git a/.gitignore b/.gitignore
index 57cf7e6a31a5..371b8758c0a1 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,3 +18,6 @@ libdmmp/docs/man/*.3.gz
 libdmmp/*.so.*
 libdmmp/test/libdmmp_test
 libdmmp/test/libdmmp_speed_test
+tests/*-test
+tests/*.out
+
diff --git a/Makefile b/Makefile
index bfb168fb172c..11c46eb4dbc9 100644
--- a/Makefile
+++ b/Makefile
@@ -27,6 +27,7 @@ recurse_clean:
 	@for dir in $(BUILDDIRS); do \
 	$(MAKE) -C $$dir clean || exit $?; \
 	done
+	$(MAKE) -C tests clean
 
 recurse_install:
 	@for dir in $(BUILDDIRS); do \
@@ -44,6 +45,9 @@ install: recurse_install
 
 uninstall: recurse_uninstall
 
+test:	all
+	$(MAKE) -C tests
+
 .PHONY:	TAGS
 TAGS:
 	etags -a libmultipath/*.c
diff --git a/tests/Makefile b/tests/Makefile
new file mode 100644
index 000000000000..ff58cb8bb207
--- /dev/null
+++ b/tests/Makefile
@@ -0,0 +1,23 @@
+include ../Makefile.inc
+
+CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
+LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
+
+TESTS := uevent
+
+.SILENT: $(TESTS:%=%.o)
+.PRECIOUS: $(TESTS:%=%-test)
+
+%-test:	%.o globals.c $(multipathdir)/libmultipath.so
+	@$(CC) -o $@ $< $(LDFLAGS) $(LIBDEPS)
+
+%.out:	%-test
+	@echo == running $< ==
+	@LD_LIBRARY_PATH=$(multipathdir):$(mpathcmddir) ./$< >$@
+
+all:	$(TESTS:%=%.out)
+
+clean:
+	rm -f $(TESTS:%=%-test) $(TESTS:%=%.out) $(TESTS:%=%.o)
+
+
diff --git a/tests/globals.c b/tests/globals.c
new file mode 100644
index 000000000000..96a56515fd09
--- /dev/null
+++ b/tests/globals.c
@@ -0,0 +1,17 @@
+#include "structs.h"
+#include "config.h"
+
+/* Required globals */
+struct udev *udev;
+int logsink = 0;
+struct config conf = {
+	.uid_attrs = "sd:ID_BOGUS",
+};
+
+struct config *get_multipath_config(void)
+{
+	return &conf;
+}
+
+void put_multipath_config(struct config* c)
+{}
diff --git a/tests/uevent.c b/tests/uevent.c
new file mode 100644
index 000000000000..a8edd710f653
--- /dev/null
+++ b/tests/uevent.c
@@ -0,0 +1,241 @@
+/*
+ * Copyright (c) 2018 SUSE Linux GmbH
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ *
+ */
+
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <cmocka.h>
+#include "list.h"
+#include "uevent.h"
+
+#include "globals.c"
+
+/* Private prototypes missing in uevent.h */
+struct uevent * alloc_uevent(void);
+void uevent_get_wwid(struct uevent *uev);
+
+/* Stringify helpers */
+#define _str_(x) #x
+#define str(x) _str_(x)
+
+#define MAJOR 17
+#define MINOR 217
+#define DISK_RO 0
+#define DM_NAME "spam"
+#define WWID "foo"
+
+static int setup_uev(void **state)
+{
+	struct uevent *uev = alloc_uevent();
+
+	if (uev == NULL)
+		return -1;
+
+	*state = uev;
+	uev->kernel = "sdo";
+	uev->envp[0] = "MAJOR=" str(MAJOR);
+	uev->envp[1] = "ID_BOGUS=" WWID;
+	uev->envp[2] = "MINOR=" str(MINOR);
+	uev->envp[3] = "DM_NAME=" DM_NAME;
+	uev->envp[4] = "DISK_RO=" str(DISK_RO);
+	uev->envp[5] = NULL;
+	return 0;
+}
+
+static int teardown(void **state)
+{
+	free(*state);
+	return 0;
+}
+
+static void test_major_good(void **state)
+{
+	struct uevent *uev = *state;
+
+	assert_int_equal(uevent_get_major(uev), MAJOR);
+}
+
+static void test_minor_good(void **state)
+{
+	struct uevent *uev = *state;
+
+	assert_int_equal(uevent_get_minor(uev), MINOR);
+}
+
+static void test_ro_good(void **state)
+{
+	struct uevent *uev = *state;
+
+	assert_int_equal(uevent_get_disk_ro(uev), DISK_RO);
+}
+
+static void test_wwid(void **state)
+{
+	struct uevent *uev = *state;
+	uevent_get_wwid(uev);
+
+	assert_string_equal(uev->wwid, WWID);
+}
+
+static void test_major_bad_0(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR" str(MAJOR);
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_1(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOr=" str(MAJOR);
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_2(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJORIE=" str(MAJOR);
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_3(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR=max";
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_4(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR=0x10";
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_5(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJO=" str(MAJOR);
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_6(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR=" str(-MAJOR);
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_7(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR=";
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_major_bad_8(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[0] = "MAJOR";
+	assert_int_equal(uevent_get_major(uev), -1);
+}
+
+static void test_dm_name_good(void **state)
+{
+	struct uevent *uev = *state;
+	const char *name = uevent_get_dm_name(uev);
+
+	assert_string_equal(name, DM_NAME);
+	free((void*)name);
+}
+
+static void test_dm_name_bad_0(void **state)
+{
+	struct uevent *uev = *state;
+	const char *name;
+
+	uev->envp[3] = "DM_NAME" DM_NAME;
+	name = uevent_get_dm_name(uev);
+	assert_ptr_equal(name, NULL);
+	free((void*)name);
+}
+
+static void test_dm_name_bad_1(void **state)
+{
+	struct uevent *uev = *state;
+	const char *name;
+
+	uev->envp[3] = "DM_NAMES=" DM_NAME;
+	name = uevent_get_dm_name(uev);
+	assert_ptr_equal(name, NULL);
+	free((void*)name);
+}
+
+static void test_dm_name_good_1(void **state)
+{
+	struct uevent *uev = *state;
+	const char *name;
+
+	/* Note we change index 2 here */
+	uev->envp[2] = "DM_NAME=" DM_NAME;
+	name = uevent_get_dm_name(uev);
+	assert_string_equal(name, DM_NAME);
+	free((void*)name);
+}
+
+int test_uevent_get_XXX(void)
+{
+	const struct CMUnitTest tests[] = {
+		cmocka_unit_test(test_major_good),
+		cmocka_unit_test(test_minor_good),
+		cmocka_unit_test(test_ro_good),
+		cmocka_unit_test(test_dm_name_good),
+		cmocka_unit_test(test_wwid),
+		cmocka_unit_test(test_major_bad_0),
+		cmocka_unit_test(test_major_bad_1),
+		cmocka_unit_test(test_major_bad_2),
+		cmocka_unit_test(test_major_bad_3),
+		cmocka_unit_test(test_major_bad_4),
+		cmocka_unit_test(test_major_bad_5),
+		cmocka_unit_test(test_major_bad_6),
+		cmocka_unit_test(test_major_bad_7),
+		cmocka_unit_test(test_major_bad_8),
+		cmocka_unit_test(test_dm_name_bad_0),
+		cmocka_unit_test(test_dm_name_bad_1),
+		cmocka_unit_test(test_dm_name_good_1),
+	};
+	return cmocka_run_group_tests(tests, setup_uev, teardown);
+}
+
+int main(void)
+{
+	int ret = 0;
+
+	ret += test_uevent_get_XXX();
+	return ret;
+}
-- 
2.15.1

[PATCH 3/7] libmultipath: refactor uevent_get_XXX

[Martin Wilck]

Use common helper functions for uevent_get_XXX.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/uevent.c | 159 +++++++++++++++++++++-----------------------------
 1 file changed, 67 insertions(+), 92 deletions(-)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 80bf1dd15372..13d468f7fce0 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -104,28 +104,69 @@ uevq_cleanup(struct list_head *tmpq)
 	}
 }
 
+static const char* uevent_get_env_var(const struct uevent *uev,
+				      const char *attr)
+{
+	int i, len;
+	const char *p = NULL;
+
+	if (attr == NULL)
+		goto invalid;
+
+	len = strlen(attr);
+	if (len == 0)
+		goto invalid;
+
+	for (i = 0; uev->envp[i] != NULL; i++) {
+		const char *var = uev->envp[i];
+
+		if (strlen(var) > len &&
+		    !memcmp(var, attr, len) && var[len] == '=') {
+			p = var + len + 1;
+			break;
+		}
+	}
+
+	condlog(4, "%s: %s -> '%s'", __func__, attr, p);
+	return p;
+
+invalid:
+	condlog(2, "%s: empty variable name", __func__);
+	return NULL;
+}
+
+static int uevent_get_env_positive_int(const struct uevent *uev,
+				       const char *attr)
+{
+	const char *p = uevent_get_env_var(uev, attr);
+	char *q;
+	int ret;
+
+	if (p == NULL || *p == '\0')
+		return -1;
+
+	ret = strtoul(p, &q, 10);
+	if (*q != '\0' || ret < 0) {
+		condlog(2, "%s: invalid %s: '%s'", __func__, attr, p);
+		return -1;
+	}
+	return ret;
+}
+
 void
 uevent_get_wwid(struct uevent *uev)
 {
-	int i;
 	char *uid_attribute;
+	const char *val;
 	struct config * conf;
 
 	conf = get_multipath_config();
 	uid_attribute = parse_uid_attribute_by_attrs(conf->uid_attrs, uev->kernel);
 	put_multipath_config(conf);
 
-	if (!uid_attribute)
-		return;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], uid_attribute, strlen(uid_attribute)) &&
-		    strlen(uev->envp[i]) > strlen(uid_attribute) &&
-		    uev->envp[i][strlen(uid_attribute)] == '=') {
-			uev->wwid = uev->envp[i] + strlen(uid_attribute) + 1;
-			break;
-		}
-	}
+	val = uevent_get_env_var(uev, uid_attribute);
+	if (val)
+		uev->wwid = (char*)val;
 	free(uid_attribute);
 }
 
@@ -852,105 +893,39 @@ out:
 
 int uevent_get_major(struct uevent *uev)
 {
-	char *p, *q;
-	int i, major = -1;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "MAJOR", 5) && strlen(uev->envp[i]) > 6) {
-			p = uev->envp[i] + 6;
-			major = strtoul(p, &q, 10);
-			if (p == q) {
-				condlog(2, "invalid major '%s'", p);
-				major = -1;
-			}
-			break;
-		}
-	}
-	return major;
+	return uevent_get_env_positive_int(uev, "MAJOR");
 }
 
 int uevent_get_minor(struct uevent *uev)
 {
-	char *p, *q;
-	int i, minor = -1;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "MINOR", 5) && strlen(uev->envp[i]) > 6) {
-			p = uev->envp[i] + 6;
-			minor = strtoul(p, &q, 10);
-			if (p == q) {
-				condlog(2, "invalid minor '%s'", p);
-				minor = -1;
-			}
-			break;
-		}
-	}
-	return minor;
+	return uevent_get_env_positive_int(uev, "MINOR");
 }
 
 int uevent_get_disk_ro(struct uevent *uev)
 {
-	char *p, *q;
-	int i, ro = -1;
+	return uevent_get_env_positive_int(uev, "DISK_RO");
+}
 
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "DISK_RO", 7) && strlen(uev->envp[i]) > 8) {
-			p = uev->envp[i] + 8;
-			ro = strtoul(p, &q, 10);
-			if (p == q) {
-				condlog(2, "invalid read_only setting '%s'", p);
-				ro = -1;
-			}
-			break;
-		}
-	}
-	return ro;
+static char *uevent_get_dm_str(const struct uevent *uev, char *attr)
+{
+	char *tmp = uevent_get_env_var(uev, attr);
+
+	if (tmp == NULL)
+		return NULL;
+	return strdup(tmp);
 }
 
 char *uevent_get_dm_name(struct uevent *uev)
 {
-	char *p = NULL;
-	int i;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "DM_NAME", 7) &&
-		    strlen(uev->envp[i]) > 8) {
-			p = MALLOC(strlen(uev->envp[i] + 8) + 1);
-			strcpy(p, uev->envp[i] + 8);
-			break;
-		}
-	}
-	return p;
+	return uevent_get_dm_str(uev, "DM_NAME");
 }
 
 char *uevent_get_dm_path(struct uevent *uev)
 {
-	char *p = NULL;
-	int i;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "DM_PATH", 7) &&
-		    strlen(uev->envp[i]) > 8) {
-			p = MALLOC(strlen(uev->envp[i] + 8) + 1);
-			strcpy(p, uev->envp[i] + 8);
-			break;
-		}
-	}
-	return p;
+	return uevent_get_dm_str(uev, "DM_PATH");
 }
 
 char *uevent_get_dm_action(struct uevent *uev)
 {
-	char *p = NULL;
-	int i;
-
-	for (i = 0; uev->envp[i] != NULL; i++) {
-		if (!strncmp(uev->envp[i], "DM_ACTION", 9) &&
-		    strlen(uev->envp[i]) > 10) {
-			p = MALLOC(strlen(uev->envp[i] + 10) + 1);
-			strcpy(p, uev->envp[i] + 10);
-			break;
-		}
-	}
-	return p;
+	return uevent_get_dm_str(uev, "DM_ACTION");
 }
-- 
2.15.1

[PATCH 4/7] libmultipath: const qualifier for wwid and alias

[Martin Wilck]

Add "const" qualifiers to some function arguments and fields,
in order to tidy up const handling of libmultipath.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/discovery.c   |  2 +-
 libmultipath/discovery.h   |  2 +-
 libmultipath/memory.h      |  2 +-
 libmultipath/structs.c     |  4 ++--
 libmultipath/structs.h     |  4 ++--
 libmultipath/structs_vec.c |  2 +-
 libmultipath/structs_vec.h |  2 +-
 libmultipath/uevent.c      | 16 ++++++++--------
 libmultipath/uevent.h      | 14 +++++++-------
 multipathd/main.c          |  8 ++++----
 multipathd/main.h          |  2 +-
 11 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 8ae170ee3a27..9b22bd94e491 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -34,7 +34,7 @@
 
 int
 alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
-			  char *wwid, int flag, struct path **pp_ptr)
+			  const char *wwid, int flag, struct path **pp_ptr)
 {
 	int err = PATHINFO_FAILED;
 	struct path * pp;
diff --git a/libmultipath/discovery.h b/libmultipath/discovery.h
index b151cc7a39a8..bd5e6678a26d 100644
--- a/libmultipath/discovery.h
+++ b/libmultipath/discovery.h
@@ -38,7 +38,7 @@ int get_state (struct path * pp, struct config * conf, int daemon, int state);
 int get_vpd_sgio (int fd, int pg, char * str, int maxlen);
 int pathinfo (struct path * pp, struct config * conf, int mask);
 int alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice,
-			      char *wwid, int flag, struct path **pp_ptr);
+			      const char *wwid, int flag, struct path **pp_ptr);
 int store_pathinfo (vector pathvec, struct config *conf,
 		    struct udev_device *udevice, int flag,
 		    struct path **pp_ptr);
diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index a3c478e24bd1..927619b58a62 100644
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -54,7 +54,7 @@ extern void dbg_free_final(char *);
 #else
 
 #define MALLOC(n)    (calloc(1,(n)))
-#define FREE(p)      do { free(p); p = NULL; } while(0)
+#define FREE(p)      do { free((void*)p); p = NULL; } while(0)
 #define REALLOC(p,n) (realloc((p),(n)))
 #define STRDUP(n)    (strdup(n))
 
diff --git a/libmultipath/structs.c b/libmultipath/structs.c
index c42d765409f9..4caad2a40302 100644
--- a/libmultipath/structs.c
+++ b/libmultipath/structs.c
@@ -374,7 +374,7 @@ find_mp_by_wwid (vector mpvec, char * wwid)
 }
 
 struct multipath *
-find_mp_by_alias (vector mpvec, char * alias)
+find_mp_by_alias (vector mpvec, const char * alias)
 {
 	int i;
 	int len;
@@ -425,7 +425,7 @@ find_path_by_dev (vector pathvec, char * dev)
 }
 
 struct path *
-find_path_by_devt (vector pathvec, char * dev_t)
+find_path_by_devt (vector pathvec, const char * dev_t)
 {
 	int i;
 	struct path * pp;
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index bfa660a16715..d132dfcebce4 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -374,12 +374,12 @@ int store_hostgroup(vector hostgroupvec, struct host_group *hgp);
 int store_path (vector pathvec, struct path * pp);
 int store_pathgroup (vector pgvec, struct pathgroup * pgp);
 
-struct multipath * find_mp_by_alias (vector mp, char * alias);
+struct multipath * find_mp_by_alias (vector mp, const char * alias);
 struct multipath * find_mp_by_wwid (vector mp, char * wwid);
 struct multipath * find_mp_by_str (vector mp, char * wwid);
 struct multipath * find_mp_by_minor (vector mp, int minor);
 
-struct path * find_path_by_devt (vector pathvec, char * devt);
+struct path * find_path_by_devt (vector pathvec, const char * devt);
 struct path * find_path_by_dev (vector pathvec, char * dev);
 struct path * first_path (struct multipath * mpp);
 
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index fbab61f1dcec..3aafee7b217b 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -449,7 +449,7 @@ fail:
 	return 0;
 }
 
-struct multipath *add_map_without_path (struct vectors *vecs, char *alias)
+struct multipath *add_map_without_path (struct vectors *vecs, const char *alias)
 {
 	struct multipath * mpp = alloc_multipath();
 	struct config *conf;
diff --git a/libmultipath/structs_vec.h b/libmultipath/structs_vec.h
index b81413b0d177..3749eb64a0f6 100644
--- a/libmultipath/structs_vec.h
+++ b/libmultipath/structs_vec.h
@@ -33,7 +33,7 @@ void remove_maps_and_stop_waiters (struct vectors * vecs);
 
 void sync_map_state (struct multipath *);
 int update_map (struct multipath *mpp, struct vectors *vecs);
-struct multipath * add_map_without_path (struct vectors * vecs, char * alias);
+struct multipath * add_map_without_path (struct vectors * vecs, const char * alias);
 struct multipath * add_map_with_path (struct vectors * vecs,
 				struct path * pp, int add_vec);
 int update_multipath (struct vectors *vecs, char *mapname, int reset);
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 13d468f7fce0..3d32e31c5c49 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -891,41 +891,41 @@ out:
 	return err;
 }
 
-int uevent_get_major(struct uevent *uev)
+int uevent_get_major(const struct uevent *uev)
 {
 	return uevent_get_env_positive_int(uev, "MAJOR");
 }
 
-int uevent_get_minor(struct uevent *uev)
+int uevent_get_minor(const struct uevent *uev)
 {
 	return uevent_get_env_positive_int(uev, "MINOR");
 }
 
-int uevent_get_disk_ro(struct uevent *uev)
+int uevent_get_disk_ro(const struct uevent *uev)
 {
 	return uevent_get_env_positive_int(uev, "DISK_RO");
 }
 
-static char *uevent_get_dm_str(const struct uevent *uev, char *attr)
+static const char *uevent_get_dm_str(const struct uevent *uev, char *attr)
 {
-	char *tmp = uevent_get_env_var(uev, attr);
+	const char *tmp = uevent_get_env_var(uev, attr);
 
 	if (tmp == NULL)
 		return NULL;
 	return strdup(tmp);
 }
 
-char *uevent_get_dm_name(struct uevent *uev)
+const char *uevent_get_dm_name(const struct uevent *uev)
 {
 	return uevent_get_dm_str(uev, "DM_NAME");
 }
 
-char *uevent_get_dm_path(struct uevent *uev)
+const char *uevent_get_dm_path(const struct uevent *uev)
 {
 	return uevent_get_dm_str(uev, "DM_PATH");
 }
 
-char *uevent_get_dm_action(struct uevent *uev)
+const char *uevent_get_dm_action(const struct uevent *uev)
 {
 	return uevent_get_dm_str(uev, "DM_ACTION");
 }
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index 6f5af0af6478..bf7017090b1c 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -23,7 +23,7 @@ struct uevent {
 	char *devpath;
 	char *action;
 	char *kernel;
-	char *wwid;
+	const char *wwid;
 	unsigned long seqnum;
 	char *envp[HOTPLUG_NUM_ENVP];
 };
@@ -33,11 +33,11 @@ int is_uevent_busy(void);
 int uevent_listen(struct udev *udev);
 int uevent_dispatch(int (*store_uev)(struct uevent *, void * trigger_data),
 		    void * trigger_data);
-int uevent_get_major(struct uevent *uev);
-int uevent_get_minor(struct uevent *uev);
-int uevent_get_disk_ro(struct uevent *uev);
-char *uevent_get_dm_name(struct uevent *uev);
-char *uevent_get_dm_path(struct uevent *uev);
-char *uevent_get_dm_action(struct uevent *uev);
+int uevent_get_major(const struct uevent *uev);
+int uevent_get_minor(const struct uevent *uev);
+int uevent_get_disk_ro(const struct uevent *uev);
+const char *uevent_get_dm_name(const struct uevent *uev);
+const char *uevent_get_dm_path(const struct uevent *uev);
+const char *uevent_get_dm_action(const struct uevent *uev);
 
 #endif /* _UEVENT_H */
diff --git a/multipathd/main.c b/multipathd/main.c
index ccbb0ad13d32..ff3ecb640487 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -387,7 +387,7 @@ flush_map(struct multipath * mpp, struct vectors * vecs, int nopaths)
 static int
 uev_add_map (struct uevent * uev, struct vectors * vecs)
 {
-	char *alias;
+	const char *alias;
 	int major = -1, minor = -1, rc;
 
 	condlog(3, "%s: add map (uevent)", uev->kernel);
@@ -413,7 +413,7 @@ uev_add_map (struct uevent * uev, struct vectors * vecs)
 }
 
 int
-ev_add_map (char * dev, char * alias, struct vectors * vecs)
+ev_add_map (char * dev, const char * alias, struct vectors * vecs)
 {
 	char * refwwid;
 	struct multipath * mpp;
@@ -500,7 +500,7 @@ ev_add_map (char * dev, char * alias, struct vectors * vecs)
 static int
 uev_remove_map (struct uevent * uev, struct vectors * vecs)
 {
-	char *alias;
+	const char *alias;
 	int minor;
 	struct multipath *mpp;
 
@@ -1001,7 +1001,7 @@ out:
 static int
 uev_pathfail_check(struct uevent *uev, struct vectors *vecs)
 {
-	char *action = NULL, *devt = NULL;
+	const char *action = NULL, *devt = NULL;
 	struct path *pp;
 	int r = 1;
 
diff --git a/multipathd/main.h b/multipathd/main.h
index ededdaba32fe..ee44dab36aec 100644
--- a/multipathd/main.h
+++ b/multipathd/main.h
@@ -24,7 +24,7 @@ int need_to_delay_reconfig (struct vectors *);
 int reconfigure (struct vectors *);
 int ev_add_path (struct path *, struct vectors *, int);
 int ev_remove_path (struct path *, struct vectors *, int);
-int ev_add_map (char *, char *, struct vectors *);
+int ev_add_map (char *, const char *, struct vectors *);
 int ev_remove_map (char *, char *, int, struct vectors *);
 int set_config_state(enum daemon_status);
 void * mpath_alloc_prin_response(int prin_sa);
-- 
2.15.1

[PATCH 5/7] libmultipath: move UUID_PREFIX to devmapper.h

[Martin Wilck]

This constant is useful elsewhere, too.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/devmapper.c | 3 ---
 libmultipath/devmapper.h | 3 +++
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 573fc75cfa54..f112e1cb0e66 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -30,9 +30,6 @@
 #define MAX_WAIT 5
 #define LOOPS_PER_SEC 5
 
-#define UUID_PREFIX "mpath-"
-#define UUID_PREFIX_LEN 6
-
 static int dm_conf_verbosity;
 
 #ifdef LIBDM_API_DEFERRED
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 62e14d1c0b97..558e6914074f 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -24,6 +24,9 @@
 #define MPATH_UDEV_NO_PATHS_FLAG 0
 #endif
 
+#define UUID_PREFIX "mpath-"
+#define UUID_PREFIX_LEN (sizeof(UUID_PREFIX) - 1)
+
 void dm_init(int verbosity);
 void libmp_dm_init(void);
 void libmp_udev_set_sync_support(int on);
-- 
2.15.1

[PATCH 6/7] libmultipath: add uevent_is_mpath

[Martin Wilck]

This function can be used to test if an uevent belongs to valid
multipath device. Unit tests are also added.

Signed-off-by: Martin Wilck <mwilck@suse.com>
---
 libmultipath/uevent.c | 12 ++++++++++
 libmultipath/uevent.h |  1 +
 tests/uevent.c        | 63 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 76 insertions(+)

diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 3d32e31c5c49..8f4129ca7fd0 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -52,6 +52,7 @@
 #include "util.h"
 #include "config.h"
 #include "blacklist.h"
+#include "devmapper.h"
 
 #define MAX_ACCUMULATION_COUNT 2048
 #define MAX_ACCUMULATION_TIME 30*1000
@@ -929,3 +930,14 @@ const char *uevent_get_dm_action(const struct uevent *uev)
 {
 	return uevent_get_dm_str(uev, "DM_ACTION");
 }
+
+bool uevent_is_mpath(const struct uevent *uev)
+{
+	const char *uuid = uevent_get_env_var(uev, "DM_UUID");
+
+	if (uuid == NULL)
+		return false;
+	if (strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN))
+		return false;
+	return uuid[UUID_PREFIX_LEN] != '\0';
+}
diff --git a/libmultipath/uevent.h b/libmultipath/uevent.h
index bf7017090b1c..cb5347e45c2b 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -39,5 +39,6 @@ int uevent_get_disk_ro(const struct uevent *uev);
 const char *uevent_get_dm_name(const struct uevent *uev);
 const char *uevent_get_dm_path(const struct uevent *uev);
 const char *uevent_get_dm_action(const struct uevent *uev);
+bool uevent_is_mpath(const struct uevent *uev);
 
 #endif /* _UEVENT_H */
diff --git a/tests/uevent.c b/tests/uevent.c
index a8edd710f653..b7d6458710f4 100644
--- a/tests/uevent.c
+++ b/tests/uevent.c
@@ -16,6 +16,7 @@
  *
  */
 
+#include <stdbool.h>
 #include <stdarg.h>
 #include <stddef.h>
 #include <setjmp.h>
@@ -208,6 +209,61 @@ static void test_dm_name_good_1(void **state)
 	free((void*)name);
 }
 
+static void test_dm_uuid_false_0(void **state)
+{
+	struct uevent *uev = *state;
+
+	assert_false(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_true_0(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UUID=mpath-foo";
+	assert_true(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_false_1(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UUID.mpath-foo";
+	assert_false(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_false_2(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UUID=mpath-";
+	assert_false(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_false_3(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UU=mpath-foo";
+	assert_false(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_false_4(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UUID=mpathfoo";
+	assert_false(uevent_is_mpath(uev));
+}
+
+static void test_dm_uuid_false_5(void **state)
+{
+	struct uevent *uev = *state;
+
+	uev->envp[3] = "DM_UUID=";
+	assert_false(uevent_is_mpath(uev));
+}
+
 int test_uevent_get_XXX(void)
 {
 	const struct CMUnitTest tests[] = {
@@ -228,6 +284,13 @@ int test_uevent_get_XXX(void)
 		cmocka_unit_test(test_dm_name_bad_0),
 		cmocka_unit_test(test_dm_name_bad_1),
 		cmocka_unit_test(test_dm_name_good_1),
+		cmocka_unit_test(test_dm_uuid_false_0),
+		cmocka_unit_test(test_dm_uuid_true_0),
+		cmocka_unit_test(test_dm_uuid_false_1),
+		cmocka_unit_test(test_dm_uuid_false_2),
+		cmocka_unit_test(test_dm_uuid_false_3),
+		cmocka_unit_test(test_dm_uuid_false_4),
+		cmocka_unit_test(test_dm_uuid_false_5),
 	};
 	return cmocka_run_group_tests(tests, setup_uev, teardown);
 }
-- 
2.15.1

[PATCH 7/7] multipathd: ignore uevents for non-mpath devices

[Martin Wilck]

multipathd can't deal with other devices anyway. Proceeding further
with events for other devices just generates log noise.

Based on an idea from Ritika Srivastava <ritika.srivastava@oracle.com>.
("multipath-tools: Skip CHANGE uevent for non-mpath devices").

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 ff3ecb640487..26632291657f 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1121,7 +1121,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	 * Add events are ignored here as the tables
 	 * are not fully initialised then.
 	 */
-	if (!strncmp(uev->kernel, "dm-", 3)) {
+	if (!strncmp(uev->kernel, "dm-", 3) && uevent_is_mpath(uev)) {
 		if (!strncmp(uev->action, "change", 6)) {
 			r = uev_add_map(uev, vecs);
 
-- 
2.15.1

Re: [PATCH 7/7] multipathd: ignore uevents for non-mpath devices

[Ritika Srivastava]

On 01/16/2018 11:49 PM, Martin Wilck wrote:
> multipathd can't deal with other devices anyway. Proceeding further
> with events for other devices just generates log noise.
>
> Based on an idea from Ritika Srivastava <ritika.srivastava@oracle.com>.
> ("multipath-tools: Skip CHANGE uevent for non-mpath devices").
>
> 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 ff3ecb640487..26632291657f 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1121,7 +1121,7 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>   	 * Add events are ignored here as the tables
>   	 * are not fully initialised then.
>   	 */
> -	if (!strncmp(uev->kernel, "dm-", 3)) {
> +	if (!strncmp(uev->kernel, "dm-", 3) && uevent_is_mpath(uev)) {
>   		if (!strncmp(uev->action, "change", 6)) {
>   			r = uev_add_map(uev, vecs);
>   

Hi Martin,
Thank you for the updated patch.
With this patch, the error "uevent trigger error" would not be 
encountered when removing lvm snapshots.

However, when the uevent is for a 'dm' device but not for a multipath 
device, then should we just return from this check in uev_trigger()?
With this patch, uev_update_path()/uev_remove_path() would still be 
called which would generate further log.
Should we avoid these function calls too?
-- 
Thanks,
Ritika

Re: [PATCH 7/7] multipathd: ignore uevents for non-mpath devices

[Martin Wilck]

On Mon, 2018-01-29 at 14:25 -0800, Ritika Srivastava wrote:
> On 01/16/2018 11:49 PM, Martin Wilck wrote:
> > multipathd can't deal with other devices anyway. Proceeding further
> > with events for other devices just generates log noise.
> > 
> > Based on an idea from Ritika Srivastava <ritika.srivastava@oracle.c
> > om>.
> > ("multipath-tools: Skip CHANGE uevent for non-mpath devices").
> > 
> > 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 ff3ecb640487..26632291657f 100644
> > --- a/multipathd/main.c
> > +++ b/multipathd/main.c
> > @@ -1121,7 +1121,7 @@ uev_trigger (struct uevent * uev, void *
> > trigger_data)
> >   	 * Add events are ignored here as the tables
> >   	 * are not fully initialised then.
> >   	 */
> > -	if (!strncmp(uev->kernel, "dm-", 3)) {
> > +	if (!strncmp(uev->kernel, "dm-", 3) &&
> > uevent_is_mpath(uev)) {
> >   		if (!strncmp(uev->action, "change", 6)) {
> >   			r = uev_add_map(uev, vecs);
> >   
> 
> Hi Martin,
> Thank you for the updated patch.
> With this patch, the error "uevent trigger error" would not be 
> encountered when removing lvm snapshots.
> 
> However, when the uevent is for a 'dm' device but not for a
> multipath 
> device, then should we just return from this check in uev_trigger()?
> With this patch, uev_update_path()/uev_remove_path() would still be 
> called which would generate further log.
> Should we avoid these function calls too?

You are right, thanks for pointing that out. I'll post an update.

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

[PATCH v2] multipathd: ignore uevents for non-mpath devices

[Martin Wilck]

multipathd can't deal with other devices anyway. Proceeding further
with events for other devices just generates log noise.

Based on an idea from Ritika Srivastava <ritika.srivastava@oracle.com>.
("multipath-tools: Skip CHANGE uevent for non-mpath devices").

Changes in v2: always return immediately for non-mpath case
  (Ritika Srivastava)

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

diff --git a/multipathd/main.c b/multipathd/main.c
index ff3ecb640487..a8a0c302e8fe 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1122,6 +1122,8 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 	 * are not fully initialised then.
 	 */
 	if (!strncmp(uev->kernel, "dm-", 3)) {
+		if (!uevent_is_mpath(uev))
+			goto out;
 		if (!strncmp(uev->action, "change", 6)) {
 			r = uev_add_map(uev, vecs);
 
@@ -1132,11 +1134,8 @@ uev_trigger (struct uevent * uev, void * trigger_data)
 			 * cess.
 			 */
 			uev_pathfail_check(uev, vecs);
-			goto out;
-		}
-		if (!strncmp(uev->action, "remove", 6)) {
+		} else if (!strncmp(uev->action, "remove", 6)) {
 			r = uev_remove_map(uev, vecs);
-			goto out;
 		}
 		goto out;
 	}
-- 
2.16.1

Re: [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests

[Benjamin Marzinski]

On Wed, Jan 17, 2018 at 08:49:32AM +0100, Martin Wilck wrote:
> Hi Christophe,
> 
> The incentive for creating this series was Ritika's patch "multipath-tools:
> Skip CHANGE uevent for non-mpath devices" from October 2017, plus the
> observation that uevent.c contains a lot of code repetition.
> I've refactored and simplified the uevent_get_XXX code, and started to use
> "const" qualifiers in the part of the code which I touched in the process.
> 
> In order to avoid regressions in the process, I took this as a starting point
> for implementing a simple unit test framework. For now, it covers only the
> stuff that I changed in this series.
> 
> The series is based on my previous patch series ("Various 
> multipath-tools fixes"), v2. The first patch doesn't belong in the series
> logically, but I thought it may help you track the ordering if I include it
> here rather then posting separately or spamming the list with the big series
> again.
> 
> Regards,
> Martin

Looks good.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>

> 
> Martin Wilck (7):
>   assemble_map: no newline at end of params string
>   tests: cmocka-based unit test for uevent_get_XXX
>   libmultipath: refactor uevent_get_XXX
>   libmultipath: const qualifier for wwid and alias
>   libmultipath: move UUID_PREFIX to devmapper.h
>   libmultipath: add uevent_is_mpath
>   multipathd: ignore uevents for non-mpath devices
> 
>  .gitignore                 |   3 +
>  Makefile                   |   4 +
>  libmultipath/devmapper.c   |   3 -
>  libmultipath/devmapper.h   |   3 +
>  libmultipath/discovery.c   |   2 +-
>  libmultipath/discovery.h   |   2 +-
>  libmultipath/dmparser.c    |   1 -
>  libmultipath/memory.h      |   2 +-
>  libmultipath/structs.c     |   4 +-
>  libmultipath/structs.h     |   4 +-
>  libmultipath/structs_vec.c |   2 +-
>  libmultipath/structs_vec.h |   2 +-
>  libmultipath/uevent.c      | 183 +++++++++++++--------------
>  libmultipath/uevent.h      |  15 +--
>  multipathd/main.c          |  10 +-
>  multipathd/main.h          |   2 +-
>  tests/Makefile             |  23 ++++
>  tests/globals.c            |  17 +++
>  tests/uevent.c             | 304 +++++++++++++++++++++++++++++++++++++++++++++
>  19 files changed, 462 insertions(+), 124 deletions(-)
>  create mode 100644 tests/Makefile
>  create mode 100644 tests/globals.c
>  create mode 100644 tests/uevent.c
> 
> -- 
> 2.15.1

Re: [PATCH v2] multipathd: ignore uevents for non-mpath devices

[Ritika Srivastava]

On 01/29/2018 03:57 PM, Martin Wilck wrote:
> multipathd can't deal with other devices anyway. Proceeding further
> with events for other devices just generates log noise.
>
> Based on an idea from Ritika Srivastava <ritika.srivastava@oracle.com>.
> ("multipath-tools: Skip CHANGE uevent for non-mpath devices").
>
> Changes in v2: always return immediately for non-mpath case
>    (Ritika Srivastava)
>
> Signed-off-by: Martin Wilck <mwilck@suse.com>
> ---
>   multipathd/main.c | 7 +++----
>   1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/multipathd/main.c b/multipathd/main.c
> index ff3ecb640487..a8a0c302e8fe 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1122,6 +1122,8 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>   	 * are not fully initialised then.
>   	 */
>   	if (!strncmp(uev->kernel, "dm-", 3)) {
> +		if (!uevent_is_mpath(uev))
> +			goto out;
>   		if (!strncmp(uev->action, "change", 6)) {
>   			r = uev_add_map(uev, vecs);
>   
> @@ -1132,11 +1134,8 @@ uev_trigger (struct uevent * uev, void * trigger_data)
>   			 * cess.
>   			 */
>   			uev_pathfail_check(uev, vecs);
> -			goto out;
> -		}
> -		if (!strncmp(uev->action, "remove", 6)) {
> +		} else if (!strncmp(uev->action, "remove", 6)) {
>   			r = uev_remove_map(uev, vecs);
> -			goto out;
>   		}
>   		goto out;
>   	}
The updated patch looks good.

Reviewed-by: Ritika Srivastava <ritika.srivastava@oracle.com>

-- 
Thanks,
Ritika