Instead of only setting the uuid, name, and dmi on DMP_OK, return them
whenever we find a device, even if that device doesn't match. To make the
code simpler, we set these values before checks that could possibly return
DMP_ERR.
The only caller of libmp_mapinfo() that could update a multipath value
incorrectly because of this is update_multipath_table(). Fix that.
Signed-off-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx>
Reviewed-by: Martin Wilck <mwilck@xxxxxxxx>
---
libmultipath/devmapper.c | 47 +++++++++++++++++++-------------------
libmultipath/devmapper.h | 5 +++-
libmultipath/structs_vec.c | 5 +++-
tests/mapinfo.c | 45 +++++++++++++++++++++++++-----------
4 files changed, 63 insertions(+), 39 deletions(-)
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index 42176667..38c49bd5 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -736,6 +736,29 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma
&& !(uuid = dm_task_get_uuid(dmt))))
return DMP_ERR;
+ if (info.name) {
+ strlcpy(info.name, name, WWID_SIZE);
+ condlog(4, "%s: %s: name: \"%s\"", fname__, map_id, info.name);
+ }
+ if (info.uuid) {
+ strlcpy(info.uuid, uuid, DM_UUID_LEN);
+ condlog(4, "%s: %s: uuid: \"%s\"", fname__, map_id, info.uuid);
+ }
+
+ if (info.dmi) {
+ memcpy(info.dmi, &dmi, sizeof(*info.dmi));
+ condlog(4, "%s: %s %d:%d, %d targets, %s table, %s, %s, %d opened, %u events",
+ fname__, map_id,
+ info.dmi->major, info.dmi->minor,
+ info.dmi->target_count,
+ info.dmi->live_table ? "live" :
+ info.dmi->inactive_table ? "inactive" : "no",
+ info.dmi->suspended ? "suspended" : "active",
+ info.dmi->read_only ? "ro" : "rw",
+ info.dmi->open_count,
+ info.dmi->event_nr);
+ }
+
if (flags & MAPINFO_CHECK_UUID &&
((flags & MAPINFO_PART_ONLY && !is_mpath_part_uuid(uuid, NULL)) ||
(!(flags & MAPINFO_PART_ONLY) && !is_mpath_uuid(uuid)))) {
@@ -768,40 +791,16 @@ static int libmp_mapinfo__(int flags, mapid_t id, mapinfo_t info, const char *ma
/*
* Check possible error conditions.
- * If error is returned, don't touch any output parameters.
*/
if ((info.status && !(tmp_status = strdup(params)))
|| (info.target && !tmp_target && !(tmp_target = strdup(params))))
return DMP_ERR;
- if (info.name) {
- strlcpy(info.name, name, WWID_SIZE);
- condlog(4, "%s: %s: name: \"%s\"", fname__, map_id, info.name);
- }
- if (info.uuid) {
- strlcpy(info.uuid, uuid, DM_UUID_LEN);
- condlog(4, "%s: %s: uuid: \"%s\"", fname__, map_id, info.uuid);
- }
-
if (info.size) {
*info.size = length;
condlog(4, "%s: %s: size: %lld", fname__, map_id, *info.size);
}
- if (info.dmi) {
- memcpy(info.dmi, &dmi, sizeof(*info.dmi));
- condlog(4, "%s: %s %d:%d, %d targets, %s table, %s, %s, %d opened, %u events",
- fname__, map_id,
- info.dmi->major, info.dmi->minor,
- info.dmi->target_count,
- info.dmi->live_table ? "live" :
- info.dmi->inactive_table ? "inactive" : "no",
- info.dmi->suspended ? "suspended" : "active",
- info.dmi->read_only ? "ro" : "rw",
- info.dmi->open_count,
- info.dmi->event_nr);
- }
-
if (info.target) {
*info.target = steal_ptr(tmp_target);
if (!tgt_set)
diff --git a/libmultipath/devmapper.h b/libmultipath/devmapper.h
index 0bd2b907..4aeefad1 100644
--- a/libmultipath/devmapper.h
+++ b/libmultipath/devmapper.h
@@ -115,7 +115,10 @@ typedef struct libmp_map_info {
*
* This function obtains the requested information for the device-mapper map
* identified by the input parameters.
- * Output parameters are only filled in if the return value is DMP_OK.
+ * If non-NULL, the name, uuid, and dmi output paramters may be filled in for
+ * any return value besides DMP_NOT_FOUND and will always be filled in for
+ * return values other than DMP_NOT_FOUND and DMP_ERR.
+ * The other parameters are only filled in if the return value is DMP_OK.
* For target / status / size information, the map's table should contain
* only one target (usually multipath or linear).
*/
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 5df495b3..d22056ca 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -504,6 +504,8 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
int r = DMP_ERR;
char __attribute__((cleanup(cleanup_charp))) *params = NULL;
char __attribute__((cleanup(cleanup_charp))) *status = NULL;
+ /* only set the actual mpp->dmi if libmp_mapinfo returns DMP_OK */
+ struct dm_info dmi;
unsigned long long size;
struct config *conf;
@@ -522,7 +524,7 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
.target = ¶ms,
.status = &status,
.size = &mpp->size,
- .dmi = &mpp->dmi,
+ .dmi = &dmi,
});
if (r != DMP_OK) {
@@ -531,6 +533,7 @@ update_multipath_table (struct multipath *mpp, vector pathvec, int flags)
} else if (size != mpp->size)
condlog(0, "%s: size changed from %llu to %llu", mpp->alias, size, mpp->size);
+ mpp->dmi = dmi;
return update_multipath_table__(mpp, pathvec, flags, params, status);
}
diff --git a/tests/mapinfo.c b/tests/mapinfo.c
index ee487989..36511607 100644
--- a/tests/mapinfo.c
+++ b/tests/mapinfo.c
@@ -451,15 +451,21 @@ static void test_mapinfo_bad_check_uuid_00(void **state)
static void test_mapinfo_bad_check_uuid_01(void **state)
{
int rc;
+ struct dm_info dmi = { .suspended = 0 };
+ char name[WWID_SIZE] = { 0 };
+ char uuid[DM_UUID_LEN] = { 0 };
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_name, MPATH_NAME_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 });
+ (mapinfo_t) { .dmi = &dmi, .name = name, .uuid = uuid });
assert_int_equal(rc, DMP_NO_MATCH);
+ assert_true(!strcmp(name, MPATH_NAME_01));
+ assert_true(!strcmp(uuid, BAD_UUID_01));
}
static void test_mapinfo_bad_check_uuid_02(void **state)
@@ -887,17 +893,26 @@ static void test_mapinfo_bad_next_target_01(void **state)
static void test_mapinfo_bad_next_target_02(void **state)
{
int rc;
- unsigned long long size;
+ struct dm_info dmi = { .suspended = 0 };
+ char name[WWID_SIZE] = { 0 };
+ char uuid[DM_UUID_LEN] = { 0 };
+ unsigned long long size = 0;
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_02);
+ will_return(__wrap_dm_task_get_name, MPATH_NAME_01);
+ will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
/* no targets */
mock_dm_get_next_target(0, NULL, NULL, NULL);
rc = libmp_mapinfo(DM_MAP_BY_NAME,
(mapid_t) { .str = "foo", },
- (mapinfo_t) { .size = &size });
+ (mapinfo_t) { .dmi = &dmi, .name = name, .uuid = uuid, .size = &size });
assert_int_equal(rc, DMP_EMPTY);
+ assert_int_equal(size, 0);
+ assert_memory_equal(&dmi, &MPATH_DMI_02, sizeof(dmi));
+ assert_true(!strcmp(name, MPATH_NAME_01));
+ assert_true(!strcmp(uuid, MPATH_UUID_01));
}
/* libmp_mapinfo needs to do a DM_DEVICE_STATUS ioctl */
@@ -946,10 +961,10 @@ static void test_mapinfo_bad_target_type_03(void **state)
(mapid_t) { .str = "foo", },
(mapinfo_t) { .dmi = &dmi, .name = name, .uuid = uuid });
assert_int_equal(rc, DMP_NO_MATCH);
- /* make sure memory content is not changed */
- assert_memory_equal(&dmi, &((struct dm_info) { .exists = 0 }), sizeof(dmi));
- assert_memory_equal(&name, &((char[WWID_SIZE]) { 0 }), WWID_SIZE);
- assert_memory_equal(&uuid, &((char[DM_UUID_LEN]) { 0 }), DM_UUID_LEN);
+ /* make sure that the ouput was filled in */
+ assert_memory_equal(&dmi, &MPATH_DMI_01, sizeof(dmi));
+ assert_true(!strcmp(name, MPATH_NAME_01));
+ assert_true(!strcmp(uuid, MPATH_UUID_01));
}
static void test_mapinfo_bad_target_type_04(void **state)
@@ -1065,23 +1080,32 @@ static void test_mapinfo_no_table_01(void **state)
(mapid_t) { .str = "foo", },
(mapinfo_t) { .dmi = &dmi });
assert_int_equal(rc, DMP_EMPTY);
+ assert_memory_equal(&dmi, &MPATH_DMI_02, sizeof(dmi));
}
static void test_mapinfo_no_table_02(void **state)
{
int rc;
struct dm_info dmi = { .suspended = 0 };
+ char name[WWID_SIZE] = { 0 };
+ char uuid[DM_UUID_LEN] = { 0 };
+ unsigned long long size = 0;
mock_mapinfo_name_1(DM_DEVICE_STATUS, 1, "foo", 1, 1, 0);
WRAP_DM_TASK_GET_INFO(1);
/* DMI with no live table, MAPINFO_CHECK_UUID set */
WRAP_DM_TASK_GET_INFO(&MPATH_DMI_02);
+ will_return(__wrap_dm_task_get_name, MPATH_NAME_01);
mock_dm_get_next_target(0, NULL, NULL, NULL);
will_return(__wrap_dm_task_get_uuid, MPATH_UUID_01);
rc = libmp_mapinfo(DM_MAP_BY_NAME | MAPINFO_CHECK_UUID | MAPINFO_MPATH_ONLY,
(mapid_t) { .str = "foo", },
- (mapinfo_t) { .dmi = &dmi });
+ (mapinfo_t) { .dmi = &dmi, .name = name, .uuid = uuid, .size = &size });
assert_int_equal(rc, DMP_EMPTY);
+ assert_int_equal(size, 0);
+ assert_memory_equal(&dmi, &MPATH_DMI_02, sizeof(dmi));
+ assert_true(!strcmp(name, MPATH_NAME_01));
+ assert_true(!strcmp(uuid, MPATH_UUID_01));
}
static void test_mapinfo_good_status_01(void **state)
@@ -1121,8 +1145,6 @@ static void test_mapinfo_bad_strdup_01(void **state)
(mapinfo_t) { .status = &status, .uuid = uuid, .name = name });
assert_int_equal(rc, DMP_ERR);
assert_null(status);
- assert_memory_equal(&name, &((char[WWID_SIZE]) { 0 }), WWID_SIZE);
- assert_memory_equal(&uuid, &((char[DM_UUID_LEN]) { 0 }), DM_UUID_LEN);
}
@@ -1284,9 +1306,6 @@ static void test_mapinfo_bad_strdup_02(void **state)
assert_int_equal(rc, DMP_ERR);
assert_null(status);
assert_null(target);
- assert_memory_equal(&dmi, &((struct dm_info) { .suspended = 0 }), sizeof(dmi));
- assert_memory_equal(&name, &((char[WWID_SIZE]) { 0 }), WWID_SIZE);
- assert_memory_equal(&uuid, &((char[DM_UUID_LEN]) { 0 }), DM_UUID_LEN);
}
static void test_mapinfo_bad_strdup_03(void **state)
--
2.46.2
