From 3de6df84109223978ac0d38973aea82ed00a5d00 Mon Sep 17 00:00:00 2001
From: Dave Wysochanski <dwysocha@redhat.com>
Date: Wed, 2 Sep 2009 21:34:11 +0000
Subject: [PATCH] Enforce an alphabetical lock ordering for vgname locks.

Add a new constraint that vgname locks must be obtained in
alphabetical order.  At this point, we have test coverage for
the 3 commands affected - vgsplit, vgmerge, and vgrename.
Tests have been updated to cover these commands.
Going forward any command or library call that must obtain
more than one vgname lock must do so in alphabetical order.
Future patches will update lvm2app to enforce this ordering.


Author: Dave Wysochanski <dwysocha@redhat.com>
---
 WHATS_NEW             |  2 ++
 lib/cache/lvmcache.c  | 41 ++++++++++++++++++++++++++++++++++++++++-
 lib/cache/lvmcache.h  |  1 +
 lib/locking/locking.c |  6 ++++++
 lib/locking/locking.h |  2 ++
 5 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/WHATS_NEW b/WHATS_NEW
index dc378fb9f..378fab255 100644
--- a/WHATS_NEW
+++ b/WHATS_NEW
@@ -1,5 +1,7 @@
 Version 2.02.52 -
 =================================
+  Enforce an alphabetical lock ordering for vgname locks.
+  Refactor vgsplit, vgmerge, and vgrename to obey vgname ordering rules.
   Implement write lock prioritisation for file locking and make it default.
   Fix clogd build direcory.
   Drop unrequired clogd Makefile.
diff --git a/lib/cache/lvmcache.c b/lib/cache/lvmcache.c
index a7889d834..9ad61a2fb 100644
--- a/lib/cache/lvmcache.c
+++ b/lib/cache/lvmcache.c
@@ -186,6 +186,45 @@ void lvmcache_drop_metadata(const char *vgname)
 		_drop_metadata(vgname);
 }
 
+/*
+ * Ensure vgname2 comes after vgname1 alphabetically.
+ * Special VG names beginning with '#' don't count.
+ */
+static int _vgname_order_correct(const char *vgname1, const char *vgname2)
+{
+	if ((*vgname1 == '#')|(*vgname2 == '#'))
+		return 1;
+
+	if (strcmp(vgname1, vgname2) < 0)
+		return 1;
+	return 0;
+}
+
+/*
+ * Ensure VG locks are acquired in alphabetical order.
+ */
+int lvmcache_verify_lock_order(const char *vgname)
+{
+	struct dm_hash_node *n;
+	const char *vgname2;
+
+	if (!_lock_hash)
+		return_0;
+
+	dm_hash_iterate(n, _lock_hash) {
+		if (!dm_hash_get_data(_lock_hash, n))
+			return_0;
+		vgname2 = dm_hash_get_key(_lock_hash, n);
+		if (!_vgname_order_correct(vgname2, vgname)) {
+			log_errno(EDEADLK, "Internal error: VG lock %s must "
+				  "be requested before %s, not after.",
+				  vgname, vgname2);
+			return_0;
+		}
+	}
+	return 1;
+}
+
 void lvmcache_lock_vgname(const char *vgname, int read_only __attribute((unused)))
 {
 	if (!_lock_hash && !lvmcache_init()) {
@@ -196,7 +235,7 @@ void lvmcache_lock_vgname(const char *vgname, int read_only __attribute((unused)
 	if (dm_hash_lookup(_lock_hash, vgname))
 		log_error("Internal error: Nested locking attempted on VG %s.",
 			  vgname);
-		
+
 	if (!dm_hash_insert(_lock_hash, vgname, (void *) 1))
 		log_error("Cache locking failure for %s", vgname);
 
diff --git a/lib/cache/lvmcache.h b/lib/cache/lvmcache.h
index 09edb3c7b..f1f1065af 100644
--- a/lib/cache/lvmcache.h
+++ b/lib/cache/lvmcache.h
@@ -84,6 +84,7 @@ int lvmcache_update_vg(struct volume_group *vg, unsigned precommitted);
 
 void lvmcache_lock_vgname(const char *vgname, int read_only);
 void lvmcache_unlock_vgname(const char *vgname);
+int lvmcache_verify_lock_order(const char *vgname);
 
 /* Queries */
 const struct format_type *fmt_from_vgname(const char *vgname, const char *vgid);
diff --git a/lib/locking/locking.c b/lib/locking/locking.c
index 40e860159..121797443 100644
--- a/lib/locking/locking.c
+++ b/lib/locking/locking.c
@@ -387,6 +387,12 @@ int lock_vol(struct cmd_context *cmd, const char *vol, uint32_t flags)
 		if (!_blocking_supported || vgs_locked())
 			flags |= LCK_NONBLOCK;
 
+		if (vol[0] != '#' &&
+		    ((flags & LCK_TYPE_MASK) != LCK_UNLOCK) &&
+		    (!(flags & LCK_CACHE)) &&
+		    !lvmcache_verify_lock_order(vol))
+			return 0;
+
 		/* Lock VG to change on-disk metadata. */
 		/* If LVM1 driver knows about the VG, it can't be accessed. */
 		if (!check_lvm1_vg_inactive(cmd, vol))
diff --git a/lib/locking/locking.h b/lib/locking/locking.h
index 9d37f135b..27cf41966 100644
--- a/lib/locking/locking.h
+++ b/lib/locking/locking.h
@@ -34,6 +34,8 @@ int remote_lock_held(const char *vol);
  *   Use VG_GLOBAL as a global lock and to wipe the internal cache.
  *   char *vol holds volume group name.
  *   Set the LCK_CACHE flag to invalidate 'vol' in the internal cache.
+ *   If more than one lock needs to be held simultaneously, they must be
+ *   acquired in alphabetical order of 'vol' (to avoid deadlocks).
  *
  * LCK_LV:
  *   Lock/unlock an individual logical volume