[lvm-devel] [PATCH] Specified PVs being ignored when performing mirror split (repost)

[Jonathan Brassow]

Thanks to Taka for his suggestions for improvement...
 brassow

Fix for bug 619221 - log device splitting regression

An incorrect fix an annoyance has caused a regression.  The offending
check-in was: 
http://sources.redhat.com/cgi-bin/cvsweb.cgi/LVM2/tools/lvconvert.c?cvsroot=lvm2&f=h#rev1.140

This check-in caused any PVs specified on the command line to be
ignored when performing a mirror split.

This patch reverses the aforementioned check-in (solving the regressions)
and posits a new solution to the list reversal problem.  The original
problem was that we would always take the lowest mimage LVs from a mirror
when performing a split, but what we really want is to take the highest
mimage LVs.  This patch accomplishes that by working through the list in
reverse order - choosing the higher numbered mimages first.  (This also
reduces the amount of processing necessary.)


Index: LVM2/lib/metadata/mirror.c
===================================================================
--- LVM2.orig/lib/metadata/mirror.c
+++ LVM2/lib/metadata/mirror.c
@@ -524,34 +524,22 @@ static int _move_removable_mimages_to_en
 					  uint32_t count,
 					  struct dm_list *removable_pvs)
 {
-	int i, images;
+	int i;
 	struct logical_volume *sub_lv;
 	struct lv_segment *mirrored_seg = first_seg(lv);
 
 	if (!removable_pvs)
 		return 1;
 
-	/*
-	 * When we shift an image to the end, we must start from
-	 * the begining of the list again.  We must visit the
-	 * images up to the last one we just moved.
-	 */
-	for (images = mirrored_seg->area_count; images && count; images--) {
-		for (i = 0; i < images; i++) {
-			sub_lv = seg_lv(mirrored_seg, i);
-
-			if (!is_temporary_mirror_layer(sub_lv) &&
-			    is_mirror_image_removable(sub_lv, removable_pvs)) {
-				if (!shift_mirror_images(mirrored_seg, i))
-					return_0;
-				count--;
-				break;
-			}
-		}
+	for (i = mirrored_seg->area_count - 1; (i >= 0) && count; i--) {
+		sub_lv = seg_lv(mirrored_seg, i);
 
-		/* Did we shift any images? */
-		if (i == images)
-			return 0;
+		if (!is_temporary_mirror_layer(sub_lv) &&
+		    is_mirror_image_removable(sub_lv, removable_pvs)) {
+			if (!shift_mirror_images(mirrored_seg, i))
+				return_0;
+			count--;
+		}
 	}
 
 	return !count;
Index: LVM2/tools/lvconvert.c
===================================================================
--- LVM2.orig/tools/lvconvert.c
+++ LVM2/tools/lvconvert.c
@@ -1015,7 +1015,7 @@ static int _lvconvert_mirrors_aux(struct
 						  lv->le_count,
 						  lp->region_size);
 
-	if (!operable_pvs && !lp->keep_mimages)
+	if (!operable_pvs)
 		operable_pvs = lp->pvh;
 
 	seg = first_seg(lv);