All of lore.kernel.org
 help / color / mirror / Atom feed
* [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path
@ 2020-10-21 21:39 Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 1/4] multipath: add libmpathvalid library Benjamin Marzinski
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-10-21 21:39 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

The main part of the this patchset is the first patch, which adds a
new library interface to check whether devices are valid paths. This
was designed for use in the Storage Instantiation Daemon (SID).

https://github.com/sid-project

The seconds patch adds unit tests for this library. The third patch adds
get_uid fallback code for dasd devices. The fourth patch just changes
the get_uid log level for devices configured with uid_attribute "". This
is because it is currently necessary to configure multipath with

overrides {
        uid_attribute ""
}

to claim multipath devices with SID (instead of using multipath.rules),
since SID doesn't currently get the UID information itself, and it is
called by udev before this information is added to the udev database.

changes from v1 to v2
---------------------
0001: This patch is now rebased on top of, and makes use of Martin's
patches that provide a default *_multipath_config, udev, and logsink.
Because of this, mpathvalid_init() now has a parameter used to set
logsink. There is also a new API function, mpathvalid_reload_config().

0003: This is completely new, since Martin pointed out that adding a new
config option to always use the fallback getuid code was unnecessary. It
just makes a uid_attribute of "" log at normal levels.

changes from v2 to v3
---------------------
0001:   rebased on top of Martin's latest patches, fixed some small bugs
        and added documentation to mpath_valid.h
0002:   New
0004:   was 0003. Untangled the logic, at Martin's suggestion.

Benjamin Marzinski (4):
  multipath: add libmpathvalid library
  multipath-tools tests: and unit tests for libmpathvalid
  libmultipath: add uid failback for dasd devices
  libmultipath: change log level for null uid_attribute

 Makefile                            |   3 +-
 Makefile.inc                        |   1 +
 libmpathvalid/Makefile              |  39 +++
 libmpathvalid/libmpathvalid.version |  10 +
 libmpathvalid/mpath_valid.c         | 202 ++++++++++++
 libmpathvalid/mpath_valid.h         | 155 +++++++++
 libmultipath/defaults.h             |   1 +
 libmultipath/discovery.c            |  45 ++-
 libmultipath/libmultipath.version   |   6 +
 tests/Makefile                      |   5 +-
 tests/mpathvalid.c                  | 467 ++++++++++++++++++++++++++++
 11 files changed, 929 insertions(+), 5 deletions(-)
 create mode 100644 libmpathvalid/Makefile
 create mode 100644 libmpathvalid/libmpathvalid.version
 create mode 100644 libmpathvalid/mpath_valid.c
 create mode 100644 libmpathvalid/mpath_valid.h
 create mode 100644 tests/mpathvalid.c

