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

[lvm-devel] [PATCH lvconvert 4/11] Fix possible mirror image corruption



collapse_mirrored_lv() is a function to move mimage LVs from
temporary sync layer to the original LV after sync completion.
However, the code did resume_lv between the removal of mimage LVs
and addition of them. As a result, it's possible to make the
mimage LVs out-of-sync while mirror log says in-sync.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
Fix the inverted order of dm table update and mimage LVs addition,
which could corrupt mimage LVs.

collapse_mirrored_lv() moves mimage LVs in temporary layer to
the original LV.
The removal from the temporary layer and addition to the original LV
should be done atomically in terms of table replacement
(i.e. should be done in the single suspend-resume set).
Otherwise, the moving mimage LVs could become out-of-sync between
the 1st resume and the 2nd suspend and corrupt the mirror.

This patch moves the execution of _merge_mirror_images() inside
the _remove_mirror_images(), before the 1st table update occurs.
(The table update is necessary for deleting orphan LVs.)
'collapse' parameter of _remove_mirror_images() controls whether
the removed mimage LVs will be merged or deleted.

When collapsing the LV,

  Before this patch:
     1. removing mimage LVs from temporary layer
     2. removing temporary layer from the original LV
     3. update kernel dm table
    *4. adding mimage LVs to the original LV (_merge_mirror_images)
     5. deleting temporary layer
     6. update kernel dm table

  After this patch:
     1. removing mimage LVs from temporary layer
     2. removing temporary layer from the original LV
    *3. adding mimage LVs to the original LV (_merge_mirror_images)
     4. update kernel dm table
     5. deleting temporary layer
     6. update kernel dm table

Index: LVM2.work/lib/metadata/mirror.c
===================================================================
--- LVM2.work.orig/lib/metadata/mirror.c
+++ LVM2.work/lib/metadata/mirror.c
@@ -134,13 +134,34 @@ static int _delete_lv(struct logical_vol
 	return 1;
 }
 
+static int _merge_mirror_images(struct logical_volume *lv,
+				const struct list *mimages)
+{
+	uint32_t addition = list_size(mimages);
+	struct logical_volume **img_lvs;
+	struct lv_list *lvl;
+	int i = 0;
+
+	if (!addition)
+		return 1;
+
+	if (!(img_lvs = alloca(sizeof(*img_lvs) * addition)))
+		return_0;
+
+	list_iterate_items(lvl, mimages)
+		img_lvs[i++] = lvl->lv;
+
+	return lv_add_mirror_lvs(lv, img_lvs, addition,
+				 MIRROR_IMAGE, first_seg(lv)->region_size);
+}
+
 /*
  * Remove num_removed images from mirrored_seg
  */
 static int _remove_mirror_images(struct logical_volume *lv,
 				 uint32_t num_removed,
 				 struct list *removable_pvs,
-				 unsigned remove_log, struct list *orphan_lvs)
+				 unsigned remove_log, unsigned collapse)
 {
 	uint32_t m;
 	uint32_t s, s1;
@@ -162,6 +183,12 @@ static int _remove_mirror_images(struct 
 			 old_area_count, old_area_count - num_removed,
 			 remove_log ? " and no log volume" : "");
 
+	if (collapse &&
+	    (removable_pvs || (old_area_count - num_removed != 1))) {
+		log_error("Incompatible parameters to _remove_mirror_images");
+		return 0;
+	}
+
 	/* Move removable_pvs to end of array */
 	if (removable_pvs) {
 		for (s = 0; s < mirrored_seg->area_count &&
@@ -233,6 +260,10 @@ static int _remove_mirror_images(struct 
 		lv->status &= ~MIRRORED;
 		lv->status &= ~MIRROR_NOTSYNCED;
 		remove_log = 1;
+		if (collapse && !_merge_mirror_images(lv, &tmp_orphan_lvs)) {
+			log_error("Failed to add mirror images");
+			return 0;
+		}
 	} else if (remove_log)
 		mirrored_seg->log_lv = NULL;
 
@@ -271,11 +302,7 @@ static int _remove_mirror_images(struct 
 	}
 
 	/* Save or delete the 'orphan' LVs */
-	if (orphan_lvs) {
-		*orphan_lvs = tmp_orphan_lvs;
-		orphan_lvs->n->p = orphan_lvs;
-		orphan_lvs->p->n = orphan_lvs;
-	} else {
+	if (!collapse) {
 		list_iterate_items(lvl, &tmp_orphan_lvs)
 			if (!_delete_lv(lv, lvl->lv))
 				return 0;
@@ -309,7 +336,7 @@ int remove_mirror_images(struct logical_
 			removed_once = first_seg(lv)->area_count - 1;
 
 		if (!_remove_mirror_images(lv, removed_once,
-				           removable_pvs, remove_log, NULL))
+				           removable_pvs, remove_log, 0))
 			return_0;
 
 		num_removed -= removed_once;
@@ -334,27 +361,6 @@ static int _mirrored_lv_in_sync(struct l
 	return 0;
 }
 
-static int _merge_mirror_images(struct logical_volume *lv,
-				const struct list *mimages)
-{
-	uint32_t addition = list_size(mimages);
-	struct logical_volume **img_lvs;
-	struct lv_list *lvl;
-	int i = 0;
-
-	if (!addition)
-		return 1;
-
-	if (!(img_lvs = alloca(sizeof(*img_lvs) * addition)))
-		return_0;
-
-	list_iterate_items(lvl, mimages)
-		img_lvs[i++] = lvl->lv;
-
-	return lv_add_mirror_lvs(lv, img_lvs, addition,
-				 MIRROR_IMAGE, first_seg(lv)->region_size);
-}
-
 /*
  * Return a temporary LV for resyncing added mirror image.
  * Add other mirror legs to lvs list.
@@ -390,7 +396,6 @@ static struct logical_volume *_find_tmp_
 int collapse_mirrored_lv(struct logical_volume *lv)
 {
 	struct logical_volume *tmp_lv, *parent_lv;
-	struct list lvlist;
 
 	while ((tmp_lv = _find_tmp_mirror(lv))) {
 		parent_lv = find_parent_for_layer(lv, tmp_lv);
@@ -400,18 +405,12 @@ int collapse_mirrored_lv(struct logical_
 			return 1;
 		}
 
-		list_init(&lvlist);
 		if (!_remove_mirror_images(parent_lv,
 					   first_seg(parent_lv)->area_count - 1,
-					   NULL, 1, &lvlist)) {
+					   NULL, 1, 1)) {
 			log_error("Failed to release mirror images");
 			return 0;
 		}
-
-		if (!_merge_mirror_images(parent_lv, &lvlist)) {
-			log_error("Failed to add mirror images");
-			return 0;
-		}
 	}
 
 	return 1;

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