restore: Hoist handler to function level

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@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@canonical.com>
Acked-by: Serge E. Hallyn <serge.hallyn@ubuntu.com>

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index fee758d2ddd6f0d709ac4eb61a8582e81cf4e5f5..4b31f5c37b4496fae15fd70365dcc337c7358c45 100644 (file)
--- 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, ...)