From: Martin Wilck <mwilck@xxxxxxxx>
As one step towards bundling all possibly racy libdm init calls into a single
function, split the code for determining and checking versions of
libdm and kernel components. Provide a generic helper
libmp_get_version() that makes sure the versions are "lazily" initialized.
External callers may use dm_prereq(), like before.
Minor API change: dm_prereq() does not nullify the argument any more
if an error is encountered.
Remove the conf->version field, which isn't needed any more after this
change. This makes it necessary to fixup the hwtable test. Also, it
represents an incompatible ABI change as offsets in "struct config" are
changed, and two symbols removed. Bump the ABI major version to 2.
Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
---
libmultipath/config.h | 1 -
libmultipath/devmapper.c | 160 ++++++++++++++++++++----------
libmultipath/devmapper.h | 11 +-
libmultipath/libmultipath.version | 5 +-
libmultipath/propsel.c | 10 +-
multipathd/dmevents.c | 2 +-
multipathd/main.c | 1 -
tests/Makefile | 2 +-
tests/hwtable.c | 3 -
tests/test-lib.c | 13 +++
10 files changed, 141 insertions(+), 67 deletions(-)
diff --git a/libmultipath/config.h b/libmultipath/config.h
index 290aea5..7af1984 100644
--- a/libmultipath/config.h
+++ b/libmultipath/config.h
@@ -192,7 +192,6 @@ struct config {
int find_multipaths_timeout;
int marginal_pathgroups;
int skip_delegate;
- unsigned int version[3];
unsigned int sequence_nr;
char * multipath_dir;
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 7394796..08aa09f 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -26,6 +26,7 @@
#include "sysfs.h"
#include "config.h"
#include "wwids.h"
+#include "version.h"
#include "log_pthread.h"
#include <sys/types.h>
@@ -34,7 +35,13 @@
#define MAX_WAIT 5
#define LOOPS_PER_SEC 5
+#define INVALID_VERSION ~0U
+static unsigned int dm_library_version[3] = { INVALID_VERSION, };
+static unsigned int dm_kernel_version[3] = { INVALID_VERSION, };
+static unsigned int dm_mpath_target_version[3] = { INVALID_VERSION, };
+
static pthread_once_t dm_initialized = PTHREAD_ONCE_INIT;
+static pthread_once_t versions_initialized = PTHREAD_ONCE_INIT;
static int dm_conf_verbosity;
@@ -102,7 +109,7 @@ dm_write_log (int level, const char *file, int line, const char *f, ...)
return;
}
-void dm_init(int v)
+static void dm_init(int v)
{
/*
* This maps libdm's standard loglevel _LOG_WARN (= 4), which is rather
@@ -112,59 +119,66 @@ void dm_init(int v)
dm_log_init(&dm_write_log);
}
+static void init_dm_library_version(void)
+{
+ char version[64];
+ unsigned int v[3];
+
+ dm_get_library_version(version, sizeof(version));
+ if (sscanf(version, "%u.%u.%u ", &v[0], &v[1], &v[2]) != 3) {
+ condlog(0, "invalid libdevmapper version %s", version);
+ return;
+ }
+ memcpy(dm_library_version, v, sizeof(dm_library_version));
+ condlog(3, "libdevmapper version %u.%.2u.%.2u",
+ dm_library_version[0], dm_library_version[1],
+ dm_library_version[2]);
+}
+
static int
dm_lib_prereq (void)
{
- char version[64];
- int v[3];
+
#if defined(LIBDM_API_HOLD_CONTROL)
- int minv[3] = {1, 2, 111};
+ unsigned int minv[3] = {1, 2, 111};
#elif defined(LIBDM_API_DEFERRED)
- int minv[3] = {1, 2, 89};
+ unsigned int minv[3] = {1, 2, 89};
#elif defined(DM_SUBSYSTEM_UDEV_FLAG0)
- int minv[3] = {1, 2, 82};
+ unsigned int minv[3] = {1, 2, 82};
#elif defined(LIBDM_API_COOKIE)
- int minv[3] = {1, 2, 38};
+ unsigned int minv[3] = {1, 2, 38};
#else
- int minv[3] = {1, 2, 8};
+ unsigned int minv[3] = {1, 2, 8};
#endif
- dm_get_library_version(version, sizeof(version));
- condlog(3, "libdevmapper version %s", version);
- if (sscanf(version, "%d.%d.%d ", &v[0], &v[1], &v[2]) != 3) {
- condlog(0, "invalid libdevmapper version %s", version);
- return 1;
- }
-
- if VERSION_GE(v, minv)
+ if (VERSION_GE(dm_library_version, minv))
return 0;
- condlog(0, "libdevmapper version must be >= %d.%.2d.%.2d",
+ condlog(0, "libdevmapper version must be >= %u.%.2u.%.2u",
minv[0], minv[1], minv[2]);
return 1;
}
-int
-dm_drv_version(unsigned int *v)
+static void init_dm_drv_version(void)
{
char buff[64];
-
- v[0] = 0;
- v[1] = 0;
- v[2] = 0;
+ unsigned int v[3];
if (!dm_driver_version(buff, sizeof(buff))) {
condlog(0, "cannot get kernel dm version");
- return 1;
+ return;
}
if (sscanf(buff, "%u.%u.%u ", &v[0], &v[1], &v[2]) != 3) {
condlog(0, "invalid kernel dm version '%s'", buff);
- return 1;
+ return;
}
- return 0;
+ memcpy(dm_kernel_version, v, sizeof(dm_library_version));
+ condlog(3, "kernel device mapper v%u.%u.%u",
+ dm_kernel_version[0],
+ dm_kernel_version[1],
+ dm_kernel_version[2]);
}
-int
-dm_tgt_version (unsigned int * version, char * str)
+static int dm_tgt_version (unsigned int *version, char *str)
{
int r = 2;
struct dm_task *dmt;
@@ -172,10 +186,11 @@ dm_tgt_version (unsigned int * version, char * str)
struct dm_versions *last_target;
unsigned int *v;
- version[0] = 0;
- version[1] = 0;
- version[2] = 0;
-
+ /*
+ * We have to call dm_task_create() and not libmp_dm_task_create()
+ * here to avoid a recursive invocation of
+ * pthread_once(&dm_initialized), which would cause a deadlock.
+ */
if (!(dmt = dm_task_create(DM_DEVICE_LIST_VERSIONS)))
return 1;
@@ -211,26 +226,25 @@ out:
return r;
}
-static int
-dm_tgt_prereq (unsigned int *ver)
+static void init_dm_mpath_version(void)
{
- unsigned int minv[3] = {1, 0, 3};
- unsigned int version[3] = {0, 0, 0};
- unsigned int * v = version;
-
- if (dm_tgt_version(v, TGT_MPATH)) {
- /* in doubt return not capable */
- return 1;
- }
+ if (!dm_tgt_version(dm_mpath_target_version, TGT_MPATH))
+ condlog(3, "DM multipath kernel driver v%u.%u.%u",
+ dm_mpath_target_version[0],
+ dm_mpath_target_version[1],
+ dm_mpath_target_version[2]);
+}
- /* test request based multipath capability */
- condlog(3, "DM multipath kernel driver v%u.%u.%u",
- v[0], v[1], v[2]);
+static int dm_tgt_prereq (unsigned int *ver)
+{
+ unsigned int minv[3] = {1, 0, 3};
- if (VERSION_GE(v, minv)) {
- ver[0] = v[0];
- ver[1] = v[1];
- ver[2] = v[2];
+ if (VERSION_GE(dm_mpath_target_version, minv)) {
+ if (ver) {
+ ver[0] = dm_mpath_target_version[0];
+ ver[1] = dm_mpath_target_version[1];
+ ver[2] = dm_mpath_target_version[2];
+ }
return 0;
}
@@ -239,13 +253,60 @@ dm_tgt_prereq (unsigned int *ver)
return 1;
}
+static void _init_versions(void)
+{
+ dlog(logsink, 3, VERSION_STRING);
+ init_dm_library_version();
+ init_dm_drv_version();
+ init_dm_mpath_version();
+}
+
+static int init_versions(void) {
+ pthread_once(&versions_initialized, _init_versions);
+ return (dm_library_version[0] == INVALID_VERSION ||
+ dm_kernel_version[0] == INVALID_VERSION ||
+ dm_mpath_target_version[0] == INVALID_VERSION);
+}
+
int dm_prereq(unsigned int *v)
{
+ if (init_versions())
+ return 1;
if (dm_lib_prereq())
return 1;
return dm_tgt_prereq(v);
}
+int libmp_get_version(int which, unsigned int version[3])
+{
+ unsigned int *src_version;
+
+ init_versions();
+ switch (which) {
+ case DM_LIBRARY_VERSION:
+ src_version = dm_library_version;
+ break;
+ case DM_KERNEL_VERSION:
+ src_version = dm_kernel_version;
+ break;
+ case DM_MPATH_TARGET_VERSION:
+ src_version = dm_mpath_target_version;
+ break;
+ case MULTIPATH_VERSION:
+ version[0] = (VERSION_CODE >> 16) & 0xff;
+ version[1] = (VERSION_CODE >> 8) & 0xff;
+ version[2] = VERSION_CODE & 0xff;
+ return 0;
+ default:
+ condlog(0, "%s: invalid value for 'which'", __func__);
+ return 1;
+ }
+ if (src_version[0] == INVALID_VERSION)
+ return 1;
+ memcpy(version, src_version, 3 * sizeof(*version));
+ return 0;
+}
+
static int libmp_dm_udev_sync = 0;
void libmp_udev_set_sync_support(int on)
@@ -263,7 +324,6 @@ static void libmp_dm_init(void)
exit(1);
conf = get_multipath_config();
verbosity = conf->verbosity;
- memcpy(conf->version, version, sizeof(version));
put_multipath_config(conf);
dm_init(verbosity);
#ifdef LIBDM_API_HOLD_CONTROL
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index f568ab5..c8b37e1 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -33,13 +33,10 @@ enum {
DMP_NOT_FOUND,
};
-void dm_init(int verbosity);
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);
-int dm_tgt_version (unsigned int * version, char * str);
int dm_simplecmd_flush (int, const char *, uint16_t);
int dm_simplecmd_noflush (int, const char *, uint16_t);
int dm_addmap_create (struct multipath *mpp, char *params);
@@ -85,6 +82,14 @@ struct multipath *dm_get_multipath(const char *name);
((v[0] == minv[0]) && (v[1] == minv[1]) && (v[2] >= minv[2])) \
)
+enum {
+ DM_LIBRARY_VERSION,
+ DM_KERNEL_VERSION,
+ DM_MPATH_TARGET_VERSION,
+ MULTIPATH_VERSION
+};
+int libmp_get_version(int which, unsigned int version[3]);
+
#define dm_log_error(lvl, cmd, dmt) \
condlog(lvl, "%s: libdm task=%d error: %s", __func__, \
cmd, strerror(dm_task_get_errno(dmt))) \
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index a6bf821..ab5bb0a 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -31,7 +31,7 @@
* The new version inherits the previous ones.
*/
-LIBMULTIPATH_1.0.0 {
+LIBMULTIPATH_2.0.0 {
global:
/* symbols referenced by multipath and multipathd */
add_foreign;
@@ -65,7 +65,6 @@ global:
disassemble_status;
dlog;
dm_cancel_deferred_remove;
- dm_drv_version;
dm_enablegroup;
dm_fail_path;
_dm_flush_map;
@@ -87,7 +86,6 @@ global:
dm_reinstate_path;
dm_simplecmd_noflush;
dm_switchgroup;
- dm_tgt_version;
domap;
ensure_directories_exist;
extract_hwe_from_path;
@@ -128,6 +126,7 @@ global:
is_path_valid;
is_quote;
libmp_dm_task_create;
+ libmp_get_version;
libmp_udev_set_sync_support;
load_config;
log_thread_reset;
diff --git a/libmultipath/propsel.c b/libmultipath/propsel.c
index 4020134..3f2c2cf 100644
--- a/libmultipath/propsel.c
+++ b/libmultipath/propsel.c
@@ -735,9 +735,10 @@ out:
int select_minio(struct config *conf, struct multipath *mp)
{
- unsigned int minv_dmrq[3] = {1, 1, 0};
+ unsigned int minv_dmrq[3] = {1, 1, 0}, version[3];
- if (VERSION_GE(conf->version, minv_dmrq))
+ if (!libmp_get_version(DM_MPATH_TARGET_VERSION, version)
+ && VERSION_GE(version, minv_dmrq))
return select_minio_rq(conf, mp);
else
return select_minio_bio(conf, mp);
@@ -820,9 +821,10 @@ out:
int select_retain_hwhandler(struct config *conf, struct multipath *mp)
{
const char *origin;
- unsigned int minv_dm_retain[3] = {1, 5, 0};
+ unsigned int minv_dm_retain[3] = {1, 5, 0}, version[3];
- if (!VERSION_GE(conf->version, minv_dm_retain)) {
+ if (!libmp_get_version(DM_MPATH_TARGET_VERSION, version) &&
+ !VERSION_GE(version, minv_dm_retain)) {
mp->retain_hwhandler = RETAIN_HWHANDLER_OFF;
origin = "(setting: WARNING, requires kernel dm-mpath version >= 1.5.0)";
goto out;
diff --git a/multipathd/dmevents.c b/multipathd/dmevents.c
index 5f2d210..fc97c8a 100644
--- a/multipathd/dmevents.c
+++ b/multipathd/dmevents.c
@@ -60,7 +60,7 @@ int dmevent_poll_supported(void)
{
unsigned int v[3];
- if (dm_drv_version(v))
+ if (libmp_get_version(DM_KERNEL_VERSION, v))
return 0;
if (VERSION_GE(v, DM_VERSION_FOR_ARM_POLL))
diff --git a/multipathd/main.c b/multipathd/main.c
index 5cc3435..00b66ba 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -2709,7 +2709,6 @@ reconfigure (struct vectors * vecs)
/* Re-read any timezone changes */
tzset();
- dm_tgt_version(conf->version, TGT_MPATH);
if (verbosity)
conf->verbosity = verbosity;
if (bindings_read_only)
diff --git a/tests/Makefile b/tests/Makefile
index f6ef16a..7a73869 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -42,7 +42,7 @@ endif
dmevents-test_LIBDEPS = -lpthread -ldevmapper -lurcu
hwtable-test_TESTDEPS := test-lib.o
hwtable-test_OBJDEPS := ../libmultipath/discovery.o ../libmultipath/blacklist.o \
- ../libmultipath/structs.o
+ ../libmultipath/structs.o ../libmultipath/propsel.o
hwtable-test_LIBDEPS := -ludev -lpthread -ldl
blacklist-test_TESTDEPS := test-log.o
blacklist-test_OBJDEPS := ../libmultipath/blacklist.o
diff --git a/tests/hwtable.c b/tests/hwtable.c
index 12660da..57f832b 100644
--- a/tests/hwtable.c
+++ b/tests/hwtable.c
@@ -30,8 +30,6 @@
#define N_CONF_FILES 2
static const char tmplate[] = "/tmp/hwtable-XXXXXX";
-/* pretend new dm, use minio_rq */
-static const unsigned int dm_tgt_version[3] = { 1, 1, 1 };
struct key_value {
const char *key;
@@ -360,7 +358,6 @@ static void write_device(FILE *ff, int nkv, const struct key_value *kv)
assert_ptr_not_equal(__cf, NULL); \
assert_ptr_not_equal(__cf->hwtable, NULL); \
__cf->verbosity = VERBOSITY; \
- memcpy(&__cf->version, dm_tgt_version, sizeof(__cf->version)); \
__cf; })
#define FREE_CONFIG(conf) do { \
diff --git a/tests/test-lib.c b/tests/test-lib.c
index b7c09cc..e7663f9 100644
--- a/tests/test-lib.c
+++ b/tests/test-lib.c
@@ -56,6 +56,15 @@ int __wrap_execute_program(char *path, char *value, int len)
return 0;
}
+int __wrap_libmp_get_version(int which, unsigned int version[3])
+{
+ unsigned int *vers = mock_ptr_type(unsigned int *);
+
+ condlog(4, "%s: %d", __func__, which);
+ memcpy(version, vers, 3 * sizeof(unsigned int));
+ return 0;
+}
+
struct udev_list_entry
*__wrap_udev_device_get_properties_list_entry(struct udev_device *ud)
{
@@ -339,6 +348,8 @@ struct multipath *__mock_multipath(struct vectors *vecs, struct path *pp)
struct multipath *mp;
struct config *conf;
struct mocked_path mop;
+ /* pretend new dm, use minio_rq, */
+ static const unsigned int fake_dm_tgt_version[3] = { 1, 1, 1 };
mocked_path_from_path(&mop, pp);
/* pathinfo() call in adopt_paths */
@@ -351,7 +362,9 @@ struct multipath *__mock_multipath(struct vectors *vecs, struct path *pp)
conf = get_multipath_config();
select_pgpolicy(conf, mp);
select_no_path_retry(conf, mp);
+ will_return(__wrap_libmp_get_version, fake_dm_tgt_version);
select_retain_hwhandler(conf, mp);
+ will_return(__wrap_libmp_get_version, fake_dm_tgt_version);
select_minio(conf, mp);
put_multipath_config(conf);
--
2.28.0
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/dm-devel
