multipath-tools/multipath-tools-make-params-local

From 28b790ea42f100c74f682f1abea6c33e7d0e3553 Mon Sep 17 00:00:00 2001
From: Hannes Reinecke <hare@suse.de>
Date: Thu, 20 Nov 2008 12:36:24 +0100
Subject: [PATCH] Make 'params' and 'status' local variables

The 'params' and 'status' fields in the multipath structure are
just scratch variables where the output from device-mapper is stored
into. And as we call device-mapper quite frequently it really looks
not thread-safe as the multipath structure can be accessed from
all threads. So better make them local variables to eliminate this
problem.

Signed-off-by: Hannes Reinecke <hare@suse.de>
---
 libmultipath/configure.c   |   30 ++++++++++++++++--------------
 libmultipath/configure.h   |    4 ++--
 libmultipath/devmapper.c   |   12 +++++++-----
 libmultipath/dmparser.c    |   19 +++++++++++--------
 libmultipath/dmparser.h    |    2 +-
 libmultipath/print.c       |    6 +++---
 libmultipath/print.h       |    2 +-
 libmultipath/structs.h     |    2 --
 libmultipath/structs_vec.c |   20 ++++++++++++++++----
 libmultipath/waiter.c      |    1 +
 multipath/main.c           |   13 +++++++++----
 multipathd/cli_handlers.c  |    6 ++++--
 multipathd/main.c          |   15 +++++++++++----
 13 files changed, 82 insertions(+), 50 deletions(-)

diff --git a/libmultipath/configure.c b/libmultipath/configure.c
index 494ea70..eb7ac03 100644
--- a/libmultipath/configure.c
+++ b/libmultipath/configure.c
@@ -37,7 +37,7 @@
 #include "util.h"
 
 extern int