-- 
2.17.2

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [dm-devel] [PATCH v3 1/4] multipath: add libmpathvalid library
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
@ 2020-10-21 21:39 ` Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 2/4] multipath-tools tests: and unit tests for libmpathvalid Benjamin Marzinski
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-10-21 21:39 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

This library allows other programs to check if a path should be claimed
by multipath. It exports an init function, that needs to be called
before and after all other library calls, an exit function, that needs
to be called after all library calls, a function to reread the multipath
configuration files, and two more functions.

mpath_get_mode() get the configured find_multipaths mode.
mpath_is_path() returns whether the device is claimed by multipath, and
optionally returns the wwid.  This code works slightly different than
the multipath -c/u code for SMART mode.  Instead of checking all the
existing paths to see if another has the same wwid, it expects the
caller to pass in an array of the already known path wwids, and checks
if the current path matches any of those.

The library also doesn't set up the device-mapper library. It leaves
this up to the caller.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile                            |   3 +-
 libmpathvalid/Makefile              |  39 ++++++
 libmpathvalid/libmpathvalid.version |  10 ++
 libmpathvalid/mpath_valid.c         | 202 ++++++++++++++++++++++++++++
 libmpathvalid/mpath_valid.h         | 155 +++++++++++++++++++++
 libmultipath/libmultipath.version   |   6 +
 6 files changed, 414 insertions(+), 1 deletion(-)
 create mode 100644 libmpathvalid/Makefile
 create mode 100644 libmpathvalid/libmpathvalid.version
 create mode 100644 libmpathvalid/mpath_valid.c
 create mode 100644 libmpathvalid/mpath_valid.h

diff --git a/Makefile b/Makefile
index 4a3491da..f127ff91 100644
--- a/Makefile
+++ b/Makefile
@@ -9,6 +9,7 @@ BUILDDIRS := \
 	libmultipath/checkers \
 	libmultipath/foreign \
 	libmpathpersist \
+	libmpathvalid \
 	multipath \
 	multipathd \
 	mpathpersist \
@@ -29,7 +30,7 @@ $(BUILDDIRS):
 	$(MAKE) -C $@
 
 libmultipath libdmmp: libmpathcmd
-libmpathpersist multipath multipathd: libmultipath
+libmpathpersist libmpathvalid multipath multipathd: libmultipath
 mpathpersist multipathd:  libmpathpersist
 
 libmultipath/checkers.install \
diff --git a/libmpathvalid/Makefile b/libmpathvalid/Makefile
new file mode 100644
index 00000000..6bea4bcd
--- /dev/null
+++ b/libmpathvalid/Makefile
@@ -0,0 +1,39 @@
+include ../Makefile.inc
+
+SONAME = 0
+DEVLIB = libmpathvalid.so
+LIBS = $(DEVLIB).$(SONAME)
+VERSION_SCRIPT := libmpathvalid.version
+
+CFLAGS += $(LIB_CFLAGS) -I$(multipathdir) -I$(mpathcmddir)
+
+LIBDEPS += -lpthread -ldevmapper -ldl -L$(multipathdir) \
+	   -lmultipath -L$(mpathcmddir) -lmpathcmd -ludev
+
+OBJS = mpath_valid.o
+
+all: $(LIBS)
+
+$(LIBS): $(OBJS) $(VERSION_SCRIPT)
+	$(CC) $(LDFLAGS) $(SHARED_FLAGS) -Wl,-soname=$@ -o $@ $(OBJS) $(LIBDEPS) -Wl,--version-script=libmpathvalid.version
+	$(LN) $(LIBS) $(DEVLIB)
+
+install: $(LIBS)
+	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(syslibdir)
+	$(INSTALL_PROGRAM) -m 755 $(LIBS) $(DESTDIR)$(syslibdir)/$(LIBS)
+	$(LN) $(LIBS) $(DESTDIR)$(syslibdir)/$(DEVLIB)
+	$(INSTALL_PROGRAM) -m 755 -d $(DESTDIR)$(includedir)
+	$(INSTALL_PROGRAM) -m 644 mpath_valid.h $(DESTDIR)$(includedir)
+
+uninstall:
+	$(RM) $(DESTDIR)$(syslibdir)/$(LIBS)
+	$(RM) $(DESTDIR)$(syslibdir)/$(DEVLIB)
+	$(RM) $(DESTDIR)$(includedir)/mpath_valid.h
+
+clean: dep_clean
+	$(RM) core *.a *.o *.so *.so.* *.gz
+
+include $(wildcard $(OBJS:.o=.d))
+
+dep_clean:
+	$(RM) $(OBJS:.o=.d)
diff --git a/libmpathvalid/libmpathvalid.version b/libmpathvalid/libmpathvalid.version
new file mode 100644
index 00000000..3bd0d3c5
--- /dev/null
+++ b/libmpathvalid/libmpathvalid.version
@@ -0,0 +1,10 @@
+MPATH_1.0 {
+	global:
+		mpathvalid_init;
+		mpathvalid_reload_config;
+		mpathvalid_exit;
+		mpathvalid_is_path;
+		mpathvalid_get_mode;
+	local:
+		*;
+};
diff --git a/libmpathvalid/mpath_valid.c b/libmpathvalid/mpath_valid.c
new file mode 100644
index 00000000..7073d17d
--- /dev/null
+++ b/libmpathvalid/mpath_valid.c
@@ -0,0 +1,202 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <libdevmapper.h>
+#include <libudev.h>
+#include <errno.h>
+
+#include "devmapper.h"
+#include "structs.h"
+#include "util.h"
+#include "config.h"
+#include "discovery.h"
+#include "wwids.h"
+#include "sysfs.h"
+#include "mpath_cmd.h"
+#include "valid.h"
+#include "mpath_valid.h"
+#include "debug.h"
+
+static unsigned int
+get_conf_mode(struct config *conf)
+{
+	if (conf->find_multipaths == FIND_MULTIPATHS_SMART)
+		return MPATH_SMART;
+	if (conf->find_multipaths == FIND_MULTIPATHS_GREEDY)
+		return MPATH_GREEDY;
+	return MPATH_STRICT;
+}
+
+static void
+set_conf_mode(struct config *conf, unsigned int mode)
+{
+	if (mode == MPATH_SMART)
+		conf->find_multipaths = FIND_MULTIPATHS_SMART;
+	else if (mode == MPATH_GREEDY)
+		conf->find_multipaths = FIND_MULTIPATHS_GREEDY;
+	else
+		conf->find_multipaths = FIND_MULTIPATHS_STRICT;
+}
+
+unsigned int
+mpathvalid_get_mode(void)
+{
+	int mode;
+	struct config *conf;
+
+	conf = get_multipath_config();
+	if (!conf)
+		mode = MPATH_MODE_ERROR;
+	else
+		mode = get_conf_mode(conf);
+	put_multipath_config(conf);
+	return mode;
+}
+
+static int
+convert_result(int result) {
+	switch (result) {
+	case PATH_IS_ERROR:
+		return MPATH_IS_ERROR;
+	case PATH_IS_NOT_VALID:
+		return MPATH_IS_NOT_VALID;
+	case PATH_IS_VALID:
+		return MPATH_IS_VALID;
+	case PATH_IS_VALID_NO_CHECK:
+		return MPATH_IS_VALID_NO_CHECK;
+	case PATH_IS_MAYBE_VALID:
+		return MPATH_IS_MAYBE_VALID;
+	}
+	return MPATH_IS_ERROR;
+}
+
+static void
+set_log_style(int log_style)
+{
+	/*
+	 * convert MPATH_LOG_* to LOGSINK_*
+	 * currently there is no work to do here.
+	 */
+	logsink = log_style;
+}
+
+static int
+load_default_config(int verbosity)
+{
+	/* need to set verbosity here to control logging during init_config() */
+	libmp_verbosity = verbosity;
+	if (init_config(DEFAULT_CONFIGFILE))
+		return -1;
+	/* Need to override verbosity from init_config() */
+	libmp_verbosity = verbosity;
+
+	return 0;
+}
+
+int
+mpathvalid_init(int verbosity, int log_style)
+{
+	unsigned int version[3];
+
+	set_log_style(log_style);
+	if (libmultipath_init())
+		return -1;
+
+	skip_libmp_dm_init();
+	if (load_default_config(verbosity))
+		goto fail;
+
+	if (dm_prereq(version))
+		goto fail_config;
+
+	return 0;
+
+fail_config:
+	uninit_config();
+fail:
+	libmultipath_exit();
+	return -1;
+}
+
+int
+mpathvalid_reload_config(void)
+{
+	uninit_config();
+	return load_default_config(libmp_verbosity);
+}
+
+int
+mpathvalid_exit(void)
+{
+	uninit_config();
+	libmultipath_exit();
+	return 0;
+}
+
+/*
+ * name: name of path to check
+ * mode: mode to use for determination. MPATH_DEFAULT uses configured mode
+ * info: on success, contains the path wwid
+ * paths: array of the returned mpath_info from other claimed paths
+ * nr_paths: the size of the paths array
+ */
+int
+mpathvalid_is_path(const char *name, unsigned int mode, char **wwid,
+	           const char **path_wwids, unsigned int nr_paths)
+{
+	struct config *conf;
+	int find_multipaths_saved, r = MPATH_IS_ERROR;
+	unsigned int i;
+	struct path *pp;
+
+	if (!name || mode >= MPATH_MODE_ERROR)
+		return r;
+	if (nr_paths > 0 && !path_wwids)
+		return r;
+	if (!udev)
+		return r;
+
+	pp = alloc_path();
+	if (!pp)
+		return r;
+
+	if (wwid) {
+		*wwid = (char *)malloc(WWID_SIZE);
+		if (!*wwid)
+			goto out;
+	}
+
+	conf = get_multipath_config();
+	if (!conf)
+		goto out_wwid;
+	find_multipaths_saved = conf->find_multipaths;
+	if (mode != MPATH_DEFAULT)
+		set_conf_mode(conf, mode);
+	r = convert_result(is_path_valid(name, conf, pp, true));
+	conf->find_multipaths = find_multipaths_saved;
+	put_multipath_config(conf);
+
+	if (r == MPATH_IS_MAYBE_VALID) {
+		for (i = 0; i < nr_paths; i++) {
+			if (path_wwids[i] &&
+			    strncmp(path_wwids[i], pp->wwid, WWID_SIZE) == 0) {
+				r = MPATH_IS_VALID;
+				break;
+			}
+		}
+	}
+
+out_wwid:
+	if (wwid) {
+		if (r == MPATH_IS_VALID || r == MPATH_IS_VALID_NO_CHECK ||
+		    r == MPATH_IS_MAYBE_VALID)
+			strlcpy(*wwid, pp->wwid, WWID_SIZE);
+		else {
+			free(*wwid);
+			*wwid = NULL;
+		}
+	}
+out:
+	free_path(pp);
+	return r;
+}
diff --git a/libmpathvalid/mpath_valid.h b/libmpathvalid/mpath_valid.h
new file mode 100644
index 00000000..63de4e1c
--- /dev/null
+++ b/libmpathvalid/mpath_valid.h
@@ -0,0 +1,155 @@
+/*
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This file is part of the device-mapper multipath userspace tools.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#ifndef LIB_MPATH_VALID_H
+#define LIB_MPATH_VALID_H
+
+#ifdef __cpluscplus
+extern "C" {
+#endif
+
+enum mpath_valid_mode {
+	MPATH_DEFAULT,
+	MPATH_STRICT,
+	MPATH_SMART,
+	MPATH_GREEDY,
+	MPATH_MODE_ERROR,
+};
+
+/*
+ * MPATH_IS_VALID_NO_CHECK is used to indicate that it is safe to skip
+ * checks to see if the device has already been released to the system
+ * for use by things other that multipath.
+ * MPATH_IS_MAYBE_VALID is used to indicate that this device would
+ * be a valid multipath path device if another device with the same
+ * wwid existed */
+enum mpath_valid_result {
+	MPATH_IS_ERROR = -1,
+	MPATH_IS_NOT_VALID,
+	MPATH_IS_VALID,
+	MPATH_IS_VALID_NO_CHECK,
+	MPATH_IS_MAYBE_VALID,
+};
+
+enum mpath_valid_log_style {
+	MPATH_LOG_STDERR = -1,		/* log to STDERR */
+	MPATH_LOG_STDERR_TIMESTAMP,	/* log to STDERR, with timestamps */
+	MPATH_LOG_SYSLOG,		/* log to system log */
+};
+
+enum mpath_valid_verbosity {
+	MPATH_LOG_PRIO_NOLOG = -1,	/* log nothing */
+	MPATH_LOG_PRIO_ERR,
+	MPATH_LOG_PRIO_WARN,
+	MPATH_LOG_PRIO_NOTICE,
+	MPATH_LOG_PRIO_INFO,
+	MPATH_LOG_PRIO_DEBUG,
+};
+
+/* Function declarations */
+
+/*
+ * DESCRIPTION:
+ * 	Initialize the device mapper multipath configuration. This
+ * 	function must be invoked before calling any other
+ * 	libmpathvalid functions. Call mpathvalid_exit() to cleanup.
+ * @verbosity: the logging level (mpath_valid_verbosity)
+ * @log_style: the logging style (mpath_valid_log_style)
+ *
+ * RESTRICTIONS:
+ * 	Calling mpathvalid_init() after calling mpathvalid_exit() has no
+ * 	effect.
+ *
+ * RETURNS: 0 = Success, -1 = Failure
+ */
+int mpathvalid_init(int verbosity, int log_style);
+
+
+/*
+ * DESCRIPTION:
+ * 	Reread the multipath configuration files and reinitalize
+ * 	the device mapper multipath configuration. This function can
+ * 	be called as many times as necessary.
+ *
+ * RETURNS: 0 = Success, -1 = Failure
+ */
+int mpathvalid_reload_config(void);
+
+
+/*
+ * DESCRIPTION:
+ * 	Release the device mapper multipath configuration. This
+ * 	function must be called to cleanup resoures allocated by
+ * 	mpathvalid_init(). After calling this function, no futher
+ * 	libmpathvalid functions may be called.
+ *
+ * RETURNS: 0 = Success, -1 = Failure
+ */
+int mpathvalid_exit(void);
+
+/*
+ * DESCRIPTION:
+ * 	Return the configured find_multipaths claim mode, using the
+ * 	configuration from either mpathvalid_init() or
+ * 	mpathvalid_reload_config()
+ *
+ * RETURNS:
+ * 	MPATH_STRICT, MPATH_SMART, MPATH_GREEDY, or MPATH_MODE_ERROR
+ *
+ * 	MPATH_STRICT     = find_multiapths (yes|on|no|off)
+ * 	MPATH_SMART      = find_multipaths smart
+ * 	MPATH_GREEDY     = find_multipaths greedy
+ * 	MPATH_MODE_ERROR = multipath configuration not initialized
+ */
+unsigned int mpathvalid_get_mode(void);
+/*
+ * DESCRIPTION:
+ * 	Return whether device-mapper multipath claims a path device,
+ * 	using the configuration read from either mpathvalid_init() or
+ * 	mpathvalid_reload_config(). If the device is either claimed or
+ * 	potentially claimed (MPATH_IS_VALID, MPATH_IS_VALID_NO_CHECK,
+ * 	or MPATH_IS_MAYBE_VALID) and wwid is not NULL, then *wiid will
+ * 	be set to point to the wwid of device. If set, *wwid must be
+ * 	freed by the caller. path_wwids is an obptional parameter that
+ * 	points to an array of wwids, that were returned from previous
+ * 	calls to mpathvalid_is_path(). These are wwids of existing
+ * 	devices that are or potentially are claimed by device-mapper
+ * 	multipath. path_wwids is used with the MPATH_SMART claim mode,
+ *	to claim devices when another device with the same wwid exists.
+ * 	nr_paths must either be set to the number of elements of
+ * 	path_wwids, or 0, if path_wwids is NULL.
+ * @name: The kernel name of the device. input argument
+ * @mode: the find_multipaths claim mode (mpath_valid_mode). input argument
+ * @wwid: address of a pointer to the path wwid, or NULL. Output argument.
+ * 	  Set if path is/may be claimed. If set, must be freed by caller
+ * @path_wwids: Array of pointers to path wwids, or NULL. input argument
+ * @nr_paths: number of elements in path_wwids array. input argument.
+ *
+ * RETURNS: device claim result (mpath_valid_result)
+ * 	    Also sets *wwid if wwid is not NULL, and the claim result is
+ * 	    MPATH_IS_VALID, MPATH_IS_VALID_NO_CHECK, or
+ * 	    MPATH_IS_MAYBE_VALID
+ */
+int mpathvalid_is_path(const char *name, unsigned int mode, char **wwid,
+		       const char **path_wwids, unsigned int nr_paths);
+
+#ifdef __cplusplus
+}
+#endif
+#endif /* LIB_PATH_VALID_H */
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 67a7379f..2e3583f5 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -264,3 +264,9 @@ LIBMULTIPATH_4.1.0 {
 global:
 	libmp_verbosity;
 } LIBMULTIPATH_4.0.0;
+
+LIBMULTIPATH_4.2.0 {
+global:
+	dm_prereq;
+	skip_libmp_dm_init;
+} LIBMULTIPATH_4.1.0;
-- 
2.17.2

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [dm-devel] [PATCH v3 2/4] multipath-tools tests: and unit tests for libmpathvalid
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 1/4] multipath: add libmpathvalid library Benjamin Marzinski
@ 2020-10-21 21:39 ` Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 3/4] libmultipath: add uid failback for dasd devices Benjamin Marzinski
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-10-21 21:39 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 Makefile.inc       |   1 +
 tests/Makefile     |   5 +-
 tests/mpathvalid.c | 467 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 472 insertions(+), 1 deletion(-)
 create mode 100644 tests/mpathvalid.c

