From: Mike Snitzer <snitzer@redhat.com>
To: lvm-devel@redhat.com
Subject: Allowing an actively merging snapshot to be removed?
Date: Fri, 23 Apr 2010 10:48:08 -0400 [thread overview]
Message-ID: <20100423144808.GA21974@redhat.com> (raw)
In-Reply-To: <20100422233605.GB23791@agk-dp.fab.redhat.com>
On Thu, Apr 22 2010 at 7:36pm -0400,
Alasdair G Kergon <agk@redhat.com> wrote:
> On Wed, Apr 21, 2010 at 08:01:32PM -0400, Mikulas Patocka wrote:
> > I think the merging snapshot LV shouldn't be accessible
>
> Ack. We are past the user-generated 'commit' point at which it goes.
The following patch is one way to address this bug. Given the current
merge support in lvm2 this is the most straight-forward fix. I've
tested this to work well. Does the following seem reasonable?
Disallow the direct removal of a merging snapshot.
Allow lv_remove_with_dependencies() to know the top-level LV that was
requested to be removed (before it recurses and loses context).
A merging snapshot cannot be removed directly but the associated origin
can be. Disallow removal of a merging snapshot unless the associated
origin is also being removed.
Signed-off-by: Mike Snitzer <snitzer@redhat.com>
---
lib/metadata/lv_manip.c | 17 +++++++++++++----
lib/metadata/metadata-exported.h | 2 +-
lib/metadata/metadata.c | 2 +-
test/t-snapshot-merge.sh | 12 +++++++++---
tools/lvremove.c | 2 +-
5 files changed, 25 insertions(+), 10 deletions(-)
Index: lvm2/lib/metadata/lv_manip.c
===================================================================
--- lvm2.orig/lib/metadata/lv_manip.c
+++ lvm2/lib/metadata/lv_manip.c
@@ -2306,16 +2306,25 @@ int lv_remove_single(struct cmd_context
* remove LVs with its dependencies - LV leaf nodes should be removed first
*/
int lv_remove_with_dependencies(struct cmd_context *cmd, struct logical_volume *lv,
- const force_t force)
+ const force_t force, unsigned level)
{
struct dm_list *snh, *snht;
- if (lv_is_origin(lv)) {
+ if (lv_is_cow(lv)) {
+ /* A merging snapshot cannot be removed directly */
+ if (lv_is_merging_cow(lv) && !level) {
+ log_error("Can't remove merging snapshot logical volume \"%s\"",
+ lv->name);
+ return 0;
+ }
+ }
+
+ if (lv_is_origin(lv)) {
/* remove snapshot LVs first */
dm_list_iterate_safe(snh, snht, &lv->snapshot_segs) {
if (!lv_remove_with_dependencies(cmd, dm_list_struct_base(snh, struct lv_segment,
- origin_list)->cow,
- force))
+ origin_list)->cow,
+ force, level + 1))
return 0;
}
}
Index: lvm2/test/t-snapshot-merge.sh
===================================================================
--- lvm2.orig/test/t-snapshot-merge.sh
+++ lvm2/test/t-snapshot-merge.sh
@@ -44,17 +44,22 @@ setup_merge() {
aux prepare_vg 1 100
-# full merge of a single LV
+# test full merge of a single LV
setup_merge $vg $lv1
-
# now that snapshot LV is created: test if snapshot-merge target is available
$(dmsetup targets | grep -q snapshot-merge) || exit 200
-
lvs -a
lvconvert --merge $vg/$(snap_lv_name_ $lv1)
lvremove -f $vg/$lv1
+# test that an actively merging snapshot may not be removed
+setup_merge $vg $lv1
+lvconvert -i+100 --merge --background $vg/$(snap_lv_name_ $lv1)
+not lvremove -f $vg/$(snap_lv_name_ $lv1)
+lvremove -f $vg/$lv1
+
+
# "onactivate merge" test
setup_merge $vg $lv1
lvs -a
@@ -99,6 +104,7 @@ lvs -a
lvconvert --merge $vg/$(snap_lv_name_ $lv1)
lvremove -f $vg/$lv1
+
# test merging multiple snapshots that share the same tag
setup_merge $vg $lv1
setup_merge $vg $lv2
Index: lvm2/lib/metadata/metadata-exported.h
===================================================================
--- lvm2.orig/lib/metadata/metadata-exported.h
+++ lvm2/lib/metadata/metadata-exported.h
@@ -534,7 +534,7 @@ int lv_remove_single(struct cmd_context
force_t force);
int lv_remove_with_dependencies(struct cmd_context *cmd, struct logical_volume *lv,
- force_t force);
+ force_t force, unsigned level);
int lv_rename(struct cmd_context *cmd, struct logical_volume *lv,
const char *new_name);
Index: lvm2/lib/metadata/metadata.c
===================================================================
--- lvm2.orig/lib/metadata/metadata.c
+++ lvm2/lib/metadata/metadata.c
@@ -484,7 +484,7 @@ int remove_lvs_in_vg(struct cmd_context
while ((lst = dm_list_first(&vg->lvs))) {
lvl = dm_list_item(lst, struct lv_list);
- if (!lv_remove_with_dependencies(cmd, lvl->lv, force))
+ if (!lv_remove_with_dependencies(cmd, lvl->lv, force, 0))
return 0;
}
Index: lvm2/tools/lvremove.c
===================================================================
--- lvm2.orig/tools/lvremove.c
+++ lvm2/tools/lvremove.c
@@ -26,7 +26,7 @@ static int lvremove_single(struct cmd_co
if (lv_is_cow(lv) && lv_is_virtual_origin(origin = origin_from_cow(lv)))
lv = origin;
- if (!lv_remove_with_dependencies(cmd, lv, arg_count(cmd, force_ARG))) {
+ if (!lv_remove_with_dependencies(cmd, lv, arg_count(cmd, force_ARG), 0)) {
stack;
return ECMD_FAILED;
}
prev parent reply other threads:[~2010-04-23 14:48 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-07 20:04 LVM2 ./WHATS_NEW ./WHATS_NEW_DM libdm/libdm-de agk
2010-04-07 22:31 ` Petr Rockai
2010-04-07 23:00 ` Alasdair G Kergon
2010-04-19 22:38 ` Mike Snitzer
2010-04-19 23:25 ` Alasdair G Kergon
2010-04-21 6:35 ` Mike Snitzer
2010-04-21 17:59 ` [PATCH] conditionally avoid preloading the origin in vg_remove_snapshot Mike Snitzer
2010-04-23 0:11 ` Alasdair G Kergon
2010-04-21 19:38 ` Allowing an actively merging snapshot to be removed? Mike Snitzer
2010-04-22 0:01 ` Mikulas Patocka
2010-04-22 23:36 ` Alasdair G Kergon
2010-04-23 14:48 ` Mike Snitzer [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20100423144808.GA21974@redhat.com \
--to=snitzer@redhat.com \
--cc=lvm-devel@redhat.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.