o Use new LCK_HOLD flag to indicate whether lock should be held on return

from lock_vol() - otherwise it now attempts to acquire the lock and then
  immediately releases it.
o Extend the id field in struct logical_volume to hold VG uuid + LV uuid
  for format1. This unique lvid can be used directly when calling lock_vol().
o Add the VG uuid to vgcache to make VG uuid lookups possible.  (Another
  step towards using them instead of VG names internally.)

lib/activate/activate.c

@@ -14,6 +14,9 @@
 #include "toolcontext.h"
 #include "dev_manager.h"
 
+/* FIXME Temporary */
+#include "vgcache.h"
+
 #include <limits.h>
 #include <linux/kdev_t.h>
 #include <fcntl.h>
@@ -270,23 +273,22 @@ int lvs_in_vg_opened(struct volume_group *vg)
 	return count;
 }
 
-/* FIXME Currently lvid is "vgname/lv_uuid". Needs to be vg_uuid/lv_uuid. */
 static struct logical_volume *_lv_from_lvid(struct cmd_context *cmd,
-					    const char *lvid)
+					    const char *lvid_s)
 {
 	struct lv_list *lvl;
 	struct volume_group *vg;
+	union lvid *lvid;
 	char *vgname;
-	char *slash;
 
-	if (!(slash = strchr(lvid, '/'))) {
+	lvid = (union lvid *) lvid_s;
-		log_error("Invalid VG/LV identifier: %s", lvid);
+
+	/* FIXME Change vgread to accept vgid directly - can't rely on cache */
+	if (!(vgname = vgname_from_vgid(cmd, &lvid->id[0]))) {
+		log_error("Volume group for uuid not found: %s", lvid_s);
 		return NULL;
 	}
 
-	vgname = pool_strdup(cmd->mem, lvid);
-	*strchr(vgname, '/') = '\0';
-
 	log_verbose("Finding volume group \"%s\"", vgname);
 	if (!(vg = cmd->fid->ops->vg_read(cmd->fid, vgname))) {
 		log_error("Volume group \"%s\" doesn't exist", vgname);
@@ -298,8 +300,8 @@ static struct logical_volume *_lv_from_lvid(struct cmd_context *cmd,
 		return NULL;
 	}
 
-	if (!(lvl = find_lv_in_vg_by_uuid(vg, slash + 1))) {
+	if (!(lvl = find_lv_in_vg_by_lvid(vg, lvid))) {
-		log_verbose("Can't find logical volume id %s", lvid);
+		log_very_verbose("Can't find logical volume id %s", lvid_s);
 		return NULL;
 	}
 
@@ -308,11 +310,11 @@ static struct logical_volume *_lv_from_lvid(struct cmd_context *cmd,
 
 /* These functions should become the new interface and the _if_active
  * bits then disappear */
-int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid)
+int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid_s)
 {
 	struct logical_volume *lv;
 
-	if (!(lv = _lv_from_lvid(cmd, lvid)))
+	if (!(lv = _lv_from_lvid(cmd, lvid_s)))
 		return 0;
 
 	if (lv_active(lv) > 0)
@@ -322,11 +324,11 @@ int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid)
 }
 
 
-int lv_resume_if_active(struct cmd_context *cmd, const char *lvid)
+int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s)
 {
 	struct logical_volume *lv;
 
-	if (!(lv = _lv_from_lvid(cmd, lvid)))
+	if (!(lv = _lv_from_lvid(cmd, lvid_s)))
 		return 0;
 
 	if ((lv_active(lv) > 0) && lv_suspended(lv))
@@ -335,11 +337,11 @@ int lv_resume_if_active(struct cmd_context *cmd, const char *lvid)
 	return 1;
 }
 
-int lv_deactivate_if_active(struct cmd_context *cmd, const char *lvid)
+int lv_deactivate_if_active(struct cmd_context *cmd, const char *lvid_s)
 {
 	struct logical_volume *lv;
 
-	if (!(lv = _lv_from_lvid(cmd, lvid)))
+	if (!(lv = _lv_from_lvid(cmd, lvid_s)))
 		return 0;
 
 	if (lv_active(lv) > 0)
@@ -348,12 +350,12 @@ int lv_deactivate_if_active(struct cmd_context *cmd, const char *lvid)
 	return 1;
 }
 
-int lv_activate_if_inactive(struct cmd_context *cmd, const char *lvid)
+int lv_activate_if_inactive(struct cmd_context *cmd, const char *lvid_s)
 {
 	struct logical_volume *lv;
 	int active;
 
-	if (!(lv = _lv_from_lvid(cmd, lvid)))
+	if (!(lv = _lv_from_lvid(cmd, lvid_s)))
 		return 0;
 
 	active = lv_active(lv);

lib/activate/activate.h

@@ -40,10 +40,10 @@ int lv_rename(const char *old_name, struct logical_volume *lv);
  * These should eventually replace some of the above and maybe
  * use config file to determine whether or not to activate
  */
-int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid);
+int lv_suspend_if_active(struct cmd_context *cmd, const char *lvid_s);
-int lv_resume_if_active(struct cmd_context *cmd, const char *lvid);
+int lv_resume_if_active(struct cmd_context *cmd, const char *lvid_s);
-int lv_activate_if_inactive(struct cmd_context *cmd, const char *lvid);
+int lv_activate_if_inactive(struct cmd_context *cmd, const char *lvid_s);
-int lv_deactivate_if_active(struct cmd_context *cmd, const char *lvid);
+int lv_deactivate_if_active(struct cmd_context *cmd, const char *lvid_s);
 
 
 /*

lib/activate/dev_manager.c

@@ -133,7 +133,7 @@ static int _load(struct dev_manager *dm, struct dev_layer *dl, int task)
 	int r;
 	struct dm_task *dmt;
 
-	log_very_verbose("Loading %s", dl->name);
+	log_verbose("Loading %s", dl->name);
 	if (!(dmt = _setup_task(dl->name, task))) {
 		stack;
 		return 0;
@@ -162,7 +162,7 @@ static int _remove(struct dev_layer *dl)
 	int r;
 	struct dm_task *dmt;
 
-	log_very_verbose("Removing %s", dl->name);
+	log_verbose("Removing %s", dl->name);
 	if (!(dmt = _setup_task(dl->name, DM_DEVICE_REMOVE))) {
 		stack;
 		return 0;

lib/display/display.c

@@ -205,7 +205,7 @@ int lvdisplay_full(struct cmd_context *cmd, struct logical_volume *lv)
 	char uuid[64];
 	struct snapshot *snap;
 
-	if (!id_write_format(&lv->id, uuid, sizeof(uuid))) {
+	if (!id_write_format(&lv->lvid.id[1], uuid, sizeof(uuid))) {
 		stack;
 		return 0;
 	}

lib/format1/disk-rep.c

@@ -305,7 +305,7 @@ static struct disk_list *__read_disk(struct device *dev, struct pool *mem,
 		log_very_verbose("%s is not a member of any VG", name);
 
 		/* Update VG cache */
-		vgcache_add(data->pvd.vg_name, dev);
+		vgcache_add(data->pvd.vg_name, NULL, dev);
 
 		return (vg_name) ? NULL : data;
 	}
