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

Re: [lvm-devel] 'Resuming before suspend' problem



Jun'ichi Nomura wrote:
> Attached is a patch to workaround the problem that resuming
> of a inserted layer occurs before the suspending of the
> original mirror.

This is an updated version of the workaround patch.
The activation of layer LV should be only done
when the layer is inserted.
That's what pvmove code currently does.
(lv->status | CONVERTING) could be true if the layer
has already been inserted.

Thanks,
-- 
Jun'ichi Nomura, NEC Corporation of America
Current status:
  - suspend_lv() in LVM2 preloads dm tables before suspending.
    It's necessary because the table loading could require memory
    while kernel may not able to allocate memory without writing
    to the device.
  - The preloading involves resuming, if the device has a parent
    node in the deptree.
    It's necessary because, for example, the new device size needs
    to be visible to load the parent table.
  - Preloading is bottom-up. Suspending is top-down.

What's the problem:
  - If a layer LV is inserted, suspend_lv() after that will
    resume the layer LV temporarily before the parent LV is
    suspended.
    It creates a window where both the parent LV and the layer LV
    have the same active table.

A workaround:
  - Don't preload (and resume) in suspend_lv() if a layer LV is just inserted.
    Instead, the caller should activate the inserted layer LV by itself
    after suspend_lv() to build sub-tree in kernel so that the following
    resume_lv() can properly load tables.

Index: LVM2.work/lib/activate/dev_manager.c
===================================================================
--- LVM2.work.orig/lib/activate/dev_manager.c
+++ LVM2.work/lib/activate/dev_manager.c
@@ -1141,6 +1141,14 @@ int dev_manager_preload(struct dev_manag
 	if ((lv->status & PVMOVE) || (lv->status & LOCKED))
 		return 1;
 
+	/* FIXME:
+	 * Preloading involves activation of the inserted layer LV.
+	 * As a result, there is a window that 2 active dm devices having
+	 * the same table.
+	 * So skip preloading for now and let the caller cope with that. */
+	if ((lv->status & CONVERTING))
+		return 1;
+
 	return _tree_action(dm, lv, PRELOAD);
 }
 
Index: LVM2.work/tools/lvconvert.c
===================================================================
--- LVM2.work.orig/tools/lvconvert.c
+++ LVM2.work/tools/lvconvert.c
@@ -382,7 +382,9 @@ static int lvconvert_mirrors(struct cmd_
 	uint32_t existing_mirrors;
 	const char *mirrorlog;
 	unsigned corelog = 0;
-	struct logical_volume *original_lv;
+	struct logical_volume *original_lv, *layer_lv;
+	struct lvinfo info;
+	int layer_inserted = 0;
 
 	seg = first_seg(lv);
 	existing_mirrors = lv_mirror_count(lv);
@@ -555,9 +557,12 @@ static int lvconvert_mirrors(struct cmd_
 		}
 		/* Insert a temporary layer for syncing,
 		 * only if the original lv is using disk log. */
-		if (seg->log_lv && !_insert_lvconvert_layer(cmd, lv)) {
-			log_error("Failed to insert resync layer");
-			return 0;
+		if (seg->log_lv) {
+			if (!_insert_lvconvert_layer(cmd, lv)) {
+				log_error("Failed to insert resync layer");
+				return 0;
+			}
+			layer_inserted = 1;
 		}
 		/* FIXME: can't have multiple mlogs. force corelog. */
 		if (!lv_add_mirrors(cmd, lv, lp->mirrors - existing_mirrors, 1,
@@ -599,6 +604,18 @@ commit_changes:
 
 	log_very_verbose("Updating \"%s\" in kernel", lv->name);
 
+	/* FIXME: see dev_manager_preload() */
+	if (layer_inserted &&
+	    lv_info(cmd, lv, &info, 1, 0) && info.exists &&
+	    (layer_lv = find_temporary_mirror(lv))) {
+		if (!activate_lv(cmd, layer_lv)) {
+			log_error("Failed to activate temporary mirror %s",
+				  layer_lv->name);
+			log_error("Not resuming the LV %s", lv->name);
+			return 0;
+		}
+	}
+
 	if (!resume_lv(cmd, lv)) {
 		log_error("Problem reactivating %s", lv->name);
 		return 0;

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