[lxc-devel] [PATCH] c/r: tell CRIU about all external mounts in the config

St├ęphane Graber stgraber at ubuntu.com
Mon Apr 6 16:09:28 UTC 2015


I'm not seeing an ack in this thread, are we expecting a new version of this?

On Fri, Apr 03, 2015 at 11:39:59PM +0000, Serge Hallyn wrote:
> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > On Fri, Apr 03, 2015 at 09:41:03PM +0000, Serge Hallyn wrote:
> > > Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > > > ---
> > > >  src/lxc/conf.c         | 18 +++++++++++---
> > > >  src/lxc/conf.h         |  2 ++
> > > >  src/lxc/list.h         | 11 +++++++++
> > > >  src/lxc/lxccontainer.c | 67 +++++++++++++++++++++++++++++++++++++++++++-------
> > > >  4 files changed, 85 insertions(+), 13 deletions(-)
> > > > 
> > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > > index 2868708..f9c7e37 100644
> > > > --- a/src/lxc/conf.c
> > > > +++ b/src/lxc/conf.c
> > > > @@ -2040,18 +2040,16 @@ static int setup_mount(const struct lxc_rootfs *rootfs, const char *fstab,
> > > >  	return ret;
> > > >  }
> > > >  
> > > > -static int setup_mount_entries(const struct lxc_rootfs *rootfs, struct lxc_list *mount,
> > > > -	const char *lxc_name)
> > > > +FILE *write_mount_file(struct lxc_list *mount)
> > > >  {
> > > >  	FILE *file;
> > > >  	struct lxc_list *iterator;
> > > >  	char *mount_entry;
> > > > -	int ret;
> > > >  
> > > >  	file = tmpfile();
> > > >  	if (!file) {
> > > >  		ERROR("tmpfile error: %m");
> > > > -		return -1;
> > > > +		return NULL;
> > > >  	}
> > > >  
> > > >  	lxc_list_for_each(iterator, mount) {
> > > > @@ -2060,6 +2058,18 @@ static int setup_mount_entries(const struct lxc_rootfs *rootfs, struct lxc_list
> > > >  	}
> > > >  
> > > >  	rewind(file);
> > > > +	return file;
> > > > +}
> > > > +
> > > > +static int setup_mount_entries(const struct lxc_rootfs *rootfs, struct lxc_list *mount,
> > > > +	const char *lxc_name)
> > > > +{
> > > > +	FILE *file;
> > > > +	int ret;
> > > > +
> > > > +	file = write_mount_file(mount);
> > > > +	if (!file)
> > > > +		return -1;
> > > >  
> > > >  	ret = mount_file_entries(rootfs, file, lxc_name);
> > > >  
> > > > diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> > > > index 334ea70..4b66045 100644
> > > > --- a/src/lxc/conf.h
> > > > +++ b/src/lxc/conf.h
> > > > @@ -25,6 +25,7 @@
> > > >  
> > > >  #include "config.h"
> > > >  
> > > > +#include <stdio.h>
> > > >  #include <netinet/in.h>
> > > >  #include <net/if.h>
> > > >  #include <sys/param.h>
> > > > @@ -422,4 +423,5 @@ extern int parse_mntopts(const char *mntopts, unsigned long *mntflags,
> > > >  extern void tmp_proc_unmount(struct lxc_conf *lxc_conf);
> > > >  void remount_all_slave(void);
> > > >  extern void suggest_default_idmap(void);
> > > > +FILE *write_mount_file(struct lxc_list *mount);
> > > >  #endif
> > > > diff --git a/src/lxc/list.h b/src/lxc/list.h
> > > > index 0882da0..f16af54 100644
> > > > --- a/src/lxc/list.h
> > > > +++ b/src/lxc/list.h
> > > > @@ -99,4 +99,15 @@ static inline void lxc_list_del(struct lxc_list *list)
> > > >  	prev->next = next;
> > > >  }
> > > >  
> > > > +static inline int lxc_list_len(struct lxc_list *list)
> > > > +{
> > > > +	 int i = 0;
> > > > +	 struct lxc_list *iter;
> > > > +	 lxc_list_for_each(iter, list) {
> > > > +		i++;
> > > > +	 }
> > > > +
> > > > +	 return i;
> > > > +}
> > > > +
> > > >  #endif
> > > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > > index d84543f..7e50372 100644
> > > > --- a/src/lxc/lxccontainer.c
> > > > +++ b/src/lxc/lxccontainer.c
> > > > @@ -35,6 +35,8 @@
> > > >  #include <libgen.h>
> > > >  #include <stdint.h>
> > > >  #include <grp.h>
> > > > +#include <stdio.h>
> > > > +#include <mntent.h>
> > > >  #include <sys/syscall.h>
> > > >  
> > > >  #include <lxc/lxccontainer.h>
> > > > @@ -3513,11 +3515,15 @@ struct criu_opts {
> > > >  
> > > >  static void exec_criu(struct criu_opts *opts)
> > > >  {
> > > > -	char **argv, log[PATH_MAX], buf[257];
> > > > +	char **argv, log[PATH_MAX];
> > > >  	int static_args = 14, argc = 0, i, ret;
> > > >  	int netnr = 0;
> > > >  	struct lxc_list *it;
> > > >  
> > > > +	struct mntent mntent;
> > > > +	char buf[4096];
> > > > +	FILE *mnts = NULL;
> > > > +
> > > >  	/* The command line always looks like:
> > > >  	 * criu $(action) --tcp-established --file-locks --link-remap --force-irmap \
> > > >  	 * --manage-cgroups action-script foo.sh -D $(directory) \
> > > > @@ -3592,6 +3598,21 @@ static void exec_criu(struct criu_opts *opts)
> > > >  	if (opts->verbose)
> > > >  		DECLARE_ARG("-vvvvvv");
> > > >  
> > > > +#define RESIZE_ARGS(additional) 						\
> > > 
> > > This seems to want to be reused, however it doesn't bump argc,
> > > so if it is used twice in a row it wont' have the desired effect.
> > 
> > Right, argc is supposedly where the current pointer is, so it
> > shouldn't bump argc. RESIZE_ARGS has an implicit assumption that
> > you're out of space when you call it, so you shouldn't call it twice
> > in a row. I can re-send a patch with a comment to that effect if you
> > like,
> 
> That (a comment) is fine.
> 
> > or do something else. This whole function is kind of messy now
> > (and long), so it may be time to rethink it.
> 
> Yes :)
> 
> > Tycho
> > 
> > > > +	do {									\
> > > > +		void *m;							\
> > > > +		if (additional < 0) {						\
> > > > +			ERROR("resizing by negative amount");			\
> > > > +			goto err;						\
> > > > +		} else if (additional == 0)					\
> > > > +			continue;						\
> > > > +										\
> > > > +		m = realloc(argv, (argc + additional + 1) * sizeof(*argv));	\
> > > > +		if (!m)								\
> > > > +			goto err;						\
> > > > +		argv = m;							\
> > > > +	} while (0)
> > > > +
> > > >  	if (strcmp(opts->action, "dump") == 0) {
> > > >  		char pid[32];
> > > >  
> > > > @@ -3623,9 +3644,10 @@ static void exec_criu(struct criu_opts *opts)
> > > >  		DECLARE_ARG("--cgroup-root");
> > > >  		DECLARE_ARG(opts->cgroup_path);
> > > >  
> > > > +		RESIZE_ARGS(lxc_list_len(&opts->c->lxc_conf->network) * 2);
> > > > +
> > > >  		lxc_list_for_each(it, &opts->c->lxc_conf->network) {
> > > >  			char eth[128], *veth;
> > > > -			void *m;
> > > >  			struct lxc_netdev *n = it->elem;
> > > >  
> > > >  			if (n->name) {
> > > > @@ -3641,18 +3663,42 @@ static void exec_criu(struct criu_opts *opts)
> > > >  			if (ret < 0 || ret >= sizeof(buf))
> > > >  				goto err;
> > > >  
> > > > -			/* final NULL and --veth-pair eth0=vethASDF */
> > > > -			m = realloc(argv, (argc + 1 + 2) * sizeof(*argv));
> > > > -			if (!m)
> > > > -				goto err;
> > > > -			argv = m;
> > > > -
> > > >  			DECLARE_ARG("--veth-pair");
> > > >  			DECLARE_ARG(buf);
> > > > -			argv[argc] = NULL;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	// CRIU wants to know about any external bind mounts the
> > > > +	// container has.
> > > > +	mnts = write_mount_file(&opts->c->lxc_conf->mount_list);
> > > > +	if (!mnts)
> > > > +		goto err;
> > > > +
> > > > +	RESIZE_ARGS(lxc_list_len(&opts->c->lxc_conf->mount_list) * 2);
> > > > +
> > > > +	while (getmntent_r(mnts, &mntent, buf, sizeof(buf))) {
> > > > +		char arg[2048], *key, *val;
> > > > +		int ret;
> > > >  
> > > > +		if (strcmp(opts->action, "dump") == 0) {
> > > > +			key = mntent.mnt_fsname;
> > > > +			val = mntent.mnt_dir;
> > > > +		} else {
> > > > +			key = mntent.mnt_dir;
> > > > +			val = mntent.mnt_fsname;
> > > >  		}
> > > > +
> > > > +		ret = snprintf(arg, sizeof(arg), "%s:%s", key, val);
> > > > +		if (ret < 0 || ret >= sizeof(arg)) {
> > > > +			goto err;
> > > > +		}
> > > > +
> > > > +		DECLARE_ARG("--ext-mount-map");
> > > > +		DECLARE_ARG(arg);
> > > >  	}
> > > > +	fclose(mnts);
> > > > +
> > > > +	argv[argc] = NULL;
> > > >  
> > > >  	netnr = 0;
> > > >  	lxc_list_for_each(it, &opts->c->lxc_conf->network) {
> > > > @@ -3688,8 +3734,11 @@ static void exec_criu(struct criu_opts *opts)
> > > >  	}
> > > >  
> > > >  #undef DECLARE_ARG
> > > > +#undef RESIZE_ARGS
> > > >  	execv(argv[0], argv);
> > > >  err:
> > > > +	if (mnts)
> > > > +		fclose(mnts);
> > > >  	for (i = 0; argv[i]; i++)
> > > >  		free(argv[i]);
> > > >  	free(argv);
> > > > -- 
> > > > 2.1.0
> > > > 
> > > > _______________________________________________
> > > > lxc-devel mailing list
> > > > lxc-devel at lists.linuxcontainers.org
> > > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

-- 
St├ęphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20150406/d35ec52f/attachment-0001.sig>


More information about the lxc-devel mailing list