[lxc-devel] [PATCH 1/2] restore: Hoist handler to function level

[Tycho Andersen]

On Tue, Oct 07, 2014 at 07:33:07PM +0000, Tycho Andersen wrote:
> This commit is in preparation for the cgroups create work, since we will need
> the handler in both the parent and the child. This commit also re-works how
> errors are propagated to be less verbose.

Here is an updated version:


>From 941623498a49551411ccf185146061f3f37d3a67 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.andersen at canonical.com>
Date: Tue, 7 Oct 2014 19:13:51 +0000
Subject: [PATCH 1/2] restore: Hoist handler to function level

This commit is in preparation for the cgroups create work, since we will need
the handler in both the parent and the child. This commit also re-works how
errors are propagated to be less verbose.

v2: rename error to has_error, handle it correctly, and remove some diff noise

Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
---
 src/lxc/lxccontainer.c | 68 +++++++++++++++++++++++---------------------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index fee758d..4b31f5c 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -3809,6 +3809,8 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
 	struct lxc_list *it;
 	struct lxc_rootfs *rootfs;
 	char pidfile[L_tmpnam];
+	struct lxc_handler *handler;
+	bool has_error = true;
 
 	if (!criu_ok(c))
 		return false;
@@ -3821,9 +3823,18 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
 	if (!tmpnam(pidfile))
 		return false;
 
+	handler = lxc_init(c->name, c->lxc_conf, c->config_path);
+	if (!handler)
+		return false;
+
+	if (!cgroup_init(handler)) {
+		ERROR("failed initing cgroups");
+		goto out_fini_handler;
+	}
+
 	pid = fork();
 	if (pid < 0)
-		return false;
+		goto out_fini_handler;
 
 	if (pid == 0) {
 		struct criu_opts os;
@@ -3862,30 +3873,22 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
 		exit(1);
 	} else {
 		int status;
-		struct lxc_handler *handler;
-		bool error = false;
 
 		pid_t w = waitpid(pid, &status, 0);
 
 		if (w == -1) {
 			perror("waitpid");
-			return false;
+			goto out_fini_handler;
 		}
 
-		handler = lxc_init(c->name, c->lxc_conf, c->config_path);
-		if (!handler)
-			return false;
-
 		if (WIFEXITED(status)) {
 			if (WEXITSTATUS(status)) {
-				error = true;
 				goto out_fini_handler;
 			}
 			else {
 				int netnr = 0, ret;
 				FILE *f = fopen(pidfile, "r");
 				if (!f) {
-					error = true;
 					perror("reading pidfile");
 					ERROR("couldn't read restore's init pidfile %s\n", pidfile);
 					goto out_fini_handler;
@@ -3894,71 +3897,62 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
 				ret = fscanf(f, "%d", (int*) &handler->pid);
 				fclose(f);
 				if (ret != 1) {
-					error = true;
 					ERROR("reading restore pid failed");
 					goto out_fini_handler;
 				}
 
-				if (!cgroup_init(handler)) {
-					error = true;
-					ERROR("failed initing cgroups");
-					goto out_fini_handler;
-				}
-
 				if (!cgroup_parse_existing_cgroups(handler)) {
 					ERROR("failed creating cgroups");
 					goto out_fini_handler;
 				}
 
-				if (container_mem_lock(c)) {
-					error = true;
+				if (container_mem_lock(c))
 					goto out_fini_handler;
-				}
 
 				lxc_list_for_each(it, &c->lxc_conf->network) {
 					char eth[128], veth[128];
 					struct lxc_netdev *netdev = it->elem;
 
 					if (read_criu_file(directory, "veth", netnr, veth)) {
-						error = true;
-						goto out_unlock;
+						container_mem_unlock(c);
+						goto out_fini_handler;
 					}
+
 					if (read_criu_file(directory, "eth", netnr, eth)) {
-						error = true;
-						goto out_unlock;
+						container_mem_unlock(c);
+						goto out_fini_handler;
 					}
+
 					netdev->priv.veth_attr.pair = strdup(veth);
 					if (!netdev->priv.veth_attr.pair) {
-						error = true;
-						goto out_unlock;
+						container_mem_unlock(c);
+						goto out_fini_handler;
 					}
+
 					netnr++;
 				}
-out_unlock:
+
 				container_mem_unlock(c);
-				if (error)
-					goto out_fini_handler;
 
-				if (lxc_set_state(c->name, handler, RUNNING)) {
-					error = true;
+				if (lxc_set_state(c->name, handler, RUNNING))
 					goto out_fini_handler;
-				}
 			}
 		} else {
 			ERROR("CRIU was killed with signal %d\n", WTERMSIG(status));
-			error = true;
 			goto out_fini_handler;
 		}
 
 		if (lxc_poll(c->name, handler)) {
 			lxc_abort(c->name, handler);
-			return false;
+			goto out_fini_handler;
 		}
+	}
+
+	has_error = false;
 
 out_fini_handler:
-		lxc_fini(c->name, handler);
-		return !error;
-	}
+	lxc_fini(c->name, handler);
+	return !has_error;
 }
 
 static int lxcapi_attach_run_waitl(struct lxc_container *c, lxc_attach_options_t *options, const char *program, const char *arg, ...)
-- 
1.9.1