We have a couple of callers that check whether the map UUID conforms
to the multipath convention ("mpath-xyz"). Move this check into
libmp_mapinfo__(). Add another flag MAPINFO_CHECK_UUID for this
purpose. This allows to simplify some callers, which only fetched
the UUID in order to check it.
Note that the UUID check is orthogonal to MAPINFO_MPATH_ONLY, which
tests the target type. We shold make sure that both tests are in
agreement, but this is postponed to a later patch set.
is_mpath_uuid() can now be converted to a static function.
Also add some unit tests for the WWID check.
Signed-off-by: Martin Wilck <mwilck@xxxxxxxx>
---
libmpathpersist/mpath_persist_int.c | 11 +-
libmultipath/devmapper.c | 52 +++----
libmultipath/devmapper.h | 3 +-
libmultipath/libmultipath.version | 1 -
multipath/main.c | 5 +-
multipathd/main.c | 7 +-
tests/mapinfo.c | 226 ++++++++++++++++++++++++++++
7 files changed, 256 insertions(+), 49 deletions(-)
diff --git a/libmpathpersist/mpath_persist_int.c b/libmpathpersist/mpath_persist_int.c
index 807415f..f4d9e7c 100644
--- a/libmpathpersist/mpath_persist_int.c
+++ b/libmpathpersist/mpath_persist_int.c
@@ -158,7 +158,7 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, struct multipath
{
int rc;
struct stat info;
- char alias[WWID_SIZE], uuid[DM_UUID_LEN];
+ char alias[WWID_SIZE];
struct multipath *mpp;
if (fstat(fd, &info) != 0){
@@ -171,14 +171,11 @@ static int mpath_get_map(vector curmp, vector pathvec, int fd, struct multipath
}
/* get alias from major:minor*/
- rc = libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY,
+ rc = libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
(mapid_t) { .devt = info.st_rdev },
- (mapinfo_t) {
- .name = alias,
- .uuid = uuid,
- });
+ (mapinfo_t) { .name = alias });
- if (rc == DMP_NO_MATCH || !is_mpath_uuid(uuid)) {
+ if (rc == DMP_NO_MATCH) {
condlog(3, "%s: not a multipath device.", alias);
return MPATH_PR_DMMP_ERROR;
} else if (rc != DMP_OK) {
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index d9d96be..bbbadee 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -611,6 +611,11 @@ int dm_addmap_reload(struct multipath *mpp, char *params, int flush)
return 0;
}
+static bool is_mpath_uuid(const char uuid[DM_UUID_LEN])
+{
+ return !strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN);
+}
+
bool
has_dm_info(const struct multipath *mpp)
{
@@ -734,11 +739,17 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma
* If error is returned, don't touch any output parameters.
*/
if ((info.name && !(name = dm_task_get_name(dmt)))
- || (info.uuid && !(uuid = dm_task_get_uuid(dmt)))
+ || ((info.uuid || flags & MAPINFO_CHECK_UUID)
+ && !(uuid = dm_task_get_uuid(dmt)))
|| (info.status && !(tmp_status = strdup(params)))
|| (info.target && !tmp_target && !(tmp_target = strdup(params))))
return DMP_ERR;
+ if (flags & MAPINFO_CHECK_UUID && !is_mpath_uuid(uuid)) {
+ condlog(3, "%s: UUID mismatch: %s", __func__, uuid);
+ return DMP_NO_MATCH;
+ }
+
if (info.name) {
strlcpy(info.name, name, WWID_SIZE);
condlog(4, "%s: %s: name: \"%s\"", __func__, map_id, info.name);
@@ -808,18 +819,6 @@ int libmp_mapinfo(int flags, mapid_t id, mapinfo_t info)
libmp_map_identifier(flags, id, idbuf));
}
-static int dm_get_dm_uuid(const char *mapname, char uuid[DM_UUID_LEN])
-{
- return libmp_mapinfo(DM_MAP_BY_NAME,
- (mapid_t) { .str = mapname },
- (mapinfo_t) { .uuid = uuid });
-}
-
-bool is_mpath_uuid(const char uuid[DM_UUID_LEN])
-{
- return !strncmp(uuid, UUID_PREFIX, UUID_PREFIX_LEN);
-}
-
/**
* dm_get_wwid(): return WWID for a multipath map
* @returns:
@@ -832,16 +831,14 @@ bool is_mpath_uuid(const char uuid[DM_UUID_LEN])
int dm_get_wwid(const char *name, char *uuid, int uuid_len)
{
char tmp[DM_UUID_LEN];
- int rc = dm_get_dm_uuid(name, tmp);
+ int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = name },
+ (mapinfo_t) { .uuid = tmp });
if (rc != DMP_OK)
return rc;
- if (is_mpath_uuid(tmp))
- strlcpy(uuid, tmp + UUID_PREFIX_LEN, uuid_len);
- else
- return DMP_NO_MATCH;
-
+ strlcpy(uuid, tmp + UUID_PREFIX_LEN, uuid_len);
return DMP_OK;
}
@@ -859,16 +856,13 @@ static bool is_mpath_part_uuid(const char part_uuid[DM_UUID_LEN],
int dm_is_mpath(const char *name)
{
- char uuid[DM_UUID_LEN];
- int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY,
+ int rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
(mapid_t) { .str = name },
- (mapinfo_t) { .uuid = uuid });
+ (mapinfo_t) { .uuid = NULL });
switch (rc) {
case DMP_OK:
- if (is_mpath_uuid(uuid))
- return DM_IS_MPATH_YES;
- /* fallthrough */
+ return DM_IS_MPATH_YES;
case DMP_NOT_FOUND:
case DMP_NO_MATCH:
return DM_IS_MPATH_NO;
@@ -974,14 +968,10 @@ int _dm_flush_map (const char *mapname, int flags, int retries)
int queue_if_no_path = 0;
int udev_flags = 0;
char *params __attribute__((cleanup(cleanup_charp))) = NULL;
- char uuid[DM_UUID_LEN];
- if (libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY,
+ if (libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
(mapid_t) { .str = mapname },
- (mapinfo_t) {
- .uuid = uuid,
- .target = ¶ms }) != DMP_OK
- || !is_mpath_uuid(uuid))
+ (mapinfo_t) { .target = ¶ms }) != DMP_OK)
return DM_FLUSH_OK; /* nothing to do */
/* if the device currently has no partitions, do not
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 6eb5ab9..c28991f 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -57,6 +57,8 @@ enum __mapinfo_flags {
/* Fail if target type is not "partition" (linear) */
MAPINFO_PART_ONLY = (1 << 4),
__MAPINFO_TGT_TYPE = (MAPINFO_MPATH_ONLY | MAPINFO_PART_ONLY),
+ /* Fail if the UUID doesn't match the multipath UUID format */
+ MAPINFO_CHECK_UUID = (1 << 8),
};
typedef union libmp_map_identifier {
@@ -138,7 +140,6 @@ enum {
DM_IS_MPATH_ERR,
};
-bool is_mpath_uuid(const char uuid[DM_UUID_LEN]);
int dm_is_mpath(const char *name);
enum {
diff --git a/libmultipath/libmultipath.version b/libmultipath/libmultipath.version
index 649c1cb..959f675 100644
--- a/libmultipath/libmultipath.version
+++ b/libmultipath/libmultipath.version
@@ -127,7 +127,6 @@ global:
init_foreign;
init_prio;
io_err_stat_handle_pathfail;
- is_mpath_uuid;
is_path_valid;
libmp_dm_task_create;
libmp_get_version;
diff --git a/multipath/main.c b/multipath/main.c
index 0d989dc..4b19d2e 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -254,7 +254,7 @@ static int check_usable_paths(struct config *conf,
if (pathvec == NULL)
return r;
- if (libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY,
+ if (libmp_mapinfo(DM_MAP_BY_DEVT | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
(mapid_t) { .devt = devt },
(mapinfo_t) {
.name = mpp->alias,
@@ -266,9 +266,6 @@ static int check_usable_paths(struct config *conf,
}) != DMP_OK)
return r;
- if (!is_mpath_uuid(uuid))
- return r;
-
strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid));
if (update_multipath_table__(mpp, pathvec, 0, params, status) != DMP_OK)
diff --git a/multipathd/main.c b/multipathd/main.c
index 13af94e..386cd07 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -720,7 +720,7 @@ static int add_map_without_path (struct vectors *vecs, const char *alias)
if (!mpp || !(mpp->alias = strdup(alias)))
return DMP_ERR;
- if ((rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY,
+ if ((rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
(mapid_t) { .str = mpp->alias },
(mapinfo_t) {
.uuid = uuid,
@@ -731,10 +731,7 @@ static int add_map_without_path (struct vectors *vecs, const char *alias)
})) != DMP_OK)
return rc;
- if (!is_mpath_uuid(uuid))
- return DMP_NO_MATCH;
- else
- strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid));
+ strlcpy(mpp->wwid, uuid + UUID_PREFIX_LEN, sizeof(mpp->wwid));
if (!strlen(mpp->wwid))
condlog(1, "%s: adding map with empty WWID", mpp->alias);
diff --git a/tests/mapinfo.c b/tests/mapinfo.c
index f3a8440..f7ad868 100644
--- a/tests/mapinfo.c
+++ b/tests/mapinfo.c
@@ -54,6 +54,16 @@ static const char MPATH_STATUS_01[] =
"A 0 3 2 65:32 A 0 0 1 67:64 A 0 0 1 69:96 A 0 0 1 "
"E 0 3 2 8:16 A 0 0 1 66:48 A 0 0 1 68:80 A 0 0 1 ";
+static const char BAD_UUID_01[] = "";
+static const char BAD_UUID_02[] = "mpath3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_03[] = " mpath-3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_04[] = "-mpath-3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_05[] = "mpth-3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_06[] = "part1-mpath-3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_07[] = "mpath 3600a098038302d414b2b4d4453474f62";
+static const char BAD_UUID_08[] = "mpath";
+static const char BAD_UUID_09[] = "mpath-";
+
char *__real_strdup(const char *str);
char *__wrap_strdup(const char *str)
{
@@ -413,6 +423,208 @@ static void test_mapinfo_good_exists(void **state)
assert_int_equal(rc, DMP_OK);
}
+static void test_mapinfo_bad_check_uuid_00(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, NULL);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_ERR);
+}
+
+static void test_mapinfo_bad_check_uuid_01(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_01);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_02(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_02);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_03(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_03);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_04(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_04);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_05(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_05);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_06(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_06);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_07(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_07);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_08(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_08);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_NO_MATCH);
+}
+
+static void test_mapinfo_bad_check_uuid_09(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, BAD_UUID_09);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_OK);
+}
+
+static void test_mapinfo_good_check_uuid_01(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_OK);
+}
+
+static void test_mapinfo_good_check_uuid_02(void **state)
+{
+ int rc;
+ char uuid[DM_UUID_LEN];
+
+ mock_mapinfo_name_1(DM_DEVICE_INFO, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .uuid = uuid });
+ assert_int_equal(rc, DMP_OK);
+}
+
+static void test_mapinfo_good_check_uuid_03(void **state)
+{
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_STATUS, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ mock_dm_get_next_target(12345, TGT_MPATH, MPATH_STATUS_01, NULL);
+ will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .name = NULL });
+ assert_int_equal(rc, DMP_OK);
+}
+
+static void test_mapinfo_good_check_uuid_04(void **state)
+{
+ char __attribute__((cleanup(cleanup_charp))) *target = NULL;
+ int rc;
+
+ mock_mapinfo_name_1(DM_DEVICE_TABLE, 1, "foo", 1, 1, 0);
+ WRAP_DM_TASK_GET_INFO(1);
+ WRAP_DM_TASK_GET_INFO(&MPATH_DMI_01);
+ mock_dm_get_next_target(12345, TGT_MPATH, MPATH_STATUS_01, NULL);
+ will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
+ will_return(__wrap_strdup, 1);
+
+ rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_MPATH_ONLY | MAPINFO_CHECK_UUID,
+ (mapid_t) { .str = "foo", },
+ (mapinfo_t) { .target = &target });
+ assert_int_equal(rc, DMP_OK);
+}
+
static void test_mapinfo_bad_set_uuid(void **state)
{
int rc;
@@ -1126,6 +1338,20 @@ static int test_mapinfo(void)
cmocka_unit_test(test_mapinfo_bad_get_info_03),
cmocka_unit_test(test_mapinfo_bad_get_info_04),
cmocka_unit_test(test_mapinfo_good_exists),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_00),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_01),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_02),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_03),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_04),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_05),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_06),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_07),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_08),
+ cmocka_unit_test(test_mapinfo_bad_check_uuid_09),
+ cmocka_unit_test(test_mapinfo_good_check_uuid_01),
+ cmocka_unit_test(test_mapinfo_good_check_uuid_02),
+ cmocka_unit_test(test_mapinfo_good_check_uuid_03),
+ cmocka_unit_test(test_mapinfo_good_check_uuid_04),
cmocka_unit_test(test_mapinfo_bad_set_uuid),
cmocka_unit_test(test_mapinfo_bad_set_dev_01),
cmocka_unit_test(test_mapinfo_bad_set_dev_02),
--
2.45.2
