clvmd: fix test mode race

When TEST_MODE flag is passed around the cluster,
it's been use in thread unprotected way, so it may have
influenced behaviour of other running parallel lvm commands
(activation/deactivation/suspend/resume).

Fix it by set/query function only under lvm mutex.
For hold_un/lock function calls check lock_flags bits directly.

4 files changed, 19 insertions, 28 deletions

diff --git a/WHATS_NEW b/WHATS_NEW
index 4a35b7e41..9c42344f8 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,6 @@
 Version 2.02.107 - 
 ==================================
+  Fix usage of --test option in clvmd.
   Skip more libraries to be mlocked in memory.
   Remove LOCKED flag for pvmove replaced with error target.
   Return invalid command when specifying negative polling interval.
diff --git a/daemons/clvmd/clvmd-command.c b/daemons/clvmd/clvmd-command.c
index 611f6bfcc..9e59e51e0 100644
--- a/daemons/clvmd/clvmd-command.c
+++ b/daemons/clvmd/clvmd-command.c
@@ -78,9 +78,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
 	unsigned char lock_cmd;
 	unsigned char lock_flags;
 
-	/* Reset test mode before we start */
-	init_test(0);
-
 	/* Do the command */
 	switch (msg->cmd) {
 		/* Just a test message */
@@ -112,8 +109,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
 		lockname = &args[2];
 		/* Check to see if the VG is in use by LVM1 */
 		status = do_check_lvm1(lockname);
-		if (lock_flags & LCK_TEST_MODE)
-			init_test(1);
 		do_lock_vg(lock_cmd, lock_flags, lockname);
 		break;
 
@@ -122,8 +117,6 @@ int do_command(struct local_client *client, struct clvm_header *msg, int msglen,
 		lock_cmd = args[0];
 		lock_flags = args[1];
 		lockname = &args[2];
-		if (lock_flags & LCK_TEST_MODE)
-			init_test(1);
 		status = do_lock_lv(lock_cmd, lock_flags, lockname);
 		/* Replace EIO with something less scary */
 		if (status == EIO) {
@@ -252,7 +245,6 @@ int do_pre_command(struct local_client *client)
 	int status = 0;
 	char *lockname;
 
-	init_test(0);
 	switch (header->cmd) {
 	case CLVMD_CMD_TEST:
 		status = sync_lock("CLVMD_TEST", LCK_EXCL, 0, &lockid);
@@ -271,8 +263,6 @@ int do_pre_command(struct local_client *client)
 		lock_cmd = args[0];
 		lock_flags = args[1];
 		lockname = &args[2];
-		if (lock_flags & LCK_TEST_MODE)
-			init_test(1);
 		status = pre_lock_lv(lock_cmd, lock_flags, lockname);
 		break;
 
@@ -304,7 +294,6 @@ int do_post_command(struct local_client *client)
 	char *args = header->node + strlen(header->node) + 1;
 	char *lockname;
 
-	init_test(0);
 	switch (header->cmd) {
 	case CLVMD_CMD_TEST:
 		status = sync_unlock("CLVMD_TEST", (int) (long) client->bits.localsock.private);
@@ -315,8 +304,6 @@ int do_post_command(struct local_client *client)
 		lock_cmd = args[0];
 		lock_flags = args[1];
 		lockname = &args[2];
-		if (lock_flags & LCK_TEST_MODE)
-			init_test(1);
 		status = post_lock_lv(lock_cmd, lock_flags, lockname);
 		break;
 
diff --git a/daemons/clvmd/clvmd.c b/daemons/clvmd/clvmd.c
index bc98ccb4c..0ad0c3ab0 100644
--- a/daemons/clvmd/clvmd.c
+++ b/daemons/clvmd/clvmd.c
@@ -1762,7 +1762,6 @@ static int process_local_command(struct clvm_header *msg, int msglen,
 	if (msg->flags & CLVMD_FLAG_REMOTE)
 		status = 0;
 	else
-		/* FIXME: usage of init_test() is unprotected */
 		status = do_command(client, msg, msglen, &replybuf, buflen, &replylen);
 
 	if (status)
diff --git a/daemons/clvmd/lvm-functions.c b/daemons/clvmd/lvm-functions.c
index f4fb7fd74..2ecf7d73e 100644
--- a/daemons/clvmd/lvm-functions.c
+++ b/daemons/clvmd/lvm-functions.c
@@ -251,9 +251,6 @@ static int hold_lock(char *resource, int mode, int flags)
 	int saved_errno;
 	struct lv_info *lvi;
 
-	if (test_mode())
-		return 0;
-
 	/* Mask off invalid options */
 	flags &= LCKF_NOQUEUE | LCKF_CONVERT;
 
@@ -319,9 +316,6 @@ static int hold_unlock(char *resource)
 	int status;
 	int saved_errno;
 
-	if (test_mode())
-		return 0;
-
 	if (!(lvi = lookup_info(resource))) {
 		DEBUGLOG("hold_unlock, lock not already held\n");
 		return 0;
@@ -381,7 +375,7 @@ static int do_activate_lv(char *resource, unsigned char command, unsigned char l
 	 * Use lock conversion only if requested, to prevent implicit conversion
 	 * of exclusive lock to shared one during activation.
 	 */
-	if (command & LCK_CLUSTER_VG) {
+	if (!test_mode() && command & LCK_CLUSTER_VG) {
 		status = hold_lock(resource, mode, LCKF_NOQUEUE | (lock_flags & LCK_CONVERT ? LCKF_CONVERT:0));
 		if (status) {
 			/* Return an LVM-sensible error for this.
@@ -415,7 +409,7 @@ static int do_activate_lv(char *resource, unsigned char command, unsigned char l
 	return 0;
 
 error:
-	if (oldmode == -1 || oldmode != mode)
+	if (!test_mode() && (oldmode == -1 || oldmode != mode))
 		(void)hold_unlock(resource);
 	return EIO;
 }
@@ -479,7 +473,7 @@ static int do_deactivate_lv(char *resource, unsigned char command, unsigned char
 	if (!lv_deactivate(cmd, resource, NULL))
 		return EIO;
 
-	if (command & LCK_CLUSTER_VG) {
+	if (!test_mode() && command & LCK_CLUSTER_VG) {
 		status = hold_unlock(resource);
 		if (status)
 			return errno;
@@ -526,6 +520,8 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 	}
 
 	pthread_mutex_lock(&lvm_lock);
+	init_test((lock_flags & LCK_TEST_MODE) ? 1 : 0);
+
 	if (lock_flags & LCK_MIRROR_NOSYNC_MODE)
 		init_mirror_in_sync(1);
 
@@ -578,6 +574,7 @@ int do_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 
 	/* clean the pool for another command */
 	dm_pool_empty(cmd->mem);
+	init_test(0);
 	pthread_mutex_unlock(&lvm_lock);
 
 	DEBUGLOG("Command return is %d, critical_section is %d\n", status, critical_section());
@@ -597,7 +594,8 @@ int pre_lock_lv(unsigned char command, unsigned char lock_flags, char *resource)
 		DEBUGLOG("pre_lock_lv: resource '%s', cmd = %s, flags = %s\n",
 			 resource, decode_locking_cmd(command), decode_flags(lock_flags));
 
-		if (hold_lock(resource, LCK_WRITE, LCKF_NOQUEUE | LCKF_CONVERT))
+		if (!(lock_flags & LCK_TEST_MODE) &&
+		    hold_lock(resource, LCK_WRITE, LCKF_NOQUEUE | LCKF_CONVERT))
 			return errno;
 	}
 	return 0;
@@ -629,11 +627,13 @@ int post_lock_lv(unsigned char command, unsigned char lock_flags,
 			if (!status)
 				return EIO;
 
-			if (lvi.exists) {
-				if (hold_lock(resource, LCK_READ, LCKF_CONVERT))
+			if (!(lock_flags & LCK_TEST_MODE)) {
+				if (lvi.exists) {
+					if (hold_lock(resource, LCK_READ, LCKF_CONVERT))
+						return errno;
+				} else if (hold_unlock(resource))
 					return errno;
-			} else if (hold_unlock(resource))
-				return errno;
+			}
 		}
 	}
 	return 0;
@@ -697,6 +697,8 @@ void do_lock_vg(unsigned char command, unsigned char lock_flags, char *resource)
 	}
 
 	pthread_mutex_lock(&lvm_lock);
+	init_test((lock_flags & LCK_TEST_MODE) ? 1 : 0);
+
 	switch (lock_cmd) {
 		case LCK_VG_COMMIT:
 			DEBUGLOG("vg_commit notification for VG %s\n", vgname);
@@ -711,6 +713,8 @@ void do_lock_vg(unsigned char command, unsigned char lock_flags, char *resource)
 			DEBUGLOG("Invalidating cached metadata for VG %s\n", vgname);
 			lvmcache_drop_metadata(vgname, 0);
 	}
+
+	init_test(0);
 	pthread_mutex_unlock(&lvm_lock);
 }