Quoting Tycho Andersen (tycho.ander...@canonical.com):
> If we don't re-open these after clone, the init process has a pointer to the
> parent's /dev/{zero,null}. CRIU seese these and wants to dump the parent's
> mount namespace, which is unnecessary. Instead, we should just re-open
> stdin/out/err after we do the clone and pivot root, to ensure that we have
> pointers to the devcies in init's rootfs instead of the host's.
> 
> v2: Only close fds if the container was daemonized. This didn't turn out as
>     nicely as described on the list because lxc_start() doesn't actually have
>     the struct lxc_container, so it cant see the flag. Instead, we just pass 
> it
>     down everywhere.
> 
> Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>

Thanks, Tycho.

Acked-by: Serge E. Hallyn <serge.hal...@ubuntu.com>

> ---
>  src/lxc/execute.c      |  4 ++--
>  src/lxc/lxc.h          | 22 +++++++++++++---------
>  src/lxc/lxc_execute.c  |  2 +-
>  src/lxc/lxccontainer.c | 10 ++--------
>  src/lxc/start.c        | 17 ++++++++++++++---
>  src/lxc/start.h        |  3 ++-
>  6 files changed, 34 insertions(+), 24 deletions(-)
> 
> diff --git a/src/lxc/execute.c b/src/lxc/execute.c
> index a0f7ff1..d14092a 100644
> --- a/src/lxc/execute.c
> +++ b/src/lxc/execute.c
> @@ -111,7 +111,7 @@ static struct lxc_operations execute_start_ops = {
>  };
>  
>  int lxc_execute(const char *name, char *const argv[], int quiet,
> -             struct lxc_conf *conf, const char *lxcpath)
> +             struct lxc_conf *conf, const char *lxcpath, bool backgrounded)
>  {
>       struct execute_args args = {
>               .argv = argv,
> @@ -122,5 +122,5 @@ int lxc_execute(const char *name, char *const argv[], int 
> quiet,
>               return -1;
>  
>       conf->is_execute = 1;
> -     return __lxc_start(name, conf, &execute_start_ops, &args, lxcpath);
> +     return __lxc_start(name, conf, &execute_start_ops, &args, lxcpath, 
> backgrounded);
>  }
> diff --git a/src/lxc/lxc.h b/src/lxc/lxc.h
> index dd1048c..b1b858d 100644
> --- a/src/lxc/lxc.h
> +++ b/src/lxc/lxc.h
> @@ -27,6 +27,7 @@
>  extern "C" {
>  #endif
>  
> +#include <stdbool.h>
>  #include <stddef.h>
>  #include <sys/select.h>
>  #include <sys/types.h>
> @@ -44,24 +45,27 @@ struct lxc_arguments;
>  
>  /*
>   * Start the specified command inside a system container
> - * @name     : the name of the container
> - * @argv     : an array of char * corresponding to the commande line
> - * @conf     : configuration
> + * @name         : the name of the container
> + * @argv         : an array of char * corresponding to the commande line
> + * @conf         : configuration
> + * @backgrounded : whether or not the container is daemonized
>   * Returns 0 on success, < 0 otherwise
>   */
>  extern int lxc_start(const char *name, char *const argv[], struct lxc_conf 
> *conf,
> -                  const char *lxcpath);
> +                  const char *lxcpath, bool backgrounded);
>  
>  /*
>   * Start the specified command inside an application container
> - * @name     : the name of the container
> - * @argv     : an array of char * corresponding to the commande line
> - * @quiet    : if != 0 then lxc-init won't produce any output
> - * @conf     : configuration
> + * @name         : the name of the container
> + * @argv         : an array of char * corresponding to the commande line
> + * @quiet        : if != 0 then lxc-init won't produce any output
> + * @conf         : configuration
> + * @backgrounded : whether or not the container is daemonized
>   * Returns 0 on success, < 0 otherwise
>   */
>  extern int lxc_execute(const char *name, char *const argv[], int quiet,
> -                    struct lxc_conf *conf, const char *lxcpath);
> +                    struct lxc_conf *conf, const char *lxcpath,
> +                    bool backgrounded);
>  
>  /*
>   * Open the monitoring mechanism for a specific container
> diff --git a/src/lxc/lxc_execute.c b/src/lxc/lxc_execute.c
> index b602793..4f1e1f6 100644
> --- a/src/lxc/lxc_execute.c
> +++ b/src/lxc/lxc_execute.c
> @@ -139,7 +139,7 @@ int main(int argc, char *argv[])
>       if (lxc_config_define_load(&defines, conf))
>               return 1;
>  
> -     ret = lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, 
> my_args.lxcpath[0]);
> +     ret = lxc_execute(my_args.name, my_args.argv, my_args.quiet, conf, 
> my_args.lxcpath[0], false);
>  
>       lxc_conf_free(conf);
>  
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 0ca5b9f..d49426f 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -584,7 +584,7 @@ static bool lxcapi_start(struct lxc_container *c, int 
> useinit, char * const argv
>       container_mem_unlock(c);
>  
>       if (useinit) {
> -             ret = lxc_execute(c->name, argv, 1, conf, c->config_path);
> +             ret = lxc_execute(c->name, argv, 1, conf, c->config_path, 
> daemonize);
>               return ret == 0 ? true : false;
>       }
>  
> @@ -642,12 +642,6 @@ static bool lxcapi_start(struct lxc_container *c, int 
> useinit, char * const argv
>                       return false;
>               }
>               lxc_check_inherited(conf, true, -1);
> -             close(0);
> -             close(1);
> -             close(2);
> -             open("/dev/zero", O_RDONLY);
> -             open("/dev/null", O_RDWR);
> -             open("/dev/null", O_RDWR);
>               setsid();
>       } else {
>               if (!am_single_threaded()) {
> @@ -687,7 +681,7 @@ reboot:
>               goto out;
>       }
>  
> -     ret = lxc_start(c->name, argv, conf, c->config_path);
> +     ret = lxc_start(c->name, argv, conf, c->config_path, daemonize);
>       c->error_num = ret;
>  
>       if (conf->reboot) {
> diff --git a/src/lxc/start.c b/src/lxc/start.c
> index d615375..8aedc7d 100644
> --- a/src/lxc/start.c
> +++ b/src/lxc/start.c
> @@ -759,6 +759,15 @@ static int do_start(void *data)
>  
>       close(handler->sigfd);
>  
> +     if (handler->backgrounded) {
> +             close(0);
> +             close(1);
> +             close(2);
> +             open("/dev/zero", O_RDONLY);
> +             open("/dev/null", O_RDWR);
> +             open("/dev/null", O_RDWR);
> +     }
> +
>       /* after this call, we are in error because this
>        * ops should not return as it execs */
>       handler->ops->start(handler, handler->data);
> @@ -1112,7 +1121,8 @@ int get_netns_fd(int pid)
>  }
>  
>  int __lxc_start(const char *name, struct lxc_conf *conf,
> -             struct lxc_operations* ops, void *data, const char *lxcpath)
> +             struct lxc_operations* ops, void *data, const char *lxcpath,
> +             bool backgrounded)
>  {
>       struct lxc_handler *handler;
>       int err = -1;
> @@ -1126,6 +1136,7 @@ int __lxc_start(const char *name, struct lxc_conf *conf,
>       }
>       handler->ops = ops;
>       handler->data = data;
> +     handler->backgrounded = backgrounded;
>  
>       if (must_drop_cap_sys_boot(handler->conf)) {
>               #if HAVE_SYS_CAPABILITY_H
> @@ -1257,12 +1268,12 @@ static struct lxc_operations start_ops = {
>  };
>  
>  int lxc_start(const char *name, char *const argv[], struct lxc_conf *conf,
> -           const char *lxcpath)
> +           const char *lxcpath, bool backgrounded)
>  {
>       struct start_args start_arg = {
>               .argv = argv,
>       };
>  
>       conf->need_utmp_watch = 1;
> -     return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath);
> +     return __lxc_start(name, conf, &start_ops, &start_arg, lxcpath, 
> backgrounded);
>  }
> diff --git a/src/lxc/start.h b/src/lxc/start.h
> index aab063a..f1a41f5 100644
> --- a/src/lxc/start.h
> +++ b/src/lxc/start.h
> @@ -74,6 +74,7 @@ struct lxc_handler {
>       const char *lxcpath;
>       void *cgroup_data;
>       int ttysock[2]; // socketpair for child->parent tty fd passing
> +     bool backgrounded; // indicates whether should we close std{in,out,err} 
> on start
>  };
>  
>  
> @@ -85,7 +86,7 @@ extern void lxc_fini(const char *name, struct lxc_handler 
> *handler);
>  
>  extern int lxc_check_inherited(struct lxc_conf *conf, bool closeall, int 
> fd_to_ignore);
>  int __lxc_start(const char *, struct lxc_conf *, struct lxc_operations *,
> -             void *, const char *);
> +             void *, const char *, bool);
>  
>  extern void resolve_clone_flags(struct lxc_handler *handler);
>  #endif
> -- 
> 2.1.4
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to