@@ -319,7 +319,7 @@ static struct disk_list *__read_disk(struct device *dev, struct pool *mem,
 	_munge_exported_vg(data);
 
 	/* Update VG cache with what we found */
-	vgcache_add(data->pvd.vg_name, dev);
+	vgcache_add(data->pvd.vg_name, data->vgd.vg_uuid, dev);
 
 	if (vg_name && strcmp(vg_name, data->pvd.vg_name)) {
 		log_very_verbose("%s is not a member of the VG %s",
@@ -582,13 +582,18 @@ static int __write_all_pvd(struct disk_list *data)
 		return 0;
 	}
 
-	if (!test_mode())
+		vgcache_add(data->pvd.vg_name, data->vgd.vg_uuid, data->dev);
-		vgcache_add(data->pvd.vg_name, data->dev);
 	/*
 	 * Stop here for orphan pv's.
 	 */
-	if (data->pvd.vg_name[0] == '\0')
+	if (data->pvd.vg_name[0] == '\0') {
+		if (!test_mode())
+			vgcache_add(data->pvd.vg_name, NULL, data->dev);
 		return 1;
+	}
+
+	if (!test_mode())
+		vgcache_add(data->pvd.vg_name, data->vgd.vg_uuid, data->dev);
 
 	if (!_write_vgd(data)) {
 		log_error("Failed to write VG data to %s", pv_name);

lib/format1/format1.c

@@ -429,7 +429,7 @@ static int _find_free_lvnum(struct logical_volume *lv)
 
 	list_iterate(lvh, &lv->vg->lvs) {
 		lvl = list_item(lvh, struct lv_list);
-		lvnum_used[lvnum_from_id(&lvl->lv->id)] = 1;
+		lvnum_used[lvnum_from_lvid(&lvl->lv->lvid)] = 1;
 	}
 
 	while (lvnum_used[i])
@@ -442,7 +442,7 @@ static int _lv_setup(struct format_instance *fi, struct logical_volume *lv)
 {
 	uint64_t max_size = UINT_MAX;
 
-	id_from_lvnum(&lv->id, _find_free_lvnum(lv));
+	lvid_from_lvnum(&lv->lvid, &lv->vg->id, _find_free_lvnum(lv));
 
 	if (lv->le_count > MAX_LE_TOTAL) {
 		log_error("logical volumes cannot contain more than "

lib/format1/import-export.c

@@ -272,9 +272,7 @@ int export_vg(struct vg_disk *vgd, struct volume_group *vg)
 
 int import_lv(struct pool *mem, struct logical_volume *lv, struct lv_disk *lvd)
 {
-	memset(&lv->id, 0, sizeof(lv->id));
+	lvid_from_lvnum(&lv->lvid, &lv->vg->id, lvd->lv_number);
-
-	id_from_lvnum(&lv->id, lvd->lv_number);
 
         if (!(lv->name = _create_lv_name(mem, lvd->lv_name))) {
 		stack;
@@ -431,6 +429,7 @@ static struct logical_volume *_add_lv(struct pool *mem,
 		return NULL;
 	}
 	lv = ll->lv;
+	lv->vg = vg;
 
 	if (!import_lv(mem, lv, lvd)) {
 		stack;
@@ -438,7 +437,6 @@ static struct logical_volume *_add_lv(struct pool *mem,
 	}
 
 	list_add(&vg->lvs, &ll->list);
-	lv->vg = vg;
 	vg->lv_count++;
 
 	return lv;
@@ -504,7 +502,7 @@ int export_lvs(struct disk_list *dl, struct volume_group *vg,
 
 		export_lv(&lvdl->lvd, vg, ll->lv, dev_dir);
 
-		lv_num = lvnum_from_id(&ll->lv->id);
+		lv_num = lvnum_from_lvid(&ll->lv->lvid);
 
 		lvdl->lvd.lv_number = lv_num;
 

lib/format_text/export.c

@@ -354,7 +354,8 @@ static int _print_lvs(struct formatter *f, struct volume_group *vg)
 		_out(f, "%s {", lv->name);
 		_inc_indent(f);
 
-		if (!id_write_format(&lv->id, buffer, sizeof(buffer))) {
+		/* FIXME: Write full lvid */
+		if (!id_write_format(&lv->lvid.id[1], buffer, sizeof(buffer))) {
 			stack;
 			return 0;
 		}

lib/format_text/import.c

@@ -364,13 +364,15 @@ static int _read_lv(struct pool *mem,
 
 	lv->vg = vg;
 
-
+	/* FIXME: read full lvid */
-	if (!_read_id(&lv->id, lvn, "id")) {
+	if (!_read_id(&lv->lvid.id[1], lvn, "id")) {
 		log_err("Couldn't read uuid for logical volume %s.",
 			lv->name);
 		return 0;
 	}
 
+	memcpy(&lv->lvid.id[0], &lv->vg->id, sizeof(lv->lvid.id[0]));
+
 	if (!(cn = find_config_node(lvn, "status", '/'))) {
 		log_err("Couldn't find status flags for logical volume.");
 		return 0;

lib/locking/locking.c

@@ -118,6 +118,21 @@ void fin_locking(void)
  * LV locking is by VG_name/LV_uuid
  * FIXME This should take a VG_uuid instead of VG_name
  */
+int _lock_vol(struct cmd_context *cmd, const char *resource, int flags)
+{
+	_ignore_signals();
+
+	if (!(_locking.lock_resource(cmd, resource, flags))) {
+		_enable_signals();
+		return 0;
+	}
+
+	_update_lock_count(flags);
+	_enable_signals();
+
+	return 1;
+}
+
 int lock_vol(struct cmd_context *cmd, const char *vol, int flags)
 {
 	char resource[258];
@@ -133,16 +148,16 @@ int lock_vol(struct cmd_context *cmd, const char *vol, int flags)
 		return 0;
 	}
 
-	_ignore_signals();
+	if (!_lock_vol(cmd, resource, flags))
+		return 0;
 
-	if (!(_locking.lock_resource(cmd, resource, flags))) {
+	/* Perform immediate unlock unless LCK_HOLD set */
-		_enable_signals();
+	if (!(flags & LCK_HOLD) && ((flags & LCK_TYPE_MASK) != LCK_NONE)) {
+		if (!_lock_vol(cmd, resource, 
+			       (flags & ~LCK_TYPE_MASK) | LCK_NONE))
 			return 0;
 	}
 
-	_update_lock_count(flags);
-	_enable_signals();
-
 	return 1;
 }
 

lib/locking/locking.h

@@ -49,13 +49,14 @@ int lock_vol(struct cmd_context *cmd, const char *vol, int flags);
 /*
  * Lock bits
  */
-#define LCK_NONBLOCK	0x00010000
+#define LCK_NONBLOCK	0x00010000	/* Don't block waiting for lock? */
+#define LCK_HOLD	0x00020000	/* Hold lock when lock_vol returns? */
 
 /*
  * Common combinations
  */
-#define LCK_VG_READ		(LCK_VG | LCK_READ)
+#define LCK_VG_READ		(LCK_VG | LCK_READ | LCK_HOLD)
-#define LCK_VG_WRITE		(LCK_VG | LCK_WRITE)
+#define LCK_VG_WRITE		(LCK_VG | LCK_WRITE | LCK_HOLD)
 #define LCK_VG_UNLOCK		(LCK_VG | LCK_NONE)
 
 #define LCK_LV_DEACTIVATE	(LCK_LV | LCK_EXCL)

lib/metadata/lv_manip.c

@@ -416,7 +416,7 @@ struct logical_volume *lv_create(struct format_instance *fi,
 
 	lv = ll->lv;
 
-	strcpy(lv->id.uuid, "");
+	lv->vg = vg;
 
 	if (!(lv->name = pool_strdup(cmd->mem, name))) {
 		stack;
@@ -428,7 +428,6 @@ struct logical_volume *lv_create(struct format_instance *fi,
 	lv->minor = -1;
 	lv->size = (uint64_t) extents * vg->extent_size;
 	lv->le_count = extents;
-	lv->vg = vg;
 	list_init(&lv->segments);
 
 	if (!_allocate(vg, lv, acceptable_pvs, 0u, stripes, stripe_size)) {

lib/metadata/metadata.c

@@ -290,14 +290,14 @@ struct lv_list *find_lv_in_vg(struct volume_group *vg, const char *lv_name)
 	return NULL;
 }
 
-struct lv_list *find_lv_in_vg_by_uuid(struct volume_group *vg, const char *uuid)
+struct lv_list *find_lv_in_vg_by_lvid(struct volume_group *vg, union lvid *lvid)
 {
 	struct list *lvh;
 	struct lv_list *lvl;
 
 	list_iterate(lvh, &vg->lvs) {
 		lvl = list_item(lvh, struct lv_list);
-		if (!strncmp(lvl->lv->id.uuid, uuid, ID_LEN))
+		if (!strncmp(lvl->lv->lvid.s, lvid->s, sizeof(*lvid)))
 			return lvl;
 	}
 
@@ -324,14 +324,3 @@ struct physical_volume *find_pv(struct volume_group *vg, struct device *dev)
 	return NULL;
 }
 
-char *lvid(struct logical_volume *lv, char *buf, int size)
-{
-	/* FIXME Create uuid.h functions for all uuid manipulation */
-	if (lvm_snprintf(buf, size, "%s/%." ID_LEN_S "s", lv->vg->name, 
-			 lv->id.uuid) < 0) {
-		log_error("Buffer too small to hold LV id: %s", buf);
-		return NULL;
-	}
-
-	return buf;
-}

lib/metadata/metadata.h

@@ -115,7 +115,7 @@ struct stripe_segment {
 };
 
 struct logical_volume {
-	struct id id;
+	union lvid lvid;
         char *name;
 
 	struct volume_group *vg;
@@ -303,11 +303,8 @@ struct pv_list *find_pv_in_vg(struct volume_group *vg, const char *pv_name);
 
 /* Find an LV within a given VG */
 struct lv_list *find_lv_in_vg(struct volume_group *vg, const char *lv_name);
-struct lv_list *find_lv_in_vg_by_uuid(struct volume_group *vg, 
+struct lv_list *find_lv_in_vg_by_lvid(struct volume_group *vg, 
-				      const char *uuid);
+				      union lvid *lvid);
-
-/* Get unique LV identifier - currently "<VG uuid>/<LV uuid>" */
-char *lvid(struct logical_volume *lv, char *buf, int size);
 
 /* Return the VG that contains a given LV (based on path given in lv_name) */
 /* or environment var */

lib/uuid/uuid.c

@@ -19,26 +19,30 @@ static unsigned char _c[] =
 static int _built_inverse;
 static unsigned char _inverse_c[256];
 
-int id_from_lvnum(struct id *id, int lv_num)
+int lvid_from_lvnum(union lvid *lvid, struct id *vgid, int lv_num)
 {
 	int i;
 
+	memcpy(lvid->id, vgid, sizeof(*lvid->id));
+
 	for (i = ID_LEN; i; i--) {
-		id->uuid[i - 1] = _c[lv_num % (sizeof(_c) - 1)];
+		lvid->id[1].uuid[i - 1] = _c[lv_num % (sizeof(_c) - 1)];
 		lv_num /= sizeof(_c) - 1;
 	}
 
+	lvid->s[sizeof(lvid->s) - 1] = '\0';
+
 	return 1;
 }
 
-int lvnum_from_id(struct id *id)
+int lvnum_from_lvid(union lvid *lvid)
 {
 	int i, lv_num = 0;
 	unsigned char *c;
 
 	for (i = 0; i < ID_LEN; i++) {
 		lv_num *= sizeof(_c) - 1;
-		if ((c = strchr(_c, id->uuid[i])))
+		if ((c = strchr(_c, lvid->id[1].uuid[i])))
 			lv_num += (int) (c - _c);
 	}
 

lib/uuid/uuid.h

@@ -16,8 +16,17 @@ struct id {
 	uint8_t uuid[ID_LEN];
 };
 
-int id_from_lvnum(struct id *id, int lv_num);
+/*
-int lvnum_from_id(struct id *id);
+ * Unique logical volume identifier
+ * With format1 this is VG uuid + LV uuid + '\0'
+ */
+union lvid {
+	struct id id[2];
+	char s[2 * sizeof(struct id) + 1];	
+};
+
+int lvid_from_lvnum(union lvid *lvid, struct id *vgid, int lv_num);
+int lvnum_from_lvid(union lvid *lvid);
 
 int id_create(struct id *id);
 int id_valid(struct id *id);

lib/vgcache/vgcache.c

@@ -10,8 +10,11 @@
 #include "hash.h"
 #include "dbg_malloc.h"
 #include "log.h"
+#include "uuid.h"
+#include "toolcontext.h"
 
 static struct hash_table *_vghash;
+static struct hash_table *_vgidhash;
 static struct hash_table *_pvhash;
 
 const char *all_devices = "\0";
@@ -21,6 +24,9 @@ int vgcache_init()
 	if (!(_vghash = hash_create(128)))
 		return 0;
 
+	if (!(_vgidhash = hash_create(128)))
+		return 0;
+
 	if (!(_pvhash = hash_create(128)))
 		return 0;
 
@@ -44,6 +50,23 @@ struct list *vgcache_find(const char *vg_name)
 	return &vgn->pvdevs;
 }
 
+struct list *vgcache_find_by_vgid(const char *vgid)
+{
+	struct vgname_entry *vgn;
+	char vgid_s[ID_LEN + 1];
+
+	if (!_vgidhash || !vgid)
+		return NULL;
+
+	memcpy(vgid_s, vgid, ID_LEN);
+	vgid_s[ID_LEN] = '\0';
+
+	if (!(vgn = hash_lookup(_vgidhash, vgid_s)))
+		return NULL;
+
+	return &vgn->pvdevs;
+}
+
 void vgcache_del_orphan(struct device *dev)
 {
 	struct pvdev_list *pvdev;
@@ -55,7 +78,7 @@ void vgcache_del_orphan(struct device *dev)
 	}
 }
 
-int vgcache_add_entry(const char *vg_name, struct device *dev)
+int vgcache_add_entry(const char *vg_name, const char *vgid, struct device *dev)
 {
 	const char *pv_name;
 	struct vgname_entry *vgn;
@@ -67,6 +90,7 @@ int vgcache_add_entry(const char *vg_name, struct device *dev)
 			log_error("struct vgname_entry allocation failed");
 			return 0;
 		}
+		memset(vgn, 0, sizeof(struct vgname_entry));
 
 		pvdevs = &vgn->pvdevs;
 		list_init(pvdevs);
@@ -80,6 +104,17 @@ int vgcache_add_entry(const char *vg_name, struct device *dev)
 			log_error("vgcache_add: VG hash insertion failed");
 			return 0;
 		}
+
+		if (vgid) {
+			memcpy(vgn->vgid, vgid, ID_LEN);
+			vgn->vgid[ID_LEN] = '\0';
+
+			if (!hash_insert(_vgidhash, vgn->vgid, vgn)) {
+				log_error("vgcache_add: vgid hash insertion "
+					  "failed");
+				return 0;
+			}
+		}
 	}
 
 	list_iterate(pvdh, pvdevs) {
@@ -115,7 +150,7 @@ int vgcache_add_entry(const char *vg_name, struct device *dev)
 }
 
 /* vg_name of "\0" is an orphan PV; NULL means only add to all_devices */
-int vgcache_add(const char *vg_name, struct device *dev)
+int vgcache_add(const char *vg_name, const char *vgid, struct device *dev)
 {
 	if (!_vghash && !vgcache_init())
 		return 0;
@@ -125,11 +160,11 @@ int vgcache_add(const char *vg_name, struct device *dev)
 		vgcache_del_orphan(dev);
 
 	/* Add PV if vg_name supplied */
-	if (vg_name && *vg_name && !vgcache_add_entry(vg_name, dev))
+	if (vg_name && *vg_name && !vgcache_add_entry(vg_name, vgid, dev))
 		return 0;
 
 	/* Always add to all_devices */
-	return vgcache_add_entry(all_devices, dev);
+	return vgcache_add_entry(all_devices, NULL, dev);
 }
 
 void vgcache_destroy_entry(struct vgname_entry *vgn)
@@ -147,6 +182,8 @@ void vgcache_destroy_entry(struct vgname_entry *vgn)
 			dbg_free(pvdev);
 		}
 		dbg_free(vgn->vgname);
+		if (_vgidhash && vgn->vgid[0])
+			hash_remove(_vgidhash, vgn->vgid);
 	}
 	dbg_free(vgn);
 }
@@ -165,9 +202,35 @@ void vgcache_del(const char *vg_name)
 		return;
 
 	hash_remove(_vghash, vg_name);
+	if (vgn->vgid[0])
+		hash_remove(_vgidhash, vgn->vgid);
+
 	vgcache_destroy_entry(vgn);
 }
 
+
+void vgcache_del_by_vgid(const char *vgid)
+{
+	struct vgname_entry *vgn;
+	char vgid_s[ID_LEN + 1];
+
+	if (!_vgidhash || !vgid)
+		return;
+
+	memcpy(vgid_s, vgid, ID_LEN);
+	vgid_s[ID_LEN] = '\0';
+
+	if (!(vgn = hash_lookup(_vghash, vgid_s)))
+		return;
+
+	hash_remove(_vgidhash, vgn->vgid);
+	if (vgn->vgname[0])
+		hash_remove(_vghash, vgn->vgname);
+
+	vgcache_destroy_entry(vgn);
+}
+
+
 void vgcache_destroy()
 {
 	if (_vghash) {
@@ -176,8 +239,31 @@ void vgcache_destroy()
 		_vghash = NULL;
 	}
 
+	if (_vgidhash) {
+		hash_destroy(_vgidhash);
+		_vgidhash = NULL;
+	}
+
 	if (_pvhash) {
 		hash_destroy(_pvhash);
 		_pvhash = NULL;
 	}
 }
+
+char *vgname_from_vgid(struct cmd_context *cmd, struct id *vgid)
+{
+	struct vgname_entry *vgn;
+	char vgid_s[ID_LEN + 1];
+
+	if (!_vgidhash || !vgid)
+		return NULL;
+
+	memcpy(vgid_s, vgid->uuid, ID_LEN);
+	vgid_s[ID_LEN] = '\0';
+
+	if (!(vgn = hash_lookup(_vgidhash, vgid_s)))
+		return NULL;
+
+	return pool_strdup(cmd->mem, vgn->vgname);
+}
+

lib/vgcache/vgcache.h

@@ -12,10 +12,13 @@
 #include <asm/page.h>
 #include "dev-cache.h"
 #include "list.h"
+#include "uuid.h"
+#include "toolcontext.h"
 
 struct vgname_entry {
 	struct list pvdevs;
 	char *vgname;
+	char vgid[ID_LEN + 1];
 };
 
 struct pvdev_list {
@@ -28,9 +31,13 @@ void vgcache_destroy();
 
 /* Return list of PVs in named VG */
 struct list *vgcache_find(const char *vg_name);
+struct list *vgcache_find_by_vgid(const char *vgid);
+
+/* FIXME Temporary function */
+char *vgname_from_vgid(struct cmd_context *cmd, struct id *vgid);
 
 /* Add/delete a device */
-int vgcache_add(const char *vg_name, struct device *dev);
+int vgcache_add(const char *vg_name, const char *vgid, struct device *dev);
 void vgcache_del(const char *vg_name);
 
 #endif

tools/lvchange.c

@@ -126,7 +126,6 @@ static int lvchange_permission(struct cmd_context *cmd,
 			       struct logical_volume *lv)
 {
 	int lv_access;
-	char lvidbuf[128];
 
 	lv_access = arg_int_value(cmd, permission_ARG, 0);
 
@@ -152,10 +151,7 @@ static int lvchange_permission(struct cmd_context *cmd,
 			    lv->name);
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD)) {
-		return 0;
-
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND)) {
 		log_error("Failed to lock %s", lv->name);
 		return 0;
 	}
@@ -163,14 +159,14 @@ static int lvchange_permission(struct cmd_context *cmd,
 	log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
 	if (!cmd->fid->ops->vg_write(cmd->fid, lv->vg)) {
                 /* FIXME: Attempt reversion? */
-                lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+                lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 		return 0;
 	}
 
 	backup(lv->vg);
 
 	log_very_verbose("Updating permissions for \"%s\" in kernel", lv->name);
-        if (!lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK)) {
+        if (!lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK)) {
                 log_error("Problem reactivating %s", lv->name);
                 return 0;
         }
@@ -182,7 +178,6 @@ static int lvchange_availability(struct cmd_context *cmd,
 				 struct logical_volume *lv)
 {
 	int activate = 0;
-	char lvidbuf[128];
 
 	if (strcmp(arg_str_value(cmd, available_ARG, "n"), "n"))
 		activate = 1;
@@ -191,21 +186,16 @@ static int lvchange_availability(struct cmd_context *cmd,
 		lv->minor = arg_int_value(cmd, minor_ARG, -1);
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
-		return 0;
-
 	if (activate) {
 		log_verbose("Activating logical volume \"%s\"", lv->name);
-		if (!lock_vol(cmd, lvidbuf, LCK_LV_ACTIVATE))
+		if (!lock_vol(cmd, lv->lvid.s, LCK_LV_ACTIVATE))
 			return 0;
 	} else {
 		log_verbose("Deactivating logical volume \"%s\"", lv->name);
-		if (!lock_vol(cmd, lvidbuf, LCK_LV_DEACTIVATE))
+		if (!lock_vol(cmd, lv->lvid.s, LCK_LV_DEACTIVATE))
 			return 0;
 	}
 
-	lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
-
 	return 1;
 }
 
@@ -213,7 +203,6 @@ static int lvchange_contiguous(struct cmd_context *cmd,
 			       struct logical_volume *lv)
 {
 	int lv_allocation = 0;
-	char lvidbuf[128];
 
 	if (strcmp(arg_str_value(cmd, contiguous_ARG, "n"), "n"))
 		lv_allocation |= ALLOC_CONTIGUOUS;
@@ -250,10 +239,7 @@ static int lvchange_contiguous(struct cmd_context *cmd,
 			    lv->name);
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD)) {
-		return 0;
-
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND)) {
 		log_error("Failed to lock %s", lv->name);
 		return 0;
 	}
@@ -261,14 +247,14 @@ static int lvchange_contiguous(struct cmd_context *cmd,
 	log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
 	if (!cmd->fid->ops->vg_write(cmd->fid, lv->vg)) {
                 /* FIXME: Attempt reversion? */
-                lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+                lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 		return 0;
 	}
 
 	backup(lv->vg);
 
 	log_very_verbose("Reactivating \"%s\" in kernel", lv->name);
-        if (!lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK)) {
+        if (!lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK)) {
                 log_error("Problem reactivating %s", lv->name);
                 return 0;
         }
@@ -281,7 +267,6 @@ static int lvchange_readahead(struct cmd_context *cmd,
 			      struct logical_volume *lv)
 {
 	int read_ahead = 0;
-	char lvidbuf[128];
 
 	read_ahead = arg_int_value(cmd, readahead_ARG, 0);
 
@@ -303,10 +288,7 @@ static int lvchange_readahead(struct cmd_context *cmd,
 	log_verbose("Setting read ahead to %u for \"%s\"", read_ahead,
 		    lv->name);
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD)) {
-		return 0;
-
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND)) {
 		log_error("Failed to lock %s", lv->name);
 		return 0;
 	}
@@ -314,14 +296,14 @@ static int lvchange_readahead(struct cmd_context *cmd,
 	log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
 	if (!cmd->fid->ops->vg_write(cmd->fid, lv->vg)) {
                 /* FIXME: Attempt reversion? */
-                lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+                lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 		return 0;
 	}
 
 	backup(lv->vg);
 
 	log_very_verbose("Reactivating \"%s\" in kernel", lv->name);
-        if (!lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK)) {
+        if (!lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK)) {
                 log_error("Problem reactivating %s", lv->name);
                 return 0;
         }
@@ -332,7 +314,6 @@ static int lvchange_readahead(struct cmd_context *cmd,
 static int lvchange_persistent(struct cmd_context *cmd,
 			       struct logical_volume *lv)
 {
-	char lvidbuf[128];
 
 	if (!strcmp(arg_str_value(cmd, persistent_ARG, "n"), "n")) {
 		if (!(lv->status & FIXED_MINOR)) {
@@ -358,10 +339,7 @@ static int lvchange_persistent(struct cmd_context *cmd,
 			    lv->minor, lv->name);
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD)) {
-		return 0;
-
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND)) {
 		log_error("Failed to lock %s", lv->name);
 		return 0;
 	}
@@ -369,14 +347,14 @@ static int lvchange_persistent(struct cmd_context *cmd,
 	log_very_verbose("Updating logical volume \"%s\" on disk(s)", lv->name);
 	if (!cmd->fid->ops->vg_write(cmd->fid, lv->vg)) {
                 /* FIXME: Attempt reversion? */
-                lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+                lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 		return 0;
 	}
 
 	backup(lv->vg);
 
 	log_very_verbose("Reactivating \"%s\" in kernel", lv->name);
-        if (!lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK)) {
+        if (!lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK)) {
                 log_error("Problem reactivating %s", lv->name);
                 return 0;
         }

tools/lvcreate.c

@@ -277,7 +277,7 @@ static int _zero_lv(struct cmd_context *cmd, struct logical_volume *lv)
 	log_verbose("Zeroing start of logical volume \"%s\"", lv->name);
 
 	if (!(dev = dev_cache_get(name, NULL))) {
-		log_error("\"%s\" not found: device not zeroed", name);
+		log_error("%s: not found: device not zeroed", name);
 		return 0;
 	}
 
@@ -297,7 +297,6 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp)
 	struct volume_group *vg;
 	struct logical_volume *lv, *org;
 	struct list *pvh;
-	char lvidbuf[128];
 
 	if (lp->contiguous)
 		status |= ALLOC_CONTIGUOUS;
@@ -395,9 +394,6 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp)
 		log_verbose("Setting minor number to %d", lv->minor);
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
-		return 0;
-
 	if (!archive(vg))
 		return 0;
 
@@ -405,7 +401,7 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp)
 	if (!cmd->fid->ops->vg_write(cmd->fid, vg))
 		return 0;
 
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_ACTIVATE))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_ACTIVATE))
 		return 0;
 
 	if (lp->zero || lp->snapshot)
@@ -413,16 +409,12 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp)
 	else
 		log_print("WARNING: \"%s\" not zeroed", lv->name);
 
-	lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
-
 	if (lp->snapshot) {
-		if (!lock_vol(cmd, lvidbuf, LCK_LV_DEACTIVATE)) {
+		if (!lock_vol(cmd, lv->lvid.s, LCK_LV_DEACTIVATE)) {
 			log_err("Couldn't lock snapshot.");
 			return 0;
 		}
 
-		lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
-
 		if (!vg_add_snapshot(org, lv, 1, lp->chunk_size)) {
 			log_err("Couldn't create snapshot.");
 			return 0;
@@ -432,9 +424,8 @@ static int _lvcreate(struct cmd_context *cmd, struct lvcreate_params *lp)
 		if (!cmd->fid->ops->vg_write(cmd->fid, vg))
 			return 0;
 
-		if (!lock_vol(cmd, lvidbuf, LCK_LV_ACTIVATE))
+		if (!lock_vol(cmd, lv->lvid.s, LCK_LV_ACTIVATE))
 			return 0;
-		lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
 	}
 
 	backup(vg);

tools/lvremove.c

@@ -36,7 +36,6 @@ static int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv)
 {
 	struct volume_group *vg;
 	int active;
-	char lvidbuf[128];
 
 	vg = lv->vg;
 
@@ -56,7 +55,7 @@ static int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv)
 		return ECMD_FAILED;
 	}
 
-	active = lv_active(lv);
+	active = (lv_active(lv) > 0);
 
 	if (active && !arg_count(cmd, force_ARG)) {
 		if (yes_no_prompt("Do you really want to remove active "
@@ -68,23 +67,22 @@ static int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv)
 		}
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
-		return 0;
-
 	if (!archive(vg))
 		return ECMD_FAILED;
 
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_DEACTIVATE)) {
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_DEACTIVATE)) {
 		log_error("Unable to deactivate logical volume \"%s\"",
 			  lv->name);
 		return ECMD_FAILED;
 	}
 
