Refactor snapshot-merge deptree and device removal to support info-by-uuid

Add a merging snapshot to the deptree, using the "error" target, rather
than avoid adding it entirely.  This allows proper cleanup of the -cow
device without having to rename the -cow to use the origin's name as a
prefix.

Move the preloading of the origin LV, after a merge, from
lv_remove_single() to vg_remove_snapshot().  Having vg_remove_snapshot()
preload the origin allows the -cow device to be released so that it can
be removed via deactivate_lv().  lv_remove_single()'s deactivate_lv()
reliably removes the -cow device because the associated snapshot LV,
that is to be removed when a snapshot-merge completes, is always added
to the deptree (and kernel -- via "error" target).

Now when the snapshot LV is removed both the -cow and -real devices
get removed using uuid rather than device name.  This paves the way
for us to switch over to info-by-uuid queries.

Signed-off-by: Mike Snitzer <snitzer@redhat.com>

WHATS_NEW

@@ -1,5 +1,6 @@
 Version 2.02.62 -
 ====================================
+  Refactor snapshot-merge deptree and device removal to support info-by-uuid.
 
 Version 2.02.61 - 15th February 2010
 ====================================

lib/activate/dev_manager.c

@@ -1035,10 +1035,6 @@ static int _add_snapshot_target_to_dtree(struct dev_manager *dm,
 		return 0;
 	}
 
-	/* cow is to be merged so skip adding it */
-	if (lv_is_merging_cow(lv))
-		return 1;
-
 	if (!(origin_dlid = build_dlid(dm, snap_seg->origin->lvid.s, "real")))
 		return_0;
 
@@ -1047,7 +1043,13 @@ static int _add_snapshot_target_to_dtree(struct dev_manager *dm,
 
 	size = (uint64_t) snap_seg->len * snap_seg->origin->vg->extent_size;
 
-	if (!dm_tree_node_add_snapshot_target(dnode, size, origin_dlid, cow_dlid, 1, snap_seg->chunk_size))
+	if (lv_is_merging_cow(lv)) {
+		/* cow is to be merged so load the error target */
+		if (!dm_tree_node_add_error_target(dnode, size))
+			return_0;
+	}
+	else if (!dm_tree_node_add_snapshot_target(dnode, size, origin_dlid,
+						   cow_dlid, 1, snap_seg->chunk_size))
 		return_0;
 
 	return 1;
@@ -1166,7 +1168,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
 	struct lv_layer *lvlayer;
 	struct dm_tree_node *dnode;
 	const struct dm_info *dinfo;
-	char *name, *dlid, *lv_name;
+	char *name, *dlid;
 	uint32_t max_stripe_size = UINT32_C(0);
 	uint32_t read_ahead = lv->read_ahead;
 	uint32_t read_ahead_flags = UINT32_C(0);
@@ -1192,22 +1194,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
 		}
 	}
 
-	lv_name = lv->name;
+	if (!(name = build_dm_name(dm->mem, lv->vg->name, lv->name, layer)))
-	if (lv_is_cow(lv) && lv_is_merging_cow(lv)) {
-		if (layer) {
-			/*
-			 * use origin's name as basis for snapshot-merge device names;
-			 * this allows _clean_tree to automatically cleanup "-cow"
-			 * when the origin is resumed (after merge completes)
-			 */
-			lv_name = origin_from_cow(lv)->name;
-		} else {
-			/* top-level snapshot device is not needed during merge */
-			return 1;
-		}
-	}
-
-	if (!(name = build_dm_name(dm->mem, lv->vg->name, lv_name, layer)))
 		return_0;
 
 	if (!(dlid = build_dlid(dm, lv->lvid.s, layer)))
@@ -1220,7 +1207,7 @@ static int _add_new_lv_to_dtree(struct dev_manager *dm, struct dm_tree *dtree,
 
 	if (!(lvlayer = dm_pool_alloc(dm->mem, sizeof(*lvlayer)))) {
 		log_error("_add_new_lv_to_dtree: pool alloc failed for %s %s.",
-			  lv_name, layer);
+			  lv->name, layer);
 		return 0;
 	}
 

lib/metadata/lv_manip.c

@@ -2064,7 +2064,6 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
 	struct volume_group *vg;
 	struct lvinfo info;
 	struct logical_volume *origin = NULL;
-	int was_merging = 0, preload_origin = 0;
 
 	vg = lv->vg;
 
@@ -2117,26 +2116,20 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
 	if (!archive(vg))
 		return 0;
 
-	/* FIXME Snapshot commit out of sequence if it fails after here? */
+	if (lv_is_cow(lv)) {
+		origin = origin_from_cow(lv);
+		log_verbose("Removing snapshot %s", lv->name);
+		/* vg_remove_snapshot() will preload origin if it was merging */
+		if (!vg_remove_snapshot(lv))
+			return_0;
+	}
+
 	if (!deactivate_lv(cmd, lv)) {
 		log_error("Unable to deactivate logical volume \"%s\"",
 			  lv->name);
 		return 0;
 	}
 
