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 <christianvanbrau...@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 <christianvanbrau...@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?
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to