-	log_verbose("Removing snapshot.");
+	if (lv_is_cow(lv)) {
-	if (lv_is_cow(lv) && !vg_remove_snapshot(lv->vg, lv)) {
+		log_verbose("Removing snapshot %s", lv->name);
+ 		if (!vg_remove_snapshot(lv->vg, lv)) {
 			stack;
 			return ECMD_FAILED;
 		}
+	}
 
 	log_verbose("Releasing logical volume \"%s\"", lv->name);
 	if (!lv_remove(vg, lv)) {
@@ -98,8 +96,6 @@ static int lvremove_single(struct cmd_context *cmd, struct logical_volume *lv)
 
 	backup(vg);
 
-	lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
-
 	log_print("Logical volume \"%s\" successfully removed", lv->name);
 	return 0;
 }

tools/lvrename.c

@@ -26,7 +26,6 @@ int lvrename(struct cmd_context *cmd, int argc, char **argv)
 	char *lv_name_old, *lv_name_new;
 	char *vg_name, *vg_name_new;
 	char *st;
-	char lvidbuf[128];
 
 	struct volume_group *vg;
 	struct logical_volume *lv;
@@ -121,13 +120,11 @@ int lvrename(struct cmd_context *cmd, int argc, char **argv)
 
 	lv = lvl->lv;
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
-		goto error;
-
 	if (!archive(lv->vg))
 		goto error;
 
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND | LCK_NONBLOCK))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD | 
+				       LCK_NONBLOCK))
 		goto error;
 
 	if (!(lv->name = pool_strdup(cmd->mem, lv_name_new))) {
@@ -139,7 +136,7 @@ int lvrename(struct cmd_context *cmd, int argc, char **argv)
 	if (!(cmd->fid->ops->vg_write(cmd->fid, vg)))
 		goto lverror;
 
-	lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+	lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 
 	backup(lv->vg);
 
@@ -152,7 +149,7 @@ int lvrename(struct cmd_context *cmd, int argc, char **argv)
 
 
       lverror:
-	lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+	lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 
       error:
 	lock_vol(cmd, vg_name, LCK_VG_UNLOCK);

tools/lvresize.c

@@ -31,7 +31,6 @@ int lvresize(struct cmd_context *cmd, int argc, char **argv)
 	uint32_t size_rest;
 	sign_t sign = SIGN_NONE;
 	char *lv_name, *vg_name;
-	char lvidbuf[128];
 	char *st;
 	char *dummy;
 	const char *cmd_name;
@@ -332,10 +331,7 @@ int lvresize(struct cmd_context *cmd, int argc, char **argv)
 			goto error;
 	}
 
