[dm-devel] [PATCH v2 18/21] libmultipath (coverity): avoid sleeping in dm_mapname()

[mwilck at suse.com]

From: Martin Wilck <mwilck at suse.com>

dm_mapname is called while holding the vecs lock, and shouldn't
sleep. All callers handle a NULL return value.

This code dates back to the initial commit creating multipathd,
66b457b ("[multipathd]") from 2005. I am pretty certain that the comment
"device map might not be ready when we get here from uevent trigger"
doesn't hold any more (if it has ever been true). Perhaps the early
code acted on "add" events for maps, which would indeed not necessarily
have been set up. On the actually relevant "change" events, the map
name has to be set.

Reviewed-by: Benjamin Marzinski <bmarzins at redhat.com>
Signed-off-by: Martin Wilck <mwilck at suse.com>
---
 libmultipath/devmapper.c | 17 +----------------
 1 file changed, 1 insertion(+), 16 deletions(-)

diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index dd293aa..1dc83df 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -1389,7 +1389,6 @@ dm_mapname(int major, int minor)
 	const char *map;
 	struct dm_task *dmt;
 	int r;
-	int loop = MAX_WAIT * LOOPS_PER_SEC;
 
 	if (!(dmt = libmp_dm_task_create(DM_DEVICE_STATUS)))
 		return NULL;
@@ -1399,23 +1398,9 @@ dm_mapname(int major, int minor)
 		goto bad;
 
 	dm_task_no_open_count(dmt);
-
-	/*
-	 * device map might not be ready when we get here from
-	 * daemon uev_trigger -> uev_add_map
-	 */
-	while (--loop) {
-		r = libmp_dm_task_run(dmt);
-
-		if (r)
-			break;
-
-		usleep(1000 * 1000 / LOOPS_PER_SEC);
-	}
-
+	r = libmp_dm_task_run(dmt);
 	if (!r) {
 		dm_log_error(2, DM_DEVICE_STATUS, dmt);
-		condlog(0, "%i:%i: timeout fetching map name", major, minor);
 		goto bad;
 	}
 
-- 
2.33.1