* [PATCH 1/7] assemble_map: no newline at end of params string
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 2/7] tests: cmocka-based unit test for uevent_get_XXX Martin Wilck
` (6 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 2/7] tests: cmocka-based unit test for uevent_get_XXX
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
2018-01-17 7:49 ` [PATCH 1/7] assemble_map: no newline at end of params string Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 3/7] libmultipath: refactor uevent_get_XXX Martin Wilck
` (5 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 3/7] libmultipath: refactor uevent_get_XXX
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
2018-01-17 7:49 ` [PATCH 1/7] assemble_map: no newline at end of params string Martin Wilck
2018-01-17 7:49 ` [PATCH 2/7] tests: cmocka-based unit test for uevent_get_XXX Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 4/7] libmultipath: const qualifier for wwid and alias Martin Wilck
` (4 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 4/7] libmultipath: const qualifier for wwid and alias
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
` (2 preceding siblings ...)
2018-01-17 7:49 ` [PATCH 3/7] libmultipath: refactor uevent_get_XXX Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 5/7] libmultipath: move UUID_PREFIX to devmapper.h Martin Wilck
` (3 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 5/7] libmultipath: move UUID_PREFIX to devmapper.h
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
` (3 preceding siblings ...)
2018-01-17 7:49 ` [PATCH 4/7] libmultipath: const qualifier for wwid and alias Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 6/7] libmultipath: add uevent_is_mpath Martin Wilck
` (2 subsequent siblings)
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 6/7] libmultipath: add uevent_is_mpath
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
` (4 preceding siblings ...)
2018-01-17 7:49 ` [PATCH 5/7] libmultipath: move UUID_PREFIX to devmapper.h Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-17 7:49 ` [PATCH 7/7] multipathd: ignore uevents for non-mpath devices Martin Wilck
2018-02-07 18:35 ` [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Benjamin Marzinski
7 siblings, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* [PATCH 7/7] multipathd: ignore uevents for non-mpath devices
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
` (5 preceding siblings ...)
2018-01-17 7:49 ` [PATCH 6/7] libmultipath: add uevent_is_mpath Martin Wilck
@ 2018-01-17 7:49 ` Martin Wilck
2018-01-29 22:25 ` Ritika Srivastava
2018-02-07 18:35 ` [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Benjamin Marzinski
7 siblings, 1 reply; 13+ messages in thread
From: Martin Wilck @ 2018-01-17 7:49 UTC (permalink / raw)
To: Christophe Varoqui, dm-devel
Cc: Ritika Srivastava, Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH 7/7] multipathd: ignore uevents for non-mpath devices
2018-01-17 7:49 ` [PATCH 7/7] multipathd: ignore uevents for non-mpath devices Martin Wilck
@ 2018-01-29 22:25 ` Ritika Srivastava
2018-01-29 23:44 ` Martin Wilck
2018-01-29 23:57 ` [PATCH v2] " Martin Wilck
0 siblings, 2 replies; 13+ messages in thread
From: Ritika Srivastava @ 2018-01-29 22:25 UTC (permalink / raw)
To: Martin Wilck, Christophe Varoqui, dm-devel; +Cc: Xose Vazquez Perez
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
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH 7/7] multipathd: ignore uevents for non-mpath devices
2018-01-29 22:25 ` Ritika Srivastava
@ 2018-01-29 23:44 ` Martin Wilck
2018-01-29 23:57 ` [PATCH v2] " Martin Wilck
1 sibling, 0 replies; 13+ messages in thread
From: Martin Wilck @ 2018-01-29 23:44 UTC (permalink / raw)
To: Ritika Srivastava, Christophe Varoqui, dm-devel; +Cc: Xose Vazquez Perez
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
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH v2] multipathd: ignore uevents for non-mpath devices
2018-01-29 22:25 ` Ritika Srivastava
2018-01-29 23:44 ` Martin Wilck
@ 2018-01-29 23:57 ` Martin Wilck
2018-02-07 19:14 ` Ritika Srivastava
1 sibling, 1 reply; 13+ messages in thread
From: Martin Wilck @ 2018-01-29 23:57 UTC (permalink / raw)
To: Christophe Varoqui, ritika.srivastava, dm-devel
Cc: Xose Vazquez Perez, 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
^ permalink raw reply related [flat|nested] 13+ messages in thread* Re: [PATCH v2] multipathd: ignore uevents for non-mpath devices
2018-01-29 23:57 ` [PATCH v2] " Martin Wilck
@ 2018-02-07 19:14 ` Ritika Srivastava
0 siblings, 0 replies; 13+ messages in thread
From: Ritika Srivastava @ 2018-02-07 19:14 UTC (permalink / raw)
To: Martin Wilck, Christophe Varoqui, dm-devel; +Cc: Xose Vazquez Perez
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
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests
2018-01-17 7:49 [PATCH 0/7] multipath-tools: uevent processing fixes and unit tests Martin Wilck
` (6 preceding siblings ...)
2018-01-17 7:49 ` [PATCH 7/7] multipathd: ignore uevents for non-mpath devices Martin Wilck
@ 2018-02-07 18:35 ` Benjamin Marzinski
7 siblings, 0 replies; 13+ messages in thread
From: Benjamin Marzinski @ 2018-02-07 18:35 UTC (permalink / raw)
To: Martin Wilck; +Cc: dm-devel, Ritika Srivastava, Xose Vazquez Perez
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
^ permalink raw reply [flat|nested] 13+ messages in thread