-	if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_SUSPEND | LCK_HOLD)) {
-		goto error;
-
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_SUSPEND)) {
 		log_error("Can't get lock for %s", lv_name);
 		goto error;
 	}
@@ -343,13 +339,13 @@ int lvresize(struct cmd_context *cmd, int argc, char **argv)
 	/* store vg on disk(s) */
 	if (!cmd->fid->ops->vg_write(cmd->fid, vg)) {
 		/* FIXME: Attempt reversion? */
-		lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
+		lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK);
 		goto error;
 	}
 
 	backup(vg);
 
-	if (!lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK)) {
+	if (!lock_vol(cmd, lv->lvid.s, LCK_LV_UNLOCK)) {
 		log_error("Problem reactivating %s", lv_name);
 		goto error;
 	}

tools/vgchange.c

@@ -31,19 +31,13 @@ static int _activate_lvs_in_vg(struct cmd_context *cmd,
 	struct list *lvh;
 	struct logical_volume *lv;
 	int count = 0;
-	char lvidbuf[128];
 
 	list_iterate(lvh, &vg->lvs) {
 		lv = list_item(lvh, struct lv_list)->lv;
 
-		if (!lvid(lv, lvidbuf, sizeof(lvidbuf)))
+		if (!lock_vol(cmd, lv->lvid.s, lock | LCK_NONBLOCK))
 			continue;
 
-		if (!lock_vol(cmd, lvidbuf, lock | LCK_NONBLOCK))
-			continue;
-
-		lock_vol(cmd, lvidbuf, LCK_LV_UNLOCK);
-
 		count++;
 	}