From 8d6e0c95621d9650e27c0821d63ce46d78cdb42a Mon Sep 17 00:00:00 2001
From: Milan Broz <mbroz@redhat.com>
Date: Fri, 4 Jun 2010 12:59:30 +0000
Subject: [PATCH] Fix restart of clvmd using -S switch

 - allocate environment dynamically (still missing some limit?)
 - try to recover, if destroy failed (do not destroy lvm here) and free memory
 - check strdup() return codes
 - report failure to log
 - do not print NULL in exclusive lock loop
---
 WHATS_NEW                     |  1 +
 daemons/clvmd/clvmd-command.c | 51 ++++++++++++++++++++++++-----------
 daemons/clvmd/lvm-functions.c |  4 ++-
 3 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index e767e0eb1..593d56226 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.67 -
 ===============================
+  Handle failed restart of clvmd using -S switch properly.
   Fix clvmd initscript restart command to start clvmd if not yet running.
   Use absolute paths for clvmd restart.
   Require partial option in lvchange --refresh for partial LVs.
diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index 180f8091c..83d03ef72 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -361,33 +361,51 @@ void cmd_client_cleanup(struct local_client *client)
 
 static int restart_clvmd(void)
 {
-	char *argv[1024];
-	int argc = 1;
+	char **argv = NULL;
+	char *debug_arg = NULL, *lv_name;
+	int i, argc = 0, max_locks = 0;
 	struct dm_hash_node *hn = NULL;
-	char *lv_name;
 
 	DEBUGLOG("clvmd restart requested\n");
 
+	/* Count exclusively-open LVs */
+	hn = NULL;
+	do {
+		hn = get_next_excl_lock(hn, &lv_name);
+		if (lv_name)
+			max_locks++;
+	} while (hn && *lv_name);
+
+	/* clvmd + locks (-E uuid) + debug (-d X) + NULL */
+	argv = malloc((max_locks * 2 + 4) * sizeof(*argv));
+	if (!argv)
+		goto_out;
+
 	/*
 	 * Build the command-line
 	 */
-	/* FIXME missing strdup error checks */
-	argv[0] = strdup("clvmd");
+	argv[argc++] = strdup("clvmd");
+	if (!argv[0])
+		goto_out;
 
 	/* Propogate debug options */
 	if (debug) {
-		char debug_level[16];
-
-		sprintf(debug_level, "-d%d", debug);
-		argv[argc++] = strdup(debug_level);
+		if (!(debug_arg = malloc(16)) ||
+		    snprintf(debug_arg, 16, "-d%d", (int)debug) < 0)
+			goto_out;
+		argv[argc++] = debug_arg;
 	}
 
 	/* Now add the exclusively-open LVs */
 	do {
 		hn = get_next_excl_lock(hn, &lv_name);
 		if (lv_name) {
-			argv[argc++] = strdup("-E");
-			argv[argc++] = strdup(lv_name);
+			argv[argc] = strdup("-E");
+			if (!argv[argc++])
+				goto_out;
+			argv[argc] = strdup(lv_name);
+			if (!argv[argc++])
+				goto_out;
 
 			DEBUGLOG("excl lock: %s\n", lv_name);
 			hn = get_next_excl_lock(hn, &lv_name);
@@ -395,13 +413,16 @@ static int restart_clvmd(void)
 	} while (hn && *lv_name);
 	argv[argc++] = NULL;
 
-	/* Tidy up */
-	destroy_lvm();
-
 	/* Exec new clvmd */
 	/* NOTE: This will fail when downgrading! */
 	execve(CLVMD_PATH, argv, NULL);
-
+out:
 	/* We failed */
+	DEBUGLOG("Restart of clvmd failed.\n");
+
+	for (i = 0; i < argc && argv[i]; i++)
+		free(argv[i]);
+	free(argv);
+
 	return 0;
 }
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index 3a0b3c183..5877a31ac 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -898,7 +898,9 @@ struct dm_hash_node *get_next_excl_lock(struct dm_hash_node *v, char **name)
 			v = dm_hash_get_next(lv_hash, v);
 		}
 	} while (v && !*name);
-	DEBUGLOG("returning EXclusive UUID %s\n", *name);
+
+	if (*name)
+		DEBUGLOG("returning EXclusive UUID %s\n", *name);
 	return v;
 }