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

[lvm-devel] 'Resuming before suspend' problem


Attached is a patch to workaround the problem that resuming
of a inserted layer occurs before the suspending of the
original mirror.

I would like to hear comments about how to fix this problem:

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
    It creates a window where both the parent LV and the layer LV
    have the same active table.

For example, in case of adding a mirror,

$ lvconvert -m+1 -vvvv vg/lvol0 2>&1 | egrep 'Suspend|Resum|Load'
#libdm-deptree.c:1463     Loading vg-lvol0_mimage_1 table
#libdm-deptree.c:1463     Loading vg-lvol0_mimage_0 table
#libdm-deptree.c:1463     Loading vg-lvol0_mlog table
#libdm-deptree.c:1463     Loading vg-lvol0_mimagetmp_2 table
#libdm-deptree.c:897     Resuming vg-lvol0_mimagetmp_2 (253:5)
#libdm-deptree.c:1463     Loading vg-lvol0_mimage_2 table
#libdm-deptree.c:897     Resuming vg-lvol0_mimage_2 (253:6)
#libdm-deptree.c:1463     Loading vg-lvol0 table
#libdm-deptree.c:940     Suspending vg-lvol0 (253:4)
#libdm-deptree.c:940     Suspending vg-lvol0_mimage_1 (253:3)
#libdm-deptree.c:940     Suspending vg-lvol0_mimage_0 (253:2)
#libdm-deptree.c:940     Suspending vg-lvol0_mlog (253:1)

lvol0_mimagetmp_2 is supposed to have the same table as
lvol0 had. lvol0 will be updated to mirror lvol0_mimagetmp_2
and lvol0_mimage_2.
However, lvol0_mimagetmp_2 is resumed before lvol0 is suspended.

The problem is not only for mirror.
For example, adding snapshot has the similar window:

$ lvcreate -l1 -s -vvvv vg/lvol0 2>&1 | egrep 'Suspend|Resum|Load' #libdm-deptree.c:1463     Loading vg-lvol1 table
#libdm-deptree.c:897     Resuming vg-lvol1 (253:2)
#libdm-deptree.c:1463     Loading vg-lvol0-real table
#libdm-deptree.c:897     Resuming vg-lvol0-real (253:3)
#libdm-deptree.c:1463     Loading vg-lvol0 table
#libdm-deptree.c:1463     Loading vg-lvol1-cow table
#libdm-deptree.c:897     Resuming vg-lvol1-cow (253:4)
#libdm-deptree.c:1463     Loading vg-lvol1 table
#libdm-deptree.c:940     Suspending vg-lvol0 (253:1) with filesystem sync without device flush
#libdm-deptree.c:940     Suspending vg-lvol0-real (253:3) with filesystem sync without device flush

But it might not be a big problem because the mapping is linear.

In case of mirror, the resuming has a side effect (e.g. synching).
So it would be a problem.

Jun'ichi Nomura, NEC Corporation of America
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,8 @@ 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;
 	seg = first_seg(lv);
 	existing_mirrors = lv_mirror_count(lv);
@@ -599,6 +600,19 @@ commit_changes:
 	log_very_verbose("Updating \"%s\" in kernel", lv->name);
+	/* FIXME: see dev_manager_preload() */
+	if ((lv->status | CONVERTING)) {
+		if (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]