[Evms-cvs-commits] evms2/engine/plugins/snapshot snap_activate.c,1.33,1.34

[<kc...@us...>]

Update of /cvsroot/evms/evms2/engine/plugins/snapshot
In directory sc8-pr-cvs1:/tmp/cvs-serv32621

Modified Files:
	snap_activate.c 
Log Message:
Minor rewrite/cleanup to how origin volumes are suspended during snapshot
activation and deactivation. This is necessary for snapshots to work correctly
on 2.6 kernels.


Index: snap_activate.c
===================================================================
RCS file: /cvsroot/evms/evms2/engine/plugins/snapshot/snap_activate.c,v
retrieving revision 1.33
retrieving revision 1.34
diff -u -d -r1.33 -r1.34
--- snap_activate.c	2 Sep 2003 21:50:08 -0000	1.33
+++ snap_activate.c	31 Oct 2003 20:56:44 -0000	1.34
@@ -393,30 +393,114 @@
 }
 
 /**
- * suspend_origin
+ * suspend_origin_volume
+ *
+ * Suspend/resume the device for the volume on top of the origin-parent, but
+ * only if it's not the same device as the origin parent.
  **/
-static int suspend_origin(snapshot_volume_t * org_volume, int suspend)
+static int suspend_origin_volume(snapshot_volume_t * org_volume, int suspend)
 {
 	storage_object_t *parent = org_volume->parent;
 	logical_volume_t *volume = parent->volume;
+	int rc = 0;
+
+	LOG_ENTRY();
+
+	if (volume && volume->dev_major &&
+	    volume->dev_minor != parent->dev_minor) {
+		rc = EngFncs->dm_suspend_volume(volume, suspend);
+	}
+
+	LOG_EXIT_INT(rc);
+	return rc;
+}
+
+/**
+ * suspend_origin_parent
+ *
+ * Suspend/resume the device for the origin-parent.
+ **/
+static int suspend_origin_parent(snapshot_volume_t * org_volume, int suspend)
+{
 	int rc;
 
 	LOG_ENTRY();
 
-	if (suspend) {
-		EngFncs->dm_set_suspended_flag(TRUE);
+	rc = EngFncs->dm_suspend(org_volume->parent, suspend);
+
+	LOG_EXIT_INT(rc);
+	return rc;
+}
+
+/**
+ * suspend_origin
+ *
+ * Suspend both the origin parent and volume. The parent must be suspended so
+ * it can later be resumed, which is when the kernel will update the origin's
+ * chunk-size. The volume must be suspended because that is when the kernel
+ * will lock the VFS. If the parent and the volume are the same device, we'll
+ * obviously only suspend once.
+ **/
+static int suspend_origin(snapshot_volume_t * org_volume)
+{
+	int rc;
+
+	LOG_ENTRY();
+
+	EngFncs->dm_set_suspended_flag(TRUE);
+
+	rc = suspend_origin_volume(org_volume, TRUE);
+	if (rc) {
+		goto out;
 	}
 
-	if (volume->dev_major && volume->dev_minor != parent->dev_minor) {
-		rc = EngFncs->dm_suspend_volume(volume, suspend);
-	} else {
-		rc = EngFncs->dm_suspend(parent, suspend);
+	rc = suspend_origin_parent(org_volume, TRUE);
+	if (rc) {
+		suspend_origin_volume(org_volume, FALSE);
+		goto out;
 	}
 
-	if ((!suspend && !rc) || (suspend && rc)) {
+out:
+	if (rc) {
 		EngFncs->dm_set_suspended_flag(FALSE);
 	}
+	LOG_EXIT_INT(rc);
+	return rc;
+}
 
+/**
+ * resume_origin
+ *
+ * Resume both the origin parent and volume. The parent must be resumed because
+ * that is when the kernel will update the origin's chunk-size. The volume must
+ * be resumed because that is when the kernel will unlock the VFS. If the parent
+ * and the volume are the same device, we'll obviously only resume once.
+ *
+ * The ordering of resume is the opposite of suspend. Since we suspend the
+ * volume first, and then the parent, all pending I/Os are queued on the volume
+ * (and none on the parent). If we resumed the volume first, all those pending
+ * I/Os would temporarily queue up on the parent before we have a chance to
+ * resume it, which would just a waste of time.
+ **/
+static int resume_origin(snapshot_volume_t * org_volume)
+{
+	int rc = 0;
+
+	LOG_ENTRY();
+
+	rc = suspend_origin_parent(org_volume, FALSE);
+	if (rc) {
+		goto out;
+	}
+
+	rc = suspend_origin_volume(org_volume, FALSE);
+	if (rc) {
+		goto out;
+	}
+
+	EngFncs->dm_set_suspended_flag(FALSE);
+
+out:
 	LOG_EXIT_INT(rc);
 	return rc;
 }
@@ -458,7 +542,7 @@
 		goto out;
 	}
 
-	rc = suspend_origin(snap_volume->origin, TRUE);
+	rc = suspend_origin(snap_volume->origin);
 	if (rc) {
 		goto out;
 	}
@@ -473,7 +557,7 @@
 
 out:
 	if (suspended) {
-		suspend_origin(snap_volume->origin, FALSE);
+		resume_origin(snap_volume->origin);
 	}
 	if (rc) {
 		EngFncs->dm_deactivate(snap_volume->parent);
@@ -547,13 +631,13 @@
 	LOG_ENTRY();
 
 	if (EngFncs->dm_get_version() != 3) {
-		suspend_origin(snap_volume->origin, TRUE);
+		suspend_origin(snap_volume->origin);
 	}
 
 	rc = EngFncs->dm_deactivate(snap_volume->parent);
 
 	if (EngFncs->dm_get_version() != 3) {
-		suspend_origin(snap_volume->origin, FALSE);
+		resume_origin(snap_volume->origin);
 	}
 
 	LOG_EXIT_INT(rc);