[lxc-devel] [PATCH 2/2] restore: create cgroups for criu

Serge Hallyn serge.hallyn at ubuntu.com
Wed Oct 8 17:41:29 UTC 2014


Quoting Tycho Andersen (tycho.andersen at canonical.com):
> Previously, we let criu create the cgroups for a container as it was restoring
> things. In some cases (i.e. migration across hosts), if the container being
> migrated was in /lxc/u1-3, it would be migrated to the target host in
> /lxc/u1-3, even if there was no /lxc/u1-2 (or worse, if there was already an
> alive container in u1-3).
> 
> Instead, we use lxc's cgroup_create, and then tell criu where to restore to.
> 
> Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/cgfs.c         | 36 +++++++++++++++++++++++-------------
>  src/lxc/cgmanager.c    | 11 ++++++++++-
>  src/lxc/cgroup.c       | 25 ++++++++++++++++---------
>  src/lxc/cgroup.h       |  8 ++++++--
>  src/lxc/lxccontainer.c | 19 ++++++++++++-------
>  5 files changed, 67 insertions(+), 32 deletions(-)
> 
> diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
> index 0f181c6..9fa47cd 100644
> --- a/src/lxc/cgfs.c
> +++ b/src/lxc/cgfs.c
> @@ -2319,6 +2319,28 @@ static const char *cgfs_get_cgroup(void *hdata, const char *subsystem)
>  	return lxc_cgroup_get_hierarchy_path_data(subsystem, d);
>  }
>  
> +static const char *cgfs_canonical_path(void *hdata)
> +{
> +	struct cgfs_data *d = hdata;
> +	struct cgroup_process_info *info_ptr;
> +	char *path = NULL;
> +
> +	if (!d)
> +		return NULL;
> +
> +	for (info_ptr = d->info; info_ptr; info_ptr = info_ptr->next) {
> +		if (!path)
> +			path = info_ptr->cgroup_path;
> +		else if (strcmp(path, info_ptr->cgroup_path) != 0) {
> +			ERROR("not all paths match %s, %s has path %s", path,
> +				info_ptr->hierarchy->subsystems[0], info_ptr->cgroup_path);
> +			return NULL;
> +		}
> +	}
> +
> +	return path;
> +}
> +
>  static bool cgfs_unfreeze(void *hdata)
>  {
>  	struct cgfs_data *d = hdata;
> @@ -2376,18 +2398,6 @@ static bool lxc_cgroupfs_attach(const char *name, const char *lxcpath, pid_t pid
>  	return true;
>  }
>  
> -static bool cgfs_parse_existing_cgroups(void *hdata, pid_t init)
> -{
> -	struct cgfs_data *d = hdata;
> -
> -	if (!d)
> -		return false;
> -
> -	d->info = lxc_cgroup_process_info_get(init, d->meta);
> -
> -	return !!(d->info);
> -}
> -
>  static struct cgroup_ops cgfs_ops = {
>  	.init = cgfs_init,
>  	.destroy = cgfs_destroy,
> @@ -2395,6 +2405,7 @@ static struct cgroup_ops cgfs_ops = {
>  	.enter = cgfs_enter,
>  	.create_legacy = cgfs_create_legacy,
>  	.get_cgroup = cgfs_get_cgroup,
> +	.canonical_path = cgfs_canonical_path,
>  	.get = lxc_cgroupfs_get,
>  	.set = lxc_cgroupfs_set,
>  	.unfreeze = cgfs_unfreeze,
> @@ -2402,7 +2413,6 @@ static struct cgroup_ops cgfs_ops = {
>  	.name = "cgroupfs",
>  	.attach = lxc_cgroupfs_attach,
>  	.chown = NULL,
> -	.parse_existing_cgroups = cgfs_parse_existing_cgroups,
>  	.mount_cgroup = cgroupfs_mount_cgroup,
>  	.nrtasks = cgfs_nrtasks,
>  };
> diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> index 4038c41..140f4cd 100644
> --- a/src/lxc/cgmanager.c
> +++ b/src/lxc/cgmanager.c
> @@ -679,6 +679,15 @@ static const char *cgm_get_cgroup(void *hdata, const char *subsystem)
>  	return d->cgroup_path;
>  }
>  
> +static const char *cgm_canonical_path(void *hdata)
> +{
> +	struct cgm_data *d = hdata;
> +
> +	if (!d || !d->cgroup_path)
> +		return NULL;
> +	return d->cgroup_path;
> +}
> +
>  #if HAVE_CGMANAGER_GET_PID_CGROUP_ABS_SYNC
>  static inline bool abs_cgroup_supported(void) {
>  	return api_version >= CGM_SUPPORTS_GET_ABS;
> @@ -1307,13 +1316,13 @@ static struct cgroup_ops cgmanager_ops = {
>  	.enter = cgm_enter,
>  	.create_legacy = NULL,
>  	.get_cgroup = cgm_get_cgroup,
> +	.canonical_path = cgm_canonical_path,
>  	.get = cgm_get,
>  	.set = cgm_set,
>  	.unfreeze = cgm_unfreeze,
>  	.setup_limits = cgm_setup_limits,
>  	.name = "cgmanager",
>  	.chown = cgm_chown,
> -	.parse_existing_cgroups = NULL,
>  	.attach = cgm_attach,
>  	.mount_cgroup = cgm_mount_cgroup,
>  	.nrtasks = cgm_get_nrtasks,
> diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
> index 56d0e56..a413832 100644
> --- a/src/lxc/cgroup.c
> +++ b/src/lxc/cgroup.c
> @@ -21,6 +21,9 @@
>   * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
>   */
>  
> +#include <unistd.h>
> +#include <sys/types.h>
> +
>  #include "cgroup.h"
>  #include "conf.h"
>  #include "log.h"
> @@ -106,6 +109,19 @@ const char *cgroup_get_cgroup(struct lxc_handler *handler, const char *subsystem
>  	return NULL;
>  }
>  
> +const char *cgroup_canonical_path(struct lxc_handler *handler)
> +{
> +	if (geteuid()) {
> +		WARN("cgroup_canonical_path only makes sense for privileged containers.\n");
> +		return NULL;
> +	}
> +
> +	if (ops)
> +		return ops->canonical_path(handler->cgroup_data);
> +
> +	return NULL;
> +}
> +
>  bool cgroup_unfreeze(struct lxc_handler *handler)
>  {
>  	if (ops)
> @@ -128,15 +144,6 @@ bool cgroup_chown(struct lxc_handler *handler)
>  	return true;
>  }
>  
> -bool cgroup_parse_existing_cgroups(struct lxc_handler *handler)
> -{
> -	if (ops && ops->parse_existing_cgroups)
> -		return ops->parse_existing_cgroups(handler->cgroup_data, handler->pid);
> -
> -	/* cgmanager does this automatically */
> -	return true;
> -}
> -
>  bool cgroup_mount(const char *root, struct lxc_handler *handler, int type)
>  {
>  	if (ops) {
> diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
> index 0c2e566..281f404 100644
> --- a/src/lxc/cgroup.h
> +++ b/src/lxc/cgroup.h
> @@ -41,12 +41,12 @@ struct cgroup_ops {
>  	bool (*enter)(void *hdata, pid_t pid);
>  	bool (*create_legacy)(void *hdata, pid_t pid);
>  	const char *(*get_cgroup)(void *hdata, const char *subsystem);
> +	const char *(*canonical_path)(void *hdata);
>  	int (*set)(const char *filename, const char *value, const char *name, const char *lxcpath);
>  	int (*get)(const char *filename, char *value, size_t len, const char *name, const char *lxcpath);
>  	bool (*unfreeze)(void *hdata);
>  	bool (*setup_limits)(void *hdata, struct lxc_list *cgroup_conf, bool with_devices);
>  	bool (*chown)(void *hdata, struct lxc_conf *conf);
> -	bool (*parse_existing_cgroups)(void *hdata, pid_t pid);
>  	bool (*attach)(const char *name, const char *lxcpath, pid_t pid);
>  	bool (*mount_cgroup)(void *hdata, const char *root, int type);
>  	int (*nrtasks)(void *hdata);
> @@ -60,12 +60,16 @@ extern bool cgroup_init(struct lxc_handler *handler);
>  extern bool cgroup_create(struct lxc_handler *handler);
>  extern bool cgroup_setup_limits(struct lxc_handler *handler, bool with_devices);
>  extern bool cgroup_chown(struct lxc_handler *handler);
> -extern bool cgroup_parse_existing_cgroups(struct lxc_handler *handler);
>  extern bool cgroup_enter(struct lxc_handler *handler);
>  extern void cgroup_cleanup(struct lxc_handler *handler);
>  extern bool cgroup_create_legacy(struct lxc_handler *handler);
>  extern int cgroup_nrtasks(struct lxc_handler *handler);
>  extern const char *cgroup_get_cgroup(struct lxc_handler *handler, const char *subsystem);
> +
> +/*
> + * Currently, this call  only makes sense for privileged containers.
> + */
> +extern const char *cgroup_canonical_path(struct lxc_handler *handler);
>  extern bool cgroup_unfreeze(struct lxc_handler *handler);
>  extern void cgroup_disconnect(void);
>  
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 7e3a6b2..74b1b12 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -3485,6 +3485,7 @@ struct criu_opts {
>  
>  	/* restore: the file to write the init process' pid into */
>  	char *pidfile;
> +	const char *cgroup_path;
>  };
>  
>  /*
> @@ -3533,8 +3534,9 @@ static void exec_criu(struct criu_opts *opts)
>  		if (!opts->stop)
>  			static_args++;
>  	} else if (strcmp(opts->action, "restore") == 0) {
> -		/* --root $(lxc_mount_point) --restore-detached --restore-sibling --pidfile $foo */
> -		static_args += 6;
> +		/* --root $(lxc_mount_point) --restore-detached
> +		 * --restore-sibling --pidfile $foo --cgroup-root $foo */
> +		static_args += 8;
>  	} else {
>  		return;
>  	}
> @@ -3603,6 +3605,8 @@ static void exec_criu(struct criu_opts *opts)
>  		DECLARE_ARG("--restore-sibling");
>  		DECLARE_ARG("--pidfile");
>  		DECLARE_ARG(opts->pidfile);
> +		DECLARE_ARG("--cgroup-root");
> +		DECLARE_ARG(opts->cgroup_path);
>  
>  		lxc_list_for_each(it, &opts->c->lxc_conf->network) {
>  			char eth[128], veth[128], buf[257];
> @@ -3828,6 +3832,11 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
>  		goto out_fini_handler;
>  	}
>  
> +	if (!cgroup_create(handler)) {
> +		ERROR("failed creating groups");
> +		goto out_fini_handler;
> +	}
> +
>  	pid = fork();
>  	if (pid < 0)
>  		goto out_fini_handler;
> @@ -3861,6 +3870,7 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
>  		os.c = c;
>  		os.pidfile = pidfile;
>  		os.verbose = verbose;
> +		os.cgroup_path = cgroup_canonical_path(handler);
>  
>  		/* exec_criu() returning is an error */
>  		exec_criu(&os);
> @@ -3897,11 +3907,6 @@ static bool lxcapi_restore(struct lxc_container *c, char *directory, bool verbos
>  					goto out_fini_handler;
>  				}
>  
> -				if (!cgroup_parse_existing_cgroups(handler)) {
> -					ERROR("failed creating groups");
> -					goto out_fini_handler;
> -				}
> -
>  				if (container_mem_lock(c))
>  					goto out_fini_handler;
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel


More information about the lxc-devel mailing list