[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[lvm-devel] [PATCH lvconvert 3/11] Multiple fixes about to-corelog conversion



_remove_mirror_images() is the core function used for both
mirror image removal and mirror log removal.
However, it has some bugs and to-corelog conversion didn't work
at all.

The patches fixes them at once.
Please see the patch header for detailed information about
each bug.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
The new lvconvert has multiple defects which prevent to-corelog conversion.

  - _remove_mirror_images() did nothing if num_removed == 0
    and removable_pvs != NULL, while it should remove log if remove_log == 1.
      * It checks 'old_area_count - new_area_count == num_removed' at
        the end of the new_area_count calculation loop.
        It should be checked at the first for the case 'num_removed == 0'.
      * It errors if old_area_count == new_area_count. However, it is
        not an error in case of num_removed == 0.

  - _remove_mirror_images() didn't set seg->log_lv NULL,
    if num_removed == 0 and remove_log == 1.

  - remove_mirror_images() did nothing if num_removed == 0.
    It was checking 'num_removed' to see how many
    mimage LVs it should remove and finishing if it's 0.
    However, the function can be used for just removing log_lv,
    where 'num_removed' is already 0 on entry.

  - lvconvert should check '--corelog' option as well as '--mirrorlog'
    to determine whether the intent of lvconvert is just to (re)start
    polldaemon.

  - lvconvert should pass NULL as 'removable_pvs' to remove_mirror_log(),
    instead of an empty list, if no PVs are specified on command line.
    This is a preventive fix because remove_mirror_images() currently
    ignores 'removable_pvs' for log_lv removal.

  - lvconvert should pass 'log_count = 1U' to lv_remove_mirrors()
    if '--corelog' or '--mirrorlog core' is specified.

Index: LVM2.work/lib/metadata/mirror.c
===================================================================
--- LVM2.work.orig/lib/metadata/mirror.c
+++ LVM2.work/lib/metadata/mirror.c
@@ -164,7 +164,8 @@ static int _remove_mirror_images(struct 
 
 	/* Move removable_pvs to end of array */
 	if (removable_pvs) {
-		for (s = 0; s < mirrored_seg->area_count; s++) {
+		for (s = 0; s < mirrored_seg->area_count &&
+			    old_area_count - new_area_count < num_removed; s++) {
 			all_pvs_removable = 1;
 			sub_lv = seg_lv(mirrored_seg, s);
 			list_iterate_items(seg, &sub_lv->segments) {
@@ -197,11 +198,8 @@ static int _remove_mirror_images(struct 
 				mirrored_seg->areas[new_area_count] = mirrored_seg->areas[s];
 				mirrored_seg->areas[s] = area;
 			}
-			/* Found enough matches? */
-			if (old_area_count - new_area_count == num_removed)
-				break;
 		}
-		if (old_area_count == new_area_count) {
+		if (num_removed && old_area_count == new_area_count) {
 			log_error("No mirror images found using specified PVs.");
 			return 0;
 		}
@@ -235,7 +233,8 @@ static int _remove_mirror_images(struct 
 		lv->status &= ~MIRRORED;
 		lv->status &= ~MIRROR_NOTSYNCED;
 		remove_log = 1;
-	}
+	} else if (remove_log)
+		mirrored_seg->log_lv = NULL;
 
 	if (remove_log && log_lv) {
 		log_lv->status &= ~MIRROR_LOG;
@@ -302,7 +301,8 @@ int remove_mirror_images(struct logical_
 
 	num_removed = existing_mirrors - num_mirrors;
 
-	while (num_removed) {
+	/* num_removed can be 0 if the function is called just to remove log */
+	do {
 		if (num_removed < first_seg(lv)->area_count)
 			removed_once = num_removed;
 		else
@@ -313,7 +313,7 @@ int remove_mirror_images(struct logical_
 			return_0;
 
 		num_removed -= removed_once;
-	}
+	} while (num_removed);
 
 	return 1;
 }
Index: LVM2.work/tools/lvconvert.c
===================================================================
--- LVM2.work.orig/tools/lvconvert.c
+++ LVM2.work/tools/lvconvert.c
@@ -370,7 +370,8 @@ static int lvconvert_mirrors(struct cmd_
 	existing_mirrors = lv_mirror_count(lv);
 
 	/* If called with no argument, try collapsing the resync layers */
-	if (!arg_count(cmd, mirrors_ARG) && !arg_count(cmd, mirrorlog_ARG)) {
+	if (!arg_count(cmd, mirrors_ARG) && !arg_count(cmd, mirrorlog_ARG) &&
+	    !arg_count(cmd, corelog_ARG)) {
 		lp->wait_daemon = 1;
 		return 1;
 	}
@@ -496,7 +497,8 @@ static int lvconvert_mirrors(struct cmd_
 					    lp->pvh, lp->alloc))
 				return_0;
 		} else if (seg->log_lv && corelog) {
-			if (!remove_mirror_log(cmd, lv, lp->pvh))
+			if (!remove_mirror_log(cmd, lv,
+					       lp->pv_count ? lp->pvh : NULL))
 				return_0;
 		} else {
 			/* No change */
@@ -529,7 +531,8 @@ static int lvconvert_mirrors(struct cmd_
 	} else {
 		/* Reduce number of mirrors */
 		if (!lv_remove_mirrors(cmd, lv, existing_mirrors - lp->mirrors,
-				       0, lp->pv_count ? lp->pvh : NULL, 0))
+				       corelog ? 1U : 0U,
+				       lp->pv_count ? lp->pvh : NULL, 0))
 			return_0;
 	}
 

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]