From a5ec3e3827b9b35da222575349a6a1be99958e14 Mon Sep 17 00:00:00 2001 From: Mike Snitzer Date: Wed, 17 Feb 2010 22:59:46 +0000 Subject: [PATCH] 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 --- WHATS_NEW | 1 + lib/activate/dev_manager.c | 33 ++++++++++--------------------- lib/metadata/lv_manip.c | 32 ++++++++++-------------------- lib/metadata/snapshot_manip.c | 37 ++++++++++++++++++++++++++++++----- tools/lvconvert.c | 5 ----- 5 files changed, 53 insertions(+), 55 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index 109fd5956..b3bdba9d9 100644 --- a/WHATS_NEW +++ b/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 ==================================== diff --git a/lib/activate/dev_manager.c b/lib/activate/dev_manager.c index b31a303bc..f21a3cc7b 100644 --- a/lib/activate/dev_manager.c +++ b/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 (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))) + 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; } diff --git a/lib/metadata/lv_manip.c b/lib/metadata/lv_manip.c index 3af605ad1..f161ddb56 100644 --- a/lib/metadata/lv_manip.c +++ b/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,24 +2116,18 @@ 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 (!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); + /* vg_remove_snapshot() will preload origin if it was merging */ 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; - } + } + + if (!deactivate_lv(cmd, lv)) { + log_error("Unable to deactivate logical volume \"%s\"", + lv->name); + return 0; } log_verbose("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)) - return_0; - - if (!preload_origin && !vg_commit(vg)) + if (!vg_write(vg) || !vg_commit(vg)) return_0; - /* If no snapshots left or if we stopped merging, reload */ - if (origin && (!lv_is_origin(origin) || was_merging)) { + /* If no snapshots left, reload without -real. */ + if (origin && (!lv_is_origin(origin))) { 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; diff --git a/lib/metadata/snapshot_manip.c b/lib/metadata/snapshot_manip.c index 19effe380..f7218c266 100644 --- a/lib/metadata/snapshot_manip.c +++ b/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()? - * - find_merging_cow() adds to code clarity in caller - */ + if (!lv_is_merging_origin(origin)) + 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)) - clear_snapshot_merge(origin_from_cow(cow)); + if (find_merging_cow(origin) == find_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; } diff --git a/tools/lvconvert.c b/tools/lvconvert.c index 4fb04a1ba..914b68173 100644 --- a/tools/lvconvert.c +++ b/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; -- 2.43.5