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