From eec663aa3bea9ecdc393b506eaa1111876b6dce4 Mon Sep 17 00:00:00 2001 From: Dave Wysochanski Date: Wed, 9 Apr 2008 20:56:06 +0000 Subject: [PATCH] Fix vgsplit to only move hidden 'snapshotN' LVs when necessary. This bug has been around for a long time as far as I can tell. Without this fix, a vgsplit would unconditionally move the 'hidden/internal' snapshot LVs, and result in corrupted metadata in the following case: vg1: contains lv1, lv1snap, both on pvset1 vg1: contains lv2, on pvset2 "vgsplit vg1 vg2 pvset2" would result in "snapshot0" hidden LV being moved to vg2, and the origin and cow being left in vg1. The tools detect the corruption in vg2, but not in vg1. --- WHATS_NEW | 1 + test/t-vgsplit-operation.sh | 29 +++++++++++++++++++++++++++++ tools/vgsplit.c | 22 ++++++++++++++++------ 3 files changed, 46 insertions(+), 6 deletions(-) diff --git a/WHATS_NEW b/WHATS_NEW index f8d74e530..2d98cdb6c 100644 --- a/WHATS_NEW +++ b/WHATS_NEW @@ -1,5 +1,6 @@ Version 2.02.34 - =================================== + Fix vgsplit to only move hidden 'snapshotN' LVs when necessary. Update vgsplit tests for lvnames on the cmdline. Update vgsplit man page to reflect lvnames on the cmdline. Update vgsplit to take "-n LogicalVolumeName" on the cmdline. diff --git a/test/t-vgsplit-operation.sh b/test/t-vgsplit-operation.sh index 5b05eeb9f..12f1b3233 100755 --- a/test/t-vgsplit-operation.sh +++ b/test/t-vgsplit-operation.sh @@ -149,6 +149,35 @@ test_expect_success \ lvremove -f $vg2/$lv1 && vgremove -f $vg2' +test_expect_success \ + "vgsplit correctly splits linear LV but not snap+origin LV into $i VG ($j args)" \ + 'vgcreate $vg1 $d1 $d2 && + if [ $i == existing ]; then + vgcreate $vg2 $d3 + fi && + lvcreate -l 64 -i 2 -n $lv1 $vg1 && + lvcreate -l 4 -i 2 -s -n $lv2 $vg1/$lv1 && + vgextend $vg1 $d4 && + lvcreate -l 64 -n $lv3 $vg1 $d4 && + vgchange -an $vg1 && + if [ $j == PV ]; then + vgsplit $vg1 $vg2 $d4 + else + vgsplit -n $lv3 $vg1 $vg2 + fi && + if [ $i == existing ]; then + vg_validate_pvlv_counts_ $vg2 2 1 0 + vg_validate_pvlv_counts_ $vg1 2 1 1 + else + vg_validate_pvlv_counts_ $vg2 1 1 0 + vg_validate_pvlv_counts_ $vg1 2 1 1 + fi && + lvremove -f $vg1/$lv2 && + lvremove -f $vg1/$lv1 && + lvremove -f $vg2/$lv3 && + vgremove -f $vg1 && + vgremove -f $vg2' + done done diff --git a/tools/vgsplit.c b/tools/vgsplit.c index 3c11b0d74..c08388934 100644 --- a/tools/vgsplit.c +++ b/tools/vgsplit.c @@ -164,6 +164,9 @@ static int _move_lvs(struct volume_group *vg_from, struct volume_group *vg_to) return 1; } +/* + * Move the hidden / internal "snapshotN" LVs.from 'vg_from' to 'vg_to'. + */ static int _move_snapshots(struct volume_group *vg_from, struct volume_group *vg_to) { @@ -191,14 +194,21 @@ static int _move_snapshots(struct volume_group *vg_from, " two Volume Groups", seg->cow->name); return 0; } + + /* + * At this point, the cow and origin should already be + * in vg_to. + */ + if (_lv_is_in_vg(vg_to, seg->cow) && + _lv_is_in_vg(vg_to, seg->origin)) { + list_del(lvh); + list_add(&vg_to->lvs, lvh); + + vg_from->snapshot_count--; + vg_to->snapshot_count++; + } } - /* Move this snapshot */ - list_del(lvh); - list_add(&vg_to->lvs, lvh); - - vg_from->snapshot_count--; - vg_to->snapshot_count++; } return 1;