-setup_map (struct multipath * mpp)
+setup_map (struct multipath * mpp, char * params, int params_size)
 {
 	struct pathgroup * pgp;
 	int i;
@@ -89,7 +89,7 @@ setup_map (struct multipath * mpp)
 	 * transform the mp->pg vector of vectors of paths
 	 * into a mp->params strings to feed the device-mapper
 	 */
-	if (assemble_map(mpp)) {
+	if (assemble_map(mpp, params, params_size)) {
 		condlog(0, "%s: problem assembing map", mpp->alias);
 		return 1;
 	}
@@ -298,7 +298,7 @@ lock_multipath (struct multipath * mpp, int lock)
 #define DOMAP_DRY	3
 
 extern int
-domap (struct multipath * mpp)
+domap (struct multipath * mpp, char * params)
 {
 	int r = 0;
 
@@ -337,25 +337,25 @@ domap (struct multipath * mpp)
 			break;
 		}
 
-		r = dm_addmap_create(mpp->alias, mpp->params, mpp->size,
+		r = dm_addmap_create(mpp->alias, params, mpp->size,
 				     mpp->wwid);
 
 		if (!r)
-			 r = dm_addmap_create_ro(mpp->alias, mpp->params,
+			 r = dm_addmap_create_ro(mpp->alias, params,
 						 mpp->size, mpp->wwid);
 
 		lock_multipath(mpp, 0);
 		break;
 
 	case ACT_RELOAD:
-		r = (dm_addmap_reload(mpp->alias, mpp->params, mpp->size, NULL)
+		r = (dm_addmap_reload(mpp->alias, params, mpp->size, NULL)
 		     && dm_simplecmd(DM_DEVICE_RESUME, mpp->alias, 1));
 		break;
 
 	case ACT_RESIZE:
-		r = dm_addmap_reload(mpp->alias, mpp->params, mpp->size, NULL);
+		r = dm_addmap_reload(mpp->alias, params, mpp->size, NULL);
 		if (!r)
-			r = dm_addmap_reload_ro(mpp->alias, mpp->params,
+			r = dm_addmap_reload_ro(mpp->alias, params,
 						mpp->size, NULL);
 		if (r)
 			r = dm_simplecmd(DM_DEVICE_RESUME, mpp->alias, 0);
@@ -383,7 +383,7 @@ domap (struct multipath * mpp)
 			/* multipath daemon mode */
 			mpp->stat_map_loads++;
 			condlog(2, "%s: load table [0 %llu %s %s]", mpp->alias,
-				mpp->size, TGT_MPATH, mpp->params);
+				mpp->size, TGT_MPATH, params);
 			/*
 			 * Required action is over, reset for the stateful daemon
 			 */
@@ -422,6 +422,7 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 	int r = 1;
 	int k, i;
 	char empty_buff[WWID_SIZE];
+	char params[PARAMS_SIZE];
 	struct multipath * mpp;
 	struct path * pp1;
 	struct path * pp2;
@@ -493,8 +494,9 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 				mpp->action = ACT_REJECT;
 		}
 		verify_paths(mpp, vecs, NULL);
-		
-		if (setup_map(mpp)) {
+
+		params[0] = '\0';
+		if (setup_map(mpp, params, PARAMS_SIZE)) {
 			remove_map(mpp, vecs, 0);
 			continue;
 		}
@@ -502,7 +504,7 @@ coalesce_paths (struct vectors * vecs, vector newmp, char * refwwid, int force_r
 		if (mpp->action == ACT_UNDEF)
 			select_action(mpp, curmp, force_reload);
 
-		r = domap(mpp);
+		r = domap(mpp, params);
 
 		if (r == DOMAP_FAIL || r == DOMAP_RETRY) {
 			condlog(3, "%s: domap (%u) failure "
@@ -610,7 +612,7 @@ get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
 
 	if (dev_type == DEV_DEVT) {
 		pp = find_path_by_devt(pathvec, dev);
-		
+
 		if (!pp) {
 			if (devt2devname(buff, dev))
 				return NULL;
@@ -624,7 +626,7 @@ get_refwwid (char * dev, enum devtypes dev_type, vector pathvec)
 
 			if (pathinfo(pp, conf->hwtable, DI_SYSFS | DI_WWID))
 				return NULL;
-			
+
 			if (store_path(pathvec, pp)) {
 				free_path(pp);
 				return NULL;
diff --git a/libmultipath/configure.h b/libmultipath/configure.h
index 25891ba..ec2800d 100644
--- a/libmultipath/configure.h
+++ b/libmultipath/configure.h
@@ -23,8 +23,8 @@ enum actions {
 #define FLUSH_ONE 1
 #define FLUSH_ALL 2
 
-int setup_map (struct multipath * mpp);
-int domap (struct multipath * mpp);
+int setup_map (struct multipath * mpp, char * params, int params_size);
+int domap (struct multipath * mpp, char * params);
 int reinstate_paths (struct multipath *mpp);
 int coalesce_paths (struct vectors *vecs, vector curmp, char * refwwid, int force_reload);
 char * get_refwwid (char * dev, enum devtypes dev_type, vector pathvec);
diff --git a/libmultipath/devmapper.c b/libmultipath/devmapper.c
index cde98eb..a7ab41f 100644
--- a/libmultipath/devmapper.c
+++ b/libmultipath/devmapper.c
@@ -206,6 +206,8 @@ dm_addmap (int task, const char *name, const char *target,
 			goto freeout;
 	}
 
+	condlog(4, "%s: addmap [0 %llu %s %s]\n", name, size, target, params);
+
 	dm_task_no_open_count(dmt);
 
 	r = dm_task_run (dmt);
@@ -323,7 +325,10 @@ dm_get_map(char * name, unsigned long long * size, char * outparams)
 	if (size)
 		*size = length;
 
-	if (snprintf(outparams, PARAMS_SIZE, "%s", params) <= PARAMS_SIZE)
+	if (outparams) {
+		if (snprintf(outparams, PARAMS_SIZE, "%s", params) <= PARAMS_SIZE)
+			r = 0;
+	} else
 		r = 0;
 out:
 	dm_task_destroy(dmt);
@@ -756,10 +761,7 @@ dm_get_maps (vector mp)
 			goto out1;
 
 		if (info > 0) {
-			if (dm_get_map(names->name, &mpp->size, mpp->params))
-				goto out1;
-
-			if (dm_get_status(names->name, mpp->status))
+			if (dm_get_map(names->name, &mpp->size, NULL))
 				goto out1;
 
 			dm_get_uuid(names->name, mpp->wwid);
diff --git a/libmultipath/dmparser.c b/libmultipath/dmparser.c
index d1face5..9441b11 100644
--- a/libmultipath/dmparser.c
+++ b/libmultipath/dmparser.c
@@ -47,7 +47,7 @@ merge_words (char ** dst, char * word, int space)
  * Transforms the path group vector into a proper device map string
  */
 int
-assemble_map (struct multipath * mp)
+assemble_map (struct multipath * mp, char * params, int len)
 {
 	int i, j;
 	int shift, freechar;
@@ -57,15 +57,15 @@ assemble_map (struct multipath * mp)
 	struct path * pp;
 
 	minio = mp->minio;
-	p = mp->params;
-	freechar = sizeof(mp->params);
+	p = params;
+	freechar = len;
 
 	shift = snprintf(p, freechar, "%s %s %i %i",
 			 mp->features, mp->hwhandler,
 			 VECTOR_SIZE(mp->pg), mp->bestpg);
 
 	if (shift >= freechar) {
-		fprintf(stderr, "mp->params too small\n");
+		condlog(0, "%s: params too small\n", mp->alias);
 		return 1;
 	}
 	p += shift;
@@ -76,7 +76,7 @@ assemble_map (struct multipath * mp)
 		shift = snprintf(p, freechar, " %s %i 1", mp->selector,
 				 VECTOR_SIZE(pgp->paths));
 		if (shift >= freechar) {
-			fprintf(stderr, "mp->params too small\n");
+			condlog(0, "%s: params too small\n", mp->alias);
 			return 1;
 		}
 		p += shift;
@@ -88,11 +88,14 @@ assemble_map (struct multipath * mp)
 			if (mp->rr_weight == RR_WEIGHT_PRIO
 			    && pp->priority > 0)
 				tmp_minio = minio * pp->priority;
-
+			if (!strlen(pp->dev_t) ) {
+				condlog(0, "dev_t not set for '%s'\n", pp->dev);
+				return 1;
+			}
 			shift = snprintf(p, freechar, " %s %d",
 					 pp->dev_t, tmp_minio);
 			if (shift >= freechar) {
-				fprintf(stderr, "mp->params too small\n");
+				condlog(0, "%s: params too small\n", mp->alias);
 				return 1;
 			}
 			p += shift;
@@ -100,7 +103,7 @@ assemble_map (struct multipath * mp)
 		}
 	}
 	if (freechar < 1) {
-		fprintf(stderr, "mp->params too small\n");
+		condlog(0, "%s: params too small\n", mp->alias);
 		return 1;
 	}
 	snprintf(p, 1, "\n");
diff --git a/libmultipath/dmparser.h b/libmultipath/dmparser.h
index bf4b2c3..1b45df0 100644
--- a/libmultipath/dmparser.h
+++ b/libmultipath/dmparser.h
@@ -1,3 +1,3 @@
-int assemble_map (struct multipath *);
+int assemble_map (struct multipath *, char *, int);
 int disassemble_map (vector, char *, struct multipath *);
 int disassemble_status (char *, struct multipath *);
diff --git a/libmultipath/print.c b/libmultipath/print.c
index 7467411..9dea392 100644
--- a/libmultipath/print.c
+++ b/libmultipath/print.c
@@ -1276,11 +1276,11 @@ print_pathgroup (struct pathgroup * pgp, char * style)
 }
 
 extern void
-print_map (struct multipath * mpp)
+print_map (struct multipath * mpp, char * params)
 {
-	if (mpp->size && mpp->params)
+	if (mpp->size && params)
 		printf("0 %llu %s %s\n",
-			 mpp->size, TGT_MPATH, mpp->params);
+			 mpp->size, TGT_MPATH, params);
 	return;
 }
 
diff --git a/libmultipath/print.h b/libmultipath/print.h
index a8be408..5fe826b 100644
--- a/libmultipath/print.h
+++ b/libmultipath/print.h
@@ -54,7 +54,7 @@ void print_multipath_topology (struct multipath * mpp, int verbosity);
 void print_path (struct path * pp, char * style);
 void print_multipath (struct multipath * mpp, char * style);
 void print_pathgroup (struct pathgroup * pgp, char * style);
-void print_map (struct multipath * mpp);
+void print_map (struct multipath * mpp, char * params);
 void print_all_paths (vector pathvec, int banner);
 void print_all_paths_custo (vector pathvec, int banner, char *fmt);
 void print_hwtable (vector hwtable);
diff --git a/libmultipath/structs.h b/libmultipath/structs.h
index 85d5109..658d6b2 100644
--- a/libmultipath/structs.h
+++ b/libmultipath/structs.h
@@ -151,8 +151,6 @@ struct multipath {
 	unsigned long long size;
 	vector paths;
 	vector pg;
-	char params[PARAMS_SIZE];
-	char status[PARAMS_SIZE];
 	struct dm_info * dmi;
 
 	/* configlet pointers */
diff --git a/libmultipath/structs_vec.c b/libmultipath/structs_vec.c
index 17cafd1..e3cace9 100644
--- a/libmultipath/structs_vec.c
+++ b/libmultipath/structs_vec.c
@@ -237,14 +237,20 @@ extract_hwe_from_path(struct multipath * mpp)
 static int
 update_multipath_table (struct multipath *mpp, vector pathvec)
 {
+	char params[PARAMS_SIZE] = {0};
+
 	if (!mpp)
 		return 1;
 
-	if (dm_get_map(mpp->alias, &mpp->size, mpp->params))
+	if (dm_get_map(mpp->alias, &mpp->size, params)) {
+		condlog(3, "%s: cannot get map", mpp->alias);
 		return 1;
+	}
 
-	if (disassemble_map(pathvec, mpp->params, mpp))
+	if (disassemble_map(pathvec, params, mpp)) {
+		condlog(3, "%s: cannot disassemble map", mpp->alias);
 		return 1;
+	}
 
 	return 0;
 }
@@ -252,14 +258,20 @@ update_multipath_table (struct multipath *mpp, vector pathvec)
 static int
 update_multipath_status (struct multipath *mpp)
 {
+	char status[PARAMS_SIZE] = {0};
+
 	if (!mpp)
 		return 1;
 
-	if(dm_get_status(mpp->alias, mpp->status))
+	if (dm_get_status(mpp->alias, status)) {
+		condlog(3, "%s: cannot get status", mpp->alias);
 		return 1;
+	}
 
-	if (disassemble_status(mpp->status, mpp))
+	if (disassemble_status(status, mpp)) {
+		condlog(3, "%s: cannot disassemble status", mpp->alias);
 		return 1;
+	}
 
 	return 0;
 }
diff --git a/libmultipath/waiter.c b/libmultipath/waiter.c
index 54cd19f..530180f 100644
--- a/libmultipath/waiter.c
+++ b/libmultipath/waiter.c
@@ -26,6 +26,7 @@ struct event_thread *alloc_waiter (void)
 	struct event_thread *wp;
 
 	wp = (struct event_thread *)MALLOC(sizeof(struct event_thread));
+	memset(wp, 0, sizeof(struct event_thread));
 
 	return wp;
 }
diff --git a/multipath/main.c b/multipath/main.c
index 60244a5..ffa6eb5 100644
--- a/multipath/main.c
+++ b/multipath/main.c
@@ -158,6 +158,7 @@ get_dm_mpvec (vector curmp, vector pathvec, char * refwwid)
 {
 	int i;
 	struct multipath * mpp;
+	char params[PARAMS_SIZE], status[PARAMS_SIZE];
 
 	if (dm_get_maps(curmp))
 		return 1;
@@ -175,10 +176,12 @@ get_dm_mpvec (vector curmp, vector pathvec, char * refwwid)
 			continue;
 		}
 
-		condlog(3, "params = %s", mpp->params);
-		condlog(3, "status = %s", mpp->status);
+		dm_get_map(mpp->alias, &mpp->size, params);
+		condlog(3, "params = %s", params);
+		dm_get_status(mpp->alias, status);
+		condlog(3, "status = %s", status);
 
-		disassemble_map(pathvec, mpp->params, mpp);
+		disassemble_map(pathvec, params, mpp);
 
 		/*
 		 * disassemble_map() can add new paths to pathvec.
@@ -191,7 +194,7 @@ get_dm_mpvec (vector curmp, vector pathvec, char * refwwid)
 		if (conf->list > 1)
 			mpp->bestpg = select_path_group(mpp);
 
-		disassemble_status(mpp->status, mpp);
+		disassemble_status(status, mpp);
 
 		if (conf->list)
 			print_multipath_topology(mpp, conf->verbosity);
@@ -495,6 +498,8 @@ out:
 	sysfs_cleanup();
 	dm_lib_release();
 	dm_lib_exit();
+	cleanup_prio();
+	cleanup_checkers();
 
 	/*
 	 * Freeing config must be done after dm_lib_exit(), because
diff --git a/multipathd/cli_handlers.c b/multipathd/cli_handlers.c
index f9b0da4..4f639b6 100644
--- a/multipathd/cli_handlers.c
+++ b/multipathd/cli_handlers.c
@@ -463,11 +463,13 @@ out:
 int resize_map(struct multipath *mpp, unsigned long long size,
 	       struct vectors * vecs)
 {
+	char params[PARAMS_SIZE] = {0};
+
 	mpp->size = size;
 	update_mpp_paths(mpp, vecs->pathvec);
-	setup_map(mpp);
+	setup_map(mpp, params, PARAMS_SIZE);
 	mpp->action = ACT_RESIZE;
-	if (domap(mpp) <= 0) {
+	if (domap(mpp, params) <= 0) {
 		condlog(0, "%s: failed to resize map : %s", mpp->alias,
 			strerror(errno));
 		return 1;
diff --git a/multipathd/main.c b/multipathd/main.c
index bb396ec..444494e 100644
--- a/multipathd/main.c
+++ b/multipathd/main.c
@@ -158,6 +158,8 @@ sync_map_state(struct multipath *mpp)
 	if (!mpp->pg)
 		return;
 
+	condlog(5, "%s: sync map state", mpp->alias);
+
 	vector_foreach_slot (mpp->pg, pgp, i){
 		vector_foreach_slot (pgp->paths, pp, j){
 			if (pp->state == PATH_UNCHECKED || 
@@ -355,6 +357,7 @@ ev_add_path (char * devname, struct vectors * vecs)
 	struct multipath * mpp;
 	struct path * pp;
 	char empty_buff[WWID_SIZE] = {0};
+	char params[PARAMS_SIZE] = {0};
 
 	pp = find_path_by_dev(vecs->pathvec, devname);
 
@@ -409,7 +412,7 @@ rescan:
 	/*
 	 * push the map to the device-mapper
 	 */
-	if (setup_map(mpp)) {
+	if (setup_map(mpp, params, PARAMS_SIZE)) {
 		condlog(0, "%s: failed to setup map for addition of new "
 			"path %s", mpp->alias, devname);
 		goto out;
@@ -417,7 +420,7 @@ rescan:
 	/*
 	 * reload the map for the multipath mapped device
 	 */
-	if (domap(mpp) <= 0) {
+	if (domap(mpp, params) <= 0) {
 		condlog(0, "%s: failed in domap for addition of new "
 			"path %s", mpp->alias, devname);
 		/*
@@ -480,6 +483,7 @@ ev_remove_path (char * devname, struct vectors * vecs)
 	struct multipath * mpp;
 	struct path * pp;
 	int i, retval = 0;
+	char params[PARAMS_SIZE] = {0};
 
 	pp = find_path_by_dev(vecs->pathvec, devname);
 
@@ -526,7 +530,7 @@ ev_remove_path (char * devname, struct vectors * vecs)
 			 */
 		}
 
-		if (setup_map(mpp)) {
+		if (setup_map(mpp, params, PARAMS_SIZE)) {
 			condlog(0, "%s: failed to setup map for"
 				" removal of path %s", mpp->alias,
 				devname);
@@ -536,7 +540,7 @@ ev_remove_path (char * devname, struct vectors * vecs)
 		 * reload the map
 		 */
 		mpp->action = ACT_RELOAD;
-		if (domap(mpp) <= 0) {
+		if (domap(mpp, params) <= 0) {
 			condlog(0, "%s: failed in domap for "
 				"removal of path %s",
 				mpp->alias, devname);
@@ -1395,6 +1399,9 @@ child (void * param)
 	cleanup_checkers();
 	cleanup_prio();
 
+	cleanup_checkers();
+	cleanup_prio();
+
 	condlog(2, "--------shut down-------");
 
 	if (logsink)
-- 
1.5.3.2