diff --git a/Makefile.inc b/Makefile.inc
index e05f3a91..13587a9f 100644
--- a/Makefile.inc
+++ b/Makefile.inc
@@ -66,6 +66,7 @@ libdir		= $(prefix)/$(LIB)/multipath
 unitdir		= $(prefix)/$(SYSTEMDPATH)/systemd/system
 mpathpersistdir	= $(TOPDIR)/libmpathpersist
 mpathcmddir	= $(TOPDIR)/libmpathcmd
+mpathvaliddir	= $(TOPDIR)/libmpathvalid
 thirdpartydir	= $(TOPDIR)/third-party
 libdmmpdir	= $(TOPDIR)/libdmmp
 nvmedir		= $(TOPDIR)/libmultipath/nvme
diff --git a/tests/Makefile b/tests/Makefile
index 73ff0f5c..73ec0702 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -13,7 +13,7 @@ CFLAGS += $(BIN_CFLAGS) -I$(multipathdir) -I$(mpathcmddir) \
 LIBDEPS += -L. -L$(mpathcmddir) -lmultipath -lmpathcmd -lcmocka
 
 TESTS := uevent parser util dmevents hwtable blacklist unaligned vpd pgpolicy \
-	 alias directio valid devt
+	 alias directio valid devt mpathvalid
 
 .SILENT: $(TESTS:%=%.o)
 .PRECIOUS: $(TESTS:%=%-test)
