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

[Serge Hallyn]

Quoting Tycho Andersen (tycho.andersen at canonical.com):
> 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>

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.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
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel