* [PATCH v2 1/6] libmultipath: make libmp_dm_init optional
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 2/6] libmultipath: make sysfs_is_multipathed able to return wwid Benjamin Marzinski
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
Move dm_initialized out of libmp_dm_task_create(), and add
a function skip_libmp_dm_init() so that users of libmultipath can skip
initializing device-mapper. This is needed for other programs that
use libmultipath (or a library that depends on it) but want to control
how device-mapper is set up.
Also make dm_prereq a global function.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/devmapper.c | 17 +++++++++++++----
libmultipath/devmapper.h | 3 ++-
2 files changed, 15 insertions(+), 5 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 13a1cf53..7ed494a1 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -33,6 +33,8 @@
#define MAX_WAIT 5
#define LOOPS_PER_SEC 5
+static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
+
static int dm_conf_verbosity;
#ifdef LIBDM_API_DEFERRED
@@ -229,7 +231,7 @@ dm_tgt_prereq (unsigned int *ver)
return 1;
}
-static int dm_prereq(unsigned int *v)
+int dm_prereq(unsigned int *v)
{
if (dm_lib_prereq())
return 1;
@@ -243,7 +245,7 @@ void libmp_udev_set_sync_support(int on)
libmp_dm_udev_sync = !!on;
}
-void libmp_dm_init(void)
+static void libmp_dm_init(void)
{
struct config *conf;
int verbosity;
@@ -262,11 +264,18 @@ void libmp_dm_init(void)
dm_udev_set_sync_support(libmp_dm_udev_sync);
}
+static void _do_skip_libmp_dm_init(void)
+{
+}
+
+void skip_libmp_dm_init(void)
+{
+ pthread_once(&dm_initialized, _do_skip_libmp_dm_init);
+}
+
struct dm_task*
libmp_dm_task_create(int task)
{
- static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
-
pthread_once(&dm_initialized, libmp_dm_init);
return dm_task_create(task);
}
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 7557a86b..17fc9faf 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -28,7 +28,8 @@
#define UUID_PREFIX_LEN (sizeof(UUID_PREFIX) - 1)
void dm_init(int verbosity);
-void libmp_dm_init(void);
+int dm_prereq(unsigned int *v);
+void skip_libmp_dm_init(void);
void libmp_udev_set_sync_support(int on);
struct dm_task *libmp_dm_task_create(int task);
int dm_drv_version (unsigned int * version);
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/6] libmultipath: make sysfs_is_multipathed able to return wwid
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 1/6] libmultipath: make libmp_dm_init optional Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 3/6] multipath: centralize validation code Benjamin Marzinski
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
sysfs_is_multipathed reads the wwid of the dm device holding a path to
check if its a multipath device. Add code to optinally set pp->wwid to
that wwid. This will be used by a future patch.
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/sysfs.c | 24 +++++++++++++++++++-----
libmultipath/sysfs.h | 2 +-
multipath/main.c | 7 ++++---
3 files changed, 24 insertions(+), 9 deletions(-)
diff --git a/libmultipath/sysfs.c b/libmultipath/sysfs.c
index 62ec2ed7..12a82d95 100644
--- a/libmultipath/sysfs.c
+++ b/libmultipath/sysfs.c
@@ -295,7 +295,7 @@ static int select_dm_devs(const struct dirent *di)
return fnmatch("dm-*", di->d_name, FNM_FILE_NAME) == 0;
}
-bool sysfs_is_multipathed(const struct path *pp)
+bool sysfs_is_multipathed(struct path *pp, bool set_wwid)
{
char pathbuf[PATH_MAX];
struct scandir_result sr;
@@ -325,7 +325,7 @@ bool sysfs_is_multipathed(const struct path *pp)
for (i = 0; i < r && !found; i++) {
long fd;
int nr;
- char uuid[6];
+ char uuid[WWID_SIZE + UUID_PREFIX_LEN];
if (safe_snprintf(pathbuf + n, sizeof(pathbuf) - n,
"/%s/dm/uuid", di[i]->d_name))
@@ -339,12 +339,26 @@ bool sysfs_is_multipathed(const struct path *pp)
pthread_cleanup_push(close_fd, (void *)fd);
nr = read(fd, uuid, sizeof(uuid));
- if (nr == sizeof(uuid) && !memcmp(uuid, "mpath-", sizeof(uuid)))
+ if (nr > (int)UUID_PREFIX_LEN &&
+ !memcmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN))
found = true;
else if (nr < 0) {
- condlog(1, "%s: error reading from %s: %s",
- __func__, pathbuf, strerror(errno));
+ condlog(1, "%s: error reading from %s: %m",
+ __func__, pathbuf);
}
+ if (found && set_wwid) {
+ nr -= UUID_PREFIX_LEN;
+ memcpy(pp->wwid, uuid + UUID_PREFIX_LEN, nr);
+ if (nr == WWID_SIZE) {
+ condlog(4, "%s: overflow while reading from %s",
+ __func__, pathbuf);
+ pp->wwid[0] = '\0';
+ } else {
+ pp->wwid[nr] = '\0';
+ strchop(pp->wwid);
+ }
+ }
+
pthread_cleanup_pop(1);
}
pthread_cleanup_pop(1);
diff --git a/libmultipath/sysfs.h b/libmultipath/sysfs.h
index 9ae30b39..72b39ab2 100644
--- a/libmultipath/sysfs.h
+++ b/libmultipath/sysfs.h
@@ -14,5 +14,5 @@ ssize_t sysfs_bin_attr_get_value(struct udev_device *dev, const char *attr_name,
unsigned char * value, size_t value_len);
int sysfs_get_size (struct path *pp, unsigned long long * size);
int sysfs_check_holders(char * check_devt, char * new_devt);
-bool sysfs_is_multipathed(const struct path *pp);
+bool sysfs_is_multipathed(struct path *pp, bool set_wwid);
#endif
diff --git a/multipath/main.c b/multipath/main.c
index 78822ee1..f25b8693 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -640,7 +640,8 @@ configure (struct config *conf, enum mpath_cmds cmd,
* Shortcut for find_multipaths smart:
* Quick check if path is already multipathed.
*/
- if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0))) {
+ if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
+ false)) {
r = RTVL_YES;
goto print_valid;
}
@@ -749,8 +750,8 @@ configure (struct config *conf, enum mpath_cmds cmd,
/*
* Check if we raced with multipathd
*/
- r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0)) ?
- RTVL_YES : RTVL_NO;
+ r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
+ false) ? RTVL_YES : RTVL_NO;
}
goto print_valid;
}
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/6] multipath: centralize validation code
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 1/6] libmultipath: make libmp_dm_init optional Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 2/6] libmultipath: make sysfs_is_multipathed able to return wwid Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 4/6] Unit tests for is_path_valid() Benjamin Marzinski
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
This code pulls the multipath path validation code out of configure(),
and puts it into its own function, check_path_valid(). This function
calls a new libmultipath function, is_path_valid() to check just path
requested. This seperation exists so that is_path_valid() can be reused
by future code. This code will give almost the same answer as the
existing code, with the exception that now, if a device is currently
multipathed, it will always be a valid multipath path.
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/Makefile | 3 +-
libmultipath/devmapper.c | 45 ++++++
libmultipath/devmapper.h | 1 +
libmultipath/structs.h | 24 +---
libmultipath/valid.c | 118 ++++++++++++++++
libmultipath/valid.h | 42 ++++++
libmultipath/wwids.c | 10 +-
multipath/main.c | 296 +++++++++++++++++----------------------
8 files changed, 344 insertions(+), 195 deletions(-)
create mode 100644 libmultipath/valid.c
create mode 100644 libmultipath/valid.h
diff --git a/libmultipath/Makefile b/libmultipath/Makefile
index ad690a49..436f479d 100644
--- a/libmultipath/Makefile
+++ b/libmultipath/Makefile
@@ -47,7 +47,8 @@ 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 file.o wwids.o prioritizers/alua_rtpg.o prkey.o \
- io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o
+ io_err_stat.o dm-generic.o generic.o foreign.o nvme-lib.o \
+ valid.o
all: $(LIBS)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 7ed494a1..27d52398 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -770,6 +770,51 @@ out:
return r;
}
+/*
+ * Return
+ * 1 : map with uuid exists
+ * 0 : map with uuid doesn't exist
+ * -1 : error
+ */
+int
+dm_map_present_by_uuid(const char *uuid)
+{
+ struct dm_task *dmt;
+ struct dm_info info;
+ char prefixed_uuid[WWID_SIZE + UUID_PREFIX_LEN];
+ int r = -1;
+
+ if (!uuid || uuid[0] == '\0')
+ return 0;
+
+ if (safe_sprintf(prefixed_uuid, UUID_PREFIX "%s", uuid))
+ goto out;
+
+ if (!(dmt = dm_task_create(DM_DEVICE_INFO)))
+ goto out;
+
+ dm_task_no_open_count(dmt);
+
+ if (!dm_task_set_uuid(dmt, prefixed_uuid))
+ goto out_task;
+
+ if (!dm_task_run(dmt))
+ goto out_task;
+
+ if (!dm_task_get_info(dmt, &info))
+ goto out_task;
+
+ r = !!info.exists;
+
+out_task:
+ dm_task_destroy(dmt);
+out:
+ if (r < 0)
+ condlog(3, "%s: dm command failed in %s: %s", uuid,
+ __FUNCTION__, strerror(errno));
+ return r;
+}
+
static int
dm_dev_t (const char * mapname, char * dev_t, int len)
{
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 17fc9faf..5ed7edc5 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -39,6 +39,7 @@ int dm_simplecmd_noflush (int, const char *, uint16_t);
int dm_addmap_create (struct multipath *mpp, char *params);
int dm_addmap_reload (struct multipath *mpp, char *params, int flush);
int dm_map_present (const char *);
+int dm_map_present_by_uuid(const char *uuid);
int dm_get_map(const char *, unsigned long long *, char *);
int dm_get_status(const char *, char *);
int dm_type(const char *, char *);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 9bd39eb1..d69bc2e9 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -101,29 +101,13 @@ enum yes_no_undef_states {
YNU_YES,
};
-#define _FIND_MULTIPATHS_F (1 << 1)
-#define _FIND_MULTIPATHS_I (1 << 2)
-#define _FIND_MULTIPATHS_N (1 << 3)
-/*
- * _FIND_MULTIPATHS_F must have the same value as YNU_YES.
- * Generate a compile time error if that isn't the case.
- */
-extern char ___error1___[-(_FIND_MULTIPATHS_F != YNU_YES)];
-
-#define find_multipaths_on(conf) \
- (!!((conf)->find_multipaths & _FIND_MULTIPATHS_F))
-#define ignore_wwids_on(conf) \
- (!!((conf)->find_multipaths & _FIND_MULTIPATHS_I))
-#define ignore_new_devs_on(conf) \
- (!!((conf)->find_multipaths & _FIND_MULTIPATHS_N))
-
enum find_multipaths_states {
FIND_MULTIPATHS_UNDEF = YNU_UNDEF,
FIND_MULTIPATHS_OFF = YNU_NO,
- FIND_MULTIPATHS_ON = _FIND_MULTIPATHS_F,
- FIND_MULTIPATHS_GREEDY = _FIND_MULTIPATHS_I,
- FIND_MULTIPATHS_SMART = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_I,
- FIND_MULTIPATHS_STRICT = _FIND_MULTIPATHS_F|_FIND_MULTIPATHS_N,
+ FIND_MULTIPATHS_ON = YNU_YES,
+ FIND_MULTIPATHS_GREEDY,
+ FIND_MULTIPATHS_SMART,
+ FIND_MULTIPATHS_STRICT,
__FIND_MULTIPATHS_LAST,
};
diff --git a/libmultipath/valid.c b/libmultipath/valid.c
new file mode 100644
index 00000000..456b1f6e
--- /dev/null
+++ b/libmultipath/valid.c
@@ -0,0 +1,118 @@
+/*
+ Copyright (c) 2020 Benjamin Marzinski, IBM
+
+ 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, see <https://www.gnu.org/licenses/>.
+ */
+#include <stddef.h>
+#include <errno.h>
+#include <libudev.h>
+
+#include "vector.h"
+#include "config.h"
+#include "debug.h"
+#include "util.h"
+#include "devmapper.h"
+#include "discovery.h"
+#include "wwids.h"
+#include "sysfs.h"
+#include "blacklist.h"
+#include "mpath_cmd.h"
+#include "valid.h"
+
+int
+is_path_valid(const char *name, struct config *conf, struct path *pp,
+ bool check_multipathd)
+{
+ int r;
+ int fd;
+
+ if (!pp || !name || !conf)
+ return PATH_IS_ERROR;
+
+ if (conf->find_multipaths <= FIND_MULTIPATHS_UNDEF ||
+ conf->find_multipaths >= __FIND_MULTIPATHS_LAST)
+ return PATH_IS_ERROR;
+
+ if (safe_sprintf(pp->dev, "%s", name))
+ return PATH_IS_ERROR;
+
+ if (sysfs_is_multipathed(pp, true)) {
+ if (pp->wwid[0] == '\0')
+ return PATH_IS_ERROR;
+ return PATH_IS_VALID_NO_CHECK;
+ }
+
+ /*
+ * "multipath -u" may be run before the daemon is started. In this
+ * case, systemd might own the socket but might delay multipathd
+ * startup until some other unit (udev settle!) has finished
+ * starting. With many LUNs, the listen backlog may be exceeded, which
+ * would cause connect() to block. This causes udev workers calling
+ * "multipath -u" to hang, and thus creates a deadlock, until "udev
+ * settle" times out. To avoid this, call connect() in non-blocking
+ * mode here, and take EAGAIN as indication for a filled-up systemd
+ * backlog.
+ */
+
+ if (check_multipathd) {
+ fd = __mpath_connect(1);
+ if (fd < 0) {
+ if (errno != EAGAIN && !systemd_service_enabled(name)) {
+ condlog(3, "multipathd not running or enabled");
+ return PATH_IS_NOT_VALID;
+ }
+ } else
+ mpath_disconnect(fd);
+ }
+
+ pp->udev = udev_device_new_from_subsystem_sysname(udev, "block", name);
+ if (!pp->udev)
+ return PATH_IS_ERROR;
+
+ r = pathinfo(pp, conf, DI_SYSFS | DI_WWID | DI_BLACKLIST);
+ if (r == PATHINFO_SKIPPED)
+ return PATH_IS_NOT_VALID;
+ else if (r)
+ return PATH_IS_ERROR;
+
+ if (pp->wwid[0] == '\0')
+ return PATH_IS_NOT_VALID;
+
+ if (pp->udev && pp->uid_attribute &&
+ filter_property(conf, pp->udev, 3, pp->uid_attribute) > 0)
+ return PATH_IS_NOT_VALID;
+
+ r = is_failed_wwid(pp->wwid);
+ if (r != WWID_IS_NOT_FAILED) {
+ if (r == WWID_IS_FAILED)
+ return PATH_IS_NOT_VALID;
+ return PATH_IS_ERROR;
+ }
+
+ if (conf->find_multipaths == FIND_MULTIPATHS_GREEDY)
+ return PATH_IS_VALID;
+
+ if (check_wwids_file(pp->wwid, 0) == 0)
+ return PATH_IS_VALID_NO_CHECK;
+
+ if (dm_map_present_by_uuid(pp->wwid) == 1)
+ return PATH_IS_VALID;
+
+ /* all these act like FIND_MULTIPATHS_STRICT for finding if a
+ * path is valid */
+ if (conf->find_multipaths != FIND_MULTIPATHS_SMART)
+ return PATH_IS_NOT_VALID;
+
+ return PATH_IS_MAYBE_VALID;
+}
diff --git a/libmultipath/valid.h b/libmultipath/valid.h
new file mode 100644
index 00000000..ce1c7cbf
--- /dev/null
+++ b/libmultipath/valid.h
@@ -0,0 +1,42 @@
+/*
+ Copyright (c) 2020 Benjamin Marzinski, IBM
+
+ 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, see <https://www.gnu.org/licenses/>.
+ */
+#ifndef _VALID_H
+#define _VALID_H
+
+/*
+ * PATH_IS_VALID_NO_CHECK is returned when multipath should claim
+ * the path, regardless of whether is has been released to systemd
+ * already.
+ * PATH_IS_VALID is returned by is_path_valid, when the path is
+ * valid only if it hasn't been released to systemd already.
+ * PATH_IS_MAYBE_VALID is returned when the the path would be valid
+ * if other paths with the same wwid existed. It is up to the caller
+ * to check for these other paths.
+ */
+enum is_path_valid_result {
+ PATH_IS_ERROR = -1,
+ PATH_IS_NOT_VALID,
+ PATH_IS_VALID,
+ PATH_IS_VALID_NO_CHECK,
+ PATH_IS_MAYBE_VALID,
+ PATH_MAX_VALID_RESULT, /* only for bounds checking */
+};
+
+int is_path_valid(const char *name, struct config *conf, struct path *pp,
+ bool check_multipathd);
+
+#endif /* _VALID_D */
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index fab6fc8f..397f6e54 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -289,19 +289,19 @@ out:
int
should_multipath(struct path *pp1, vector pathvec, vector mpvec)
{
- int i, ignore_new_devs, find_multipaths;
+ int i, find_multipaths;
struct path *pp2;
struct config *conf;
conf = get_multipath_config();
- ignore_new_devs = ignore_new_devs_on(conf);
- find_multipaths = find_multipaths_on(conf);
+ find_multipaths = conf->find_multipaths;
put_multipath_config(conf);
- if (!find_multipaths && !ignore_new_devs)
+ if (find_multipaths == FIND_MULTIPATHS_OFF ||
+ find_multipaths == FIND_MULTIPATHS_GREEDY)
return 1;
condlog(4, "checking if %s should be multipathed", pp1->dev);
- if (!ignore_new_devs) {
+ if (find_multipaths != FIND_MULTIPATHS_STRICT) {
char tmp_wwid[WWID_SIZE];
struct multipath *mp = find_mp_by_wwid(mpvec, pp1->wwid);
diff --git a/multipath/main.c b/multipath/main.c
index f25b8693..5eac757a 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -63,21 +63,18 @@
#include "propsel.h"
#include "time-util.h"
#include "file.h"
+#include "valid.h"
int logsink;
struct udev *udev;
struct config *multipath_conf;
/*
- * Return values of configure(), print_cmd_valid(), and main().
- * RTVL_{YES,NO} are synonyms for RTVL_{OK,FAIL} for the CMD_VALID_PATH case.
+ * Return values of configure(), check_path_valid(), and main().
*/
enum {
RTVL_OK = 0,
- RTVL_YES = RTVL_OK,
RTVL_FAIL = 1,
- RTVL_NO = RTVL_FAIL,
- RTVL_MAYBE, /* only used internally, never returned */
RTVL_RETRY, /* returned by configure(), not by main() */
};
@@ -271,9 +268,6 @@ get_dm_mpvec (enum mpath_cmds cmd, vector curmp, vector pathvec, char * refwwid)
continue;
}
- if (cmd == CMD_VALID_PATH)
- continue;
-
dm_get_map(mpp->alias, &mpp->size, params);
condlog(3, "params = %s", params);
dm_get_status(mpp->alias, status);
@@ -493,10 +487,11 @@ static int print_cmd_valid(int k, const vector pathvec,
struct timespec until;
struct path *pp;
- if (k != RTVL_YES && k != RTVL_NO && k != RTVL_MAYBE)
- return RTVL_NO;
+ if (k != PATH_IS_VALID && k != PATH_IS_NOT_VALID &&
+ k != PATH_IS_MAYBE_VALID)
+ return PATH_IS_NOT_VALID;
- if (k == RTVL_MAYBE) {
+ if (k == PATH_IS_MAYBE_VALID) {
/*
* Caller ensures that pathvec[0] is the path to
* examine.
@@ -506,7 +501,7 @@ static int print_cmd_valid(int k, const vector pathvec,
wait = find_multipaths_check_timeout(
pp, pp->find_multipaths_timeout, &until);
if (wait != FIND_MULTIPATHS_WAITING)
- k = RTVL_NO;
+ k = PATH_IS_NOT_VALID;
} else if (pathvec != NULL && (pp = VECTOR_SLOT(pathvec, 0)))
wait = find_multipaths_check_timeout(pp, 0, &until);
if (wait == FIND_MULTIPATHS_WAITING)
@@ -515,9 +510,9 @@ static int print_cmd_valid(int k, const vector pathvec,
else if (wait == FIND_MULTIPATHS_WAIT_DONE)
printf("FIND_MULTIPATHS_WAIT_UNTIL=\"0\"\n");
printf("DM_MULTIPATH_DEVICE_PATH=\"%d\"\n",
- k == RTVL_MAYBE ? 2 : k == RTVL_YES ? 1 : 0);
+ k == PATH_IS_MAYBE_VALID ? 2 : k == PATH_IS_VALID ? 1 : 0);
/* Never return RTVL_MAYBE */
- return k == RTVL_NO ? RTVL_NO : RTVL_YES;
+ return k == PATH_IS_NOT_VALID ? PATH_IS_NOT_VALID : PATH_IS_VALID;
}
/*
@@ -550,7 +545,6 @@ configure (struct config *conf, enum mpath_cmds cmd,
int di_flag = 0;
char * refwwid = NULL;
char * dev = NULL;
- bool released = released_to_systemd();
/*
* allocate core vectors to store paths and multipaths
@@ -575,7 +569,7 @@ configure (struct config *conf, enum mpath_cmds cmd,
cmd != CMD_REMOVE_WWID &&
(filter_devnode(conf->blist_devnode,
conf->elist_devnode, dev) > 0)) {
- goto print_valid;
+ goto out;
}
/*
@@ -583,14 +577,10 @@ configure (struct config *conf, enum mpath_cmds cmd,
* failing the translation is fatal (by policy)
*/
if (devpath) {
- int failed = get_refwwid(cmd, devpath, dev_type,
- pathvec, &refwwid);
+ get_refwwid(cmd, devpath, dev_type, pathvec, &refwwid);
if (!refwwid) {
condlog(4, "%s: failed to get wwid", devpath);
- if (failed == 2 && cmd == CMD_VALID_PATH)
- goto print_valid;
- else
- condlog(3, "scope is null");
+ condlog(3, "scope is null");
goto out;
}
if (cmd == CMD_REMOVE_WWID) {
@@ -616,53 +606,6 @@ configure (struct config *conf, enum mpath_cmds cmd,
goto out;
}
condlog(3, "scope limited to %s", refwwid);
- /* If you are ignoring the wwids file and find_multipaths is
- * set, you need to actually check if there are two available
- * paths to determine if this path should be multipathed. To
- * do this, we put off the check until after discovering all
- * the paths.
- * Paths listed in the wwids file are always considered valid.
- */
- if (cmd == CMD_VALID_PATH) {
- if (is_failed_wwid(refwwid) == WWID_IS_FAILED) {
- r = RTVL_NO;
- goto print_valid;
- }
- if ((!find_multipaths_on(conf) &&
- ignore_wwids_on(conf)) ||
- check_wwids_file(refwwid, 0) == 0)
- r = RTVL_YES;
- if (!ignore_wwids_on(conf))
- goto print_valid;
- /* At this point, either r==0 or find_multipaths_on. */
-
- /*
- * Shortcut for find_multipaths smart:
- * Quick check if path is already multipathed.
- */
- if (sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
- false)) {
- r = RTVL_YES;
- goto print_valid;
- }
-
- /*
- * DM_MULTIPATH_DEVICE_PATH=="0" means that we have
- * been called for this device already, and have
- * released it to systemd. Unless the device is now
- * already multipathed (see above), we can't try to
- * grab it, because setting SYSTEMD_READY=0 would
- * cause file systems to be unmounted.
- * Leave DM_MULTIPATH_DEVICE_PATH="0".
- */
- if (released) {
- r = RTVL_NO;
- goto print_valid;
- }
- if (r == RTVL_YES)
- goto print_valid;
- /* find_multipaths_on: Fall through to path detection */
- }
}
/*
@@ -703,59 +646,6 @@ configure (struct config *conf, enum mpath_cmds cmd,
goto out;
}
- if (cmd == CMD_VALID_PATH) {
- struct path *pp;
- int fd;
-
- /* This only happens if find_multipaths and
- * ignore_wwids is set, and the path is not in WWIDs
- * file, not currently multipathed, and has
- * never been released to systemd.
- * If there is currently a multipath device matching
- * the refwwid, or there is more than one path matching
- * the refwwid, then the path is valid */
- if (VECTOR_SIZE(curmp) != 0) {
- r = RTVL_YES;
- goto print_valid;
- } else if (VECTOR_SIZE(pathvec) > 1)
- r = RTVL_YES;
- else
- r = RTVL_MAYBE;
-
- /*
- * If opening the path with O_EXCL fails, the path
- * is in use (e.g. mounted during initramfs processing).
- * We know that it's not used by dm-multipath.
- * We may not set SYSTEMD_READY=0 on such devices, it
- * might cause systemd to umount the device.
- * Use O_RDONLY, because udevd would trigger another
- * uevent for close-after-write.
- *
- * The O_EXCL check is potentially dangerous, because it may
- * race with other tasks trying to access the device. Therefore
- * this code is only executed if the path hasn't been released
- * to systemd earlier (see above).
- *
- * get_refwwid() above stores the path we examine in slot 0.
- */
- pp = VECTOR_SLOT(pathvec, 0);
- fd = open(udev_device_get_devnode(pp->udev),
- O_RDONLY|O_EXCL);
- if (fd >= 0)
- close(fd);
- else {
- condlog(3, "%s: path %s is in use: %s",
- __func__, pp->dev,
- strerror(errno));
- /*
- * Check if we raced with multipathd
- */
- r = sysfs_is_multipathed(VECTOR_SLOT(pathvec, 0),
- false) ? RTVL_YES : RTVL_NO;
- }
- goto print_valid;
- }
-
if (cmd != CMD_CREATE && cmd != CMD_DRY_RUN) {
r = RTVL_OK;
goto out;
@@ -768,10 +658,6 @@ configure (struct config *conf, enum mpath_cmds cmd,
conf->force_reload, cmd);
r = rc == CP_RETRY ? RTVL_RETRY : rc == CP_OK ? RTVL_OK : RTVL_FAIL;
-print_valid:
- if (cmd == CMD_VALID_PATH)
- r = print_cmd_valid(r, pathvec, conf);
-
out:
if (refwwid)
FREE(refwwid);
@@ -782,6 +668,112 @@ out:
return r;
}
+static int
+check_path_valid(const char *name, struct config *conf, bool is_uevent)
+{
+ int fd, r = PATH_IS_ERROR;
+ struct path *pp = NULL;
+ vector pathvec = NULL;
+
+ pp = alloc_path();
+ if (!pp)
+ return RTVL_FAIL;
+
+ r = is_path_valid(name, conf, pp, is_uevent);
+ if (r <= PATH_IS_ERROR || r >= PATH_MAX_VALID_RESULT)
+ goto fail;
+
+ /* set path values if is_path_valid() didn't */
+ if (!pp->udev)
+ pp->udev = udev_device_new_from_subsystem_sysname(udev, "block",
+ name);
+ if (!pp->udev)
+ goto fail;
+
+ if (!strlen(pp->dev_t)) {
+ dev_t devt = udev_device_get_devnum(pp->udev);
+ if (major(devt) == 0 && minor(devt) == 0)
+ goto fail;
+ snprintf(pp->dev_t, BLK_DEV_SIZE, "%d:%d", major(devt),
+ minor(devt));
+ }
+
+ pathvec = vector_alloc();
+ if (!pathvec)
+ goto fail;
+
+ if (store_path(pathvec, pp) != 0) {
+ free_path(pp);
+ goto fail;
+ }
+
+ if ((r == PATH_IS_VALID || r == PATH_IS_MAYBE_VALID) &&
+ released_to_systemd())
+ r = PATH_IS_NOT_VALID;
+
+ /* This state is only used to skip the released_to_systemd() check */
+ if (r == PATH_IS_VALID_NO_CHECK)
+ r = PATH_IS_VALID;
+
+ if (r != PATH_IS_MAYBE_VALID)
+ goto out;
+
+ /*
+ * If opening the path with O_EXCL fails, the path
+ * is in use (e.g. mounted during initramfs processing).
+ * We know that it's not used by dm-multipath.
+ * We may not set SYSTEMD_READY=0 on such devices, it
+ * might cause systemd to umount the device.
+ * Use O_RDONLY, because udevd would trigger another
+ * uevent for close-after-write.
+ *
+ * The O_EXCL check is potentially dangerous, because it may
+ * race with other tasks trying to access the device. Therefore
+ * this code is only executed if the path hasn't been released
+ * to systemd earlier (see above).
+ */
+ fd = open(udev_device_get_devnode(pp->udev), O_RDONLY|O_EXCL);
+ if (fd >= 0)
+ close(fd);
+ else {
+ condlog(3, "%s: path %s is in use: %m", __func__, pp->dev);
+ /* Check if we raced with multipathd */
+ if (sysfs_is_multipathed(pp, false))
+ r = PATH_IS_VALID;
+ else
+ r = PATH_IS_NOT_VALID;
+ goto out;
+ }
+
+ /* For find_multipaths = SMART, if there is more than one path
+ * matching the refwwid, then the path is valid */
+ if (path_discovery(pathvec, DI_SYSFS | DI_WWID) < 0)
+ goto fail;
+ filter_pathvec(pathvec, pp->wwid);
+ if (VECTOR_SIZE(pathvec) > 1)
+ r = PATH_IS_VALID;
+ else
+ r = PATH_IS_MAYBE_VALID;
+
+out:
+ r = print_cmd_valid(r, pathvec, conf);
+ free_pathvec(pathvec, FREE_PATHS);
+ /*
+ * multipath -u must exit with status 0, otherwise udev won't
+ * import its output.
+ */
+ if (!is_uevent && r == PATH_IS_NOT_VALID)
+ return RTVL_FAIL;
+ return RTVL_OK;
+
+fail:
+ if (pathvec)
+ free_pathvec(pathvec, FREE_PATHS);
+ else
+ free_path(pp);
+ return RTVL_FAIL;
+}
+
static int
get_dev_type(char *dev) {
struct stat buf;
@@ -863,32 +855,6 @@ out:
return r;
}
-static int test_multipathd_socket(void)
-{
- int fd;
- /*
- * "multipath -u" may be run before the daemon is started. In this
- * case, systemd might own the socket but might delay multipathd
- * startup until some other unit (udev settle!) has finished
- * starting. With many LUNs, the listen backlog may be exceeded, which
- * would cause connect() to block. This causes udev workers calling
- * "multipath -u" to hang, and thus creates a deadlock, until "udev
- * settle" times out. To avoid this, call connect() in non-blocking
- * mode here, and take EAGAIN as indication for a filled-up systemd
- * backlog.
- */
-
- fd = __mpath_connect(1);
- if (fd == -1) {
- if (errno == EAGAIN)
- condlog(3, "daemon backlog exceeded");
- else
- return 0;
- } else
- close(fd);
- return 1;
-}
-
int
main (int argc, char *argv[])
{
@@ -972,7 +938,11 @@ main (int argc, char *argv[])
conf->force_reload = FORCE_RELOAD_YES;
break;
case 'i':
- conf->find_multipaths |= _FIND_MULTIPATHS_I;
+ if (conf->find_multipaths == FIND_MULTIPATHS_ON ||
+ conf->find_multipaths == FIND_MULTIPATHS_STRICT)
+ conf->find_multipaths = FIND_MULTIPATHS_SMART;
+ else if (conf->find_multipaths == FIND_MULTIPATHS_OFF)
+ conf->find_multipaths = FIND_MULTIPATHS_GREEDY;
break;
case 't':
r = dump_config(conf, NULL, NULL) ? RTVL_FAIL : RTVL_OK;
@@ -1070,15 +1040,10 @@ main (int argc, char *argv[])
condlog(0, "the -c option requires a path to check");
goto out;
}
- if (cmd == CMD_VALID_PATH &&
- dev_type == DEV_UEVENT) {
- if (!test_multipathd_socket()) {
- condlog(3, "%s: daemon is not running", dev);
- if (!systemd_service_enabled(dev)) {
- r = print_cmd_valid(RTVL_NO, NULL, conf);
- goto out;
- }
- }
+ if (cmd == CMD_VALID_PATH) {
+ char * name = convert_dev(dev, (dev_type == DEV_DEVNODE));
+ r = check_path_valid(name, conf, dev_type == DEV_UEVENT);
+ goto out;
}
if (cmd == CMD_REMOVE_WWID && !dev) {
@@ -1142,13 +1107,6 @@ out:
cleanup_prio();
cleanup_checkers();
- /*
- * multipath -u must exit with status 0, otherwise udev won't
- * import its output.
- */
- if (cmd == CMD_VALID_PATH && dev_type == DEV_UEVENT && r == RTVL_NO)
- r = RTVL_OK;
-
if (dev_type == DEV_UEVENT)
closelog();
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/6] Unit tests for is_path_valid()
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
` (2 preceding siblings ...)
2020-05-19 4:57 ` [PATCH v2 3/6] multipath: centralize validation code Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 5/6] libmultipath: simplify failed wwid code Benjamin Marzinski
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
tests/Makefile | 4 +-
tests/valid.c | 486 +++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 489 insertions(+), 1 deletion(-)
create mode 100644 tests/valid.c
diff --git a/tests/Makefile b/tests/Makefile
index 77ff3249..7fc261c3 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -13,7 +13,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \
LIBDEPS += -L$(multipathdir) -lmultipath -lcmocka
TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
- alias directio
+ alias directio valid
.SILENT: $(TESTS:%=%.o)
.PRECIOUS: $(TESTS:%=%-test)
@@ -50,6 +50,8 @@ vpd-test_OBJDEPS := ../libmultipath/discovery.o
vpd-test_LIBDEPS := -ludev -lpthread -ldl
alias-test_TESTDEPS := test-log.o
alias-test_LIBDEPS := -lpthread -ldl
+valid-test_OBJDEPS := ../libmultipath/valid.o
+valid-test_LIBDEPS := -ludev -lpthread -ldl
ifneq ($(DIO_TEST_DEV),)
directio-test_LIBDEPS := -laio
endif
diff --git a/tests/valid.c b/tests/valid.c
new file mode 100644
index 00000000..693c72c5
--- /dev/null
+++ b/tests/valid.c
@@ -0,0 +1,486 @@
+/*
+ * Copyright (c) 2020 Benjamin Marzinski, Redhat
+ *
+ * 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, see <https://www.gnu.org/licenses/>.
+ *
+ */
+
+#define _GNU_SOURCE
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <errno.h>
+#include <cmocka.h>
+#include "globals.c"
+#include "util.h"
+#include "discovery.h"
+#include "wwids.h"
+#include "blacklist.h"
+#include "valid.h"
+
+int test_fd;
+struct udev_device {
+ int unused;
+} test_udev;
+
+bool __wrap_sysfs_is_multipathed(struct path *pp, bool set_wwid)
+{
+ bool is_multipathed = mock_type(bool);
+ assert_non_null(pp);
+ assert_int_not_equal(strlen(pp->dev), 0);
+ if (is_multipathed && set_wwid)
+ strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
+ return is_multipathed;
+}
+
+int __wrap___mpath_connect(int nonblocking)
+{
+ bool connected = mock_type(bool);
+ assert_int_equal(nonblocking, 1);
+ if (connected)
+ return test_fd;
+ errno = mock_type(int);
+ return -1;
+}
+
+int __wrap_systemd_service_enabled(const char *dev)
+{
+ return (int)mock_type(bool);
+}
+
+/* There's no point in checking the return value here */
+int __wrap_mpath_disconnect(int fd)
+{
+ assert_int_equal(fd, test_fd);
+ return 0;
+}
+
+struct udev_device *__wrap_udev_device_new_from_subsystem_sysname(struct udev *udev, const char *subsystem, const char *sysname)
+{
+ bool passed = mock_type(bool);
+ assert_string_equal(sysname, mock_ptr_type(char *));
+ if (passed)
+ return &test_udev;
+ return NULL;
+}
+
+int __wrap_pathinfo(struct path *pp, struct config *conf, int mask)
+{
+ int ret = mock_type(int);
+ assert_string_equal(pp->dev, mock_ptr_type(char *));
+ assert_int_equal(mask, DI_SYSFS | DI_WWID | DI_BLACKLIST);
+ if (ret == PATHINFO_OK) {
+ pp->uid_attribute = "ID_TEST";
+ strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
+ } else
+ memset(pp->wwid, 0, WWID_SIZE);
+ return ret;
+}
+
+int __wrap_filter_property(struct config *conf, struct udev_device *udev,
+ int lvl, const char *uid_attribute)
+{
+ int ret = mock_type(int);
+ assert_string_equal(uid_attribute, "ID_TEST");
+ return ret;
+}
+
+int __wrap_is_failed_wwid(const char *wwid)
+{
+ int ret = mock_type(int);
+ assert_string_equal(wwid, mock_ptr_type(char *));
+ return ret;
+}
+
+int __wrap_check_wwids_file(char *wwid, int write_wwid)
+{
+ bool passed = mock_type(bool);
+ assert_int_equal(write_wwid, 0);
+ assert_string_equal(wwid, mock_ptr_type(char *));
+ if (passed)
+ return 0;
+ else
+ return -1;
+}
+
+int __wrap_dm_map_present_by_uuid(const char *uuid)
+{
+ int ret = mock_type(int);
+ assert_string_equal(uuid, mock_ptr_type(char *));
+ return ret;
+}
+
+enum {
+ STAGE_IS_MULTIPATHED,
+ STAGE_CHECK_MULTIPATHD,
+ STAGE_GET_UDEV_DEVICE,
+ STAGE_PATHINFO,
+ STAGE_FILTER_PROPERTY,
+ STAGE_IS_FAILED,
+ STAGE_CHECK_WWIDS,
+ STAGE_UUID_PRESENT,
+};
+
+enum {
+ CHECK_MPATHD_RUNNING,
+ CHECK_MPATHD_EAGAIN,
+ CHECK_MPATHD_ENABLED,
+ CHECK_MPATHD_SKIP,
+};
+
+/* setup the test to continue past the given stage in is_path_valid() */
+static void setup_passing(char *name, char *wwid, unsigned int check_multipathd,
+ unsigned int stage)
+{
+ will_return(__wrap_sysfs_is_multipathed, false);
+ if (stage == STAGE_IS_MULTIPATHED)
+ return;
+ if (check_multipathd == CHECK_MPATHD_RUNNING)
+ will_return(__wrap___mpath_connect, true);
+ else if (check_multipathd == CHECK_MPATHD_EAGAIN) {
+ will_return(__wrap___mpath_connect, false);
+ will_return(__wrap___mpath_connect, EAGAIN);
+ } else if (check_multipathd == CHECK_MPATHD_ENABLED) {
+ will_return(__wrap___mpath_connect, false);
+ will_return(__wrap___mpath_connect, ECONNREFUSED);
+ will_return(__wrap_systemd_service_enabled, true);
+ }
+ /* nothing for CHECK_MPATHD_SKIP */
+ if (stage == STAGE_CHECK_MULTIPATHD)
+ return;
+ will_return(__wrap_udev_device_new_from_subsystem_sysname, true);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname,
+ name);
+ if (stage == STAGE_GET_UDEV_DEVICE)
+ return;
+ will_return(__wrap_pathinfo, PATHINFO_OK);
+ will_return(__wrap_pathinfo, name);
+ will_return(__wrap_pathinfo, wwid);
+ if (stage == STAGE_PATHINFO)
+ return;
+ will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_EXCEPT);
+ if (stage == STAGE_FILTER_PROPERTY)
+ return;
+ will_return(__wrap_is_failed_wwid, WWID_IS_NOT_FAILED);
+ will_return(__wrap_is_failed_wwid, wwid);
+ if (stage == STAGE_IS_FAILED)
+ return;
+ will_return(__wrap_check_wwids_file, false);
+ will_return(__wrap_check_wwids_file, wwid);
+ if (stage == STAGE_CHECK_WWIDS)
+ return;
+ will_return(__wrap_dm_map_present_by_uuid, 0);
+ will_return(__wrap_dm_map_present_by_uuid, wwid);
+}
+
+static void test_bad_arguments(void **state)
+{
+ struct path pp;
+ char too_long[FILE_NAME_SIZE + 1];
+
+ memset(&pp, 0, sizeof(pp));
+ /* test NULL pointers */
+ assert_int_equal(is_path_valid("test", &conf, NULL, true),
+ PATH_IS_ERROR);
+ assert_int_equal(is_path_valid("test", NULL, &pp, true),
+ PATH_IS_ERROR);
+ assert_int_equal(is_path_valid(NULL, &conf, &pp, true),
+ PATH_IS_ERROR);
+ /* test undefined find_multipaths */
+ conf.find_multipaths = FIND_MULTIPATHS_UNDEF;
+ assert_int_equal(is_path_valid("test", &conf, &pp, true),
+ PATH_IS_ERROR);
+ /* test name too long */
+ memset(too_long, 'x', sizeof(too_long));
+ too_long[sizeof(too_long) - 1] = '\0';
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ assert_int_equal(is_path_valid(too_long, &conf, &pp, true),
+ PATH_IS_ERROR);
+}
+
+static void test_sysfs_is_multipathed(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test_wwid";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ /* test for already existing multiapthed device */
+ will_return(__wrap_sysfs_is_multipathed, true);
+ will_return(__wrap_sysfs_is_multipathed, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_VALID_NO_CHECK);
+ assert_string_equal(pp.dev, name);
+ assert_string_equal(pp.wwid, wwid);
+ /* test for wwid device with empty wwid */
+ will_return(__wrap_sysfs_is_multipathed, true);
+ will_return(__wrap_sysfs_is_multipathed, "");
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_ERROR);
+}
+
+static void test_check_multipathd(void **state)
+{
+ struct path pp;
+ char *name = "test";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ /* test failed check to see if multipathd is active */
+ will_return(__wrap_sysfs_is_multipathed, false);
+ will_return(__wrap___mpath_connect, false);
+ will_return(__wrap___mpath_connect, ECONNREFUSED);
+ will_return(__wrap_systemd_service_enabled, false);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_NOT_VALID);
+ assert_string_equal(pp.dev, name);
+ /* test pass because service is enabled. fail getting udev */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, NULL, CHECK_MPATHD_ENABLED, STAGE_CHECK_MULTIPATHD);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname,
+ name);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_ERROR);
+ assert_string_equal(pp.dev, name);
+ /* test pass because connect returned EAGAIN. fail getting udev */
+ setup_passing(name, NULL, CHECK_MPATHD_EAGAIN, STAGE_CHECK_MULTIPATHD);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname,
+ name);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_ERROR);
+ /* test pass because connect succeeded. fail getting udev */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, NULL, CHECK_MPATHD_RUNNING, STAGE_CHECK_MULTIPATHD);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname, false);
+ will_return(__wrap_udev_device_new_from_subsystem_sysname,
+ name);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_ERROR);
+ assert_string_equal(pp.dev, name);
+}
+
+static void test_pathinfo(void **state)
+{
+ struct path pp;
+ char *name = "test";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ /* Test pathinfo blacklisting device */
+ setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE);
+ will_return(__wrap_pathinfo, PATHINFO_SKIPPED);
+ will_return(__wrap_pathinfo, name);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ /* Test pathinfo failing */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE);
+ will_return(__wrap_pathinfo, PATHINFO_FAILED);
+ will_return(__wrap_pathinfo, name);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_ERROR);
+ /* Test blank wwid */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, NULL, CHECK_MPATHD_SKIP, STAGE_GET_UDEV_DEVICE);
+ will_return(__wrap_pathinfo, PATHINFO_OK);
+ will_return(__wrap_pathinfo, name);
+ will_return(__wrap_pathinfo, "");
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+}
+
+static void test_filter_property(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ /* test blacklist property */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+ /* test missing property */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ will_return(__wrap_filter_property, MATCH_PROPERTY_BLIST_MISSING);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ /* test MATCH_NOTHING fail on is_failed_wwid */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_PATHINFO);
+ will_return(__wrap_filter_property, MATCH_NOTHING);
+ will_return(__wrap_is_failed_wwid, WWID_IS_FAILED);
+ will_return(__wrap_is_failed_wwid, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+}
+
+static void test_is_failed_wwid(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ /* Test wwid failed */
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_FILTER_PROPERTY);
+ will_return(__wrap_is_failed_wwid, WWID_IS_FAILED);
+ will_return(__wrap_is_failed_wwid, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+ /* test is_failed_wwid error */
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_FILTER_PROPERTY);
+ will_return(__wrap_is_failed_wwid, WWID_FAILED_ERROR);
+ will_return(__wrap_is_failed_wwid, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_ERROR);
+}
+
+static void test_greedy(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ /* test greedy success with checking multipathd */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_GREEDY;
+ setup_passing(name, wwid, CHECK_MPATHD_RUNNING, STAGE_IS_FAILED);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+ /* test greedy success without checking multiapthd */
+ memset(&pp, 0, sizeof(pp));
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_IS_FAILED);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_VALID);
+}
+
+static void test_check_wwids(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ setup_passing(name, wwid, CHECK_MPATHD_EAGAIN, STAGE_IS_FAILED);
+ will_return(__wrap_check_wwids_file, true);
+ will_return(__wrap_check_wwids_file, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_VALID_NO_CHECK);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+}
+
+static void test_check_uuid_present(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ setup_passing(name, wwid, CHECK_MPATHD_ENABLED, STAGE_CHECK_WWIDS);
+ will_return(__wrap_dm_map_present_by_uuid, 1);
+ will_return(__wrap_dm_map_present_by_uuid, wwid);
+ assert_int_equal(is_path_valid(name, &conf, &pp, true),
+ PATH_IS_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+}
+
+
+static void test_find_multipaths(void **state)
+{
+ struct path pp;
+ char *name = "test";
+ char *wwid = "test-wwid";
+
+ /* test find_multipaths = FIND_MULTIPATHS_STRICT */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_STRICT;
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+ /* test find_multipaths = FIND_MULTIPATHS_OFF */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_OFF;
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ /* test find_multipaths = FIND_MULTIPATHS_ON */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_ON;
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_NOT_VALID);
+ /* test find_multipaths = FIND_MULTIPATHS_SMART */
+ memset(&pp, 0, sizeof(pp));
+ conf.find_multipaths = FIND_MULTIPATHS_SMART;
+ setup_passing(name, wwid, CHECK_MPATHD_SKIP, STAGE_UUID_PRESENT);
+ assert_int_equal(is_path_valid(name, &conf, &pp, false),
+ PATH_IS_MAYBE_VALID);
+ assert_string_equal(pp.dev, name);
+ assert_ptr_equal(pp.udev, &test_udev);
+ assert_string_equal(pp.wwid, wwid);
+}
+
+int test_valid(void)
+{
+ const struct CMUnitTest tests[] = {
+ cmocka_unit_test(test_bad_arguments),
+ cmocka_unit_test(test_sysfs_is_multipathed),
+ cmocka_unit_test(test_check_multipathd),
+ cmocka_unit_test(test_pathinfo),
+ cmocka_unit_test(test_filter_property),
+ cmocka_unit_test(test_is_failed_wwid),
+ cmocka_unit_test(test_greedy),
+ cmocka_unit_test(test_check_wwids),
+ cmocka_unit_test(test_check_uuid_present),
+ cmocka_unit_test(test_find_multipaths),
+ };
+ return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+int main(void)
+{
+ int ret = 0;
+ ret += test_valid();
+ return ret;
+}
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 5/6] libmultipath: simplify failed wwid code
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
` (3 preceding siblings ...)
2020-05-19 4:57 ` [PATCH v2 4/6] Unit tests for is_path_valid() Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 4:57 ` [PATCH v2 6/6] libmultipath: use atomic linkat() in mark_failed_wwid() Benjamin Marzinski
2020-05-19 12:55 ` [PATCH v2 0/6] multipath: path validation library prep work Martin Wilck
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck
The (is|mark|unmark)_failed_wwid code is needlessly complicated.
Locking a file is necssary if multiple processes could otherwise be
writing to it at the same time. That is not the case with the
failed_wwids files. They can simply be empty files in a directory. Even
with all the locking in place, two processes accessing or modifying a
file at the same time will still race. And even without the locking, if
two processes try to access or modify a file at the same time, they will
both see a reasonable result, and will leave the files in a valid state
afterwards.
Instead of doing all the locking work (which made it necessary to write
a file, even just to check if a file existed), simply check for files
with lstat(), create them with open(), and remove them with unlink().
Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/wwids.c | 131 ++++++++++++++++++-------------------------
1 file changed, 56 insertions(+), 75 deletions(-)
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index 397f6e54..da55924b 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -6,6 +6,7 @@
#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
+#include <fcntl.h>
#include "util.h"
#include "checkers.h"
@@ -348,113 +349,93 @@ remember_wwid(char *wwid)
}
static const char shm_dir[] = MULTIPATH_SHM_BASE "failed_wwids";
-static const char shm_lock[] = ".lock";
-static const char shm_header[] = "multipath shm lock file, don't edit";
-static char _shm_lock_path[sizeof(shm_dir)+sizeof(shm_lock)];
-static const char *shm_lock_path = &_shm_lock_path[0];
-static void init_shm_paths(void)
+static void print_failed_wwid_result(const char * msg, const char *wwid, int r)
{
- snprintf(_shm_lock_path, sizeof(_shm_lock_path),
- "%s/%s", shm_dir, shm_lock);
+ switch(r) {
+ case WWID_FAILED_ERROR:
+ condlog(1, "%s: %s: %m", msg, wwid);
+ return;
+ case WWID_IS_FAILED:
+ case WWID_IS_NOT_FAILED:
+ condlog(4, "%s: %s is %s", msg, wwid,
+ r == WWID_IS_FAILED ? "failed" : "good");
+ return;
+ case WWID_FAILED_CHANGED:
+ condlog(3, "%s: %s", msg, wwid);
+ }
}
-static pthread_once_t shm_path_once = PTHREAD_ONCE_INIT;
-
-static int multipath_shm_open(bool rw)
+int is_failed_wwid(const char *wwid)
{
- int fd;
- int can_write;
-
- pthread_once(&shm_path_once, init_shm_paths);
- fd = open_file(shm_lock_path, &can_write, shm_header);
+ struct stat st;
+ char path[PATH_MAX];
+ int r;
- if (fd >= 0 && rw && !can_write) {
- close(fd);
- condlog(1, "failed to open %s for writing", shm_dir);
+ if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
+ condlog(1, "%s: path name overflow", __func__);
return -1;
}
- return fd;
-}
-
-static void multipath_shm_close(void *arg)
-{
- long fd = (long)arg;
+ if (lstat(path, &st) == 0)
+ r = WWID_IS_FAILED;
+ else if (errno == ENOENT)
+ r = WWID_IS_NOT_FAILED;
+ else
+ r = WWID_FAILED_ERROR;
- close(fd);
- unlink(shm_lock_path);
+ print_failed_wwid_result("is_failed", wwid, r);
+ return r;
}
-static int _failed_wwid_op(const char *wwid, bool rw,
- int (*func)(const char *), const char *msg)
+int mark_failed_wwid(const char *wwid)
{
char path[PATH_MAX];
- long lockfd;
- int r = -1;
+ int r, fd;
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
return -1;
}
-
- lockfd = multipath_shm_open(rw);
- if (lockfd == -1)
+ if (ensure_directories_exist(path, 0700) < 0) {
+ condlog(1, "%s: can't setup directories", __func__);
return -1;
+ }
- pthread_cleanup_push(multipath_shm_close, (void *)lockfd);
- r = func(path);
- pthread_cleanup_pop(1);
-
- if (r == WWID_FAILED_ERROR)
- condlog(1, "%s: %s: %s", msg, wwid, strerror(errno));
- else if (r == WWID_FAILED_CHANGED)
- condlog(3, "%s: %s", msg, wwid);
- else if (!rw)
- condlog(4, "%s: %s is %s", msg, wwid,
- r == WWID_IS_FAILED ? "failed" : "good");
+ fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
+ if (fd >= 0) {
+ close(fd);
+ r = WWID_FAILED_CHANGED;
+ } else if (errno == EEXIST)
+ r = WWID_FAILED_UNCHANGED;
+ else
+ r = WWID_FAILED_ERROR;
+ print_failed_wwid_result("mark_failed", wwid, r);
return r;
}
-static int _is_failed(const char *path)
+int unmark_failed_wwid(const char *wwid)
{
- struct stat st;
+ char path[PATH_MAX];
+ int r;
- if (lstat(path, &st) == 0)
- return WWID_IS_FAILED;
+ if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
+ condlog(1, "%s: path name overflow", __func__);
+ return -1;
+ }
+
+ if (unlink(path) == 0)
+ r = WWID_FAILED_CHANGED;
else if (errno == ENOENT)
- return WWID_IS_NOT_FAILED;
+ r = WWID_FAILED_UNCHANGED;
else
- return WWID_FAILED_ERROR;
-}
-
-static int _mark_failed(const char *path)
-{
- /* Called from _failed_wwid_op: we know that shm_lock_path exists */
- if (_is_failed(path) == WWID_IS_FAILED)
- return WWID_FAILED_UNCHANGED;
- return (link(shm_lock_path, path) == 0 ? WWID_FAILED_CHANGED :
- WWID_FAILED_ERROR);
-}
+ r = WWID_FAILED_ERROR;
-static int _unmark_failed(const char *path)
-{
- if (_is_failed(path) == WWID_IS_NOT_FAILED)
- return WWID_FAILED_UNCHANGED;
- return (unlink(path) == 0 ? WWID_FAILED_CHANGED : WWID_FAILED_ERROR);
-}
-
-#define declare_failed_wwid_op(op, rw) \
-int op ## _wwid(const char *wwid) \
-{ \
- return _failed_wwid_op(wwid, (rw), _ ## op, #op); \
+ print_failed_wwid_result("unmark_failed", wwid, r);
+ return r;
}
-declare_failed_wwid_op(is_failed, false)
-declare_failed_wwid_op(mark_failed, true)
-declare_failed_wwid_op(unmark_failed, true)
-
int remember_cmdline_wwid(void)
{
FILE *f = NULL;
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 6/6] libmultipath: use atomic linkat() in mark_failed_wwid()
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
` (4 preceding siblings ...)
2020-05-19 4:57 ` [PATCH v2 5/6] libmultipath: simplify failed wwid code Benjamin Marzinski
@ 2020-05-19 4:57 ` Benjamin Marzinski
2020-05-19 12:55 ` [PATCH v2 0/6] multipath: path validation library prep work Martin Wilck
6 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 4:57 UTC (permalink / raw)
To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck, Martin Wilck
From: Martin Wilck <mwilck@suse.com>
This keeps (almost) the simplicity of the previous patch, while
making sure that the return value of mark_failed_wwid()
(WWID_FAILED_CHANGED vs. WWID_FAILED_UNCHANGED) is correct, even
if several processes access this WWID at the same time.
Signed-off-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
libmultipath/wwids.c | 42 +++++++++++++++++++++++++++++-------------
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/libmultipath/wwids.c b/libmultipath/wwids.c
index da55924b..c7a16636 100644
--- a/libmultipath/wwids.c
+++ b/libmultipath/wwids.c
@@ -374,7 +374,7 @@ int is_failed_wwid(const char *wwid)
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
- return -1;
+ return WWID_FAILED_ERROR;
}
if (lstat(path, &st) == 0)
@@ -390,27 +390,43 @@ int is_failed_wwid(const char *wwid)
int mark_failed_wwid(const char *wwid)
{
- char path[PATH_MAX];
- int r, fd;
+ char tmpfile[WWID_SIZE + 2 * sizeof(long) + 1];
+ int r = WWID_FAILED_ERROR, fd, dfd;
- if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
- condlog(1, "%s: path name overflow", __func__);
- return -1;
+ dfd = open(shm_dir, O_RDONLY|O_DIRECTORY);
+ if (dfd == -1 && errno == ENOENT) {
+ char path[sizeof(shm_dir) + 2];
+
+ /* arg for ensure_directories_exist() must not end with "/" */
+ safe_sprintf(path, "%s/_", shm_dir);
+ ensure_directories_exist(path, 0700);
+ dfd = open(shm_dir, O_RDONLY|O_DIRECTORY);
}
- if (ensure_directories_exist(path, 0700) < 0) {
- condlog(1, "%s: can't setup directories", __func__);
- return -1;
+ if (dfd == -1) {
+ condlog(1, "%s: can't setup %s: %m", __func__, shm_dir);
+ return WWID_FAILED_ERROR;
}
- fd = open(path, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
- if (fd >= 0) {
+ safe_sprintf(tmpfile, "%s.%lx", wwid, (long)getpid());
+ fd = openat(dfd, tmpfile, O_RDONLY | O_CREAT | O_EXCL, S_IRUSR);
+ if (fd >= 0)
close(fd);
+ else
+ goto out_closedir;
+
+ if (linkat(dfd, tmpfile, dfd, wwid, 0) == 0)
r = WWID_FAILED_CHANGED;
- } else if (errno == EEXIST)
+ else if (errno == EEXIST)
r = WWID_FAILED_UNCHANGED;
else
r = WWID_FAILED_ERROR;
+ if (unlinkat(dfd, tmpfile, 0) == -1)
+ condlog(2, "%s: failed to unlink %s/%s: %m",
+ __func__, shm_dir, tmpfile);
+
+out_closedir:
+ close(dfd);
print_failed_wwid_result("mark_failed", wwid, r);
return r;
}
@@ -422,7 +438,7 @@ int unmark_failed_wwid(const char *wwid)
if (safe_sprintf(path, "%s/%s", shm_dir, wwid)) {
condlog(1, "%s: path name overflow", __func__);
- return -1;
+ return WWID_FAILED_ERROR;
}
if (unlink(path) == 0)
--
2.17.2
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH v2 0/6] multipath: path validation library prep work
2020-05-19 4:57 [PATCH v2 0/6] multipath: path validation library prep work Benjamin Marzinski
` (5 preceding siblings ...)
2020-05-19 4:57 ` [PATCH v2 6/6] libmultipath: use atomic linkat() in mark_failed_wwid() Benjamin Marzinski
@ 2020-05-19 12:55 ` Martin Wilck
2020-05-19 14:23 ` Benjamin Marzinski
6 siblings, 1 reply; 9+ messages in thread
From: Martin Wilck @ 2020-05-19 12:55 UTC (permalink / raw)
To: Benjamin Marzinski, Christophe Varoqui; +Cc: device-mapper development
On Mon, 2020-05-18 at 23:57 -0500, Benjamin Marzinski wrote:
> I've been playing around with the SID code more and I've decided to
> hold
> off on submitting the library until I have it working with the SID
> multipath module better. Instead, I've pulled out the common code
> thatremember_cmdline_wwid
> multipath -u/-c and the library can use, and put it into
> libmultipath.
>
> I've also removed some of the ordering differences between the
> existing
> code and my new code. Right now, the only difference is that if a
> path
> is currently multipathed, it will always be claimed as a valid path.
>
> Patches 0001 & 0002 are the same as in my "RFC PATCH v2" set, and
> patch
> 0005 is the same as my "libmultipath: simplify failed wwid code"
> patch.
>
> Only patches 0003 and 0004 haven't been posted before.
>
> Changes from v1:
> 0003: Minor fixes suggested by Martin Wilck
> 0004: Fixed typo, added tests for filter_property() and switched some
> tests to pass the check_multipathd code in various ways,
> instead
> of skipping it, as suggested by Martin Wilck
>
This set (v2) doesn't apply cleanly to upstream, neither with or
without my late patches. It's been generated against a tree that
included "Make multipath add wwids from kernel cmdline mpath.wwids with
-A" (https://patchwork.kernel.org/patch/4445691/). From my series, it's
missing "libmultipath: move libsg into libmultipath".
Apart from that, for the series:
Reviewed-by: Martin Wilck <mwilck@suse.com>
Martin
--
Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
SUSE Software Solutions Germany GmbH
HRB 36809, AG Nürnberg GF: Felix
Imendörffer
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 0/6] multipath: path validation library prep work
2020-05-19 12:55 ` [PATCH v2 0/6] multipath: path validation library prep work Martin Wilck
@ 2020-05-19 14:23 ` Benjamin Marzinski
0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-05-19 14:23 UTC (permalink / raw)
To: Martin Wilck; +Cc: device-mapper development
On Tue, May 19, 2020 at 02:55:05PM +0200, Martin Wilck wrote:
> On Mon, 2020-05-18 at 23:57 -0500, Benjamin Marzinski wrote:
> > I've been playing around with the SID code more and I've decided to
> > hold
> > off on submitting the library until I have it working with the SID
> > multipath module better. Instead, I've pulled out the common code
> > thatremember_cmdline_wwid
> > multipath -u/-c and the library can use, and put it into
> > libmultipath.
> >
> > I've also removed some of the ordering differences between the
> > existing
> > code and my new code. Right now, the only difference is that if a
> > path
> > is currently multipathed, it will always be claimed as a valid path.
> >
> > Patches 0001 & 0002 are the same as in my "RFC PATCH v2" set, and
> > patch
> > 0005 is the same as my "libmultipath: simplify failed wwid code"
> > patch.
> >
> > Only patches 0003 and 0004 haven't been posted before.
> >
> > Changes from v1:
> > 0003: Minor fixes suggested by Martin Wilck
> > 0004: Fixed typo, added tests for filter_property() and switched some
> > tests to pass the check_multipathd code in various ways,
> > instead
> > of skipping it, as suggested by Martin Wilck
> >
>
> This set (v2) doesn't apply cleanly to upstream, neither with or
> without my late patches. It's been generated against a tree that
> included "Make multipath add wwids from kernel cmdline mpath.wwids with
> -A" (https://patchwork.kernel.org/patch/4445691/). From my series, it's
> missing "libmultipath: move libsg into libmultipath".
>
> Apart from that, for the series:
>
> Reviewed-by: Martin Wilck <mwilck@suse.com>
Whoops. I accidentally sent patches from the branch that's built on top
of my latest RHEL code. Resending.
-Ben
> Martin
>
> --
> Dr. Martin Wilck <mwilck@suse.com>, Tel. +49 (0)911 74053 2107
> SUSE Software Solutions Germany GmbH
> HRB 36809, AG Nürnberg GF: Felix
> Imendörffer
>
>
^ permalink raw reply [flat|nested] 9+ messages in thread