Re: [dm-devel] [RFC PATCH] multipathd: strict_timing without signals
On Wed, Mar 07, 2018 at 03:17:51PM +0100, Martin Wilck wrote:
> The internal usage of SIGALRM by setitimer() function may cause subtle
> conflicts with other uses of SIGALRM, either by multipath-tools code itself
> (e.g. lock_file()) or libc (e.g. glob()).
>
> This patch changes the checkerloop to use an interval timer and a pthread
> condition variable. No signals are used except those used by libpthread
> behind the scenes.
>
I'm not sure that I'd call this simpler. Personally, I prefer using
nanosleep(), since it is just a system call instead of a library that
creates a thread every second to wake up the checker thread. But I don't
actually think that there's anything wrong with this, and I'm sure it
will come closer to waking the checker loop on the second, so if you're
strongly in favor of it, that's fine. Otherwise, I'll just rebase my
nanosleep patch.
-Ben
> Signed-off-by: Martin Wilck
> ---
> multipathd/Makefile | 2 +-
> multipathd/main.c | 113
> ++--
> 2 files changed, 85 insertions(+), 30 deletions(-)
>
> diff --git a/multipathd/Makefile b/multipathd/Makefile
> index 4c9d29634160..c348f02ef7f8 100644
> --- a/multipathd/Makefile
> +++ b/multipathd/Makefile
> @@ -10,7 +10,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir)
> -I$(mpathpersistdir) \
> -I$(mpathcmddir) -I$(thirdpartydir)
> LDFLAGS += $(BIN_LDFLAGS)
> LIBDEPS += -L$(multipathdir) -lmultipath -L$(mpathpersistdir) -lmpathpersist
> \
> --L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread \
> +-L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread -lrt \
> -ldevmapper -lreadline
>
> ifdef SYSTEMD
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 6e6c52a52783..c9c57c5baece 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -1848,6 +1848,68 @@ static void init_path_check_interval(struct vectors
> *vecs)
> }
> }
>
> +static pthread_mutex_t checker_lock = PTHREAD_MUTEX_INITIALIZER;
> +static pthread_cond_t checker_cond = PTHREAD_COND_INITIALIZER;
> +
> +static void checker_notify(union sigval sv) {
> +
> + pthread_mutex_lock(&checker_lock);
> + pthread_cond_broadcast(&checker_cond);
> + pthread_mutex_unlock(&checker_lock);
> +}
> +
> +static void unlock_mutex(void *arg)
> +{
> + pthread_mutex_unlock((pthread_mutex_t*)arg);
> +}
> +
> +void cleanup_timer(void *arg)
> +{
> + timer_t tmr = (timer_t)arg;
> +
> + if (tmr != 0)
> + timer_delete(tmr);
> +}
> +
> +static timer_t setup_check_timer(void)
> +{
> + timer_t tmr = (timer_t)0;
> + struct sigevent se;
> +
> + memset(&se, 0, sizeof(se));
> + se.sigev_notify = SIGEV_THREAD;
> + se.sigev_notify_function = checker_notify;
> +
> + if (timer_create(CLOCK_MONOTONIC, &se, &tmr)) {
> + condlog(2, "%s: errno %d creating monotonic timer",
> + __func__, errno);
> + if (timer_create(CLOCK_REALTIME, &se, &tmr))
> + condlog(0, "%s: errno %d creating timer",
> + __func__, errno);
> + }
> + return tmr;
> +}
> +
> +static int set_check_timer(timer_t tmr, bool arm)
> +{
> + struct itimerspec is;
> +
> + if (tmr == 0)
> + return -1;
> +
> + if (arm) {
> + is.it_value.tv_sec = 1;
> + is.it_interval.tv_sec = 1;
> + } else {
> + is.it_value.tv_sec = 0;
> + is.it_interval.tv_sec = 0;
> + }
> + is.it_value.tv_nsec = 0;
> + is.it_interval.tv_nsec = 0;
> +
> + return timer_settime(tmr, 0, &is, NULL);
> +}
> +
> static void *
> checkerloop (void *ap)
> {
> @@ -1855,9 +1917,10 @@ checkerloop (void *ap)
> struct path *pp;
> int count = 0;
> unsigned int i;
> - struct itimerval timer_tick_it;
> struct timespec last_time;
> struct config *conf;
> + timer_t check_timer;
> + int old_strict_timing = 0;
>
> pthread_cleanup_push(rcu_unregister, NULL);
> rcu_register_thread();
> @@ -1865,6 +1928,9 @@ checkerloop (void *ap)
> vecs = (struct vectors *)ap;
> condlog(2, "path checkers start up");
>
> + check_timer = setup_check_timer();
> + pthread_cleanup_push(cleanup_timer, (void*)check_timer);
> +
> /* Tweak start time for initial path check */
> if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0)
> last_time.tv_sec = 0;
> @@ -1873,8 +1939,7 @@ checkerloop (void *ap)
>
> while (1) {
> struct timespec diff_time, start_time, end_time;
> - int num_paths = 0, ticks = 0, signo, strict_timing, rc = 0;
> - sigset_t mask;
> + int num_paths = 0, ticks = 0, strict_timing, rc = 0;
>
> if (clock_gettime(CLOCK_MONOTONIC, &start_time) != 0)
> start_time.tv_sec = 0;
> @@ -1956,40 +2021,30 @@ checkerloop (void *ap)
> }
> check_foreign
[dm-devel] [PATCH v2 1/5] tests: add unit tests for config file parser
Add test cases for parsing the config file.
Some of these tests currently fail. The patches that follow fix them.
Signed-off-by: Martin Wilck
---
tests/Makefile | 2 +-
tests/globals.c | 1 +
tests/parser.c | 479
3 files changed, 481 insertions(+), 1 deletion(-)
create mode 100644 tests/parser.c
diff --git a/tests/Makefile b/tests/Makefile
index 7ae6b9012b5a..81f5518b05d0 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -3,7 +3,7 @@ include ../Makefile.inc
CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
-TESTS := uevent
+TESTS := uevent parser
.SILENT: $(TESTS:%=%.o)
.PRECIOUS: $(TESTS:%=%-test)
diff --git a/tests/globals.c b/tests/globals.c
index 96a56515fd09..80f57bd3639a 100644
--- a/tests/globals.c
+++ b/tests/globals.c
@@ -6,6 +6,7 @@ struct udev *udev;
int logsink = 0;
struct config conf = {
.uid_attrs = "sd:ID_BOGUS",
+ .verbosity = 4,
};
struct config *get_multipath_config(void)
diff --git a/tests/parser.c b/tests/parser.c
new file mode 100644
index ..8e73cebd878a
--- /dev/null
+++ b/tests/parser.c
@@ -0,0 +1,479 @@
+/*
+ * 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
+#include
+#include
+#include
+#include
+#include
+// #include "list.h"
+#include "parser.h"
+#include "vector.h"
+
+#include "globals.c"
+
+/* Set these to 1 to get success for current broken behavior */
+/* Strip leading whitespace between quotes */
+#define LSTRIP_QUOTED_WSP 0
+/* Stop parsing at 2nd quote */
+#define TWO_QUOTES_ONLY 0
+
+static bool is_quote(const char *s)
+{
+ return *s == '"';
+}
+
+static char *test_file = "test.conf";
+
+/* Missing declaration */
+int validate_config_strvec(vector strvec, char *file);
+
+/* Stringify helpers */
+#define _str_(x) #x
+#define str(x) _str_(x)
+
+static int setup(void **state)
+{
+ return 0;
+}
+
+static int teardown(void **state)
+{
+ return 0;
+}
+
+static void test01(void **state)
+{
+ vector v = alloc_strvec("keyword value");
+ char *val;
+
+ assert_int_equal(validate_config_strvec(v, test_file), 0);
+ assert_int_equal(VECTOR_SIZE(v), 2);
+ assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+ assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+ val = set_value(v);
+ assert_string_equal(val, "value");
+
+ free(val);
+ free_strvec(v);
+}
+
+static void test02(void **state)
+{
+ vector v = alloc_strvec("keyword \"value\"");
+ char *val;
+
+ assert_int_equal(validate_config_strvec(v, test_file), 0);
+ assert_int_equal(VECTOR_SIZE(v), 4);
+ assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+ assert_true(is_quote(VECTOR_SLOT(v, 1)));;
+ assert_string_equal(VECTOR_SLOT(v, 2), "value");
+ assert_true(is_quote(VECTOR_SLOT(v, 3)));;
+
+ val = set_value(v);
+ assert_string_equal(val, "value");
+
+ free(val);
+ free_strvec(v);
+}
+
+static void test03(void **state)
+{
+ vector v = alloc_strvec("keyword value\n");
+ char *val;
+
+ assert_int_equal(validate_config_strvec(v, test_file), 0);
+ assert_int_equal(VECTOR_SIZE(v), 2);
+ assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+ assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+ val = set_value(v);
+ assert_string_equal(val, "value");
+
+ free(val);
+ free_strvec(v);
+}
+
+static void test04(void **state)
+{
+ vector v = alloc_strvec("keyword \t value \t \n ");
+ char *val;
+
+ assert_int_equal(validate_config_strvec(v, test_file), 0);
+ assert_int_equal(VECTOR_SIZE(v), 2);
+ assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+ assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+ val = set_value(v);
+ assert_string_equal(val, "value");
+
+ free(val);
+ free_strvec(v);
+}
+
+static void test05(void **state)
+{
+ vector v = alloc_strvec("keyword \t value \t ! comment ");
+ char *val;
+
+ assert_int_equal(validate_config_strvec(v, test_file), 0);
+ assert_int_equal(VECTOR_SIZE(v), 2);
+ assert_string_equal(VECTOR_SLOT(v, 0), "keyword");
+ assert_string_equal(VECTOR_SLOT(v, 1), "value");
+
+ val
[dm-devel] [PATCH v2 4/5] libmultipath: config parser: fix corner case for double quotes
A corner case of the previous patch are strings starting with a double quote,
such as '"prepended to itself is false" prepended to itself is false' or
'"" is the empty string', and in particular, the string '"' ("\"" in C
notation), which is indistinguishable from the "QUOTE" token in the parsed
strvec.
This patch fixes that by introducing a special token that can't occur as part
of a normal string to indicate the beginning and end of a quoted string.
'"' is admittedly not a very likely keyword value for multipath.conf, but
a) this is a matter of correctness, b) we didn't think of '2.5"' before,
either, and
c) the (*str != '"') expressions would need to be patched anyway to fix the
'string starting with "' case.
Signed-off-by: Martin Wilck
---
libmultipath/parser.c | 31 ++-
libmultipath/parser.h | 1 +
multipathd/cli.c | 2 +-
tests/parser.c| 5 -
4 files changed, 20 insertions(+), 19 deletions(-)
diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 21151a16ad74..cee1c9681361 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -186,6 +186,12 @@ snprint_keyword(char *buff, int len, char *fmt, struct
keyword *kw,
return fwd;
}
+static const char quote_marker[] = { '\0', '"', '\0' };
+bool is_quote(const char* token)
+{
+ return !memcmp(token, quote_marker, sizeof(quote_marker));
+}
+
vector
alloc_strvec(char *string)
{
@@ -227,13 +233,12 @@ alloc_strvec(char *string)
start = cp;
if (*cp == '"' && !(in_string && *(cp + 1) == '"')) {
cp++;
- token = MALLOC(2);
+ token = MALLOC(sizeof(quote_marker));
if (!token)
goto out;
- *(token) = '"';
- *(token + 1) = '\0';
+ memcpy(token, quote_marker, sizeof(quote_marker));
if (in_string)
in_string = 0;
else
@@ -324,13 +329,13 @@ set_value(vector strvec)
(char *)VECTOR_SLOT(strvec, 0));
return NULL;
}
- size = strlen(str);
- if (size == 0) {
- condlog(0, "option '%s' has empty value",
- (char *)VECTOR_SLOT(strvec, 0));
- return NULL;
- }
- if (*str != '"') {
+ if (!is_quote(str)) {
+ size = strlen(str);
+ if (size == 0) {
+ condlog(0, "option '%s' has empty value",
+ (char *)VECTOR_SLOT(strvec, 0));
+ return NULL;
+ }
alloc = MALLOC(sizeof (char) * (size + 1));
if (alloc)
memcpy(alloc, str, size);
@@ -354,7 +359,7 @@ set_value(vector strvec)
(char *)VECTOR_SLOT(strvec, 0));
return NULL;
}
- if (*str == '"')
+ if (is_quote(str))
break;
tmp = alloc;
/* The first +1 is for the NULL byte. The rest are for the
@@ -460,7 +465,7 @@ validate_config_strvec(vector strvec, char *file)
(char *)VECTOR_SLOT(strvec, 0), line_nr, file);
return -1;
}
- if (*str != '"') {
+ if (!is_quote(str)) {
if (VECTOR_SIZE(strvec) > 2)
condlog(0, "ignoring extra data starting with '%s' on
line %d of %s", (char *)VECTOR_SLOT(strvec, 2), line_nr, file);
return 0;
@@ -472,7 +477,7 @@ validate_config_strvec(vector strvec, char *file)
line_nr, file);
return -1;
}
- if (*str == '"') {
+ if (is_quote(str)) {
if (VECTOR_SIZE(strvec) > i + 1)
condlog(0, "ignoring extra data starting with
'%s' on line %d of %s", (char *)VECTOR_SLOT(strvec, (i + 1)), line_nr, file);
return 0;
diff --git a/libmultipath/parser.h b/libmultipath/parser.h
index 0a747507d7be..62906e98c1f7 100644
--- a/libmultipath/parser.h
+++ b/libmultipath/parser.h
@@ -81,5 +81,6 @@ extern int process_file(struct config *conf, char *conf_file);
extern struct keyword * find_keyword(vector keywords, vector v, char * name);
int snprint_keyword(char *buff, int len, char *fmt, struct keyword *kw,
const void *data);
+bool is_quote(const char* token);
#endif
diff --git a/multipathd/cli.c b/multipathd/cli.c
index f10f862cd14f..bf25b41fd5dd 100644
--- a/multipathd/cli.c
+++ b/multipathd/cli.c
@@ -279,7 +279,7 @@ get_cmdvec (char * cmd, vector *v)
}
vector_foreach_slot(strvec, buff, i) {
- if (*buff == '"')
+ if (is_quote(buff))
[dm-devel] [PATCH v2 5/5] multipath.conf(5): improve syntax documentation
Describe the syntax of attribute / value pairs, comments, and quoted
strings, as well as the peculiarities of section beginnings and ends.
Also describe the newly added '""' feature.
Signed-off-by: Martin Wilck
---
multipath/multipath.conf.5 | 17 +
1 file changed, 17 insertions(+)
diff --git a/multipath/multipath.conf.5 b/multipath/multipath.conf.5
index ab151e720d75..c4d0789475a3 100644
--- a/multipath/multipath.conf.5
+++ b/multipath/multipath.conf.5
@@ -67,6 +67,23 @@ recognized keywords for attributes or subsections depend on
the
section in which they occur.
.LP
.
+\fB\fR and \fB\fR must be on a single line.
+\fB\fR is one of the keywords listed in this man page.
+\fB\fR is either a simple word (containing no whitespace and none of the
+characters '\(dq', '#', and '!') or \fIone\fR string enclosed in double
+quotes ("..."). Outside a quoted string, text starting with '#', and '!' is
+regarded as a comment and ignored until the end of the line. Inside a quoted
+string, '#' and '!' are normal characters, and whitespace is preserved.
+To represent a double quote character inside a double quoted string, use two
+consecutive double quotes ('""'). Thus '2.5\(dq SSD' can be written as "2.5""
SSD".
+.LP
+.
+Opening braces ('{') must follow the (sub)section name on the same line.
Closing
+braces ('}') that mark the end of a (sub)section must be the only
non-whitespace
+character on the line. Whitespace is ignored except inside double quotes, thus
+the indentation shown in the above example is helpful for human readers but
+not mandatory.
+.LP
.
The following \fIsection\fP keywords are recognized:
.TP 17
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 0/5] Fixes for config file parsing
This series was motivated by the real-world problem that a user couldn't figure out how to write a blacklist entry for a device called '1.8" SSD'. Fixing this for good turned out to be a little tricky, therefore I also added a test suite. Changes since v1: - fixed a problem with parsing the command strings from cli handlers. Martin Wilck (5): tests: add unit tests for config file parser libmultipath: config parser: don't strip whitepace between quotes libmultipath: config parser: Allow '"' in strings libmultipath: config parser: fix corner case for double quotes multipath.conf(5): improve syntax documentation libmultipath/parser.c | 60 -- libmultipath/parser.h | 1 + multipath/multipath.conf.5 | 17 ++ multipathd/cli.c | 2 +- tests/Makefile | 2 +- tests/globals.c| 1 + tests/parser.c | 474 + 7 files changed, 540 insertions(+), 17 deletions(-) create mode 100644 tests/parser.c -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 3/5] libmultipath: config parser: Allow '"' in strings
We have seen model strings lile '2.5" SSD' which can't be parsed
by the current config parser. This patch fixes this by allowing
'""' to represent a double quote character inside a a string.
The above model string could now be entered in the config file like this:
blacklist {
vendor SomeCorp
product "2.5"" SSD"
}
Signed-off-by: Martin Wilck
---
libmultipath/parser.c | 26 +-
1 file changed, 25 insertions(+), 1 deletion(-)
diff --git a/libmultipath/parser.c b/libmultipath/parser.c
index 3d9656f47945..21151a16ad74 100644
--- a/libmultipath/parser.c
+++ b/libmultipath/parser.c
@@ -219,11 +219,13 @@ alloc_strvec(char *string)
in_string = 0;
while (1) {
+ int two_quotes = 0;
+
if (!vector_alloc_slot(strvec))
goto out;
start = cp;
- if (*cp == '"') {
+ if (*cp == '"' && !(in_string && *(cp + 1) == '"')) {
cp++;
token = MALLOC(2);
@@ -246,11 +248,23 @@ alloc_strvec(char *string)
*(token + 1) = '\0';
cp++;
} else {
+
+ move_on:
while ((in_string ||
(!isspace((int) *cp) && isascii((int) *cp) &&
*cp != '!' && *cp != '#' && *cp != '{' &&
*cp != '}')) && *cp != '\0' && *cp != '"')
cp++;
+
+ /* Two consecutive double quotes - don't end string */
+ if (in_string && *cp == '"') {
+ if (*(cp + 1) == '"') {
+ two_quotes = 1;
+ cp += 2;
+ goto move_on;
+ }
+ }
+
strlen = cp - start;
token = MALLOC(strlen + 1);
@@ -259,6 +273,16 @@ alloc_strvec(char *string)
memcpy(token, start, strlen);
*(token + strlen) = '\0';
+
+ /* Replace "" by " */
+ if (two_quotes) {
+ char *qq = strstr(token, "\"\"");
+ while (qq != NULL) {
+ memmove(qq + 1, qq + 2,
+ strlen + 1 - (qq + 2 - token));
+ qq = strstr(qq + 1, "\"\"");
+ }
+ }
}
vector_set_slot(strvec, token);
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v2 2/5] libmultipath: config parser: don't strip whitepace between quotes
Between double quotes, the parser currently strips leading (but not trailing) whitespace. That's inconsistent and unexpected. Fix it. Signed-off-by: Martin Wilck --- libmultipath/parser.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/libmultipath/parser.c b/libmultipath/parser.c index 5caa2019a1a4..3d9656f47945 100644 --- a/libmultipath/parser.c +++ b/libmultipath/parser.c @@ -262,7 +262,8 @@ alloc_strvec(char *string) } vector_set_slot(strvec, token); - while ((isspace((int) *cp) || !isascii((int) *cp)) + while ((!in_string && + (isspace((int) *cp) || !isascii((int) *cp))) && *cp != '\0') cp++; if (*cp == '\0' || *cp == '!' || *cp == '#') -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH RESEND 2/4] libmultipath: get_uid: don't quit prematurely without udev
Not all the implemented methods to derive the UID rely on udev
information being present. For example getuid callout, rbd,
and the SCSI vpd code work fine without it. It's unlikely that
we don't get udev data, but we want to be as good as possible
at deriving the uid.
Signed-off-by: Martin Wilck
---
libmultipath/discovery.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 53182a85fa10..9f2a9c907914 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1853,11 +1853,6 @@ get_uid (struct path * pp, int path_state, struct
udev_device *udev)
put_multipath_config(conf);
}
- if (!udev) {
- condlog(1, "%s: no udev information", pp->dev);
- return 1;
- }
-
memset(pp->wwid, 0, WWID_SIZE);
if (pp->getuid) {
char buff[CALLOUT_MAX_SIZE];
@@ -1881,7 +1876,7 @@ get_uid (struct path * pp, int path_state, struct
udev_device *udev)
origin = "sysfs";
} else {
- if (pp->uid_attribute) {
+ if (udev && pp->uid_attribute) {
len = get_udev_uid(pp, pp->uid_attribute, udev);
origin = "udev";
if (len <= 0)
@@ -1900,6 +1895,7 @@ get_uid (struct path * pp, int path_state, struct
udev_device *udev)
condlog(1, "%s: failed to get %s uid: %s",
pp->dev, origin, strerror(-len));
memset(pp->wwid, 0x0, WWID_SIZE);
+ return 1;
} else {
/* Strip any trailing blanks */
c = strchr(pp->wwid, '\0');
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH RESEND 1/4] libmultipath: get_uid: check VPD pages for SCSI only
The VPD code won't work for non-SCSI devices, anyway. For indentation
reasons, I moved the "retrigger_tries" case to a separate function,
which is also called only for SCSI devices.
Signed-off-by: Martin Wilck
---
libmultipath/discovery.c | 50 +++-
1 file changed, 32 insertions(+), 18 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 3d38a2550980..53182a85fa10 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1807,9 +1807,38 @@ get_vpd_uid(struct path * pp)
parent = udev_device_get_parent(parent);
}
+ if (!parent)
+ return -EINVAL;
+
return get_vpd_sysfs(parent, 0x83, pp->wwid, WWID_SIZE);
}
+static ssize_t scsi_uid_fallback(struct path *pp, int path_state,
+const char **origin)
+{
+ ssize_t len = 0;
+ int retrigger;
+ struct config *conf;
+
+ conf = get_multipath_config();
+ retrigger = conf->retrigger_tries;
+ put_multipath_config(conf);
+ if (pp->retriggers >= retrigger &&
+ !strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
+ len = get_vpd_uid(pp);
+ *origin = "sysfs";
+ pp->uid_attribute = NULL;
+ if (len < 0 && path_state == PATH_UP) {
+ condlog(1, "%s: failed to get sysfs uid: %s",
+ pp->dev, strerror(-len));
+ len = get_vpd_sgio(pp->fd, 0x83, pp->wwid,
+ WWID_SIZE);
+ *origin = "sgio";
+ }
+ }
+ return len;
+}
+
int
get_uid (struct path * pp, int path_state, struct udev_device *udev)
{
@@ -1851,7 +1880,6 @@ get_uid (struct path * pp, int path_state, struct
udev_device *udev)
len = get_rbd_uid(pp);
origin = "sysfs";
} else {
- int retrigger;
if (pp->uid_attribute) {
len = get_udev_uid(pp, pp->uid_attribute, udev);
@@ -1861,26 +1889,12 @@ get_uid (struct path * pp, int path_state, struct
udev_device *udev)
"%s: failed to get udev uid: %s",
pp->dev, strerror(-len));
- } else {
+ } else if (pp->bus == SYSFS_BUS_SCSI) {
len = get_vpd_uid(pp);
origin = "sysfs";
}
- conf = get_multipath_config();
- retrigger = conf->retrigger_tries;
- put_multipath_config(conf);
- if (len <= 0 && pp->retriggers >= retrigger &&
- !strcmp(pp->uid_attribute, DEFAULT_UID_ATTRIBUTE)) {
- len = get_vpd_uid(pp);
- origin = "sysfs";
- pp->uid_attribute = NULL;
- if (len < 0 && path_state == PATH_UP) {
- condlog(1, "%s: failed to get sysfs uid: %s",
- pp->dev, strerror(-len));
- len = get_vpd_sgio(pp->fd, 0x83, pp->wwid,
- WWID_SIZE);
- origin = "sgio";
- }
- }
+ if (len <= 0 && pp->bus == SYSFS_BUS_SCSI)
+ len = scsi_uid_fallback(pp, path_state, &origin);
}
if ( len < 0 ) {
condlog(1, "%s: failed to get %s uid: %s",
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH RESEND 3/4] libmultipath: uev_update_path: always warn if WWID changed
Print the warning about changed WWID not only if disable_changed_wwids
is set, but always. It's actually more dangerous if that option is
not set.
Signed-off-by: Martin Wilck
---
multipathd/main.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/multipathd/main.c b/multipathd/main.c
index f85995810950..e9668205ce92 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -940,15 +940,18 @@ uev_update_path (struct uevent *uev, struct vectors *
vecs)
pp = find_path_by_dev(vecs->pathvec, uev->kernel);
if (pp) {
struct multipath *mpp = pp->mpp;
+ char wwid[WWID_SIZE];
- if (disable_changed_wwids &&
- (strlen(pp->wwid) || pp->wwid_changed)) {
- char wwid[WWID_SIZE];
+ strcpy(wwid, pp->wwid);
+ get_uid(pp, pp->state, uev->udev);
- strcpy(wwid, pp->wwid);
- get_uid(pp, pp->state, uev->udev);
- if (strcmp(wwid, pp->wwid) != 0) {
- condlog(0, "%s: path wwid changed from '%s' to
'%s'. disallowing", uev->kernel, wwid, pp->wwid);
+ if (strncmp(wwid, pp->wwid, WWID_SIZE) != 0) {
+ condlog(0, "%s: path wwid changed from '%s' to '%s'.
%s",
+ uev->kernel, wwid, pp->wwid,
+ (disable_changed_wwids ? "disallowing" :
+"continuing"));
+ if (disable_changed_wwids &&
+ (strlen(wwid) || pp->wwid_changed)) {
strcpy(pp->wwid, wwid);
if (!pp->wwid_changed) {
pp->wwid_changed = 1;
@@ -957,7 +960,9 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
dm_fail_path(pp->mpp->alias,
pp->dev_t);
}
goto out;
- } else
+ } else if (!disable_changed_wwids)
+ strcpy(pp->wwid, wwid);
+ else
pp->wwid_changed = 0;
}
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH RESEND 0/4] multipath-tools: fixes for path wwid detection and path change uevents
Hi Christophe, this small series fixes some minor glitches I found in the current path discovery code, and attempts to implement the safe part of the functionality discussed in the thread "multipathd: update path's udev in uev_update_path" in January (based on an idea from Wu Chongyun). Resending the series, unchanged, rebased on 0.7.5. Regards, Martin Martin Wilck (4): libmultipath: get_uid: check VPD pages for SCSI only libmultipath: get_uid: don't quit prematurely without udev libmultipath: uev_update_path: always warn if WWID changed libmultipath: uev_update_path: update path properties libmultipath/discovery.c | 58 multipathd/main.c| 29 +--- 2 files changed, 55 insertions(+), 32 deletions(-) -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH RESEND 4/4] libmultipath: uev_update_path: update path properties
Update pp->udev and those path attributes that can be cheaply
updated from sysfs, i.e. without IO to the disk.
Signed-off-by: Martin Wilck
---
multipathd/main.c | 8
1 file changed, 8 insertions(+)
diff --git a/multipathd/main.c b/multipathd/main.c
index e9668205ce92..6e6c52a52783 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -964,6 +964,14 @@ uev_update_path (struct uevent *uev, struct vectors * vecs)
strcpy(pp->wwid, wwid);
else
pp->wwid_changed = 0;
+ } else {
+ udev_device_unref(pp->udev);
+ pp->udev = udev_device_ref(uev->udev);
+ conf = get_multipath_config();
+ if (pathinfo(pp, conf, DI_SYSFS|DI_NOIO) != PATHINFO_OK)
+ condlog(1, "%s: pathinfo failed after change
uevent",
+ uev->kernel);
+ put_multipath_config(conf);
}
if (pp->initialized == INIT_REQUESTED_UDEV)
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 8/8] libmultipath: remove FREE_CONST() again
The FREE_CONST macro is of questionable value, as reviewers have pointed
out. The users of this macro were mostly functions that called
uevent_get_dm_xyz(). But these functions don't need to return const char*,
as they allocate the strings they return. So my change of the prototype
was wrong. This patch reverts it. The few other users of FREE_CONST can
also be reverted to use char* instead of const char* with negligible risk.
Fixes: "libmultipath: fix compiler warnings for -Wcast-qual"
Fixes: "libmultipath: const qualifier for wwid and alias"
(Note: this reverts changes not committed upstream. But as these changes are
deeply in the middle of my large-ish series of patches, it's probably easier
to simply add this patch on top than to rebase the whole series).
Signed-off-by: Martin Wilck
---
libmultipath/devmapper.c | 7 +++
libmultipath/memory.h| 6 --
libmultipath/uevent.c| 12 ++--
libmultipath/uevent.h| 6 +++---
multipathd/main.c| 16
tests/uevent.c | 16
6 files changed, 28 insertions(+), 35 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 607aea8dc1fc..767d87c8399f 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -27,7 +27,6 @@
#include
#include
-#define FREE_CONST(p) do { free((void*)(unsigned long)p); p = NULL; } while(0)
#define MAX_WAIT 5
#define LOOPS_PER_SEC 5
@@ -1415,8 +1414,8 @@ out:
void dm_reassign_deps(char *table, const char *dep, const char *newdep)
{
- char *n;
- const char *p, *newtable;
+ char *n, *newtable;
+ const char *p;
newtable = strdup(table);
if (!newtable)
@@ -1427,7 +1426,7 @@ void dm_reassign_deps(char *table, const char *dep, const
char *newdep)
n += strlen(newdep);
p += strlen(dep);
strcat(n, p);
- FREE_CONST(newtable);
+ FREE(newtable);
}
int dm_reassign_table(const char *name, char *old, char *new)
diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index 63f59d80584c..a3c478e24bd1 100644
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -43,7 +43,6 @@ int debug;
(__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
#define STRDUP(n)( dbg_strdup((n), \
(__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
-#define FREE_CONST(p) do { FREE((void*)(unsigned long)p); } while(0)
/* Memory debug prototypes defs */
extern void *dbg_malloc(unsigned long, char *, char *, int);
@@ -56,11 +55,6 @@ extern void dbg_free_final(char *);
#define MALLOC(n)(calloc(1,(n)))
#define FREE(p) do { free(p); p = NULL; } while(0)
-/*
- * Double cast to avoid warnings with -Wcast-qual
- * use this for valid free() operations on const pointers
- */
-#define FREE_CONST(p) do { free((void*)(unsigned long)p); p = NULL; } while(0)
#define REALLOC(p,n) (realloc((p),(n)))
#define STRDUP(n)(strdup(n))
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 685ef3362c6d..59d4cad88ca3 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -157,7 +157,7 @@ static int uevent_get_env_positive_int(const struct uevent
*uev,
void
uevent_get_wwid(struct uevent *uev)
{
- const char *uid_attribute;
+ char *uid_attribute;
const char *val;
struct config * conf;
@@ -168,7 +168,7 @@ uevent_get_wwid(struct uevent *uev)
val = uevent_get_env_var(uev, uid_attribute);
if (val)
uev->wwid = val;
- FREE_CONST(uid_attribute);
+ FREE(uid_attribute);
}
bool
@@ -907,7 +907,7 @@ int uevent_get_disk_ro(const struct uevent *uev)
return uevent_get_env_positive_int(uev, "DISK_RO");
}
-static const char *uevent_get_dm_str(const struct uevent *uev, char *attr)
+static char *uevent_get_dm_str(const struct uevent *uev, char *attr)
{
const char *tmp = uevent_get_env_var(uev, attr);
@@ -916,17 +916,17 @@ static const char *uevent_get_dm_str(const struct uevent
*uev, char *attr)
return strdup(tmp);
}
-const char *uevent_get_dm_name(const struct uevent *uev)
+char *uevent_get_dm_name(const struct uevent *uev)
{
return uevent_get_dm_str(uev, "DM_NAME");
}
-const char *uevent_get_dm_path(const struct uevent *uev)
+char *uevent_get_dm_path(const struct uevent *uev)
{
return uevent_get_dm_str(uev, "DM_PATH");
}
-const char *uevent_get_dm_action(const struct uevent *uev)
+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 cb5347e45c2b..0aa867510f28 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -36,9 +36,9 @@ int uevent_dispatch(int (*store_uev)(struct uevent *, void *
trigger_data),
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);
[dm-devel] [PATCH 5/8] libmultipath: Fix sgio_get_vpd()
From: Bart Van Assche
Pass the VPD page number to sgio_get_vpd() such that the page needed
by the caller is queried instead of page 0x83. Fix the statement that
computes the length of the page returned by do_inq(). Fix the return
code check in the caller of sgio_get_vpd().
Signed-off-by: Bart Van Assche
Signed-off-by: Martin Wilck
---
libmultipath/discovery.c | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 71c75872c937..23e99f05b406 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -837,6 +837,8 @@ detect_alua(struct path * pp, struct config *conf)
#define DEFAULT_SGIO_LEN 254
+/* Query VPD page @pg. Returns number of INQUIRY bytes
+ upon success and -1 upon failure. */
static int
sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int pg)
{
@@ -848,7 +850,7 @@ sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int
pg)
}
retry:
if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
- len = buff[3] + (buff[2] << 8);
+ len = buff[3] + (buff[2] << 8) + 4;
if (len >= maxlen)
return len;
if (len > DEFAULT_SGIO_LEN)
@@ -1100,7 +1102,7 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
unsigned char buff[4096];
memset(buff, 0x0, 4096);
- if (sgio_get_vpd(buff, 4096, fd, pg) <= 0) {
+ if (sgio_get_vpd(buff, 4096, fd, pg) < 0) {
condlog(3, "failed to issue vpd inquiry for pg%02x",
pg);
return -errno;
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 2/8] libmultipath: enable feature disable changed wwid by default
From: Chongyun Wu enable feature disable changed wwid by default. Signed-off-by: Chongyun Wu Signed-off-by: Martin Wilck --- libmultipath/defaults.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h index c9e3411aa579..2b270ca46f48 100644 --- a/libmultipath/defaults.h +++ b/libmultipath/defaults.h @@ -38,7 +38,7 @@ #define DEFAULT_FORCE_SYNC 0 #define DEFAULT_PARTITION_DELIMNULL #define DEFAULT_SKIP_KPARTX SKIP_KPARTX_OFF -#define DEFAULT_DISABLE_CHANGED_WWIDS 0 +#define DEFAULT_DISABLE_CHANGED_WWIDS 1 #define DEFAULT_MAX_SECTORS_KB MAX_SECTORS_KB_UNDEF #define DEFAULT_GHOST_DELAY GHOST_DELAY_OFF -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 7/8] libmultipath: fix wrong output of "multipath -t"
The default values printed by "multipath -t" or "multipathd show config" for "detect_prio", "detect_checker", and "retain_attached_hw_handler" don't match the actual compiled-in defaults. Moreover, several other options would also be displayed wrongly if the defaults were changed. Signed-off-by: Martin Wilck --- libmultipath/dict.c | 25 - 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/libmultipath/dict.c b/libmultipath/dict.c index 47dc2a38f1ac..ea273dd91962 100644 --- a/libmultipath/dict.c +++ b/libmultipath/dict.c @@ -340,7 +340,7 @@ declare_def_handler(checker_timeout, set_int) declare_def_snprint(checker_timeout, print_nonzero) declare_def_handler(flush_on_last_del, set_yes_no_undef) -declare_def_snprint_defint(flush_on_last_del, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(flush_on_last_del, print_yes_no_undef, DEFAULT_FLUSH) declare_ovr_handler(flush_on_last_del, set_yes_no_undef) declare_ovr_snprint(flush_on_last_del, print_yes_no_undef) declare_hw_handler(flush_on_last_del, set_yes_no_undef) @@ -349,7 +349,8 @@ declare_mp_handler(flush_on_last_del, set_yes_no_undef) declare_mp_snprint(flush_on_last_del, print_yes_no_undef) declare_def_handler(user_friendly_names, set_yes_no_undef) -declare_def_snprint_defint(user_friendly_names, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(user_friendly_names, print_yes_no_undef, + DEFAULT_USER_FRIENDLY_NAMES) declare_ovr_handler(user_friendly_names, set_yes_no_undef) declare_ovr_snprint(user_friendly_names, print_yes_no_undef) declare_hw_handler(user_friendly_names, set_yes_no_undef) @@ -367,21 +368,24 @@ declare_def_handler(prkeys_file, set_str) declare_def_snprint(prkeys_file, print_str) declare_def_handler(retain_hwhandler, set_yes_no_undef) -declare_def_snprint_defint(retain_hwhandler, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(retain_hwhandler, print_yes_no_undef, + DEFAULT_RETAIN_HWHANDLER) declare_ovr_handler(retain_hwhandler, set_yes_no_undef) declare_ovr_snprint(retain_hwhandler, print_yes_no_undef) declare_hw_handler(retain_hwhandler, set_yes_no_undef) declare_hw_snprint(retain_hwhandler, print_yes_no_undef) declare_def_handler(detect_prio, set_yes_no_undef) -declare_def_snprint_defint(detect_prio, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(detect_prio, print_yes_no_undef, + DEFAULT_DETECT_PRIO) declare_ovr_handler(detect_prio, set_yes_no_undef) declare_ovr_snprint(detect_prio, print_yes_no_undef) declare_hw_handler(detect_prio, set_yes_no_undef) declare_hw_snprint(detect_prio, print_yes_no_undef) declare_def_handler(detect_checker, set_yes_no_undef) -declare_def_snprint_defint(detect_checker, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(detect_checker, print_yes_no_undef, + DEFAULT_DETECT_CHECKER) declare_ovr_handler(detect_checker, set_yes_no_undef) declare_ovr_snprint(detect_checker, print_yes_no_undef) declare_hw_handler(detect_checker, set_yes_no_undef) @@ -391,7 +395,8 @@ declare_def_handler(force_sync, set_yes_no) declare_def_snprint(force_sync, print_yes_no) declare_def_handler(deferred_remove, set_yes_no_undef) -declare_def_snprint_defint(deferred_remove, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(deferred_remove, print_yes_no_undef, + DEFAULT_DEFERRED_REMOVE) declare_ovr_handler(deferred_remove, set_yes_no_undef) declare_ovr_snprint(deferred_remove, print_yes_no_undef) declare_hw_handler(deferred_remove, set_yes_no_undef) @@ -412,7 +417,8 @@ declare_def_handler(strict_timing, set_yes_no) declare_def_snprint(strict_timing, print_yes_no) declare_def_handler(skip_kpartx, set_yes_no_undef) -declare_def_snprint_defint(skip_kpartx, print_yes_no_undef, YNU_NO) +declare_def_snprint_defint(skip_kpartx, print_yes_no_undef, + DEFAULT_SKIP_KPARTX) declare_ovr_handler(skip_kpartx, set_yes_no_undef) declare_ovr_snprint(skip_kpartx, print_yes_no_undef) declare_hw_handler(skip_kpartx, set_yes_no_undef) @@ -629,7 +635,8 @@ print_fast_io_fail(char * buff, int len, long v) } declare_def_handler(fast_io_fail, set_fast_io_fail) -declare_def_snprint_defint(fast_io_fail, print_fast_io_fail, DEFAULT_FAST_IO_FAIL) +declare_def_snprint_defint(fast_io_fail, print_fast_io_fail, + DEFAULT_FAST_IO_FAIL) declare_ovr_handler(fast_io_fail, set_fast_io_fail) declare_ovr_snprint(fast_io_fail, print_fast_io_fail) declare_hw_handler(fast_io_fail, set_fast_io_fail) @@ -816,7 +823,7 @@ print_rr_weight (char * buff, int len, long v) } declare_def_handler(rr_weight, set_rr_weight) -declare_def_snprint_defint(rr_weight, print_rr_weight, RR_WEIGHT_NONE) +declare_def_snprint_defint(rr_weight, print_rr_weight, DEFAULT_RR_WEIGHT) declare_ovr_handler(rr_weight, set_rr_weight) declare_ovr_snprint(rr_weight, print_rr_weight) declare_hw_handler
[dm-devel] [PATCH 0/8] multipath-tools: missing patches in 0.7.5
Hi Christophe, I'm reposting the following patches that have been reviewed already. I think they should be merged. (Well, the last one has not officially been reviewed but it makes no functional changes and I'm quite positive Bart would approve). Regards, Martin Bart Van Assche (4): libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is '\0'-terminated kpartx: Improve reliability of find_loop_by_file() libmultipath: Fix sgio_get_vpd() Introduce the ibmultipath/unaligned.h header file Chongyun Wu (2): multipathd: add lock protection for cli_list_status libmultipath: enable feature disable changed wwid by default Martin Wilck (2): libmultipath: fix wrong output of "multipath -t" libmultipath: remove FREE_CONST() again kpartx/lopart.c | 7 --- libmpathpersist/mpath_pr_ioctl.c | 8 libmultipath/checkers/hp_sw.c | 4 ++-- libmultipath/defaults.h | 2 +- libmultipath/devmapper.c | 7 +++ libmultipath/dict.c | 25 ++- libmultipath/discovery.c | 17 +--- libmultipath/memory.h | 6 -- libmultipath/prioritizers/alua_rtpg.c | 13 ++-- libmultipath/prioritizers/alua_spc3.h | 35 ++-- libmultipath/prioritizers/ontap.c | 4 ++-- libmultipath/uevent.c | 12 +-- libmultipath/uevent.h | 6 +++--- libmultipath/unaligned.h | 38 +++ multipathd/main.c | 18 - tests/uevent.c| 16 +++ 16 files changed, 115 insertions(+), 103 deletions(-) create mode 100644 libmultipath/unaligned.h -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 3/8] libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is '\0'-terminated
From: Bart Van Assche
Discovered by Coverity (CID 173257).
Signed-off-by: Bart Van Assche
Signed-off-by: Martin Wilck
---
libmultipath/discovery.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 9efcaac81dc1..71c75872c937 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -53,8 +53,8 @@ alloc_path_with_pathinfo (struct config *conf, struct
udev_device *udevice,
if (!pp)
return PATHINFO_FAILED;
- if(wwid)
- strncpy(pp->wwid, wwid, sizeof(pp->wwid));
+ if (wwid)
+ strlcpy(pp->wwid, wwid, sizeof(pp->wwid));
if (safe_sprintf(pp->dev, "%s", devname)) {
condlog(0, "pp->dev too small");
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 4/8] kpartx: Improve reliability of find_loop_by_file()
From: Bart Van Assche
Avoid that the strchr() call in this function examines uninitialized
data on the stack. This patch avoids that Coverity reports the following:
CID 173252: Error handling issues (CHECKED_RETURN)
"read(int, void *, size_t)" returns the number of bytes read, but it is
ignored.
Signed-off-by: Bart Van Assche
Signed-off-by: Martin Wilck
---
kpartx/lopart.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kpartx/lopart.c b/kpartx/lopart.c
index 02b29e8cf91d..69c1eda1cc5c 100644
--- a/kpartx/lopart.c
+++ b/kpartx/lopart.c
@@ -64,7 +64,7 @@ char *find_loop_by_file(const char *filename)
DIR *dir;
struct dirent *dent;
char dev[64], *found = NULL, *p;
- int fd;
+ int fd, bytes_read;
struct stat statbuf;
struct loop_info loopinfo;
const char VIRT_BLOCK[] = "/sys/devices/virtual/block";
@@ -86,14 +86,15 @@ char *find_loop_by_file(const char *filename)
if (fd < 0)
continue;
- if (read(fd, dev, sizeof(dev)) <= 0) {
+ bytes_read = read(fd, dev, sizeof(dev) - 1);
+ if (bytes_read <= 0) {
close(fd);
continue;
}
close(fd);
- dev[sizeof(dev)-1] = '\0';
+ dev[bytes_read] = '\0';
p = strchr(dev, '\n');
if (p != NULL)
*p = '\0';
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 1/8] multipathd: add lock protection for cli_list_status
From: Chongyun Wu cli_list_status will access vecs->pathvec which should have lock protection, otherwise might get inconsistent data or other problem. Signed-off-by: Chongyun Wu Signed-off-by: Martin Wilck --- multipathd/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multipathd/main.c b/multipathd/main.c index 327cc191214c..6d502aceb252 100644 --- a/multipathd/main.c +++ b/multipathd/main.c @@ -1212,7 +1212,7 @@ uxlsnrloop (void * ap) set_handler_callback(LIST+PATHS+RAW+FMT, cli_list_paths_raw); set_handler_callback(LIST+PATH, cli_list_path); set_handler_callback(LIST+MAPS, cli_list_maps); - set_unlocked_handler_callback(LIST+STATUS, cli_list_status); + set_handler_callback(LIST+STATUS, cli_list_status); set_unlocked_handler_callback(LIST+DAEMON, cli_list_daemon); set_handler_callback(LIST+MAPS+STATUS, cli_list_maps_status); set_handler_callback(LIST+MAPS+STATS, cli_list_maps_stats); -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 6/8] Introduce the ibmultipath/unaligned.h header file
From: Bart Van Assche
This patch avoids that Coverity reports the following for the code
in libmultipath/prioritizers/alua_rtpg.c:
CID 173256: Integer handling issues (SIGN_EXTENSION)
Suspicious implicit sign extension: "buf[0]" with type "unsigned char" (8
bits, unsigned) is promoted in "((buf[0] << 24) | (buf[1] << 16) | (buf[2] <<
8) | buf[3]) + 4" to type "int" (32 bits, signed), then sign-extended to type
"unsigned long" (64 bits, unsigned). If "((buf[0] << 24) | (buf[1] << 16) |
(buf[2] << 8) | buf[3]) + 4" is greater than 0x7FFF, the upper bits of the
result will all be 1.
Signed-off-by: Bart Van Assche
Signed-off-by: Martin Wilck
---
libmpathpersist/mpath_pr_ioctl.c | 8
libmultipath/checkers/hp_sw.c | 4 ++--
libmultipath/discovery.c | 9 +
libmultipath/prioritizers/alua_rtpg.c | 13 ++--
libmultipath/prioritizers/alua_spc3.h | 35 ++--
libmultipath/prioritizers/ontap.c | 4 ++--
libmultipath/unaligned.h | 38 +++
7 files changed, 60 insertions(+), 51 deletions(-)
create mode 100644 libmultipath/unaligned.h
diff --git a/libmpathpersist/mpath_pr_ioctl.c b/libmpathpersist/mpath_pr_ioctl.c
index 29df8c6f2da1..dbed4ca7fad4 100644
--- a/libmpathpersist/mpath_pr_ioctl.c
+++ b/libmpathpersist/mpath_pr_ioctl.c
@@ -13,6 +13,7 @@
#include
#include "mpath_pr_ioctl.h"
#include "mpath_persist.h"
+#include "unaligned.h"
#include "debug.h"
@@ -243,10 +244,9 @@ void mpath_format_readfullstatus(struct prin_resp
*pr_buff, int len, int noisy)
memcpy(&fdesc.key, p, 8 );
fdesc.flag = p[12];
fdesc.scope_type = p[13];
- fdesc.rtpi = ((p[18] << 8) | p[19]);
+ fdesc.rtpi = get_unaligned_be16(&p[18]);
- tid_len_len = ((p[20] << 24) | (p[21] << 16) |
- (p[22] << 8) | p[23]);
+ tid_len_len = get_unaligned_be32(&p[20]);
if (tid_len_len > 0)
decode_transport_id( &fdesc, &p[24], tid_len_len);
@@ -277,7 +277,7 @@ decode_transport_id(struct prin_fulldescr *fdesc, unsigned
char * p, int length)
jump = 24;
break;
case MPATH_PROTOCOL_ID_ISCSI:
- num = ((p[2] << 8) | p[3]);
+ num = get_unaligned_be16(&p[2]);
memcpy(&fdesc->trnptid.iscsi_name, &p[4], num);
jump = (((num + 4) < 24) ? 24 : num + 4);
break;
diff --git a/libmultipath/checkers/hp_sw.c b/libmultipath/checkers/hp_sw.c
index 6019c9dbcd9d..cee9aab8d089 100644
--- a/libmultipath/checkers/hp_sw.c
+++ b/libmultipath/checkers/hp_sw.c
@@ -14,6 +14,7 @@
#include "checkers.h"
#include "../libmultipath/sg_include.h"
+#include "../libmultipath/unaligned.h"
#define TUR_CMD_LEN6
#define INQUIRY_CMDLEN 6
@@ -63,8 +64,7 @@ do_inq(int sg_fd, int cmddt, int evpd, unsigned int pg_op,
if (evpd)
inqCmdBlk[1] |= 1;
inqCmdBlk[2] = (unsigned char) pg_op;
- inqCmdBlk[3] = (unsigned char)((mx_resp_len >> 8) & 0xff);
- inqCmdBlk[4] = (unsigned char) (mx_resp_len & 0xff);
+ put_unaligned_be16(mx_resp_len, &inqCmdBlk[3]);
memset(&io_hdr, 0, sizeof (struct sg_io_hdr));
memset(sense_b, 0, SENSE_BUFF_LEN);
io_hdr.interface_id = 'S';
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index 23e99f05b406..3d38a2550980 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -30,6 +30,7 @@
#include "discovery.h"
#include "prio.h"
#include "defaults.h"
+#include "unaligned.h"
#include "prioritizers/alua_rtpg.h"
#include "foreign.h"
@@ -850,7 +851,7 @@ sgio_get_vpd (unsigned char * buff, int maxlen, int fd, int
pg)
}
retry:
if (0 == do_inq(fd, 0, 1, pg, buff, len)) {
- len = buff[3] + (buff[2] << 8) + 4;
+ len = get_unaligned_be16(&buff[2]) + 4;
if (len >= maxlen)
return len;
if (len > DEFAULT_SGIO_LEN)
@@ -881,7 +882,7 @@ static int
parse_vpd_pg80(const unsigned char *in, char *out, size_t out_len)
{
char *p = NULL;
- int len = in[3] + (in[2] << 8);
+ int len = get_unaligned_be16(&in[2]);
if (len >= out_len) {
condlog(2, "vpd pg80 overflow, %d/%d bytes required",
@@ -1081,7 +1082,7 @@ get_vpd_sysfs (struct udev_device *parent, int pg, char *
str, int maxlen)
pg, buff[1]);
return -ENODATA;
}
- buff_len = (buff[2] << 8) + buff[3] + 4;
+ buff_len = get_unaligned_be16(&buff[2]) + 4;
if (buff_len > 4096)
condlog(3, "vpd pg%02x page truncated", pg);
@@ -1113,7 +1114,7 @@ get_vpd_sgio (int fd, int pg, char * str, int maxlen)
[dm-devel] [PATCH 2/2] multipath-tools: reformat and update comments in hwtable
Cc: Christophe Varoqui
Cc: device-mapper development
Signed-off-by: Xose Vazquez Perez
---
libmultipath/hwtable.c | 24
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 65b7835..fe71d14 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -72,13 +72,13 @@
.delay_wait_checks = DELAY_CHECKS_OFF,
.skip_kpartx = SKIP_KPARTX_OFF,
.max_sectors_kb = MAX_SECTORS_KB_UNDEF,
- .ghost_delay = GHOST_DELAY_OFF
+ .ghost_delay = GHOST_DELAY_OFF,
},
#endif
static struct hwentry default_hw[] = {
/*
- * Generic NVMe devices
+ * Generic NVMe
*
* Due to the parsing logic in find_hwe(), generic entries
* have to be put on top of this list, and more specific ones
@@ -178,7 +178,7 @@ static struct hwentry default_hw[] = {
.prio_name = PRIO_ALUA,
},
{
- /* MSA 1040, 2040 and 2050 families */
+ /* MSA 1040, 1050, 2040 and 2050 families */
.vendor= "HP",
.product = "MSA [12]0[45]0 SA[NS]",
.pgpolicy = GROUP_BY_PRIO,
@@ -264,7 +264,7 @@ static struct hwentry default_hw[] = {
.no_path_retry = 30,
},
{
- /* DDN */
+ /* (DDN) */
.vendor= "SGI",
.product = "^DD[46]A-",
.pgpolicy = GROUP_BY_PRIO,
@@ -397,7 +397,7 @@ static struct hwentry default_hw[] = {
* Mail : matthias.rudo...@hds.com
*/
{
- /* USP-V, HUS VM, VSP, VSP G1000 and VSP GX00 families */
+ /* USP-V, HUS VM, VSP, VSP G1X00 and VSP GX00 families */
.vendor= "(HITACHI|HP)",
.product = "^OPEN-",
.pgpolicy = MULTIBUS,
@@ -646,7 +646,7 @@ static struct hwentry default_hw[] = {
.pgpolicy = MULTIBUS,
},
{
- /* DDN */
+ /* (DDN) DCS9900, SONAS 2851-DR1 */
.vendor= "IBM",
.product = "^(DCS9900|2851)",
.pgpolicy = GROUP_BY_PRIO,
@@ -731,12 +731,12 @@ static struct hwentry default_hw[] = {
.pgpolicy = MULTIBUS,
.no_path_retry = 24,
},
- /*
-* NetApp NVMe-FC namespace devices: MULTIBUS, queueing preferred
-*
-* The table is searched backwards, so place this after generic NVMe
-*/
{
+ /*
+* NVMe-FC namespace devices: MULTIBUS, queueing preferred
+*
+* The hwtable is searched backwards, so place this after
"Generic NVMe"
+*/
.vendor= "NVME",
.product = "^NetApp ONTAP Controller",
.uid_attribute = "ID_WWN",
@@ -1158,7 +1158,7 @@ static struct hwentry default_hw[] = {
.no_path_retry = 30,
},
/*
-* Dot Hill Systems - Seagate Technology
+* Seagate Technology (Dot Hill Systems)
*/
{
/* SANnet family */
--
2.14.3
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 0/4] multipath-tools: important fixes for 0.7.5
Hi Christophe, thanks for catching up. This series fixes immediate problems with 0.7.5. The first one corresponds to my previously posted "[PATCH v3 2/2] multipathd: start marginal path checker thread lazily", and fixes a crash. The others fix the problens reported by Xose. Regards Martin Martin Wilck (4): libmultipath: fix crash on shutdown if io_err thread isn't running multipathd: fix -Wpointer-to-int-cast warning in uxlsnr multipath: fix clang warning in delegate_to_multipathd multipath-tools: build: prevent intermediate file deletion Makefile.inc | 1 + libmultipath/checkers/Makefile | 2 ++ libmultipath/foreign/Makefile | 2 ++ libmultipath/io_err_stat.c | 3 +++ libmultipath/prioritizers/Makefile | 2 ++ multipath/main.c | 2 +- multipathd/uxlsnr.c| 4 ++-- tests/Makefile | 5 ++--- 8 files changed, 15 insertions(+), 6 deletions(-) -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 1/4] libmultipath: fix crash on shutdown if io_err thread isn't running
If we've never created the io_error checker thread, we shouldn't
cancel it.
Fixes: 160da9fa4339 "multipathd: start marginal path checker thread
lazily"
Signed-off-by: Martin Wilck
---
libmultipath/io_err_stat.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index ac81b4b9390d..02b1453ea527 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -793,6 +793,9 @@ destroy_ctx:
void stop_io_err_stat_thread(void)
{
+ if (io_err_stat_thr == (pthread_t)0)
+ return;
+
if (uatomic_read(&io_err_thread_running) == 1)
pthread_cancel(io_err_stat_thr);
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 1/2] multipath-tools: move Nimble and SGI to HPE section
They were absorbed by HPE time ago.
Cc: Christophe Varoqui
Cc: device-mapper development
Signed-off-by: Xose Vazquez Perez
---
libmultipath/hwtable.c | 106 -
1 file changed, 51 insertions(+), 55 deletions(-)
diff --git a/libmultipath/hwtable.c b/libmultipath/hwtable.c
index 6ba70c8..65b7835 100644
--- a/libmultipath/hwtable.c
+++ b/libmultipath/hwtable.c
@@ -221,6 +221,57 @@ static struct hwentry default_hw[] = {
.no_path_retry = 18,
.prio_name = PRIO_ALUA,
},
+ {
+ /* Nimble Storage */
+ .vendor= "Nimble",
+ .product = "Server",
+ .hwhandler = "1 alua",
+ .pgpolicy = GROUP_BY_PRIO,
+ .pgfailback= -FAILBACK_IMMEDIATE,
+ .prio_name = PRIO_ALUA,
+ .no_path_retry = NO_PATH_RETRY_QUEUE,
+ },
+ /* SGI */
+ {
+ .vendor= "SGI",
+ .product = "TP9100",
+ .pgpolicy = MULTIBUS,
+ },
+ {
+ /* Total Performance family */
+ .vendor= "SGI",
+ .product = "TP9[3457]00",
+ .bl_product= "Universal Xport",
+ .pgpolicy = GROUP_BY_PRIO,
+ .checker_name = RDAC,
+ .features = "2 pg_init_retries 50",
+ .hwhandler = "1 rdac",
+ .prio_name = PRIO_RDAC,
+ .pgfailback= -FAILBACK_IMMEDIATE,
+ .no_path_retry = 30,
+ },
+ {
+ /* InfiniteStorage family */
+ .vendor= "SGI",
+ .product = "IS",
+ .bl_product= "Universal Xport",
+ .pgpolicy = GROUP_BY_PRIO,
+ .checker_name = RDAC,
+ .features = "2 pg_init_retries 50",
+ .hwhandler = "1 rdac",
+ .prio_name = PRIO_RDAC,
+ .pgfailback= -FAILBACK_IMMEDIATE,
+ .no_path_retry = 30,
+ },
+ {
+ /* DDN */
+ .vendor= "SGI",
+ .product = "^DD[46]A-",
+ .pgpolicy = GROUP_BY_PRIO,
+ .pgfailback= -FAILBACK_IMMEDIATE,
+ .prio_name = PRIO_ALUA,
+ .no_path_retry = 30,
+ },
/*
* DataDirect Networks
*/
@@ -706,49 +757,6 @@ static struct hwentry default_hw[] = {
.pgpolicy = GROUP_BY_SERIAL,
.no_path_retry = 30,
},
- /*
-* SGI
-*/
- {
- .vendor= "SGI",
- .product = "TP9100",
- .pgpolicy = MULTIBUS,
- },
- {
- /* Total Performance family */
- .vendor= "SGI",
- .product = "TP9[3457]00",
- .bl_product= "Universal Xport",
- .pgpolicy = GROUP_BY_PRIO,
- .checker_name = RDAC,
- .features = "2 pg_init_retries 50",
- .hwhandler = "1 rdac",
- .prio_name = PRIO_RDAC,
- .pgfailback= -FAILBACK_IMMEDIATE,
- .no_path_retry = 30,
- },
- {
- /* InfiniteStorage family */
- .vendor= "SGI",
- .product = "IS",
- .bl_product= "Universal Xport",
- .pgpolicy = GROUP_BY_PRIO,
- .checker_name = RDAC,
- .features = "2 pg_init_retries 50",
- .hwhandler = "1 rdac",
- .prio_name = PRIO_RDAC,
- .pgfailback= -FAILBACK_IMMEDIATE,
- .no_path_retry = 30,
- },
- {
- /* DDN */
- .vendor= "SGI",
- .product = "^DD[46]A-",
- .pgpolicy = GROUP_BY_PRIO,
- .pgfailback= -FAILBACK_IMMEDIATE,
- .prio_name = PRIO_ALUA,
- .no_path_retry = 30,
- },
/*
* NEC
*/
@@ -1013,18 +1021,6 @@ static struct hwentry default_hw[] = {
.fast_io_fail = 15,
.dev_loss = 15,
},
- /*
-* Nimble Storage
-*/
- {
- .vendor= "Nimble",
- .product = "Server",
- .hwhandler = "1 alua",
- .pgpolicy = GROUP_BY_PRIO,
- .pgfailback= -FAILBACK_IMMEDIATE,
- .prio_name = PRIO_ALUA,
- .no_path_retry = NO_PATH_RETRY_QUEUE,
- },
/*
* Kaminario
*/
--
2.14.3
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 3/4] multipath: fix clang warning in delegate_to_multipathd
Fixes this warning from clang:
main.c:628:11: warning: variable 'reply' is used uninitialized
whenever 'if' condition is true [-Wsometimes-uninitialized]
...
main.c:609:32: note: initialize the variable 'reply' to silence this warning
Fixes: 506d253b7f89 "multipath: delegate dangerous commands to multipathd"
Reported-by: Xose Vazquez Perez
Signed-off-by: Martin Wilck
---
multipath/main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/multipath/main.c b/multipath/main.c
index 8732cf8bd930..716203eab56c 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -606,7 +606,7 @@ int delegate_to_multipathd(enum mpath_cmds cmd, const char
*dev,
enum devtypes dev_type, const struct config *conf)
{
int fd;
- char command[1024], *p, *reply;
+ char command[1024], *p, *reply = NULL;
int n, r = 0;
fd = mpath_connect();
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 4/4] multipath-tools: build: prevent intermediate file deletion
By default, "make" removes intermediate files from implicit rules if they are the only dependency. Prevent that by using .SECONDARY. Otherwise some files will be re-built upon second invocation of "make". Fixes: e39283ebd79b "multipath-tools: add dependency tracking to Makefiles" Reported-by: Xose Vazquez Perez Signed-off-by: Martin Wilck --- Makefile.inc | 1 + libmultipath/checkers/Makefile | 2 ++ libmultipath/foreign/Makefile | 2 ++ libmultipath/prioritizers/Makefile | 2 ++ tests/Makefile | 5 ++--- 5 files changed, 9 insertions(+), 3 deletions(-) diff --git a/Makefile.inc b/Makefile.inc index a5b9d4e3fa74..5d6123ded51a 100644 --- a/Makefile.inc +++ b/Makefile.inc @@ -127,4 +127,5 @@ check_file = $(shell\ ) %.o: %.c + @echo building $@ because of $? $(CC) $(CFLAGS) -c -o $@ $< diff --git a/libmultipath/checkers/Makefile b/libmultipath/checkers/Makefile index 9559038a770d..87c15bd777c3 100644 --- a/libmultipath/checkers/Makefile +++ b/libmultipath/checkers/Makefile @@ -40,6 +40,8 @@ clean: dep_clean $(RM) core *.a *.o *.gz *.so OBJS := $(LIBS:libcheck%.so=%.o) libsg.o directio.o +.SECONDARY: $(OBJS) + include $(wildcard $(OBJS:.o=.d)) dep_clean: diff --git a/libmultipath/foreign/Makefile b/libmultipath/foreign/Makefile index dfba11e86d76..fe98ddf7e4e9 100644 --- a/libmultipath/foreign/Makefile +++ b/libmultipath/foreign/Makefile @@ -24,6 +24,8 @@ clean: dep_clean $(RM) core *.a *.o *.gz *.so OBJS := $(LIBS:libforeign-%.so=%.o) +.SECONDARY: $(OBJS) + include $(wildcard $(OBJS:.o=.d)) dep_clean: diff --git a/libmultipath/prioritizers/Makefile b/libmultipath/prioritizers/Makefile index b3cc944c810d..ab7bc07572ec 100644 --- a/libmultipath/prioritizers/Makefile +++ b/libmultipath/prioritizers/Makefile @@ -42,6 +42,8 @@ clean: dep_clean $(RM) core *.a *.o *.gz *.so OBJS = $(LIBS:libprio%.so=%.o) alua_rtpg.o +.SECONDARY: $(OBJS) + include $(wildcard $(OBJS:.o=.d)) dep_clean: diff --git a/tests/Makefile b/tests/Makefile index f6b55836a434..7ae6b9012b5a 100644 --- a/tests/Makefile +++ b/tests/Makefile @@ -21,10 +21,9 @@ clean: dep_clean rm -f $(TESTS:%=%-test) $(TESTS:%=%.out) $(TESTS:%=%.o) OBJS = $(TESTS:%=%.o) +.SECONDARY: $(OBJS) + include $(wildcard $(OBJS:.o=.d)) - - - dep_clean: $(RM) $(OBJS:.o=.d) -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH 2/4] multipathd: fix -Wpointer-to-int-cast warning in uxlsnr
Fixes: "multipathd: release uxsocket and resource when cancel thread"
Signed-off-by: Martin Wilck
---
multipathd/uxlsnr.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
index 0531061912b3..cdafd82943e7 100644
--- a/multipathd/uxlsnr.c
+++ b/multipathd/uxlsnr.c
@@ -148,7 +148,7 @@ void uxsock_cleanup(void *arg)
{
struct client *client_loop;
struct client *client_tmp;
- int ux_sock = (int)arg;
+ long ux_sock = (long)arg;
close(ux_sock);
@@ -167,7 +167,7 @@ void uxsock_cleanup(void *arg)
*/
void * uxsock_listen(uxsock_trigger_fn uxsock_trigger, void * trigger_data)
{
- int ux_sock;
+ long ux_sock;
int rlen;
char *inbuf;
char *reply;
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] multipath-tools: broken build system + warnings
On Wed, 2018-03-07 at 23:30 +0100, Xose Vazquez Perez wrote: > On 03/07/2018 10:16 PM, Martin Wilck wrote: > > > On Wed, 2018-03-07 at 17:40 +0100, Xose Vazquez Perez wrote: > > > Hi, > > > > > > It has to type "make" *twice* to build the full > > > source code. > > > > Hm, strange. I need to double check. It doesn't happen with my tree > > AFAICS. > > These are removed by "make": > rm random.o weightedpath.o ontap.o hp_sw.o hds.o sysfs.o rdac.o > const.o datacore.o emc.o iet.o > rm tur.o emc_clariion.o hp_sw.o rdac.o readsector0.o cciss_tur.o > rm nvme.o > Yes. That used to be the case before too, but now, on second invocation, "make" sees the generated dependency file and thus builds again. I have a fix for it. Will post soon. Martin -- Dr. Martin Wilck , 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: [dm-devel] multipath-tools: broken build system + warnings
On 03/07/2018 10:16 PM, Martin Wilck wrote: > On Wed, 2018-03-07 at 17:40 +0100, Xose Vazquez Perez wrote: >> Hi, >> >> It has to type "make" *twice* to build the full >> source code. > > Hm, strange. I need to double check. It doesn't happen with my tree > AFAICS. These are removed by "make": rm random.o weightedpath.o ontap.o hp_sw.o hds.o sysfs.o rdac.o const.o datacore.o emc.o iet.o rm tur.o emc_clariion.o hp_sw.o rdac.o readsector0.o cciss_tur.o rm nvme.o -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] multipath-tools: broken build system + warnings
On Wed, 2018-03-07 at 17:40 +0100, Xose Vazquez Perez wrote:
> Hi,
>
> It has to type "make" *twice* to build the full
> source code.
Hm, strange. I need to double check. It doesn't happen with my tree
AFAICS.
>
> And new warnings:
>
> 2 gcc warnings ===
> make[1]: Entering directory '/home/xose/curre/arrays/multipath-
> tools/multipathd'
>
> cc -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
> -Werror=implicit-function-declaration -Werror=format-security -Wno-
> sign-compare -Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
> -Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-
> protector-strong --param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\"
> -DLIB_STRING=\"lib64\" -DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
> -I../libmultipath -I../libmpathpersist -I../libmpathcmd -I../third-
> party -DUSE_SYSTEMD=234 -c -o uxlsnr.o uxlsnr.c
> uxlsnr.c: In function ‘uxsock_cleanup’:
> uxlsnr.c:151:16: warning: cast from pointer to integer of different
> size [-Wpointer-to-int-cast]
> int ux_sock = (int)arg;
This is fixed by my recent
[PATCH] multipathd: fix -Wpointer-to-int-cast warning in uxlsnr
> ^
> In file included from ../libmultipath/debug.h:5:0,
> from uxlsnr.c:28:
> uxlsnr.c: In function ‘uxsock_listen’:
> uxlsnr.c:184:39: warning: cast to pointer from integer of different
> size [-Wint-to-pointer-cast]
> pthread_cleanup_push(uxsock_cleanup, (void *)ux_sock);
>^
> end 2 gcc warnings ===
> 2 clang warnings ===
> make[1]: Entering directory '/home/xose/curre/arrays/multipath-
> tools/multipath'
> clang -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
> -Werror=implicit-function-declaration -Werror=format-security -Wno-
> sign-compare -Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
> -Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-
> protector-strong --param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\"
> -DLIB_STRING=\"lib64\" -DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
> -I../libmultipath -I../libmpathcmd -c -o main.o main.c
> main.c:628:11: warning: variable 'reply' is used uninitialized
> whenever 'if' condition is true [-Wsometimes-uninitialized]
> else if (p >= command + sizeof(command)) {
> ^~
> main.c:648:7: note: uninitialized use occurs here
> FREE(reply);
> ^
> ../libmultipath/memory.h:58:32: note: expanded from macro 'FREE'
> #define FREE(p) do { free(p); p = NULL; } while(0)
>^
> main.c:628:7: note: remove the 'if' if its condition is always false
> else if (p >= command + sizeof(command)) {
> ^
> main.c:609:32: note: initialize the variable 'reply' to silence this
> warning
> char command[1024], *p, *reply;
> ^
>= NULL
>
> clang -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
> -Werror=implicit-function-declaration -Werror=format-security -Wno-
> sign-compare -Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
> -Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-
> protector-strong --param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\"
> -DLIB_STRING=\"lib64\" -DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
> -I../libmultipath -I../libmpathpersist -I../libmpathcmd -I../third-
> party -DUSE_SYSTEMD=234 -c -o uxlsnr.o uxlsnr.c
> uxlsnr.c:184:39: warning: cast to 'void *' from smaller integer type
> 'int' [-Wint-to-void-pointer-cast]
> pthread_cleanup_push(uxsock_cleanup, (void *)ux_sock);
> ^
> end 2 clang warnings ===
Thanks for pointing this out. I'll send a fixup patch (setting reply =
NULL, as clang helpfully suggested).
Martin
--
Dr. Martin Wilck , 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: [dm-devel] [PATCH v2 23/23] multipathd: fix signal blocking logic
On Tue, Mar 06, 2018 at 12:15:07AM +0100, Martin Wilck wrote:
> multipathd is supposed to block all signals in all threads, except
> the uxlsnr thread which handles termination and reconfiguration
> signals (SIGUSR1) in its ppoll() call, SIGUSR2 in the waiter thread
> and the marginal path checker thread, and occasional SIGALRM. The current
> logic does exactly the oppsite, it blocks termination signals in SIGPOLL and
> allows multipathd to be killed e.g. by SIGALRM.
>
> Fix that by inverting the logic. The argument to pthread_sigmask and
> ppoll is the set of *blocked* signals, not vice versa.
>
> The marginal paths code needs to unblock SIGUSR2 now explicity, as
> the dm-event waiter code already does. Doing this with pselect()
> avoids asynchronous cancellation.
>
> Fixes: 810082e "libmultipath, multipathd: Rework SIGPIPE handling"
> Fixes: 534ec4c "multipathd: Ensure that SIGINT, SIGTERM, SIGHUP and SIGUSR1
> are delivered to the uxsock thread"
>
Reviewed-by: Benjamin Marzinski
> Signed-off-by: Martin Wilck
> ---
> libmultipath/io_err_stat.c | 17 -
> multipathd/main.c | 7 +--
> multipathd/uxlsnr.c| 10 +-
> 3 files changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
> index 5b10f0372f9f..00bac9e0e755 100644
> --- a/libmultipath/io_err_stat.c
> +++ b/libmultipath/io_err_stat.c
> @@ -21,6 +21,7 @@
> #include
> #include
> #include
> +#include
>
> #include "vector.h"
> #include "memory.h"
> @@ -691,14 +692,28 @@ static void service_paths(void)
>
> static void *io_err_stat_loop(void *data)
> {
> + sigset_t set;
> +
> vecs = (struct vectors *)data;
> pthread_cleanup_push(rcu_unregister, NULL);
> rcu_register_thread();
>
> + sigfillset(&set);
> + sigdelset(&set, SIGUSR2);
> +
> mlockall(MCL_CURRENT | MCL_FUTURE);
> while (1) {
> + struct timespec ts;
> +
> service_paths();
> - usleep(10);
> +
> + ts.tv_sec = 0;
> + ts.tv_nsec = 100 * 1000 * 1000;
> + /*
> + * pselect() with no fds, a timeout, and a sigmask:
> + * sleep for 100ms and react on SIGUSR2.
> + */
> + pselect(1, NULL, NULL, NULL, &ts, &set);
> }
>
> pthread_cleanup_pop(1);
> diff --git a/multipathd/main.c b/multipathd/main.c
> index 4abdd8f071c3..68595836d723 100644
> --- a/multipathd/main.c
> +++ b/multipathd/main.c
> @@ -2261,10 +2261,13 @@ signal_init(void)
> {
> sigset_t set;
>
> - sigemptyset(&set);
> - sigaddset(&set, SIGUSR2);
> + /* block all signals */
> + sigfillset(&set);
> + /* SIGPIPE occurs if logging fails */
> + sigdelset(&set, SIGPIPE);
> pthread_sigmask(SIG_SETMASK, &set, NULL);
>
> + /* Other signals will be unblocked in the uxlsnr thread */
> signal_set(SIGHUP, sighup);
> signal_set(SIGUSR1, sigusr1);
> signal_set(SIGUSR2, sigusr2);
> diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c
> index 98ac25a68c43..a2ca36ba1653 100644
> --- a/multipathd/uxlsnr.c
> +++ b/multipathd/uxlsnr.c
> @@ -170,11 +170,11 @@ void * uxsock_listen(uxsock_trigger_fn uxsock_trigger,
> void * trigger_data)
> condlog(0, "uxsock: failed to allocate poll fds");
> return NULL;
> }
> - sigemptyset(&mask);
> - sigaddset(&mask, SIGINT);
> - sigaddset(&mask, SIGTERM);
> - sigaddset(&mask, SIGHUP);
> - sigaddset(&mask, SIGUSR1);
> + sigfillset(&mask);
> + sigdelset(&mask, SIGINT);
> + sigdelset(&mask, SIGTERM);
> + sigdelset(&mask, SIGHUP);
> + sigdelset(&mask, SIGUSR1);
> while (1) {
> struct client *c, *tmp;
> int i, poll_count, num_clients;
> --
> 2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 22/23] multipathd: update path group prio in check_path
On Tue, Mar 06, 2018 at 12:15:06AM +0100, Martin Wilck wrote: > The previous patch "libmultipath: don't update path groups when printing" > removed the call to path_group_prio_update() in the printing code path. > To compensate for that, recalculate path group prio also when it's not > strictly necessary (i.e. if failback "manual" is set). > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > multipathd/main.c | 8 ++-- > 1 file changed, 6 insertions(+), 2 deletions(-) > > diff --git a/multipathd/main.c b/multipathd/main.c > index 465a1e291226..4abdd8f071c3 100644 > --- a/multipathd/main.c > +++ b/multipathd/main.c > @@ -252,8 +252,9 @@ need_switch_pathgroup (struct multipath * mpp, int > refresh) > struct path * pp; > unsigned int i, j; > struct config *conf; > + int bestpg; > > - if (!mpp || mpp->pgfailback == -FAILBACK_MANUAL) > + if (!mpp) > return 0; > > /* > @@ -272,8 +273,11 @@ need_switch_pathgroup (struct multipath * mpp, int > refresh) > if (!mpp->pg || VECTOR_SIZE(mpp->paths) == 0) > return 0; > > - mpp->bestpg = select_path_group(mpp); > + bestpg = select_path_group(mpp); > + if (mpp->pgfailback == -FAILBACK_MANUAL) > + return 0; > > + mpp->bestpg = bestpg; > if (mpp->bestpg != mpp->nextpg) > return 1; > > -- > 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 21/23] libmultipath: foreign/nvme: implement path display
On Tue, Mar 06, 2018 at 12:15:05AM +0100, Martin Wilck wrote:
> implement display of path information for NVMe foreign paths and maps.
> With this patch, I get output like this for Linux NVMe soft targets:
>
> nvme-submultipathd show topology
> sys0:NQN:subsysname (uuid.96926ba3-b207-437c-902c-4a4df6538c3f) [nvme]
> nvme0n1 NVMe,Linux,4.15.0-r
> size=2097152 features='n/a' hwhandler='n/a' wp=rw
> `-+- policy='n/a' prio=n/a status=n/a
> |- 0:1:1 nvme0c1n1 0:0 n/a n/a live
> |- 0:2:1 nvme0c2n1 0:0 n/a n/a live
> |- 0:3:1 nvme0c3n1 0:0 n/a n/a live
> `- 0:4:1 nvme0c4n1 0:0 n/a n/a live
>
> multipathd show paths format '%G %d %i %o %z %m %N'
> foreign dev hcil dev_st serial multipath host WWNN
> [nvme] nvme0c1n1 0:1:1 live 1c2c86659503a02f nvme0n1
> rdma:traddr=192.168.201.101,trsvcid=4420
> [nvme] nvme0c2n1 0:2:1 live 1c2c86659503a02f nvme0n1
> rdma:traddr=192.168.202.101,trsvcid=4420
> [nvme] nvme0c3n1 0:3:1 live 1c2c86659503a02f nvme0n1
> rdma:traddr=192.168.203.101,trsvcid=4420
> [nvme] nvme0c4n1 0:4:1 live 1c2c86659503a02f nvme0n1
> rdma:traddr=192.168.204.101,trsvcid=4420
>
> (admittedly, I abused the 'WWNN' wildcard here a bit to display information
> which is helpful for NVMe over RDMA).
>
Reviewed-by: Benjamin Marzinski
> Signed-off-by: Martin Wilck
> ---
> libmultipath/foreign/nvme.c | 334
> ++--
> 1 file changed, 320 insertions(+), 14 deletions(-)
>
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> index 32bd5c96c44a..235f75dd2add 100644
> --- a/libmultipath/foreign/nvme.c
> +++ b/libmultipath/foreign/nvme.c
> @@ -25,42 +25,98 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include "vector.h"
> #include "generic.h"
> #include "foreign.h"
> #include "debug.h"
> +#include "structs.h"
> +#include "sysfs.h"
>
> +static const char nvme_vendor[] = "NVMe";
> +static const char N_A[] = "n/a";
> const char *THIS;
>
> +struct nvme_map;
> +struct nvme_path {
> + struct gen_path gen;
> + struct udev_device *udev;
> + struct udev_device *ctl;
> + struct nvme_map *map;
> + bool seen;
> +};
> +
> +struct nvme_pathgroup {
> + struct gen_pathgroup gen;
> + vector pathvec;
> +};
> +
> struct nvme_map {
> struct gen_multipath gen;
> struct udev_device *udev;
> struct udev_device *subsys;
> dev_t devt;
> + /* Just one static pathgroup for NVMe for now */
> + struct nvme_pathgroup pg;
> + struct gen_pathgroup *gpg;
> + struct _vector pgvec;
> + vector pathvec;
> + int nr_live;
> };
>
> -#define NAME_LEN 64 /* buffer length temp model name */
> +#define NAME_LEN 64 /* buffer length for temp attributes */
> #define const_gen_mp_to_nvme(g) ((const struct nvme_map*)(g))
> #define gen_mp_to_nvme(g) ((struct nvme_map*)(g))
> #define nvme_mp_to_gen(n) &((n)->gen)
> +#define const_gen_pg_to_nvme(g) ((const struct nvme_pathgroup*)(g))
> +#define gen_pg_to_nvme(g) ((struct nvme_pathgroup*)(g))
> +#define nvme_pg_to_gen(n) &((n)->gen)
> +#define const_gen_path_to_nvme(g) ((const struct nvme_path*)(g))
> +#define gen_path_to_nvme(g) ((struct nvme_path*)(g))
> +#define nvme_path_to_gen(n) &((n)->gen)
> +
> +static void cleanup_nvme_path(struct nvme_path *path)
> +{
> + condlog(5, "%s: %p %p", __func__, path, path->udev);
> + if (path->udev)
> + udev_device_unref(path->udev);
> + /* ctl is implicitly referenced by udev, no need to unref */
> + free(path);
> +}
>
> static void cleanup_nvme_map(struct nvme_map *map)
> {
> + if (map->pathvec) {
> + struct nvme_path *path;
> + int i;
> +
> + vector_foreach_slot_backwards(map->pathvec, path, i) {
> + condlog(5, "%s: %d %p", __func__, i, path);
> + cleanup_nvme_path(path);
> + vector_del_slot(map->pathvec, i);
> + }
> + }
> + vector_free(map->pathvec);
> if (map->udev)
> udev_device_unref(map->udev);
> - if (map->subsys)
> - udev_device_unref(map->subsys);
> + /* subsys is implicitly referenced by udev, no need to unref */
> free(map);
> }
>
> static const struct _vector*
> nvme_mp_get_pgs(const struct gen_multipath *gmp) {
> - return NULL;
> + const struct nvme_map *nvme = const_gen_mp_to_nvme(gmp);
> +
> + /* This is all used under the lock, no need to copy */
> + return &nvme->pgvec;
> }
>
> static void
> nvme_mp_rel_pgs(const struct gen_multipath *gmp, const struct _vector *v)
> {
> + /* empty */
> }
>
> static void rstrip(char *str)
> @@ -75,7 +131,6 @@ static int snprint_nvme_map(const struct gen_multipath
> *gmp,
> char *buff, int len, char wildcard)
> {
> const struct nvme_map *nvm = const_gen_mp_to_nvme(gmp);
> - static const char nvme_vendor[] = "NVMe";
> ch
Re: [dm-devel] [PATCH v2 20/23] multipathd: use foreign API
On Tue, Mar 06, 2018 at 12:15:04AM +0100, Martin Wilck wrote:
> Call into the foreign library code when paths are discovered, uevents
> are received, and in the checker loop. Furthermore, use the foreign
> code to print information in the "multipathd show paths", "multipathd
> show maps", and "multipathd show topology" client commands.
>
> We don't support foreign data in the individual "show map" and "show path"
> commands, and neither in the "json" commands. The former is a deliberate
> decision, the latter could be added if desired.
>
Reviewed-by: Benjamin Marzinski
> Signed-off-by: Martin Wilck
> ---
> multipathd/cli_handlers.c | 39 ++-
> multipathd/main.c | 34 +++---
> 2 files changed, 65 insertions(+), 8 deletions(-)
>
> diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
> index 78f2a12bc2f8..c0ae54aae841 100644
> --- a/multipathd/cli_handlers.c
> +++ b/multipathd/cli_handlers.c
> @@ -27,6 +27,7 @@
> #include "main.h"
> #include "cli.h"
> #include "uevent.h"
> +#include "foreign.h"
>
> int
> show_paths (char ** r, int * len, struct vectors * vecs, char * style,
> @@ -35,11 +36,13 @@ show_paths (char ** r, int * len, struct vectors * vecs,
> char * style,
> int i;
> struct path * pp;
> char * c;
> - char * reply;
> + char * reply, * header;
> unsigned int maxlen = INITIAL_REPLY_LEN;
> int again = 1;
>
> get_path_layout(vecs->pathvec, 1);
> + foreign_path_layout();
> +
> reply = MALLOC(maxlen);
>
> while (again) {
> @@ -48,18 +51,29 @@ show_paths (char ** r, int * len, struct vectors * vecs,
> char * style,
>
> c = reply;
>
> - if (pretty && VECTOR_SIZE(vecs->pathvec) > 0)
> + if (pretty)
> c += snprint_path_header(c, reply + maxlen - c,
>style);
> + header = c;
>
> vector_foreach_slot(vecs->pathvec, pp, i)
> c += snprint_path(c, reply + maxlen - c,
> style, pp, pretty);
>
> + c += snprint_foreign_paths(c, reply + maxlen - c,
> +style, pretty);
> +
> again = ((c - reply) == (maxlen - 1));
>
> REALLOC_REPLY(reply, again, maxlen);
> }
> +
> + if (pretty && c == header) {
> + /* No output - clear header */
> + *reply = '\0';
> + c = reply;
> + }
> +
> *r = reply;
> *len = (int)(c - reply + 1);
> return 0;
> @@ -134,6 +148,8 @@ show_maps_topology (char ** r, int * len, struct vectors
> * vecs)
> int again = 1;
>
> get_path_layout(vecs->pathvec, 0);
> + foreign_path_layout();
> +
> reply = MALLOC(maxlen);
>
> while (again) {
> @@ -150,11 +166,13 @@ show_maps_topology (char ** r, int * len, struct
> vectors * vecs)
> c += snprint_multipath_topology(c, reply + maxlen - c,
> mpp, 2);
> }
> + c += snprint_foreign_topology(c, reply + maxlen - c, 2);
>
> again = ((c - reply) == (maxlen - 1));
>
> REALLOC_REPLY(reply, again, maxlen);
> }
> +
> *r = reply;
> *len = (int)(c - reply + 1);
> return 0;
> @@ -499,12 +517,14 @@ show_maps (char ** r, int *len, struct vectors * vecs,
> char * style,
> {
> int i;
> struct multipath * mpp;
> - char * c;
> + char * c, *header;
> char * reply;
> unsigned int maxlen = INITIAL_REPLY_LEN;
> int again = 1;
>
> get_multipath_layout(vecs->mpvec, 1);
> + foreign_multipath_layout();
> +
> reply = MALLOC(maxlen);
>
> while (again) {
> @@ -512,9 +532,10 @@ show_maps (char ** r, int *len, struct vectors * vecs,
> char * style,
> return 1;
>
> c = reply;
> - if (pretty && VECTOR_SIZE(vecs->mpvec) > 0)
> + if (pretty)
> c += snprint_multipath_header(c, reply + maxlen - c,
> style);
> + header = c;
>
> vector_foreach_slot(vecs->mpvec, mpp, i) {
> if (update_multipath(vecs, mpp->alias, 0)) {
> @@ -523,12 +544,20 @@ show_maps (char ** r, int *len, struct vectors * vecs,
> char * style,
> }
> c += snprint_multipath(c, reply + maxlen - c,
> style, mpp, pretty);
> - }
>
> + }
> + c += snprint_foreign_multipaths(c, reply + maxlen - c,
> + style, pretty);
> again = ((c - reply) == (maxlen - 1));
>
> REALLOC_REPLY(reply, again, maxlen);
> }
>
Re: [dm-devel] [PATCH v2 19/23] multipath: use foreign API
On Tue, Mar 06, 2018 at 12:15:03AM +0100, Martin Wilck wrote:
> Use the "foreign" code to print information about multipath maps
> owned by foreign libraries in print mode (multipath -ll, -l).
>
> Signed-off-by: Martin Wilck
Reviewed-by: Benjamin Marzinski
> ---
> multipath/main.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/multipath/main.c b/multipath/main.c
> index a0c750e6f623..4fae49ee4325 100644
> --- a/multipath/main.c
> +++ b/multipath/main.c
> @@ -59,6 +59,7 @@
> #include "wwids.h"
> #include "uxsock.h"
> #include "mpath_cmd.h"
> +#include "foreign.h"
>
> int logsink;
> struct udev *udev;
> @@ -257,6 +258,14 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector
> pathvec, char * refwwid)
> if (cmd == CMD_CREATE)
> reinstate_paths(mpp);
> }
> +
> + if (cmd == CMD_LIST_SHORT || cmd == CMD_LIST_LONG) {
> + struct config *conf = get_multipath_config();
> +
> + print_foreign_topology(conf->verbosity);
> + put_multipath_config(conf);
> + }
> +
> return 0;
> }
>
> @@ -460,6 +469,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
> print_all_paths(pathvec, 1);
>
> get_path_layout(pathvec, 0);
> + foreign_path_layout();
>
> if (get_dm_mpvec(cmd, curmp, pathvec, refwwid))
> goto out;
> @@ -817,6 +827,8 @@ main (int argc, char *argv[])
> condlog(0, "failed to initialize prioritizers");
> goto out;
> }
> + /* Failing here is non-fatal */
> + init_foreign(conf->multipath_dir);
> if (cmd == CMD_USABLE_PATHS) {
> r = check_usable_paths(conf, dev, dev_type);
> goto out;
> @@ -892,6 +904,7 @@ out:
> dm_lib_release();
> dm_lib_exit();
>
> + cleanup_foreign();
> cleanup_prio();
> cleanup_checkers();
>
> --
> 2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 18/23] libmultipath: pathinfo: call into foreign library
On Tue, Mar 06, 2018 at 12:15:02AM +0100, Martin Wilck wrote: > This actually enables the use of foreign paths. > Reviewed-by: Benjamin Marzinski > Signed-off-by: Martin Wilck > --- > libmultipath/discovery.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c > index 645224c1029c..45a4d8378893 100644 > --- a/libmultipath/discovery.c > +++ b/libmultipath/discovery.c > @@ -31,6 +31,7 @@ > #include "prio.h" > #include "defaults.h" > #include "prioritizers/alua_rtpg.h" > +#include "foreign.h" > > int > alloc_path_with_pathinfo (struct config *conf, struct udev_device *udevice, > @@ -1909,7 +1910,8 @@ int pathinfo(struct path *pp, struct config *conf, int > mask) >* limited by DI_BLACKLIST and occurs before this debug >* message with the mask value. >*/ > - if (pp->udev && filter_property(conf, pp->udev) > 0) > + if (pp->udev && (is_claimed_by_foreign(pp->udev) || > + filter_property(conf, pp->udev) > 0)) > return PATHINFO_SKIPPED; > > if (filter_devnode(conf->blist_devnode, > -- > 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH v2 17/23] libmultipath/foreign: nvme foreign library
On Tue, Mar 06, 2018 at 12:15:01AM +0100, Martin Wilck wrote:
> This still contains stubs for path handling and checking, but it's functional
> for printing already.
>
Reviewed-by: Benjamin Marzinski
> Signed-off-by: Martin Wilck
> ---
> Makefile | 1 +
> libmultipath/foreign/Makefile | 30 +++
> libmultipath/foreign/nvme.c | 455
> ++
> 3 files changed, 486 insertions(+)
> create mode 100644 libmultipath/foreign/Makefile
> create mode 100644 libmultipath/foreign/nvme.c
>
> diff --git a/Makefile b/Makefile
> index 11c46eb4dbc9..4b145c593605 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -7,6 +7,7 @@ BUILDDIRS = \
> libmultipath \
> libmultipath/prioritizers \
> libmultipath/checkers \
> + libmultipath/foreign \
> libmpathpersist \
> multipath \
> multipathd \
> diff --git a/libmultipath/foreign/Makefile b/libmultipath/foreign/Makefile
> new file mode 100644
> index ..dfba11e86d76
> --- /dev/null
> +++ b/libmultipath/foreign/Makefile
> @@ -0,0 +1,30 @@
> +#
> +# Copyright (C) 2003 Christophe Varoqui,
> +#
> +include ../../Makefile.inc
> +
> +CFLAGS += $(LIB_CFLAGS) -I..
> +
> +# If you add or remove a checker also update multipath/multipath.conf.5
> +LIBS= \
> + libforeign-nvme.so
> +
> +all: $(LIBS)
> +
> +libforeign-%.so: %.o
> + $(CC) $(LDFLAGS) $(SHARED_FLAGS) -o $@ $^
> +
> +install:
> + $(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(libdir)
> +
> +uninstall:
> + for file in $(LIBS); do $(RM) $(DESTDIR)$(libdir)/$$file; done
> +
> +clean: dep_clean
> + $(RM) core *.a *.o *.gz *.so
> +
> +OBJS := $(LIBS:libforeign-%.so=%.o)
> +include $(wildcard $(OBJS:.o=.d))
> +
> +dep_clean:
> + $(RM) $(OBJS:.o=.d)
> diff --git a/libmultipath/foreign/nvme.c b/libmultipath/foreign/nvme.c
> new file mode 100644
> index ..32bd5c96c44a
> --- /dev/null
> +++ b/libmultipath/foreign/nvme.c
> @@ -0,0 +1,455 @@
> +/*
> + Copyright (c) 2018 Martin Wilck, 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
> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
> + USA.
> +*/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "vector.h"
> +#include "generic.h"
> +#include "foreign.h"
> +#include "debug.h"
> +
> +const char *THIS;
> +
> +struct nvme_map {
> + struct gen_multipath gen;
> + struct udev_device *udev;
> + struct udev_device *subsys;
> + dev_t devt;
> +};
> +
> +#define NAME_LEN 64 /* buffer length temp model name */
> +#define const_gen_mp_to_nvme(g) ((const struct nvme_map*)(g))
> +#define gen_mp_to_nvme(g) ((struct nvme_map*)(g))
> +#define nvme_mp_to_gen(n) &((n)->gen)
> +
> +static void cleanup_nvme_map(struct nvme_map *map)
> +{
> + if (map->udev)
> + udev_device_unref(map->udev);
> + if (map->subsys)
> + udev_device_unref(map->subsys);
> + free(map);
> +}
> +
> +static const struct _vector*
> +nvme_mp_get_pgs(const struct gen_multipath *gmp) {
> + return NULL;
> +}
> +
> +static void
> +nvme_mp_rel_pgs(const struct gen_multipath *gmp, const struct _vector *v)
> +{
> +}
> +
> +static void rstrip(char *str)
> +{
> + int n;
> +
> + for (n = strlen(str) - 1; n >= 0 && str[n] == ' '; n--);
> + str[n+1] = '\0';
> +}
> +
> +static int snprint_nvme_map(const struct gen_multipath *gmp,
> + char *buff, int len, char wildcard)
> +{
> + const struct nvme_map *nvm = const_gen_mp_to_nvme(gmp);
> + static const char nvme_vendor[] = "NVMe";
> + char fld[NAME_LEN];
> + const char *val;
> +
> + switch (wildcard) {
> + case 'd':
> + return snprintf(buff, len, "%s",
> + udev_device_get_sysname(nvm->udev));
> + case 'n':
> + return snprintf(buff, len, "%s:NQN:%s",
> + udev_device_get_sysname(nvm->subsys),
> + udev_device_get_sysattr_value(nvm->subsys,
> + "subsysnqn"));
> + case 'w':
> + return snprintf(buff, len, "%s",
> + udev_device_get_sysattr_value(nvm->udev,
> + "wwid"));
> +
Re: [dm-devel] [PATCH v2 15/23] libmultipath: API for foreign multipath handling
On Tue, Mar 06, 2018 at 12:14:59AM +0100, Martin Wilck wrote:
> Add an API for "foreign" multipaths. Foreign libraries are loaded
> from ${multipath_dir}/libforeign-*.so, as we do for checkers.
>
> Refer to "foreign.h" for details about the API itself. Like we do for
> checkers, high-level multipath code isn't supposed to call the API directly,
> but rather the wrapper functions declared in "foreign.h".
>
> This API is used only for displaying information and for logging. An
> extension to
> other functionality (such as monitoring or administration) might be feasible,
> but is not planned.
>
> Foreign libraries communicate with libmultipath through the API defined in
> "foreign.h". The foreign library can implement multipath maps, pathgroups,
> and paths as it likes, they just need to provide the simple interfaces
> defined in "generic.h" to libmultipath. These interfaces are used in
> libmultipath's
> "print" implementation to convey various bits of information to users. By
> using the same interfaces for printing that libmultipath uses internally,
> foreign library implementations can focus on the technical side without
> worrying about output formatting compatibility.
>
Reviewed-by: Benjamin Marzinski
I don't think the foreign_lock is strictly necessary, since I can't see
any way of any for the readers to be running at the same time as the
writers, simply because the readers all happen in threads that have
either not been created yet, or have already been joined when the
writers run. But the locking code all makes sense, and I'm fine with
it.
> Signed-off-by: Martin Wilck
> ---
> libmultipath/Makefile | 2 +-
> libmultipath/foreign.c | 602
> +
> libmultipath/foreign.h | 322 ++
> 3 files changed, 925 insertions(+), 1 deletion(-)
> create mode 100644 libmultipath/foreign.c
> create mode 100644 libmultipath/foreign.h
>
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index 0099d9d6cc39..806aaa24f84e 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -43,7 +43,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
> switchgroup.o uxsock.o print.o alias.o log_pthread.o \
> log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
> lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
> - io_err_stat.o dm-generic.o generic.o
> + io_err_stat.o dm-generic.o generic.o foreign.o
>
> all: $(LIBS)
>
> diff --git a/libmultipath/foreign.c b/libmultipath/foreign.c
> new file mode 100644
> index ..72171840e995
> --- /dev/null
> +++ b/libmultipath/foreign.c
> @@ -0,0 +1,602 @@
> +/*
> + Copyright (c) 2018 Martin Wilck, 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
> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
> + USA.
> +*/
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "vector.h"
> +#include "debug.h"
> +#include "util.h"
> +#include "foreign.h"
> +#include "structs.h"
> +#include "structs_vec.h"
> +#include "print.h"
> +
> +static vector foreigns;
> +
> +/* This protects vector foreigns */
> +static pthread_rwlock_t foreign_lock = PTHREAD_RWLOCK_INITIALIZER;
> +
> +static void rdlock_foreigns(void)
> +{
> + pthread_rwlock_rdlock(&foreign_lock);
> +}
> +
> +static void wrlock_foreigns(void)
> +{
> + pthread_rwlock_wrlock(&foreign_lock);
> +}
> +
> +static void unlock_foreigns(void *unused)
> +{
> + pthread_rwlock_unlock(&foreign_lock);
> +}
> +
> +#define get_dlsym(foreign, sym, lbl) \
> + do {\
> + foreign->sym = dlsym(foreign->handle, #sym); \
> + if (foreign->sym == NULL) { \
> + condlog(0, "%s: symbol \"%s\" not found in \"%s\"", \
> + __func__, #sym, foreign->name); \
> + goto lbl; \
> + } \
> + } while(0)
> +
> +static void free_foreign(struct foreign *fgn)
> +{
> + struct context *ctx;
> +
> + if (fgn == NULL)
>
Re: [dm-devel] [PATCH v2 13/23] libmultipath: print: convert API to generic data type
On Tue, Mar 06, 2018 at 12:14:57AM +0100, Martin Wilck wrote:
> Convert higher level API (snprint_multipath_topology() etc) to
> using the generic multipath API. This will allow "foreign"
> multipath objects that implement the generic API to be printed
> exactly like native multipathd objects.
>
> The previous API (using "struct multipath*" and "struct path" remains
> in place through macros mapping to the new functions. By doing this
> and testing in regular setups, it's easily verified that the new
> API works and produces the same results.
>
> Moreover, abstract out the code to determine the output format from multipath
> properties into snprint_multipath_style(), to be able to use it as generic
> ->style() method.
>
> Signed-off-by: Martin Wilck
Reviewed-by: Benjamin Marzinski
> ---
> libmultipath/configure.c | 1 +
> libmultipath/dm-generic.c | 2 +-
> libmultipath/print.c | 115
> +-
> libmultipath/print.h | 24 +++---
> multipath/main.c | 1 +
> multipathd/cli_handlers.c | 1 +
> 6 files changed, 95 insertions(+), 49 deletions(-)
>
> diff --git a/libmultipath/configure.c b/libmultipath/configure.c
> index 13e14cc25fff..42b7c896ee65 100644
> --- a/libmultipath/configure.c
> +++ b/libmultipath/configure.c
> @@ -30,6 +30,7 @@
> #include "discovery.h"
> #include "debug.h"
> #include "switchgroup.h"
> +#include "dm-generic.h"
> #include "print.h"
> #include "configure.h"
> #include "pgpolicies.h"
> diff --git a/libmultipath/dm-generic.c b/libmultipath/dm-generic.c
> index 42a26085d087..bdc9ca0a488b 100644
> --- a/libmultipath/dm-generic.c
> +++ b/libmultipath/dm-generic.c
> @@ -56,7 +56,7 @@ const struct gen_multipath_ops dm_gen_multipath_ops = {
> .get_pathgroups = dm_mp_get_pgs,
> .rel_pathgroups = dm_mp_rel_pgs,
> .snprint = snprint_multipath_attr,
> - /* .style = snprint_multipath_style, TBD */
> + .style = snprint_multipath_style,
> };
>
> const struct gen_pathgroup_ops dm_gen_pathgroup_ops = {
> diff --git a/libmultipath/print.c b/libmultipath/print.c
> index e9b8fdd6e581..8701a3584859 100644
> --- a/libmultipath/print.c
> +++ b/libmultipath/print.c
> @@ -30,7 +30,8 @@
> #include "debug.h"
> #include "discovery.h"
>
> -#define MAX(x,y) (x > y) ? x : y
> +#define MAX(x,y) (((x) > (y)) ? (x) : (y))
> +#define MIN(x,y) (((x) > (y)) ? (y) : (x))
> #define TAIL (line + len - 1 - c)
> #define NOPADs = c
> #define PAD(x) \
> @@ -845,8 +846,8 @@ snprint_multipath_header (char * line, int len, const
> char * format)
> }
>
> int
> -snprint_multipath (char * line, int len, const char * format,
> - const struct multipath * mpp, int pad)
> +_snprint_multipath (const struct gen_multipath * gmp,
> + char * line, int len, const char * format, int pad)
> {
> char * c = line; /* line cursor */
> char * s = line; /* for padding */
> @@ -869,7 +870,7 @@ snprint_multipath (char * line, int len, const char *
> format,
> if (!(data = mpd_lookup(*f)))
> continue;
>
> - data->snprint(buff, MAX_FIELD_LEN, mpp);
> + gmp->ops->snprint(gmp, buff, MAX_FIELD_LEN, *f);
> PRINT(c, TAIL, "%s", buff);
> if (pad)
> PAD(data->width);
> @@ -912,8 +913,8 @@ snprint_path_header (char * line, int len, const char *
> format)
> }
>
> int
> -snprint_path (char * line, int len, const char * format,
> - const struct path * pp, int pad)
> +_snprint_path (const struct gen_path * gp, char * line, int len,
> +const char * format, int pad)
> {
> char * c = line; /* line cursor */
> char * s = line; /* for padding */
> @@ -936,7 +937,7 @@ snprint_path (char * line, int len, const char * format,
> if (!(data = pd_lookup(*f)))
> continue;
>
> - data->snprint(buff, MAX_FIELD_LEN, pp);
> + gp->ops->snprint(gp, buff, MAX_FIELD_LEN, *f);
> PRINT(c, TAIL, "%s", buff);
> if (pad)
> PAD(data->width);
> @@ -947,8 +948,8 @@ snprint_path (char * line, int len, const char * format,
> }
>
> int
> -snprint_pathgroup (char * line, int len, char * format,
> -const struct pathgroup * pgp)
> +_snprint_pathgroup (const struct gen_pathgroup * ggp, char * line, int len,
> + char * format)
> {
> char * c = line; /* line cursor */
> char * s = line; /* for padding */
> @@ -971,7 +972,7 @@ snprint_pathgroup (char * line, int len, char * format,
> if (!(data = pgd_lookup(*f)))
> continue;
>
> - data->snprint(buff, MAX_FIELD_LEN, pgp);
> + ggp->ops->snprint(ggp, buff, MAX_FIELD_LEN, *f);
> PRINT(c, TAIL, "%s", buff);
> PAD(data->width);
> } while (*f++);
> @@ -979,8 +980,10 @@ snprint_
Re: [dm-devel] [PATCH v2 12/23] libmultipath: "generic multipath" interface
On Tue, Mar 06, 2018 at 12:14:56AM +0100, Martin Wilck wrote:
> This patch adds a simplified abstract interface to the multipath data
> structures.
> The idea is to allow "foreign" data structures to be treated by libmultipath
> if they implement the same interface. Currently, the intention is to use this
> only to provide formatted output about from this interface.
>
> This interface assumes only that the data structure is organized in maps
> containing path groups containing paths, and that formatted printing (using
> the wildcards defined in libmultipath) is possible on each level of the data
> structure.
>
> The patch also implements the interface for the internal dm_multipath data
> structure.
>
> The style() method looks a bit exotic, but it's necessary because
> print_multipath_topology() uses different formats depending on the mpp
> properties. This needs to be in the generic interface, too, if we want to
> produce identical output.
>
Reviewed-by: Benjamin Marzinski
> Signed-off-by: Martin Wilck
> ---
> libmultipath/Makefile | 2 +-
> libmultipath/dm-generic.c | 70
> libmultipath/dm-generic.h | 41 ++
> libmultipath/generic.c| 39 +
> libmultipath/generic.h| 136
> ++
> libmultipath/list.h | 4 ++
> libmultipath/print.c | 33 +++
> libmultipath/print.h | 12
> libmultipath/structs.c| 4 ++
> libmultipath/structs.h| 4 ++
> 10 files changed, 344 insertions(+), 1 deletion(-)
> create mode 100644 libmultipath/dm-generic.c
> create mode 100644 libmultipath/dm-generic.h
> create mode 100644 libmultipath/generic.c
> create mode 100644 libmultipath/generic.h
>
> diff --git a/libmultipath/Makefile b/libmultipath/Makefile
> index 25b052729d48..0099d9d6cc39 100644
> --- a/libmultipath/Makefile
> +++ b/libmultipath/Makefile
> @@ -43,7 +43,7 @@ OBJS = memory.o parser.o vector.o devmapper.o callout.o \
> switchgroup.o uxsock.o print.o alias.o log_pthread.o \
> log.o configure.o structs_vec.o sysfs.o prio.o checkers.o \
> lock.o waiter.o file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
> - io_err_stat.o
> + io_err_stat.o dm-generic.o generic.o
>
> all: $(LIBS)
>
> diff --git a/libmultipath/dm-generic.c b/libmultipath/dm-generic.c
> new file mode 100644
> index ..42a26085d087
> --- /dev/null
> +++ b/libmultipath/dm-generic.c
> @@ -0,0 +1,70 @@
> +/*
> + Copyright (c) 2018 Martin Wilck, 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
> + Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301,
> + USA.
> + */
> +
> +#include
> +#include
> +#include "generic.h"
> +#include "dm-generic.h"
> +#include "structs.h"
> +#include "structs_vec.h"
> +#include "config.h"
> +#include "print.h"
> +
> +static const struct _vector*
> +dm_mp_get_pgs(const struct gen_multipath *gmp)
> +{
> + return vector_convert(NULL, gen_multipath_to_dm(gmp)->pg,
> + struct pathgroup, dm_pathgroup_to_gen);
> +}
> +
> +static void dm_mp_rel_pgs(const struct gen_multipath *gmp,
> + const struct _vector* v)
> +{
> + vector_free_const(v);
> +}
> +
> +static const struct _vector*
> +dm_pg_get_paths(const struct gen_pathgroup *gpg)
> +{
> + return vector_convert(NULL, gen_pathgroup_to_dm(gpg)->paths,
> + struct path, dm_path_to_gen);
> +}
> +
> +static void dm_mp_rel_paths(const struct gen_pathgroup *gpg,
> + const struct _vector* v)
> +{
> + vector_free_const(v);
> +}
> +
> +const struct gen_multipath_ops dm_gen_multipath_ops = {
> + .get_pathgroups = dm_mp_get_pgs,
> + .rel_pathgroups = dm_mp_rel_pgs,
> + .snprint = snprint_multipath_attr,
> + /* .style = snprint_multipath_style, TBD */
> +};
> +
> +const struct gen_pathgroup_ops dm_gen_pathgroup_ops = {
> + .get_paths = dm_pg_get_paths,
> + .rel_paths = dm_mp_rel_paths,
> + .snprint = snprint_pathgroup_attr,
> +};
> +
> +const struct gen_path_ops dm_gen_path_ops = {
> + .snprint = snprint_path_attr,
> +};
> diff --git a/libmultipath/dm-generic.h b/libmultipath/dm-generic.h
> new file mode 100644
> index ..5d5972406819
> --- /dev/null
> +++ b/libmultipath/dm-generic.h
> @@ -0,0 +1,41 @@
> +/*
Re: [dm-devel] [PATCH 0/6] Fixes for config file parsing
On Tue, 2018-03-06 at 23:56 +0100, Martin Wilck wrote: > This series was motivated by the real-world problem that a user > couldn't > figure out how to write a blacklist entry for a device called '1.8" > SSD'. > Fixing this for good turned out to be a little tricky, therefore I > also > added a test suite. Christophe, don't apply this yet. It breaks multipathd command parsing. Looking into it. Martin -- Dr. Martin Wilck , 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
[dm-devel] multipath-tools: broken build system + warnings
Hi,
It has to type "make" *twice* to build the full
source code.
And new warnings:
2 gcc warnings ===
make[1]: Entering directory '/home/xose/curre/arrays/multipath-tools/multipathd'
cc -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
-Werror=implicit-function-declaration -Werror=format-security -Wno-sign-compare
-Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
-Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong
--param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\" -DLIB_STRING=\"lib64\"
-DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
-I../libmultipath -I../libmpathpersist -I../libmpathcmd -I../third-party
-DUSE_SYSTEMD=234 -c -o uxlsnr.o uxlsnr.c
uxlsnr.c: In function ‘uxsock_cleanup’:
uxlsnr.c:151:16: warning: cast from pointer to integer of different size
[-Wpointer-to-int-cast]
int ux_sock = (int)arg;
^
In file included from ../libmultipath/debug.h:5:0,
from uxlsnr.c:28:
uxlsnr.c: In function ‘uxsock_listen’:
uxlsnr.c:184:39: warning: cast to pointer from integer of different size
[-Wint-to-pointer-cast]
pthread_cleanup_push(uxsock_cleanup, (void *)ux_sock);
^
end 2 gcc warnings ===
2 clang warnings ===
make[1]: Entering directory '/home/xose/curre/arrays/multipath-tools/multipath'
clang -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
-Werror=implicit-function-declaration -Werror=format-security -Wno-sign-compare
-Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
-Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong
--param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\" -DLIB_STRING=\"lib64\"
-DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
-I../libmultipath -I../libmpathcmd -c -o main.o main.c
main.c:628:11: warning: variable 'reply' is used uninitialized whenever 'if'
condition is true [-Wsometimes-uninitialized]
else if (p >= command + sizeof(command)) {
^~
main.c:648:7: note: uninitialized use occurs here
FREE(reply);
^
../libmultipath/memory.h:58:32: note: expanded from macro 'FREE'
#define FREE(p) do { free(p); p = NULL; } while(0)
^
main.c:628:7: note: remove the 'if' if its condition is always false
else if (p >= command + sizeof(command)) {
^
main.c:609:32: note: initialize the variable 'reply' to silence this warning
char command[1024], *p, *reply;
^
= NULL
clang -O2 -g -pipe -Wall -Wextra -Wformat=2 -Werror=implicit-int
-Werror=implicit-function-declaration -Werror=format-security -Wno-sign-compare
-Wno-unused-parameter -Wno-clobbered -Werror=cast-qual
-Werror=discarded-qualifiers -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector-strong
--param=ssp-buffer-size=4 -DBIN_DIR=\"/sbin\" -DLIB_STRING=\"lib64\"
-DRUN_DIR=\"run\" -MMD -MP -fPIE -DPIE
-I../libmultipath -I../libmpathpersist -I../libmpathcmd -I../third-party
-DUSE_SYSTEMD=234 -c -o uxlsnr.o uxlsnr.c
uxlsnr.c:184:39: warning: cast to 'void *' from smaller integer type 'int'
[-Wint-to-void-pointer-cast]
pthread_cleanup_push(uxsock_cleanup, (void *)ux_sock);
^
end 2 clang warnings ===
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipathd: use nanosleep for sleeping
On Tue, 2018-03-06 at 19:21 -0600, Benjamin Marzinski wrote: > On Tue, Mar 06, 2018 at 09:37:13PM +0100, Martin Wilck wrote: > > > > I'm sure this works, but have you considered achieving the same > > result > > simply by using create_timer with a different signal than SIGALRM? > > Thinking about it, I'm sure the strict_timing code can be > > drastically > > simplified by using an interval timer and a realtime signal. > > I don't see how it could get much simpler, and all things being > equal, > I'd rather that multipathd did less with signals. But if you want to > redo this with a timer, I don't haven any strong feelings against it. I just sent it with the subject "[RFC PATCH] multipathd: strict_timing without signals". Please have a look. The patch actually does two different things - a) convert the current setitimer-based code to a custom timer that works without SIGALRM (without any explicit signal, actually), and b) use the it_interval field to wakeup automatically at fixed intervals, rather than re-calculate the wait time from tick to tick. Whether or not this is simpler than the current code (or your approach, for that matter) is up to personal judgement. My patch has a positive line count, but it requires less code inside the checkerloop itself. Cheers, Martin -- Dr. Martin Wilck , 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: [dm-devel] [PATCH] multipath-tools: add info about how to get a release directly from gitweb
Thanks, applied. On Wed, Mar 7, 2018 at 2:28 PM, Xose Vazquez Perez wrote: > On 01/12/2018 05:56 PM, Xose Vazquez Perez wrote: > > > gitweb is able to extract and serve a release right away. > > This one is missing. > > > Cc: Christophe Varoqui > > Cc: device-mapper development > > Signed-off-by: Xose Vazquez Perez > > --- > > README | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/README b/README > > index 570224d..89bab74 100644 > > --- a/README > > +++ b/README > > @@ -20,7 +20,8 @@ git archive --format=tar.gz > --prefix=multipath-tools-X.Y.Z/ X.Y.Z > ../multipath > > > > Alternatively it may be obtained from gitweb, go to: > > https://git.opensvc.com/?p=multipath-tools/.git;a=tags > > -select a release-tag and then click on "snapshot". > > +select a release-tag and then click on "snapshot". Or get it with > > +wget "https://git.opensvc.com/?p=multipath-tools/.git;a= > snapshot;sf=tgz;h=refs/tags/X.Y.Z" -O multipath-tools-X.Y.Z.tar.gz > > > > > > Source code > > > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [RFC PATCH] multipathd: strict_timing without signals
The internal usage of SIGALRM by setitimer() function may cause subtle
conflicts with other uses of SIGALRM, either by multipath-tools code itself
(e.g. lock_file()) or libc (e.g. glob()).
This patch changes the checkerloop to use an interval timer and a pthread
condition variable. No signals are used except those used by libpthread
behind the scenes.
Signed-off-by: Martin Wilck
---
multipathd/Makefile | 2 +-
multipathd/main.c | 113 ++--
2 files changed, 85 insertions(+), 30 deletions(-)
diff --git a/multipathd/Makefile b/multipathd/Makefile
index 4c9d29634160..c348f02ef7f8 100644
--- a/multipathd/Makefile
+++ b/multipathd/Makefile
@@ -10,7 +10,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir)
-I$(mpathpersistdir) \
-I$(mpathcmddir) -I$(thirdpartydir)
LDFLAGS += $(BIN_LDFLAGS)
LIBDEPS += -L$(multipathdir) -lmultipath -L$(mpathpersistdir) -lmpathpersist \
- -L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread \
+ -L$(mpathcmddir) -lmpathcmd -ludev -ldl -lurcu -lpthread -lrt \
-ldevmapper -lreadline
ifdef SYSTEMD
diff --git a/multipathd/main.c b/multipathd/main.c
index 6e6c52a52783..c9c57c5baece 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -1848,6 +1848,68 @@ static void init_path_check_interval(struct vectors
*vecs)
}
}
+static pthread_mutex_t checker_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t checker_cond = PTHREAD_COND_INITIALIZER;
+
+static void checker_notify(union sigval sv) {
+
+ pthread_mutex_lock(&checker_lock);
+ pthread_cond_broadcast(&checker_cond);
+ pthread_mutex_unlock(&checker_lock);
+}
+
+static void unlock_mutex(void *arg)
+{
+ pthread_mutex_unlock((pthread_mutex_t*)arg);
+}
+
+void cleanup_timer(void *arg)
+{
+ timer_t tmr = (timer_t)arg;
+
+ if (tmr != 0)
+ timer_delete(tmr);
+}
+
+static timer_t setup_check_timer(void)
+{
+ timer_t tmr = (timer_t)0;
+ struct sigevent se;
+
+ memset(&se, 0, sizeof(se));
+ se.sigev_notify = SIGEV_THREAD;
+ se.sigev_notify_function = checker_notify;
+
+ if (timer_create(CLOCK_MONOTONIC, &se, &tmr)) {
+ condlog(2, "%s: errno %d creating monotonic timer",
+ __func__, errno);
+ if (timer_create(CLOCK_REALTIME, &se, &tmr))
+ condlog(0, "%s: errno %d creating timer",
+ __func__, errno);
+ }
+ return tmr;
+}
+
+static int set_check_timer(timer_t tmr, bool arm)
+{
+ struct itimerspec is;
+
+ if (tmr == 0)
+ return -1;
+
+ if (arm) {
+ is.it_value.tv_sec = 1;
+ is.it_interval.tv_sec = 1;
+ } else {
+ is.it_value.tv_sec = 0;
+ is.it_interval.tv_sec = 0;
+ }
+ is.it_value.tv_nsec = 0;
+ is.it_interval.tv_nsec = 0;
+
+ return timer_settime(tmr, 0, &is, NULL);
+}
+
static void *
checkerloop (void *ap)
{
@@ -1855,9 +1917,10 @@ checkerloop (void *ap)
struct path *pp;
int count = 0;
unsigned int i;
- struct itimerval timer_tick_it;
struct timespec last_time;
struct config *conf;
+ timer_t check_timer;
+ int old_strict_timing = 0;
pthread_cleanup_push(rcu_unregister, NULL);
rcu_register_thread();
@@ -1865,6 +1928,9 @@ checkerloop (void *ap)
vecs = (struct vectors *)ap;
condlog(2, "path checkers start up");
+ check_timer = setup_check_timer();
+ pthread_cleanup_push(cleanup_timer, (void*)check_timer);
+
/* Tweak start time for initial path check */
if (clock_gettime(CLOCK_MONOTONIC, &last_time) != 0)
last_time.tv_sec = 0;
@@ -1873,8 +1939,7 @@ checkerloop (void *ap)
while (1) {
struct timespec diff_time, start_time, end_time;
- int num_paths = 0, ticks = 0, signo, strict_timing, rc = 0;
- sigset_t mask;
+ int num_paths = 0, ticks = 0, strict_timing, rc = 0;
if (clock_gettime(CLOCK_MONOTONIC, &start_time) != 0)
start_time.tv_sec = 0;
@@ -1956,40 +2021,30 @@ checkerloop (void *ap)
}
check_foreign();
post_config_state(DAEMON_IDLE);
+
conf = get_multipath_config();
strict_timing = conf->strict_timing;
put_multipath_config(conf);
- if (!strict_timing)
+
+ if (check_timer && (old_strict_timing != strict_timing) &&
+ set_check_timer(check_timer, !!strict_timing) != 0) {
+ condlog(1, "%s: errno %d %sarming interrupt timer",
+ __func__, errno, strict_timing ? "" : "dis");
+ strict_timing = 0;
+ }
+ old_strict_timing = strict_timing;
+
+
[dm-devel] [PATCH v3 1/2] libmultipath: fix race in stop_io_err_stat_thread
It's wrong, and unnecessary, to call pthread_kill() after
pthread_cancel(). I have observed cases where the io_err checker
thread hung in libpthread after receiving the USR2 signal, in particular
when multipathd is run under strace. (If multipathd is killed with
SIGINT under strace, and the io_error thread is running, it happens
almost every time). If this happens, the io_err thread
tries to obtain a mutex in the urcu code (presumably rcu_unregister_thread())
and the main thread hangs in pthread_join(). multipathd can only be
terminated with kill -KILL in this situation.
With the change from this patch, the thread is shut down cleanly. I haven't
observed the hang under strace with the patch.
Fixes: 95d594fd "multipath-tools: intermittent IO error accounting to improve
reliability"
Signed-off-by: Martin Wilck
---
libmultipath/io_err_stat.c | 1 -
1 file changed, 1 deletion(-)
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index 00bac9e0e755..536ba87968fd 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -749,7 +749,6 @@ destroy_ctx:
void stop_io_err_stat_thread(void)
{
pthread_cancel(io_err_stat_thr);
- pthread_kill(io_err_stat_thr, SIGUSR2);
pthread_join(io_err_stat_thr, NULL);
free_io_err_pathvec(paths);
io_destroy(ioctx);
--
2.16.1
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH v3 2/2] multipathd: start marginal path checker thread lazily
I noticed that the io_error checker thread accounts for most of the
activity of multipathd even if the marginal path checking paramters
are not set (which is still the default in most installations I assume).
Therefore, start the io_error checker thread only if there's at least
one map with marginal error path checking configured. Also, make sure
the thread is really up when start_io_err_stat_thread() returns.
This requires adding a "vecs" argument to setup_map, because vecs
needs to be passed to the io_error checking code.
Signed-off-by: Martin Wilck
---
libmultipath/configure.c | 14 ---
libmultipath/configure.h | 3 ++-
libmultipath/io_err_stat.c | 58 ++
libmultipath/structs_vec.c | 3 ++-
multipathd/cli_handlers.c | 2 +-
multipathd/main.c | 8 ++-
6 files changed, 72 insertions(+), 16 deletions(-)
diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 42b7c896ee65..fa6e21cb31af 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -41,6 +41,7 @@
#include "uxsock.h"
#include "wwids.h"
#include "sysfs.h"
+#include "io_err_stat.h"
/* group paths in pg by host adapter
*/
@@ -255,7 +256,8 @@ int rr_optimize_path_order(struct pathgroup *pgp)
return 0;
}
-int setup_map(struct multipath *mpp, char *params, int params_size)
+int setup_map(struct multipath *mpp, char *params, int params_size,
+ struct vectors *vecs)
{
struct pathgroup * pgp;
struct config *conf;
@@ -315,6 +317,12 @@ int setup_map(struct multipath *mpp, char *params, int
params_size)
sysfs_set_scsi_tmo(mpp, conf->checkint);
put_multipath_config(conf);
+
+ if (mpp->marginal_path_double_failed_time > 0 &&
+ mpp->marginal_path_err_sample_time > 0 &&
+ mpp->marginal_path_err_recheck_gap_time > 0 &&
+ mpp->marginal_path_err_rate_threshold >= 0)
+ start_io_err_stat_thread(vecs);
/*
* assign paths to path groups -- start with no groups and all paths
* in mpp->paths
@@ -1019,7 +1027,7 @@ int coalesce_paths (struct vectors * vecs, vector newmp,
char * refwwid,
verify_paths(mpp, vecs);
params[0] = '\0';
- if (setup_map(mpp, params, PARAMS_SIZE)) {
+ if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {
remove_map(mpp, vecs, 0);
continue;
}
@@ -1348,7 +1356,7 @@ int reload_map(struct vectors *vecs, struct multipath
*mpp, int refresh,
}
}
}
- if (setup_map(mpp, params, PARAMS_SIZE)) {
+ if (setup_map(mpp, params, PARAMS_SIZE, vecs)) {
condlog(0, "%s: failed to setup map", mpp->alias);
return 1;
}
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 0f5d30a540ca..27a7e6f60a63 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -28,7 +28,8 @@ enum actions {
struct vectors;
-int setup_map (struct multipath * mpp, char * params, int params_size );
+int setup_map (struct multipath * mpp, char * params, int params_size,
+ struct vectors *vecs );
int domap (struct multipath * mpp, char * params, int is_daemon);
int reinstate_paths (struct multipath *mpp);
int coalesce_paths (struct vectors *vecs, vector curmp, char * refwwid, int
force_reload, enum mpath_cmds cmd);
diff --git a/libmultipath/io_err_stat.c b/libmultipath/io_err_stat.c
index 536ba87968fd..02b1453ea527 100644
--- a/libmultipath/io_err_stat.c
+++ b/libmultipath/io_err_stat.c
@@ -74,6 +74,10 @@ struct io_err_stat_path {
pthread_t io_err_stat_thr;
pthread_attr_t io_err_stat_attr;
+static pthread_mutex_t io_err_thread_lock = PTHREAD_MUTEX_INITIALIZER;
+static pthread_cond_t io_err_thread_cond = PTHREAD_COND_INITIALIZER;
+static int io_err_thread_running = 0;
+
static struct io_err_stat_pathvec *paths;
struct vectors *vecs;
io_context_t ioctx;
@@ -316,6 +320,9 @@ int io_err_stat_handle_pathfail(struct path *path)
struct timespec curr_time;
int res;
+ if (uatomic_read(&io_err_thread_running) == 0)
+ return 1;
+
if (path->io_err_disable_reinstate) {
io_err_stat_log(3, "%s: reinstate is already disabled",
path->dev);
@@ -380,6 +387,8 @@ int hit_io_err_recheck_time(struct path *pp)
struct timespec curr_time;
int r;
+ if (uatomic_read(&io_err_thread_running) == 0)
+ return 0;
if (pp->mpp->nr_active <= 0) {
io_err_stat_log(2, "%s: recover path early", pp->dev);
goto recover;
@@ -690,6 +699,16 @@ static void service_paths(void)
pthread_mutex_unlock(&paths->mutex);
}
+static void cleanup_unlock(void *arg)
+{
+ pthread_mutex_unlock((pthread_mutex_t*) arg);
+}
+
+static
[dm-devel] [PATCH v3 0/2] marginal path fixes
Changes wrt v2: - 2/2: Fixed bug that would cause a crash if marginal path checker was off. (if we don't start the thread, we'd better not kill it, either). Martin Wilck (2): libmultipath: fix race in stop_io_err_stat_thread multipathd: start marginal path checker thread lazily libmultipath/configure.c | 14 --- libmultipath/configure.h | 3 ++- libmultipath/io_err_stat.c | 59 ++ libmultipath/structs_vec.c | 3 ++- multipathd/cli_handlers.c | 2 +- multipathd/main.c | 8 ++- 6 files changed, 72 insertions(+), 17 deletions(-) -- 2.16.1 -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] multipath-tools: add info about how to get a release directly from gitweb
On 01/12/2018 05:56 PM, Xose Vazquez Perez wrote: > gitweb is able to extract and serve a release right away. This one is missing. > Cc: Christophe Varoqui > Cc: device-mapper development > Signed-off-by: Xose Vazquez Perez > --- > README | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/README b/README > index 570224d..89bab74 100644 > --- a/README > +++ b/README > @@ -20,7 +20,8 @@ git archive --format=tar.gz --prefix=multipath-tools-X.Y.Z/ > X.Y.Z > ../multipath > > Alternatively it may be obtained from gitweb, go to: > https://git.opensvc.com/?p=multipath-tools/.git;a=tags > -select a release-tag and then click on "snapshot". > +select a release-tag and then click on "snapshot". Or get it with > +wget > "https://git.opensvc.com/?p=multipath-tools/.git;a=snapshot;sf=tgz;h=refs/tags/X.Y.Z"; > -O multipath-tools-X.Y.Z.tar.gz > > > Source code > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] libmultipath: remove FREE_CONST() again
The FREE_CONST macro is of questionable value, as reviewers have pointed
out. The users of this macro were mostly functions that called
uevent_get_dm_xyz(). But these functions don't need to return const char*,
as they allocate the strings they return. So my change of the prototype
was wrong. This patch reverts it. The few other users of FREE_CONST can
also be reverted to use char* instead of const char* with negligible risk.
Fixes: "libmultipath: fix compiler warnings for -Wcast-qual"
Fixes: "libmultipath: const qualifier for wwid and alias"
(Note: this reverts changes not committed upstream. But as these changes are
deeply in the middle of my large-ish series of patches, it's probably easier
to simply add this patch on top than to rebase the whole series).
Signed-off-by: Martin Wilck
---
libmultipath/devmapper.c | 7 +++
libmultipath/memory.h| 6 --
libmultipath/uevent.c| 12 ++--
libmultipath/uevent.h| 6 +++---
multipathd/main.c| 16
tests/uevent.c | 16
6 files changed, 28 insertions(+), 35 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 607aea8dc1fc..767d87c8399f 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -27,7 +27,6 @@
#include
#include
-#define FREE_CONST(p) do { free((void*)(unsigned long)p); p = NULL; } while(0)
#define MAX_WAIT 5
#define LOOPS_PER_SEC 5
@@ -1415,8 +1414,8 @@ out:
void dm_reassign_deps(char *table, const char *dep, const char *newdep)
{
- char *n;
- const char *p, *newtable;
+ char *n, *newtable;
+ const char *p;
newtable = strdup(table);
if (!newtable)
@@ -1427,7 +1426,7 @@ void dm_reassign_deps(char *table, const char *dep, const
char *newdep)
n += strlen(newdep);
p += strlen(dep);
strcat(n, p);
- FREE_CONST(newtable);
+ FREE(newtable);
}
int dm_reassign_table(const char *name, char *old, char *new)
diff --git a/libmultipath/memory.h b/libmultipath/memory.h
index 63f59d80584c..a3c478e24bd1 100644
--- a/libmultipath/memory.h
+++ b/libmultipath/memory.h
@@ -43,7 +43,6 @@ int debug;
(__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
#define STRDUP(n)( dbg_strdup((n), \
(__FILE__), (char *)(__FUNCTION__), (__LINE__)) )
-#define FREE_CONST(p) do { FREE((void*)(unsigned long)p); } while(0)
/* Memory debug prototypes defs */
extern void *dbg_malloc(unsigned long, char *, char *, int);
@@ -56,11 +55,6 @@ extern void dbg_free_final(char *);
#define MALLOC(n)(calloc(1,(n)))
#define FREE(p) do { free(p); p = NULL; } while(0)
-/*
- * Double cast to avoid warnings with -Wcast-qual
- * use this for valid free() operations on const pointers
- */
-#define FREE_CONST(p) do { free((void*)(unsigned long)p); p = NULL; } while(0)
#define REALLOC(p,n) (realloc((p),(n)))
#define STRDUP(n)(strdup(n))
diff --git a/libmultipath/uevent.c b/libmultipath/uevent.c
index 685ef3362c6d..59d4cad88ca3 100644
--- a/libmultipath/uevent.c
+++ b/libmultipath/uevent.c
@@ -157,7 +157,7 @@ static int uevent_get_env_positive_int(const struct uevent
*uev,
void
uevent_get_wwid(struct uevent *uev)
{
- const char *uid_attribute;
+ char *uid_attribute;
const char *val;
struct config * conf;
@@ -168,7 +168,7 @@ uevent_get_wwid(struct uevent *uev)
val = uevent_get_env_var(uev, uid_attribute);
if (val)
uev->wwid = val;
- FREE_CONST(uid_attribute);
+ FREE(uid_attribute);
}
bool
@@ -907,7 +907,7 @@ int uevent_get_disk_ro(const struct uevent *uev)
return uevent_get_env_positive_int(uev, "DISK_RO");
}
-static const char *uevent_get_dm_str(const struct uevent *uev, char *attr)
+static char *uevent_get_dm_str(const struct uevent *uev, char *attr)
{
const char *tmp = uevent_get_env_var(uev, attr);
@@ -916,17 +916,17 @@ static const char *uevent_get_dm_str(const struct uevent
*uev, char *attr)
return strdup(tmp);
}
-const char *uevent_get_dm_name(const struct uevent *uev)
+char *uevent_get_dm_name(const struct uevent *uev)
{
return uevent_get_dm_str(uev, "DM_NAME");
}
-const char *uevent_get_dm_path(const struct uevent *uev)
+char *uevent_get_dm_path(const struct uevent *uev)
{
return uevent_get_dm_str(uev, "DM_PATH");
}
-const char *uevent_get_dm_action(const struct uevent *uev)
+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 cb5347e45c2b..0aa867510f28 100644
--- a/libmultipath/uevent.h
+++ b/libmultipath/uevent.h
@@ -36,9 +36,9 @@ int uevent_dispatch(int (*store_uev)(struct uevent *, void *
trigger_data),
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);
Re: [dm-devel] [PATCH 0/6] Various multipath-tools patches
Hi Bart, I've finally merged the queued patches, so you can rebase this patchset over the new head. Thanks. On Tue, Mar 6, 2018 at 4:25 PM, Benjamin Marzinski wrote: > On Thu, Mar 01, 2018 at 11:29:29AM -0800, Bart Van Assche wrote: > > Hello Christophe, > > > > This series contains the following patches: > > - Four patches address Coverity complaints. > > - One patch makes a function argument const. > > - One patch addresses a bug that I discovered while inspecting the > multipath > > command line tool output. > > > > Please consider these patches for the multipath-tools project. > > Reviewed-by: Benjamin Marzinski > > For the set. Buy I agree with Martin that we should fold some of these > into his already posted patches. > > -Ben > > > > > Thanks, > > > > Bart. > > > > Bart Van Assche (6): > > multipathd/main.c: Fix indentation > > libmultipath, alloc_path_with_pathinfo(): Ensure that pp->wwid is > > '\0'-terminated > > libmultipath, alloc_path_with_pathinfo(): Declare third argument const > > kpartx: Improve reliability of find_loop_by_file() > > libmultipath: Fix sgio_get_vpd() > > Introduce the ibmultipath/unaligned.h header file > > > > kpartx/lopart.c | 7 --- > > libmpathpersist/mpath_pr_ioctl.c | 8 > > libmultipath/checkers/hp_sw.c | 4 ++-- > > libmultipath/discovery.c | 22 +++- > > libmultipath/discovery.h | 2 +- > > libmultipath/prioritizers/alua_rtpg.c | 13 ++-- > > libmultipath/prioritizers/alua_spc3.h | 35 > ++-- > > libmultipath/prioritizers/ontap.c | 4 ++-- > > libmultipath/unaligned.h | 38 > +++ > > multipathd/main.c | 4 ++-- > > 10 files changed, 74 insertions(+), 63 deletions(-) > > create mode 100644 libmultipath/unaligned.h > > > > -- > > 2.16.2 > > > > -- > > dm-devel mailing list > > dm-devel@redhat.com > > https://www.redhat.com/mailman/listinfo/dm-devel > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [RFC PATCH 00/16] multipath path classification
Hello Christophe, On Wed, 2018-03-07 at 09:53 +0100, Christophe Varoqui wrote: > Martin, Ben, > > can you update me on the status of this patchset ? > Is it ready for inclusion ? No. I am preparing an updated set that's much improved. Regards Martin -- Dr. Martin Wilck , 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: [dm-devel] confusion about multipath_prepare_ioctl
On 2018年03月06日 13:44, Mike Snitzer wrote:
> On Mon, Mar 05 2018 at 10:35pm -0500,
> Wang Sheng-Hui wrote:
>
>> Dear,
>>
>> Sorry to trouble you.
>>
>> I noticed some code in dm-*.c like:
>> "
>> static int multipath_prepare_ioctl(struct dm_target *ti,
>> struct block_device **bdev, fmode_t *mode)
>> {
>> ...
>> /*
>> * Only pass ioctls through if the device sizes match exactly.
>> */
>> if (!r && ti->len != i_size_read((*bdev)->bd_inode) >> SECTOR_SHIFT)
>> return 1;
>> ...
>> }
>> "
>> Here, return value 1 means
>> "ioctl is being issued against a subset of the parent bdev; require extra
>> privileges."
>> (comment in dm_blk_ioctl)
>>
>> I'm confused by the comment and '!=' test for multipath.
>> In which cases, the size of low level single device is not equal to the
>> parent
>> size of multipath device?
> Given that ti->len is sent down from userspace, the DM multipath
> target's ti->len _could_ be smaller than the underlying path(s). But in
> practice that doesn't occur with multipathd.. a partitioned multipath
> device is generally done, via kpartx, in terms of linear mappings ontop
> of the multipath device.
>
> The same != test is done in the dm linear target and is much more
> relevant to concerns about ioctls being sent to partition.
>
> Mike
>
>
Thanks for your explanation, Mike!
Regards,
shenghui
--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel
[dm-devel] [PATCH] Add an option to dm-verity to validate hashes at most once
Add an option to dm-verity to validate hashes at most once
to allow platforms that is CPU/memory contraint to be
protected by dm-verity against offline attacks.
The option introduces a bitset that is used to check if
a block has been validated before or not. A block can
be validated more than once as there is no thread protection
for the bitset.
This patch has been developed and tested on entry-level
Android Go devices.
Signed-off-by: Patrik Torstensson
---
drivers/md/dm-verity-target.c | 58 +--
drivers/md/dm-verity.h| 1 +
2 files changed, 56 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index aedb8222836b..479d0af212bf 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -32,6 +32,7 @@
#define DM_VERITY_OPT_LOGGING "ignore_corruption"
#define DM_VERITY_OPT_RESTART "restart_on_corruption"
#define DM_VERITY_OPT_IGN_ZEROES "ignore_zero_blocks"
+#define DM_VERITY_OPT_AT_MOST_ONCE "check_at_most_once"
#define DM_VERITY_OPTS_MAX (2 + DM_VERITY_OPTS_FEC)
@@ -432,6 +433,19 @@ static int verity_bv_zero(struct dm_verity *v, struct
dm_verity_io *io,
return 0;
}
+/*
+ * Moves the bio iter one data block forward.
+ */
+static inline void verity_bv_skip_block(struct dm_verity *v,
+ struct dm_verity_io *io,
+ struct bvec_iter *iter)
+{
+ struct bio *bio = dm_bio_from_per_bio_data(io,
+ v->ti->per_io_data_size);
+
+ bio_advance_iter(bio, iter, 1 << v->data_dev_block_bits);
+}
+
/*
* Verify one "dm_verity_io" structure.
*/
@@ -445,8 +459,15 @@ static int verity_verify_io(struct dm_verity_io *io)
for (b = 0; b < io->n_blocks; b++) {
int r;
+ sector_t cur_block = io->block + b;
struct ahash_request *req = verity_io_hash_req(v, io);
+ if (v->validated_blocks &&
+ likely(test_bit(cur_block, v->validated_blocks))) {
+ verity_bv_skip_block(v, io, &io->iter);
+ continue;
+ }
+
r = verity_hash_for_block(v, io, io->block + b,
verity_io_want_digest(v, io),
&is_zero);
@@ -481,13 +502,17 @@ static int verity_verify_io(struct dm_verity_io *io)
return r;
if (likely(memcmp(verity_io_real_digest(v, io),
- verity_io_want_digest(v, io), v->digest_size)
== 0))
+ verity_io_want_digest(v, io),
+ v->digest_size) == 0)) {
+ if (v->validated_blocks)
+ set_bit(cur_block, v->validated_blocks);
continue;
+ }
else if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_DATA,
- io->block + b, NULL, &start) == 0)
+ cur_block, NULL, &start) == 0)
continue;
else if (verity_handle_err(v, DM_VERITY_BLOCK_TYPE_DATA,
- io->block + b))
+ cur_block))
return -EIO;
}
@@ -740,6 +765,7 @@ static void verity_dtr(struct dm_target *ti)
if (v->bufio)
dm_bufio_client_destroy(v->bufio);
+ kvfree(v->validated_blocks);
kfree(v->salt);
kfree(v->root_digest);
kfree(v->zero_digest);
@@ -760,6 +786,26 @@ static void verity_dtr(struct dm_target *ti)
kfree(v);
}
+static int verity_alloc_most_once(struct dm_verity *v)
+{
+ struct dm_target *ti = v->ti;
+
+ /* the bitset can only handle INT_MAX blocks */
+ if (v->data_blocks > INT_MAX) {
+ ti->error = "device too large to use check_at_most_once";
+ return -E2BIG;
+ }
+
+ v->validated_blocks = kvzalloc(BITS_TO_LONGS(v->data_blocks)
+* sizeof(unsigned long), GFP_KERNEL);
+ if (!v->validated_blocks) {
+ ti->error = "failed to allocate bitset for check_at_most_once";
+ return -ENOMEM;
+ }
+
+ return 0;
+}
+
static int verity_alloc_zero_digest(struct dm_verity *v)
{
int r = -ENOMEM;
@@ -829,6 +875,12 @@ static int verity_parse_opt_args(struct dm_arg_set *as,
struct dm_verity *v)
}
continue;
+ } else if (!strcasecmp(arg_name, DM_VERITY_OPT_AT_MOST_ONCE)) {
+ r = verity_alloc_most_once(v);
+ if (r)
+ return r;
+ continue;
Re: [dm-devel] [RFC PATCH 00/16] multipath path classification
Martin, Ben, can you update me on the status of this patchset ? Is it ready for inclusion ? Thanks, Christophe On Fri, Jan 19, 2018 at 1:29 AM, Martin Wilck wrote: > This patch series implements the recommendation in my recent posting > "Multipath path classification revisited". My testing has been surprisingly > successful so far; more testing is of course needed. > Anyway, I think it's in a shape that I can ask for review. > > I've seen Ben's detailed reply to my posting, but his comments haven't been > taken into account in this series yet. > > Benjamin Marzinski (1): > libmultipath: trigger change uevent on new device creation > > Martin Wilck (15): > Revert "multipath: ignore -i if find_multipaths is set" > Revert "multipathd: imply -n if find_multipaths is set" > libmultipath: add mpvec param to should_multipath() > libmultipath: should_multipath: keep existing maps > multipath -u -i: change logic for find_multipaths > libmultipath: let ignore_wwids be set in config file > multipathd: replace -n with !ignore_wwids > multipath.conf.5: document "ignore_wwids" > multipath.8: adapt documentation of '-i' > multipathd.8: document that '-n' is now ignored > multipath: common code path for CMD_VALID_PATH > multipath -u/-c: change output to environment/key format > multipath -u/-c: add "$DEV is maybe a valid path" > multipath.rules: find_multipaths+ignore_wwids logic > libmultipath: trigger path uevent only when necessary > > libmultipath/config.c | 1 + > libmultipath/config.h | 1 - > libmultipath/configure.c | 48 --- > libmultipath/configure.h | 1 + > libmultipath/defaults.h| 1 + > libmultipath/dict.c| 4 +++ > libmultipath/wwids.c | 13 +++--- > libmultipath/wwids.h | 2 +- > multipath/main.c | 58 ++ > > multipath/multipath.8 | 3 ++- > multipath/multipath.conf.5 | 31 +++ > multipath/multipath.rules | 63 ++ > +--- > multipathd/main.c | 16 +++- > multipathd/multipathd.8| 5 ++-- > 14 files changed, 192 insertions(+), 55 deletions(-) > > -- > 2.15.1 > > -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] dm-bufio: avoid false-positive Wmaybe-uninitialized warning
On Wed, Mar 7, 2018 at 2:29 AM, Mike Snitzer wrote: > On Tue, Mar 06 2018 at 4:33pm -0500, > Arnd Bergmann wrote: > >> On Thu, Feb 22, 2018 at 5:04 PM, Mike Snitzer wrote: >> > On Thu, Feb 22 2018 at 10:56am -0500, >> > Arnd Bergmann wrote: >> >> > >> > Mikulas already sent a fix for this: >> > https://patchwork.kernel.org/patch/10211631/ >> > >> > But I like yours a bit better, though I'll likely move the declaration >> > of 'noio_flag' temporary inside the conditional. >> > >> > Anyway, I'll get this fixed up shortly, thanks. >> >> I see the fix made it into linux-next on the same day, but the build bots >> still >> report the warning for mainline kernels and now also for stable kernels >> that got a backport of the patch that introduced it on arm64. >> >> I assume you had not planned to send it for mainline, any chance you >> could change that and send it as a bugfix with a 'Cc: >> sta...@vger.kernel.org' tag to restore a clean build? > > I did/do plan to send to Linus this week. > > But I've updated the commit header to include the CC: stable like you > asked. Got it, thanks a lot, that brings us down to two kernelci warnings for 4.16 and one for 4.15-stable then. Arnd -- dm-devel mailing list dm-devel@redhat.com https://www.redhat.com/mailman/listinfo/dm-devel