-	if (lv_is_cow(lv)) {
-		origin = origin_from_cow(lv);
-		was_merging = lv_is_merging_origin(origin);
-		log_verbose("Removing snapshot %s", lv->name);
-		if (!vg_remove_snapshot(lv))
-			return_0;
-		if (was_merging && lv_is_origin(origin)) {
-			/* snapshot(s) still exist. preload new origin prior to suspend. */
-			/* FIXME Seek a simpler way of dealing with this within the tree. */
-			preload_origin = 1;
-		}
-	}
-
 	log_verbose("Releasing logical volume \"%s\"", lv->name);
 	if (!lv_remove(lv)) {
 		log_error("Error releasing logical volume \"%s\"", lv->name);
@@ -2144,20 +2137,15 @@ int lv_remove_single(struct cmd_context *cmd, struct logical_volume *lv,
 	}
 
 	/* store it on disks */
-	if (!vg_write(vg))
+	if (!vg_write(vg) || !vg_commit(vg))
 		return_0;
 
-	if (!preload_origin && !vg_commit(vg))
+	/* If no snapshots left, reload without -real. */
-		return_0;
+	if (origin && (!lv_is_origin(origin))) {
-
-	/* If no snapshots left or if we stopped merging, reload */
-	if (origin && (!lv_is_origin(origin) || was_merging)) {
 		if (!suspend_lv(cmd, origin)) {
 			log_error("Failed to refresh %s without snapshot.", origin->name);
 			return 0;
 		}
-		if (preload_origin && !vg_commit(vg))
-			return_0;
 		if (!resume_lv(cmd, origin)) {
 			log_error("Failed to resume %s.", origin->name);
 			return 0;

lib/metadata/snapshot_manip.c

@@ -15,6 +15,7 @@
 
 #include "lib.h"
 #include "metadata.h"
+#include "locking.h"
 #include "toolcontext.h"
 #include "lv_alloc.h"
 
@@ -58,9 +59,9 @@ int lv_is_merging_origin(const struct logical_volume *origin)
 
 struct lv_segment *find_merging_cow(const struct logical_volume *origin)
 {
-	/* FIXME: eliminate this wrapper and just use find_cow()?
+	if (!lv_is_merging_origin(origin))
-	 * - find_merging_cow() adds to code clarity in caller
+		return NULL;
-	 */
+
 	return find_cow(origin);
 }
 
@@ -170,12 +171,22 @@ int vg_add_snapshot(struct logical_volume *origin,
 
 int vg_remove_snapshot(struct logical_volume *cow)
 {
+	int preload_origin = 0;
 	struct logical_volume *origin = origin_from_cow(cow);
 
 	dm_list_del(&cow->snapshot->origin_list);
 	origin->origin_count--;
-	if (find_merging_cow(origin) == find_cow(cow))
+	if (find_merging_cow(origin) == find_cow(cow)) {
-		clear_snapshot_merge(origin_from_cow(cow));
+		clear_snapshot_merge(origin);
+		/*
+		 * preload origin to:
+		 * - allow proper release of -cow
+		 * - avoid allocations with other devices suspended
+		 *   when transitioning from "snapshot-merge" to
+		 *   "snapshot-origin after a merge completes.
+		 */
+		preload_origin = 1;
+	}
 
 	if (!lv_remove(cow->snapshot->lv)) {
 		log_error("Failed to remove internal snapshot LV %s",
@@ -186,5 +197,21 @@ int vg_remove_snapshot(struct logical_volume *cow)
 	cow->snapshot = NULL;
 	lv_set_visible(cow);
 
+	if (preload_origin) {
+		if (!vg_write(origin->vg))
+			return_0;
+		if (!suspend_lv(origin->vg->cmd, origin)) {
+			log_error("Failed to refresh %s without snapshot.",
+				  origin->name);
+			return 0;
+		}
+		if (!vg_commit(origin->vg))
+			return_0;
+		if (!resume_lv(origin->vg->cmd, origin)) {
+			log_error("Failed to resume %s.", origin->name);
+			return 0;
+		}
+	}
+
 	return 1;
 }

tools/lvconvert.c

@@ -1210,11 +1210,6 @@ static int lvconvert_merge(struct cmd_context *cmd,
 		goto out;
 	}
 
-	if (!deactivate_lv(cmd, lv)) {
-		log_warn("WARNING: Unable to deactivate merging snapshot %s", lv->name);
-		/* merge is running regardless of this deactivation failure */
-	}
-
 	lp->need_polling = 1;
 	lp->lv_to_poll = origin;