[lxc-devel] [PATCH 6/6] Enable lxc_fini() to destroy container on shutdown This works for any bdev-type but is only used for overlayfs and aufs now

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> Fixes for the return value checks.
> 
> On Sun, Sep 06, 2015 at 10:38:21AM +0200, Christian Brauner wrote:
> > Now we can e.g. implement ephemeral containers in a consistent way.
> > 
> > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > 
> >  100.0% src/lxc/
> > diff --git a/src/lxc/start.c b/src/lxc/start.c
> > index ffb8d12..1179d2c 100644
> > --- a/src/lxc/start.c
> > +++ b/src/lxc/start.c
> > @@ -83,6 +83,11 @@ const struct ns_info ns_info[LXC_NS_MAX] = {
> >  	[LXC_NS_NET] = {"net", CLONE_NEWNET}
> >  };
> >  
> > +static int bdev_destroy_wrapper(void *data);
> > +static int lxc_rmdir_onedev_wrapper(void *data);
> > +static void lxc_destroy_container_on_signal(struct lxc_handler *handler,
> > +					    const char *name);
> > +
> >  static void print_top_failing_dir(const char *path)
> >  {
> >  	size_t len = strlen(path);
> > @@ -495,6 +500,13 @@ void lxc_fini(const char *name, struct lxc_handler *handler)
> >  		close(handler->ttysock[0]);
> >  		close(handler->ttysock[1]);
> >  	}
> > +	if (handler->conf->ephemeral > 0) {
> > +		/* Only destroy when rootfs is overlayfs or aufs for now. The
> > +		 * function however, works for other bdev types as well. */
> > +		if ((strncmp(handler->conf->rootfs.path, "overlayfs:", 10) == 0) ||
> > +		   (strncmp(handler->conf->rootfs.path, "aufs:", 5) == 0))
> > +			lxc_destroy_container_on_signal(handler, name);
> > +	}
> >  	cgroup_destroy(handler);
> >  	free(handler);
> >  }
> > @@ -1291,3 +1303,61 @@ int lxc_start(const char *name, char *const argv[], struct lxc_conf *conf,
> >  	conf->need_utmp_watch = 1;
> >  	return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath, backgrounded);
> >  }
> > +
> > +static void lxc_destroy_container_on_signal(struct lxc_handler *handler,
> > +					    const char *name)
> > +{
> > +	char destroy[MAXPATHLEN];
> > +	bool bret = true;
> > +	int ret = 0;
> > +	if (handler->conf && handler->conf->rootfs.path && handler->conf->rootfs.mount) {
> > +		if (am_unpriv())
> > +			ret = userns_exec_1(handler->conf, bdev_destroy_wrapper, handler->conf);
> > +		else
> > +			bret = bdev_destroy(handler->conf);
> > +		if (ret < 0 || !bret) {
> > +			ERROR("Error destroying rootfs for %s", name);
> > +		} else {
> > +			INFO("Destroyed rootfs for %s", name);
> > +		}
> > +	}
> 
> The check for the return value here is wrong but it is fixed in the PR on github.
> 
> > +	ret = snprintf(destroy, MAXPATHLEN, "%s/%s", handler->lxcpath, name);
> > +	if (ret < 0 || ret >= MAXPATHLEN)
> > +		ERROR("Error creating string");
> > +	if (am_unpriv())
> > +		ret = userns_exec_1(handler->conf, lxc_rmdir_onedev_wrapper, destroy);
> > +	else
> > +		ret = lxc_rmdir_onedev(destroy, NULL);
> > +	if (ret < 0) {
> > +		ERROR("Error destroying container directory for %s", name);
> > +	} else {
> > +		INFO("Destroyed directory for %s", name);
> > +	}
> > +}
> > +
> > +static int bdev_destroy_wrapper(void *data)
> > +{
> > +	struct lxc_conf *conf = data;
> > +
> > +	if (setgid(0) < 0) {
> > +		ERROR("Failed to setgid to 0");
> > +		return -1;
> > +	}
> > +	if (setgroups(0, NULL) < 0)
> > +		WARN("Failed to clear groups");
> > +	if (setuid(0) < 0) {
> > +		ERROR("Failed to setuid to 0");
> > +		return -1;
> > +	}
> > +	if (!bdev_destroy(conf))
> > +		return -1;
> > +	else
> > +		return 0;
> > +}
> > +
> > +static int lxc_rmdir_onedev_wrapper(void *data)
> > +{
> > +	char *arg = (char *) data;
> > +	return lxc_rmdir_onedev(arg, NULL);
> > +}
> > +
> > -- 
> > 2.5.1
> > 
> 
> On Sun, Sep 06, 2015 at 10:38:20AM +0200, Christian Brauner wrote:
> > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > 
> >  100.0% src/lxc/
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 2103437..9f22fdc 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -2213,21 +2213,6 @@ static int lxc_rmdir_onedev_wrapper(void *data)
> >  	return lxc_rmdir_onedev(arg, "snaps");
> >  }
> >  
> > -static int do_bdev_destroy(struct lxc_conf *conf)
> > -{
> > -	struct bdev *r;
> > -	int ret = 0;
> > -
> > -	r = bdev_init(conf, conf->rootfs.path, conf->rootfs.mount, NULL);
> > -	if (!r)
> > -		return -1;
> > -
> > -	if (r->ops->destroy(r) < 0)
> > -		ret = -1;
> > -	bdev_put(r);
> > -	return ret;
> > -}
> > -
> >  static int bdev_destroy_wrapper(void *data)
> >  {
> >  	struct lxc_conf *conf = data;
> > @@ -2242,13 +2227,16 @@ static int bdev_destroy_wrapper(void *data)
> >  		ERROR("Failed to setuid to 0");
> >  		return -1;
> >  	}
> > -	return do_bdev_destroy(conf);
> > +	if (!bdev_destroy(conf))
> > +		return -1;
> > +	else
> > +		return 0;
> >  }
> >  
> >  static bool container_destroy(struct lxc_container *c)
> >  {
> >  	bool bret = false;
> > -	int ret;
> > +	int ret = 0;
> >  	struct lxc_conf *conf;
> >  
> >  	if (!c || !do_lxcapi_is_defined(c))
> > @@ -2304,8 +2292,8 @@ static bool container_destroy(struct lxc_container *c)
> >  		if (am_unpriv())
> >  			ret = userns_exec_1(conf, bdev_destroy_wrapper, conf);
> >  		else
> > -			ret = do_bdev_destroy(conf);
> > -		if (ret < 0) {
> > +			bret = bdev_destroy(conf);
> > +		if (ret < 0 || !bret) {
> >  			ERROR("Error destroying rootfs for %s", c->name);
> >  			goto out;
> >  		}
> 
> The check for the return value here is wrong but it is fixed in the PR on github.

Oh.  It's much better if you simply reply to the email with the actual
patch, as that's how I follow the conversation :)

Once the rest of the conversation dies down, can you send a clean
new rebased v2 patchset as a new thread?