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

[lvm-devel] [PATCH] Fix mirror -> mirror up-converts



Please refer to the following previous threads for history on this
subject:
https://www.redhat.com/archives/lvm-devel/2008-July/msg00026.html
https://www.redhat.com/archives/lvm-devel/2008-February/msg00007.html

Associated Bug:
https://bugzilla.redhat.com/show_bug.cgi?id=455670
(potentially others, including 460222)

The main issue is that with the current code, it is possible to activate
2 device-mapper tables that evaluate to the same thing... meaning you
can have 2 uncoordinated recovery processes happening at the same time
in conjunction with I/O streams.  (I.e.  corruption is a distinct
possibility.)  The attached patches prevent this "double activation".

There are 2 patches attached - one device-mapper patch adding the
'preload, but don't resume' function and another with the required LVM
changes.  (Please note that I've disallowed the resizing of LVs while
they have the 'CONVERTING' flag.)

 brassow


Index: device-mapper/lib/libdevmapper.h
===================================================================
--- device-mapper.orig/lib/libdevmapper.h
+++ device-mapper/lib/libdevmapper.h
@@ -311,6 +311,14 @@ int dm_tree_preload_children(struct dm_t
 			     size_t uuid_prefix_len);
 
 /*
+ * Preload, but do not resume, device plus dependencies.
+ * Ignores devices that don't have a uuid starting with uuid_prefix.
+ */
+int dm_tree_preload_children_noresume(struct dm_tree_node *dnode,
+				      const char *uuid_prefix,
+				      size_t uuid_prefix_len);
+
+/*
  * Resume a device plus all dependencies.
  * Ignores devices that don't have a uuid starting with uuid_prefix.
  */
Index: device-mapper/lib/libdm-deptree.c
===================================================================
--- device-mapper.orig/lib/libdm-deptree.c
+++ device-mapper/lib/libdm-deptree.c
@@ -1504,9 +1504,10 @@ out:
 	return r;
 }
 
-int dm_tree_preload_children(struct dm_tree_node *dnode,
-				 const char *uuid_prefix,
-				 size_t uuid_prefix_len)
+static int _dm_tree_preload_children(struct dm_tree_node *dnode,
+				     const char *uuid_prefix,
+				     size_t uuid_prefix_len,
+				     int do_resume)
 {
 	void *handle = NULL;
 	struct dm_tree_node *child;
@@ -1524,7 +1525,8 @@ int dm_tree_preload_children(struct dm_t
 			continue;
 
 		if (dm_tree_node_num_children(child, 0))
-			dm_tree_preload_children(child, uuid_prefix, uuid_prefix_len);
+			_dm_tree_preload_children(child, uuid_prefix,
+						  uuid_prefix_len, do_resume);
 
 		/* FIXME Cope if name exists with no uuid? */
 		if (!child->info.exists) {
@@ -1541,6 +1543,9 @@ int dm_tree_preload_children(struct dm_t
 			}
 		}
 
+		if (!do_resume)
+			continue;
+
 		/* Resume device immediately if it has parents */
 		if (!dm_tree_node_num_children(child, 1))
 			continue;
@@ -1566,6 +1571,22 @@ int dm_tree_preload_children(struct dm_t
 	return 1;
 }
 
+int dm_tree_preload_children(struct dm_tree_node *dnode,
+			     const char *uuid_prefix,
+			     size_t uuid_prefix_len)
+{
+	return _dm_tree_preload_children(dnode, uuid_prefix,
+					 uuid_prefix_len, 1);
+}
+
+int dm_tree_preload_children_noresume(struct dm_tree_node *dnode,
+				      const char *uuid_prefix,
+				      size_t uuid_prefix_len)
+{
+	return _dm_tree_preload_children(dnode, uuid_prefix,
+					 uuid_prefix_len, 0);
+}
+
 /*
  * Returns 1 if unsure.
  */
Index: device-mapper/lib/.exported_symbols
===================================================================
--- device-mapper.orig/lib/.exported_symbols
+++ device-mapper/lib/.exported_symbols
@@ -58,6 +58,7 @@ dm_tree_next_parent
 dm_tree_deactivate_children
 dm_tree_activate_children
 dm_tree_preload_children
+dm_tree_preload_children_noresume
 dm_tree_suspend_children
 dm_tree_children_use_uuid
 dm_tree_node_add_snapshot_origin_target
Index: LVM2/lib/metadata/mirror.c
===================================================================
--- LVM2.orig/lib/metadata/mirror.c
+++ LVM2/lib/metadata/mirror.c
@@ -598,6 +598,22 @@ static int _remove_mirror_images(struct 
 
 	log_very_verbose("Updating \"%s\" in kernel", mirrored_seg->lv->name);
 
+	/*
+	 * Activate the removed layer LV (lv1) first.
+	 * FIXME: generic stacking support should handle this
+	 *
+	 * remove_layer_from_lv() will fill lv1 by error segments.
+	 * However, since lv1 is no longer a part of the mirrored_seg->lv,
+	 * the new dm table ("error" mapping) for lv1 will not be loaded
+	 * until it's explicitly activated.
+	 * I.e. resume_lv(mirrored_seg->lv) below will end up creating
+	 * 2 mirrros sharing the same sub LVs temporarily.
+	 */
+	if (lv1 && !activate_lv(lv1->vg->cmd, lv1)) {
+		log_error("Problem reactivating removed %s", lv1->name);
+		return 0;
+	}
+
 	if (!resume_lv(mirrored_seg->lv->vg->cmd, mirrored_seg->lv)) {
 		log_error("Problem reactivating %s", mirrored_seg->lv->name);
 		return 0;
Index: LVM2/lib/activate/dev_manager.c
===================================================================
--- LVM2.orig/lib/activate/dev_manager.c
+++ LVM2/lib/activate/dev_manager.c
@@ -1084,8 +1084,15 @@ static int _tree_action(struct dev_manag
 			goto_out;
 
 		/* Preload any devices required before any suspensions */
-		if (!dm_tree_preload_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1))
-			goto_out;
+		if (strstr(lv->name, "_mimagetmp")) {
+			if (!dm_tree_preload_children_noresume(root, dlid,
+							       ID_LEN + sizeof(UUID_PREFIX) - 1))
+				goto_out;
+		} else {
+			if (!dm_tree_preload_children(root, dlid,
+						      ID_LEN + sizeof(UUID_PREFIX) - 1))
+				goto_out;
+		}
 
 		if ((action == ACTIVATE) &&
 		    !dm_tree_activate_children(root, dlid, ID_LEN + sizeof(UUID_PREFIX) - 1))
Index: LVM2/tools/lvresize.c
===================================================================
--- LVM2.orig/tools/lvresize.c
+++ LVM2/tools/lvresize.c
@@ -308,6 +308,11 @@ static int _lvresize(struct cmd_context 
 		return ECMD_FAILED;
 	}
 
+	if (lv->status & CONVERTING) {
+		log_error("Can't resize %s while it is being converted", lv->name);
+		return ECMD_FAILED;
+	}
+
 	alloc = arg_uint_value(cmd, alloc_ARG, lv->alloc);
 
 	if (lp->size) {

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