Re: [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 (christianvanbrau...@gmail.com):
> Now we can e.g. implement ephemeral containers in a consistent way.
> 
> Signed-off-by: Christian Brauner 
> 
>  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) {

This doesn't match what was advertised - < 0 should also mean
ephemeral.  (I really think a boolean makes the most sense,
enforced in the conffile as must be 0|1.)

> + /* Only destroy when rootfs is overlayfs or aufs for now. The
> +  * function however, works for other bdev types as well. */

What is the reason for that limitation?

> + 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, _ops, _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);
> + }
> + }
> + 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)

The one in lxccontainers.c should somehow be shared.

> +{
> + 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)

Hm, I guess it's easier just to keep this one static in 
the two places like you're doing.

> +{
> + char *arg = (char *) data;
> + return lxc_rmdir_onedev(arg, NULL);
> +}
> +
> -- 
> 2.5.1
> 
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Re: [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

[Christian Brauner]

On Mon, Sep 07, 2015 at 05:25:03PM +, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > Now we can e.g. implement ephemeral containers in a consistent way.
> > 
> > Signed-off-by: Christian Brauner 
> > 
> >  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) {
> 
> This doesn't match what was advertised - < 0 should also mean
> ephemeral.  (I really think a boolean makes the most sense,
> enforced in the conffile as must be 0|1.)

Ok.

> 
> > +   /* Only destroy when rootfs is overlayfs or aufs for now. The
> > +* function however, works for other bdev types as well. */
> 
> What is the reason for that limitation?

So far we only seem to use it for overlayfs and aufs based containers and I
didn't want to be hasty making it usable for any kind of container.

> 
> > +   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, _ops, _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);
> > +   }
> > +   }
> > +   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)
> 
> The one in lxccontainers.c should somehow be shared.
We can put it in bdev.c and bdev.h

> 
> > +{
> > +   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)
> 
> Hm, I guess it's easier just to keep this one static in 
> the two places like you're doing.

Ok.

> 
> > +{
> > +   char *arg = (char *) data;
> > +   return lxc_rmdir_onedev(arg, NULL);
> > +}
> > +
> > -- 
> > 2.5.1
> > 


signature.asc
Description: PGP signature
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Re: [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

[Christian Brauner]

On Mon, Sep 07, 2015 at 05:13:56PM +, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@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 
> > > 
> > >  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, _ops, _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 
> > > 
> > >  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 

Re: [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

[Christian Brauner]

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 
> 
>  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, _ops, _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 
> 
>  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;
>  }