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.

diff --git a/WHATS_NEW b/WHATS_NEW
index f8d74e5304c4714dac0bbf62ed4c8281a6dd35a4..2d98cdb6c2ba55d50cb0377ba9a3d8cec4519e27 100644 (file)
--- 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 5b05eeb9f06a935ec1b1838bd3558ad0d4636302..12f1b323331861651a70f07c427776ff80a4ffd4 100755 (executable)
--- 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 3c11b0d748ca986a6b12dcb80137848111837efb..c0838893418c8a12d4fa42bdae83326e2ccf6985 100644 (file)
--- 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;
                        }
-               }
 
-               /* Move this snapshot */
-               list_del(lvh);
-               list_add(&vg_to->lvs, lvh);
+                       /*
+                        * 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++;
+                       }
+               }
 
-               vg_from->snapshot_count--;
-               vg_to->snapshot_count++;
        }
 
        return 1;