@@ -30,6 +30,7 @@ endif
 ifneq ($(DIO_TEST_DEV),)
 directio-test_FLAGS := -DDIO_TEST_DEV=\"$(DIO_TEST_DEV)\"
 endif
+mpathvalid-test_FLAGS := -I$(mpathvaliddir)
 
 # test-specific linker flags
 # XYZ-test_TESTDEPS: test libraries containing __wrap_xyz functions
@@ -55,6 +56,8 @@ alias-test_LIBDEPS := -lpthread -ldl
 valid-test_OBJDEPS := ../libmultipath/valid.o
 valid-test_LIBDEPS := -ludev -lpthread -ldl
 devt-test_LIBDEPS := -ludev
+mpathvalid-test_LIBDEPS := -ludev -lpthread -ldl
+mpathvalid-test_OBJDEPS := ../libmpathvalid/mpath_valid.o
 ifneq ($(DIO_TEST_DEV),)
 directio-test_LIBDEPS := -laio
 endif
diff --git a/tests/mpathvalid.c b/tests/mpathvalid.c
new file mode 100644
index 00000000..5ffabb9d
--- /dev/null
+++ b/tests/mpathvalid.c
@@ -0,0 +1,467 @@
+/*
+ * Copyright (c) 2020 Benjamin Marzinski, Red Hat
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ */
+#include <stdbool.h>
+#include <stdarg.h>
+#include <stddef.h>
+#include <setjmp.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <libudev.h>
+#include <cmocka.h>
+#include "structs.h"
+#include "config.h"
+#include "mpath_valid.h"
+#include "util.h"
+#include "debug.h"
+
+const char *test_dev = "test_name";
+#define TEST_WWID "WWID_123"
+#define CONF_TEMPLATE "mpathvalid-testconf-XXXXXXXX"
+char conf_name[] = CONF_TEMPLATE;
+bool initialized;
+
+#if 0
+static int mode_to_findmp(unsigned int mode)
+{
+	switch (mode) {
+	case MPATH_SMART:
+		return FIND_MULTIPATHS_SMART;
+	case MPATH_GREEDY:
+		return FIND_MULTIPATHS_GREEDY;
+	case MPATH_STRICT:
+		return FIND_MULTIPATHS_STRICT;
+	}
+	fail_msg("invalid mode: %u", mode);
+	return FIND_MULTIPATHS_UNDEF;
+}
+#endif
+
+static unsigned int findmp_to_mode(int findmp)
+{
+	switch (findmp) {
+	case FIND_MULTIPATHS_SMART:
+		return MPATH_SMART;
+	case FIND_MULTIPATHS_GREEDY:
+		return MPATH_GREEDY;
+	case FIND_MULTIPATHS_STRICT:
+	case FIND_MULTIPATHS_OFF:
+	case FIND_MULTIPATHS_ON:
+		return MPATH_STRICT;
+	}
+	fail_msg("invalid find_multipaths value: %d", findmp);
+	return MPATH_DEFAULT;
+}
+
+int __wrap_is_path_valid(const char *name, struct config *conf, struct path *pp,
+			 bool check_multipathd)
+{
+	int r = mock_type(int);
+	int findmp = mock_type(int);
+
+	assert_ptr_equal(name, test_dev);
+	assert_ptr_not_equal(conf, NULL);
+	assert_ptr_not_equal(pp, NULL);
+	assert_true(check_multipathd);
+
+	assert_int_equal(findmp, conf->find_multipaths);	
+	if (r == MPATH_IS_ERROR || r == MPATH_IS_NOT_VALID)
+		return r;
+	
+	strlcpy(pp->wwid, mock_ptr_type(char *), WWID_SIZE);
+	return r;
+}
+
+int __wrap_libmultipath_init(void)
+{
+	int r = mock_type(int);
+
+	assert_false(initialized);
+	if (r != 0)
+		return r;
+	initialized = true;
+	return 0;
+}
+
+void __wrap_libmultipath_exit(void)
+{
+	assert_true(initialized);
+	initialized = false;
+}
+
+int __wrap_dm_prereq(unsigned int *v)
+{
+	assert_ptr_not_equal(v, NULL);
+	return mock_type(int);
+}
+
+int __real_init_config(const char *file);
+
+int __wrap_init_config(const char *file)
+{
+	int r = mock_type(int);
+	struct config *conf;
+
+	assert_ptr_equal(file, DEFAULT_CONFIGFILE);
+	if (r != 0)
+		return r;
+
+	assert_string_not_equal(conf_name, CONF_TEMPLATE);
+	r = __real_init_config(conf_name);
+	conf = get_multipath_config();
+	assert_ptr_not_equal(conf, NULL);
+	assert_int_equal(conf->find_multipaths, mock_type(int));
+	return 0;
+}
+
+static const char * const find_multipaths_optvals[] = {
+        [FIND_MULTIPATHS_OFF] = "off",
+        [FIND_MULTIPATHS_ON] = "on",
+        [FIND_MULTIPATHS_STRICT] = "strict",
+        [FIND_MULTIPATHS_GREEDY] = "greedy",
+        [FIND_MULTIPATHS_SMART] = "smart",
+};
+
+void make_config_file(int findmp)
+{
+	int r, fd;
+	char buf[64];
+
+	assert_true(findmp > FIND_MULTIPATHS_UNDEF &&
+		    findmp < __FIND_MULTIPATHS_LAST);
+
+	r = snprintf(buf, sizeof(buf), "defaults {\nfind_multipaths %s\n}\n",
+		     find_multipaths_optvals[findmp]);
+	assert_true(r > 0 && (long unsigned int)r < sizeof(buf));
+
+	memcpy(conf_name, CONF_TEMPLATE, sizeof(conf_name));
+	fd = mkstemp(conf_name);
+	assert_true(fd >= 0);
+	assert_int_equal(safe_write(fd, buf, r), 0);
+	assert_int_equal(close(fd), 0);
+}
+
+int setup(void **state)
+{
+	initialized = false;
+	udev = udev_new();
+	if (udev == NULL)
+		return -1;
+	return 0;
+}
+
+int teardown(void **state)
+{
+	struct config *conf;
+	conf = get_multipath_config();
+	put_multipath_config(conf);
+	if (conf)
+		uninit_config();
+	if (strcmp(conf_name, CONF_TEMPLATE) != 0)
+		unlink(conf_name);
+	udev_unref(udev);
+	udev = NULL;
+	return 0;
+}
+
+static void check_config(bool valid_config)
+{
+	struct config *conf;
+
+	conf = get_multipath_config();
+	put_multipath_config(conf);
+	if (valid_config)
+		assert_ptr_not_equal(conf, NULL);
+}
+
+/* libmultipath_init fails */
+static void test_mpathvalid_init_bad1(void **state)
+{
+	will_return(__wrap_libmultipath_init, 1);
+	assert_int_equal(mpathvalid_init(MPATH_LOG_PRIO_DEBUG,
+					 MPATH_LOG_STDERR), -1);
+	assert_false(initialized);
+	check_config(false);
+}
+
+/* init_config fails */
+static void test_mpathvalid_init_bad2(void **state)
+{
+	will_return(__wrap_libmultipath_init, 0);
+	will_return(__wrap_init_config, 1);
+	assert_int_equal(mpathvalid_init(MPATH_LOG_PRIO_ERR,
+					 MPATH_LOG_STDERR_TIMESTAMP), -1);
+	assert_false(initialized);
+	check_config(false);
+}
+
+/* dm_prereq fails */
+static void test_mpathvalid_init_bad3(void **state)
+{
+	make_config_file(FIND_MULTIPATHS_STRICT);
+	will_return(__wrap_libmultipath_init, 0);
+	will_return(__wrap_init_config, 0);
+	will_return(__wrap_init_config, FIND_MULTIPATHS_STRICT);
+	will_return(__wrap_dm_prereq, 1);
+	assert_int_equal(mpathvalid_init(MPATH_LOG_STDERR, MPATH_LOG_PRIO_ERR),
+			 -1);
+	assert_false(initialized);
+	check_config(false);
+}
+
+static void check_mpathvalid_init(int findmp, int prio, int log_style)
+{
+	make_config_file(findmp);
+	will_return(__wrap_libmultipath_init, 0);
+	will_return(__wrap_init_config, 0);
+	will_return(__wrap_init_config, findmp);
+	will_return(__wrap_dm_prereq, 0);
+	assert_int_equal(mpathvalid_init(prio, log_style), 0);	
+	assert_true(initialized);
+	check_config(true);
+	assert_int_equal(logsink, log_style);
+	assert_int_equal(libmp_verbosity, prio);
+	assert_int_equal(findmp_to_mode(findmp), mpathvalid_get_mode());
+}
+
+static void check_mpathvalid_exit(void)
+{
+	assert_int_equal(mpathvalid_exit(), 0);
+	assert_false(initialized);
+	check_config(false);
+}
+
+static void test_mpathvalid_init_good1(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_OFF, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR_TIMESTAMP);
+}
+
+static void test_mpathvalid_init_good2(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_STRICT, MPATH_LOG_PRIO_DEBUG,
+			      MPATH_LOG_STDERR);
+}
+
+static void test_mpathvalid_init_good3(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_ON, MPATH_LOG_PRIO_NOLOG,
+			      MPATH_LOG_SYSLOG);
+}
+
+static void test_mpathvalid_exit(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_ON, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	check_mpathvalid_exit();
+}
+
+/* fails if config hasn't been set */
+static void test_mpathvalid_get_mode_bad(void **state)
+{
+#if 1
+	assert_int_equal(mpathvalid_get_mode(), MPATH_MODE_ERROR);
+#else
+	assert_int_equal(mpathvalid_get_mode(), 1);
+#endif
+}
+
+/*fails if config hasn't been set */
+static void test_mpathvalid_reload_config_bad1(void **state)
+{
+#if 1
+	will_return(__wrap_init_config, 1);
+#endif
+	assert_int_equal(mpathvalid_reload_config(), -1);
+	check_config(false);
+}
+
+/* init_config fails */
+static void test_mpathvalid_reload_config_bad2(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_ON, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_init_config, 1);
+	assert_int_equal(mpathvalid_reload_config(), -1);
+	check_config(false);
+	check_mpathvalid_exit();
+}
+
+static void check_mpathvalid_reload_config(int findmp)
+{
+	assert_string_not_equal(conf_name, CONF_TEMPLATE);
+	unlink(conf_name);
+	make_config_file(findmp);
+	will_return(__wrap_init_config, 0);
+	will_return(__wrap_init_config, findmp);
+	assert_int_equal(mpathvalid_reload_config(), 0);
+	check_config(true);
+	assert_int_equal(findmp_to_mode(findmp), mpathvalid_get_mode());
+}
+
+static void test_mpathvalid_reload_config_good(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_OFF, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	check_mpathvalid_reload_config(FIND_MULTIPATHS_ON);
+	check_mpathvalid_reload_config(FIND_MULTIPATHS_GREEDY);
+	check_mpathvalid_reload_config(FIND_MULTIPATHS_SMART);
+	check_mpathvalid_reload_config(FIND_MULTIPATHS_STRICT);
+	check_mpathvalid_exit();
+}
+
+/* NULL name */
+static void test_mpathvalid_is_path_bad1(void **state)
+{
+	assert_int_equal(mpathvalid_is_path(NULL, MPATH_STRICT, NULL, NULL, 0),
+			 MPATH_IS_ERROR);
+}
+
+/* bad mode */
+static void test_mpathvalid_is_path_bad2(void **state)
+{
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_MODE_ERROR, NULL,
+					    NULL, 0), MPATH_IS_ERROR);
+}
+
+/* NULL path_wwids and non-zero nr_paths */
+static void test_mpathvalid_is_path_bad3(void **state)
+{
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_MODE_ERROR, NULL,
+			 		    NULL, 1), MPATH_IS_ERROR);
+}
+
+/*fails if config hasn't been set */
+static void test_mpathvalid_is_path_bad4(void **state)
+{
+#if 0
+	will_return(__wrap_is_path_valid, MPATH_IS_ERROR);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_STRICT);
+#endif
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_STRICT, NULL,
+					    NULL, 0), MPATH_IS_ERROR);
+}
+
+/* is_path_valid fails */
+static void test_mpathvalid_is_path_bad5(void **state)
+{
+	check_mpathvalid_init(FIND_MULTIPATHS_OFF, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_ERROR);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_GREEDY);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_GREEDY, NULL,
+					    NULL, 0), MPATH_IS_ERROR);
+	check_mpathvalid_exit();
+}
+
+static void test_mpathvalid_is_path_good1(void **state)
+{
+	char *wwid;
+	check_mpathvalid_init(FIND_MULTIPATHS_STRICT, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_NOT_VALID);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_STRICT);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_DEFAULT, &wwid,
+			 		    NULL, 0), MPATH_IS_NOT_VALID);
+	assert_ptr_equal(wwid, NULL);
+	check_mpathvalid_exit();
+}
+
+static void test_mpathvalid_is_path_good2(void **state)
+{
+	const char *wwids[] = { "WWID_A", "WWID_B", "WWID_C", "WWID_D" };
+	char *wwid;
+	check_mpathvalid_init(FIND_MULTIPATHS_ON, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_VALID);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_ON);
+	will_return(__wrap_is_path_valid, TEST_WWID);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_DEFAULT, &wwid,
+					    wwids, 4), MPATH_IS_VALID);
+	assert_string_equal(wwid, TEST_WWID);
+}
+
+static void test_mpathvalid_is_path_good3(void **state)
+{
+	const char *wwids[] = { "WWID_A", "WWID_B", "WWID_C", "WWID_D" };
+	char *wwid;
+	check_mpathvalid_init(FIND_MULTIPATHS_OFF, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_VALID);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_SMART);
+	will_return(__wrap_is_path_valid, TEST_WWID);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_SMART, &wwid,
+					    wwids, 4), MPATH_IS_VALID);
+	assert_string_equal(wwid, TEST_WWID);
+}
+
+/* mabybe valid with no matching paths */
+static void test_mpathvalid_is_path_good4(void **state)
+{
+	const char *wwids[] = { "WWID_A", "WWID_B", "WWID_C", "WWID_D" };
+	char *wwid;
+	check_mpathvalid_init(FIND_MULTIPATHS_SMART, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_MAYBE_VALID);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_SMART);
+	will_return(__wrap_is_path_valid, TEST_WWID);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_DEFAULT, &wwid,
+					    wwids, 4), MPATH_IS_MAYBE_VALID);
+	assert_string_equal(wwid, TEST_WWID);
+}
+
+/* maybe valid with matching paths */
+static void test_mpathvalid_is_path_good5(void **state)
+{
+	const char *wwids[] = { "WWID_A", "WWID_B", TEST_WWID, "WWID_D" };
+	char *wwid;
+	check_mpathvalid_init(FIND_MULTIPATHS_SMART, MPATH_LOG_PRIO_ERR,
+			      MPATH_LOG_STDERR);
+	will_return(__wrap_is_path_valid, MPATH_IS_MAYBE_VALID);
+	will_return(__wrap_is_path_valid, FIND_MULTIPATHS_SMART);
+	will_return(__wrap_is_path_valid, TEST_WWID);
+	assert_int_equal(mpathvalid_is_path(test_dev, MPATH_DEFAULT, &wwid,
+					    wwids, 4), MPATH_IS_VALID);
+	assert_string_equal(wwid, TEST_WWID);
+}
+
+#define setup_test(name) \
+	cmocka_unit_test_setup_teardown(name, setup, teardown)
+
+int test_mpathvalid(void)
+{
+	const struct CMUnitTest tests[] = {
+		setup_test(test_mpathvalid_init_bad1),
+		setup_test(test_mpathvalid_init_bad2),
+		setup_test(test_mpathvalid_init_bad3),
+		setup_test(test_mpathvalid_init_good1),
+		setup_test(test_mpathvalid_init_good2),
+		setup_test(test_mpathvalid_init_good3),
+		setup_test(test_mpathvalid_exit),
+		setup_test(test_mpathvalid_get_mode_bad),
+		setup_test(test_mpathvalid_reload_config_bad1),
+		setup_test(test_mpathvalid_reload_config_bad2),
+		setup_test(test_mpathvalid_reload_config_good),
+		setup_test(test_mpathvalid_is_path_bad1),
+		setup_test(test_mpathvalid_is_path_bad2),
+		setup_test(test_mpathvalid_is_path_bad3),
+		setup_test(test_mpathvalid_is_path_bad4),
+		setup_test(test_mpathvalid_is_path_bad5),
+		setup_test(test_mpathvalid_is_path_good1),
+		setup_test(test_mpathvalid_is_path_good2),
+		setup_test(test_mpathvalid_is_path_good3),
+		setup_test(test_mpathvalid_is_path_good4),
+		setup_test(test_mpathvalid_is_path_good5),
+	};
+	return cmocka_run_group_tests(tests, NULL, NULL);
+}
+
+int main(void)
+{
+	int r = 0;
+
+	r += test_mpathvalid();
+	return r;
+}
-- 
2.17.2

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [dm-devel] [PATCH v3 3/4] libmultipath: add uid failback for dasd devices
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 1/4] multipath: add libmpathvalid library Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 2/4] multipath-tools tests: and unit tests for libmpathvalid Benjamin Marzinski
@ 2020-10-21 21:39 ` Benjamin Marzinski
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute Benjamin Marzinski
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-10-21 21:39 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

Add failback code to get the uid for dasd devices from sysfs. Copied
from dasdinfo

Reviewed-by: Martin Wilck <mwilck@suse.com>
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/defaults.h  |  1 +
 libmultipath/discovery.c | 37 ++++++++++++++++++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/libmultipath/defaults.h b/libmultipath/defaults.h
index 39a5e415..947ba467 100644
--- a/libmultipath/defaults.h
+++ b/libmultipath/defaults.h
@@ -8,6 +8,7 @@
  */
 #define DEFAULT_UID_ATTRIBUTE	"ID_SERIAL"
 #define DEFAULT_NVME_UID_ATTRIBUTE	"ID_WWN"
+#define DEFAULT_DASD_UID_ATTRIBUTE	"ID_UID"
 #define DEFAULT_UDEVDIR		"/dev"
 #define DEFAULT_MULTIPATHDIR	"/" LIB_STRING "/multipath"
 #define DEFAULT_SELECTOR	"service-time 0"
diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index cbde3585..d7e8577f 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -1957,12 +1957,44 @@ get_vpd_uid(struct path * pp)
 	return get_vpd_sysfs(parent, 0x83, pp->wwid, WWID_SIZE);
 }
 
+/* based on code from s390-tools/dasdinfo/dasdinfo.c */
+static ssize_t dasd_get_uid(struct path *pp)
+{
+	struct udev_device *parent;
+	char value[80];
+	char *p;
+	int i;
+
+	parent = udev_device_get_parent_with_subsystem_devtype(pp->udev, "ccw",
+							       NULL);
+	if (!parent)
+		return -1;
+
+	if (sysfs_attr_get_value(parent, "uid", value, 80) < 0)
+		return -1;
+
+	p = value - 1;
+	/* look for the 4th '.' and cut there */
+	for (i = 0; i < 4; i++) {
+		p = index(p + 1, '.');
+		if (!p)
+			break;
+	}
+	if (p)
+		*p = '\0';
+
+	return strlcpy(pp->wwid, value, WWID_SIZE);
+}
+
 static ssize_t uid_fallback(struct path *pp, int path_state,
 			    const char **origin)
 {
 	ssize_t len = -1;
 
-	if (pp->bus == SYSFS_BUS_SCSI) {
+	if (pp->bus == SYSFS_BUS_CCW) {
+		len = dasd_get_uid(pp);
+		*origin = "sysfs";
+	} else if (pp->bus == SYSFS_BUS_SCSI) {
 		len = get_vpd_uid(pp);
 		*origin = "sysfs";
 		if (len < 0 && path_state == PATH_UP) {
@@ -2010,6 +2042,9 @@ static bool has_uid_fallback(struct path *pp)
 		  !strcmp(pp->uid_attribute, ""))) ||
 		(pp->bus == SYSFS_BUS_NVME &&
 		 (!strcmp(pp->uid_attribute, DEFAULT_NVME_UID_ATTRIBUTE) ||
+		  !strcmp(pp->uid_attribute, ""))) ||
+		(pp->bus == SYSFS_BUS_CCW &&
+		 (!strcmp(pp->uid_attribute, DEFAULT_DASD_UID_ATTRIBUTE) ||
 		  !strcmp(pp->uid_attribute, ""))));
 }
 
-- 
2.17.2

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
                   ` (2 preceding siblings ...)
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 3/4] libmultipath: add uid failback for dasd devices Benjamin Marzinski
@ 2020-10-21 21:39 ` Benjamin Marzinski
  2020-11-01 20:48   ` Martin Wilck
  2020-11-01 21:33 ` [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Martin Wilck
  2020-12-16 20:52 ` Martin Wilck
  5 siblings, 1 reply; 9+ messages in thread
From: Benjamin Marzinski @ 2020-10-21 21:39 UTC (permalink / raw)
  To: Christophe Varoqui; +Cc: device-mapper development, Martin Wilck

If uid_attribute is explicitly set to an empty string, multipath should
log the uid at the default log level, since using the fallback code is
the expected behavior.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
---
 libmultipath/discovery.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/libmultipath/discovery.c b/libmultipath/discovery.c
index d7e8577f..950b1586 100644
--- a/libmultipath/discovery.c
+++ b/libmultipath/discovery.c
@@ -2084,8 +2084,11 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev,
 			len = strlen(pp->wwid);
 		origin = "callout";
 	} else {
-		bool udev_available = udev && pp->uid_attribute
+		bool valid_uid_attr = pp->uid_attribute
 			&& *pp->uid_attribute;
+		bool empty_uid_attr = pp->uid_attribute
+			&& !*pp->uid_attribute;
+		bool udev_available = udev && valid_uid_attr;
 
 		if (udev_available) {
 			len = get_udev_uid(pp, pp->uid_attribute, udev);
@@ -2095,7 +2098,8 @@ get_uid (struct path * pp, int path_state, struct udev_device *udev,
 		}
 		if ((!udev_available || (len <= 0 && allow_fallback))
 		    && has_uid_fallback(pp)) {
-			used_fallback = 1;
+			if (!udev || !empty_uid_attr)
+				used_fallback = 1;
 			len = uid_fallback(pp, path_state, &origin);
 		}
 	}
-- 
2.17.2

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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute Benjamin Marzinski
@ 2020-11-01 20:48   ` Martin Wilck
  0 siblings, 0 replies; 9+ messages in thread
From: Martin Wilck @ 2020-11-01 20:48 UTC (permalink / raw)
  To: Benjamin Marzinski, Christophe Varoqui; +Cc: device-mapper development

On Wed, 2020-10-21 at 16:39 -0500, Benjamin Marzinski wrote:
> If uid_attribute is explicitly set to an empty string, multipath
> should
> log the uid at the default log level, since using the fallback code
> is
> the expected behavior.
> 
> Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by:Martin Wilck <mwilck@suse.com>


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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
                   ` (3 preceding siblings ...)
  2020-10-21 21:39 ` [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute Benjamin Marzinski
@ 2020-11-01 21:33 ` Martin Wilck
  2020-11-02 20:22   ` Benjamin Marzinski
  2020-12-16 20:52 ` Martin Wilck
  5 siblings, 1 reply; 9+ messages in thread
From: Martin Wilck @ 2020-11-01 21:33 UTC (permalink / raw)
  To: bmarzins, christophe.varoqui; +Cc: dm-devel

On Wed, 2020-10-21 at 16:39 -0500, Benjamin Marzinski wrote:
> The main part of the this patchset is the first patch, which adds a
> new library interface to check whether devices are valid paths. This
> was designed for use in the Storage Instantiation Daemon (SID).
> 
> https://github.com/sid-project
> 
> The seconds patch adds unit tests for this library. The third patch
> adds
> get_uid fallback code for dasd devices. The fourth patch just changes
> the get_uid log level for devices configured with uid_attribute "".
> This
> is because it is currently necessary to configure multipath with
> 
> overrides {
>         uid_attribute ""
> }
> 
> to claim multipath devices with SID (instead of using
> multipath.rules),
> since SID doesn't currently get the UID information itself, and it is
> called by udev before this information is added to the udev database.
> 
> changes from v1 to v2
> ---------------------
> 0001: This patch is now rebased on top of, and makes use of Martin's
> patches that provide a default *_multipath_config, udev, and logsink.
> Because of this, mpathvalid_init() now has a parameter used to set
> logsink. There is also a new API function,
> mpathvalid_reload_config().
> 
> 0003: This is completely new, since Martin pointed out that adding a
> new
> config option to always use the fallback getuid code was unnecessary.
> It
> just makes a uid_attribute of "" log at normal levels.
> 
> changes from v2 to v3
> ---------------------
> 0001:   rebased on top of Martin's latest patches, fixed some small
> bugs
>         and added documentation to mpath_valid.h
> 0002:   New
> 0004:   was 0003. Untangled the logic, at Martin's suggestion.
> 
> Benjamin Marzinski (4):
>   multipath: add libmpathvalid library
>   multipath-tools tests: and unit tests for libmpathvalid
>   libmultipath: add uid failback for dasd devices
>   libmultipath: change log level for null uid_attribute
> 
>  Makefile                            |   3 +-
>  Makefile.inc                        |   1 +
>  libmpathvalid/Makefile              |  39 +++
>  libmpathvalid/libmpathvalid.version |  10 +
>  libmpathvalid/mpath_valid.c         | 202 ++++++++++++
>  libmpathvalid/mpath_valid.h         | 155 +++++++++
>  libmultipath/defaults.h             |   1 +
>  libmultipath/discovery.c            |  45 ++-
>  libmultipath/libmultipath.version   |   6 +
>  tests/Makefile                      |   5 +-
>  tests/mpathvalid.c                  | 467
> ++++++++++++++++++++++++++++
>  11 files changed, 929 insertions(+), 5 deletions(-)
>  create mode 100644 libmpathvalid/Makefile
>  create mode 100644 libmpathvalid/libmpathvalid.version
>  create mode 100644 libmpathvalid/mpath_valid.c
>  create mode 100644 libmpathvalid/mpath_valid.h
>  create mode 100644 tests/mpathvalid.c

As you probably saw, all acked. However there's a small problem with
the rebase on my recent patches. They aren't all acked yet, and Xose's
report about uclibc made me realize that there are more issues with
uclibc in my series. I don't think this will require major changes,
but e.g. on_exit() is unavailable in uclibc. I'd like to rework those.

Also, I'd wish that Christophe tags a new libmultipath version before
applying the "library version" series and everything thereafter.

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



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path
  2020-11-01 21:33 ` [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Martin Wilck
@ 2020-11-02 20:22   ` Benjamin Marzinski
  0 siblings, 0 replies; 9+ messages in thread
From: Benjamin Marzinski @ 2020-11-02 20:22 UTC (permalink / raw)
  To: Martin Wilck; +Cc: dm-devel

On Sun, Nov 01, 2020 at 09:33:09PM +0000, Martin Wilck wrote:
> On Wed, 2020-10-21 at 16:39 -0500, Benjamin Marzinski wrote:
> > The main part of the this patchset is the first patch, which adds a
> > new library interface to check whether devices are valid paths. This
> > was designed for use in the Storage Instantiation Daemon (SID).
> > 
> > https://github.com/sid-project
> > 
> > The seconds patch adds unit tests for this library. The third patch
> > adds
> > get_uid fallback code for dasd devices. The fourth patch just changes
> > the get_uid log level for devices configured with uid_attribute "".
> > This
> > is because it is currently necessary to configure multipath with
> > 
> > overrides {
> >         uid_attribute ""
> > }
> > 
> > to claim multipath devices with SID (instead of using
> > multipath.rules),
> > since SID doesn't currently get the UID information itself, and it is
> > called by udev before this information is added to the udev database.
> > 
> > changes from v1 to v2
> > ---------------------
> > 0001: This patch is now rebased on top of, and makes use of Martin's
> > patches that provide a default *_multipath_config, udev, and logsink.
> > Because of this, mpathvalid_init() now has a parameter used to set
> > logsink. There is also a new API function,
> > mpathvalid_reload_config().
> > 
> > 0003: This is completely new, since Martin pointed out that adding a
> > new
> > config option to always use the fallback getuid code was unnecessary.
> > It
> > just makes a uid_attribute of "" log at normal levels.
> > 
> > changes from v2 to v3
> > ---------------------
> > 0001:   rebased on top of Martin's latest patches, fixed some small
> > bugs
> >         and added documentation to mpath_valid.h
> > 0002:   New
> > 0004:   was 0003. Untangled the logic, at Martin's suggestion.
> > 
> > Benjamin Marzinski (4):
> >   multipath: add libmpathvalid library
> >   multipath-tools tests: and unit tests for libmpathvalid
> >   libmultipath: add uid failback for dasd devices
> >   libmultipath: change log level for null uid_attribute
> > 
> >  Makefile                            |   3 +-
> >  Makefile.inc                        |   1 +
> >  libmpathvalid/Makefile              |  39 +++
> >  libmpathvalid/libmpathvalid.version |  10 +
> >  libmpathvalid/mpath_valid.c         | 202 ++++++++++++
> >  libmpathvalid/mpath_valid.h         | 155 +++++++++
> >  libmultipath/defaults.h             |   1 +
> >  libmultipath/discovery.c            |  45 ++-
> >  libmultipath/libmultipath.version   |   6 +
> >  tests/Makefile                      |   5 +-
> >  tests/mpathvalid.c                  | 467
> > ++++++++++++++++++++++++++++
> >  11 files changed, 929 insertions(+), 5 deletions(-)
> >  create mode 100644 libmpathvalid/Makefile
> >  create mode 100644 libmpathvalid/libmpathvalid.version
> >  create mode 100644 libmpathvalid/mpath_valid.c
> >  create mode 100644 libmpathvalid/mpath_valid.h
> >  create mode 100644 tests/mpathvalid.c
> 
> As you probably saw, all acked. However there's a small problem with
> the rebase on my recent patches. They aren't all acked yet, and Xose's
> report about uclibc made me realize that there are more issues with
> uclibc in my series. I don't think this will require major changes,
> but e.g. on_exit() is unavailable in uclibc. I'd like to rework those.
> 
> Also, I'd wish that Christophe tags a new libmultipath version before
> applying the "library version" series and everything thereafter.

I'll rebase it, and I'm fine with this going in after the the version bump.

-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
> 

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path
  2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
                   ` (4 preceding siblings ...)
  2020-11-01 21:33 ` [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Martin Wilck
@ 2020-12-16 20:52 ` Martin Wilck
  5 siblings, 0 replies; 9+ messages in thread
From: Martin Wilck @ 2020-12-16 20:52 UTC (permalink / raw)
  To: bmarzins, christophe.varoqui; +Cc: dm-devel

Hi Ben:

On Wed, 2020-10-21 at 16:39 -0500, Benjamin Marzinski wrote:
> The main part of the this patchset is the first patch, which adds a
> new library interface to check whether devices are valid paths. This
> was designed for use in the Storage Instantiation Daemon (SID).
> 
> https://github.com/sid-project
> 
> The seconds patch adds unit tests for this library. The third patch
> adds
> get_uid fallback code for dasd devices. The fourth patch just changes
> the get_uid log level for devices configured with uid_attribute "".
> This
> is because it is currently necessary to configure multipath with
> 
> overrides {
>         uid_attribute ""
> }
> 
> to claim multipath devices with SID (instead of using
> multipath.rules),
> since SID doesn't currently get the UID information itself, and it is
> called by udev before this information is added to the udev database.
> 
> changes from v1 to v2
> ---------------------
> 0001: This patch is now rebased on top of, and makes use of Martin's
> patches that provide a default *_multipath_config, udev, and logsink.
> Because of this, mpathvalid_init() now has a parameter used to set
> logsink. There is also a new API function,
> mpathvalid_reload_config().
> 
> 0003: This is completely new, since Martin pointed out that adding a
> new
> config option to always use the fallback getuid code was unnecessary.
> It
> just makes a uid_attribute of "" log at normal levels.
> 
> changes from v2 to v3
> ---------------------
> 0001:   rebased on top of Martin's latest patches, fixed some small
> bugs
>         and added documentation to mpath_valid.h
> 0002:   New
> 0004:   was 0003. Untangled the logic, at Martin's suggestion.
> 
> Benjamin Marzinski (4):
>   multipath: add libmpathvalid library
>   multipath-tools tests: and unit tests for libmpathvalid
>   libmultipath: add uid failback for dasd devices
>   libmultipath: change log level for null uid_attribute
> 
>  Makefile                            |   3 +-
>  Makefile.inc                        |   1 +
>  libmpathvalid/Makefile              |  39 +++
>  libmpathvalid/libmpathvalid.version |  10 +
>  libmpathvalid/mpath_valid.c         | 202 ++++++++++++
>  libmpathvalid/mpath_valid.h         | 155 +++++++++
>  libmultipath/defaults.h             |   1 +
>  libmultipath/discovery.c            |  45 ++-
>  libmultipath/libmultipath.version   |   6 +
>  tests/Makefile                      |   5 +-
>  tests/mpathvalid.c                  | 467
> ++++++++++++++++++++++++++++
>  11 files changed, 929 insertions(+), 5 deletions(-)
>  create mode 100644 libmpathvalid/Makefile
>  create mode 100644 libmpathvalid/libmpathvalid.version
>  create mode 100644 libmpathvalid/mpath_valid.c
>  create mode 100644 libmpathvalid/mpath_valid.h
>  create mode 100644 tests/mpathvalid.c
> 

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

I have created a new branch 
https://github.com/openSUSE/multipath-tools/tree/upstream-tip

where this is series applied on my recently posted series
"libmultipath: improve cleanup on exit" (v3), in the hope that it will
pass review, too. It has to be this way around because your set
requires libmp_verbosity.

As soon as my series is finalized, this will be pushed to upstream-
queue.

Regards,
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



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


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-12-16 20:52 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-21 21:39 [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Benjamin Marzinski
2020-10-21 21:39 ` [dm-devel] [PATCH v3 1/4] multipath: add libmpathvalid library Benjamin Marzinski
2020-10-21 21:39 ` [dm-devel] [PATCH v3 2/4] multipath-tools tests: and unit tests for libmpathvalid Benjamin Marzinski
2020-10-21 21:39 ` [dm-devel] [PATCH v3 3/4] libmultipath: add uid failback for dasd devices Benjamin Marzinski
2020-10-21 21:39 ` [dm-devel] [PATCH v3 4/4] libmultipath: change log level for null uid_attribute Benjamin Marzinski
2020-11-01 20:48   ` Martin Wilck
2020-11-01 21:33 ` [dm-devel] [PATCH v3 0/4] add library to check if device is a valid path Martin Wilck
2020-11-02 20:22   ` Benjamin Marzinski
2020-12-16 20:52 ` Martin Wilck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.