The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/1194
This e-mail was sent by the LXC bot, direct replies will not reach the author
unless they happen to be subscribed to this list.
=== Description (from pull-request) ===
This is a fix for one of the issues mentioned in #1156; see the 4th patch in the series for the bulk of the fix. The CRIU commit in question went into criu-dev today, and should be in 2.7.
From 5f178bc983a7050205b0ba374163646e9e378cf6 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Mon, 12 Sep 2016 18:04:18 +0000
Subject: [PATCH 1/6] c/r: fix typo in comment
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/criu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/criu.c b/src/lxc/criu.c
index 71c9b9c..2799102 100644
--- a/src/lxc/criu.c
+++ b/src/lxc/criu.c
@@ -140,7 +140,7 @@ static void exec_criu(struct criu_opts *opts)
/* The command line always looks like:
* criu $(action) --tcp-established --file-locks --link-remap \
- * --manage-cgroups=full action-script foo.sh -D $(directory) \
+ * --manage-cgroups=full --action-script foo.sh -D $(directory) \
* -o $(directory)/$(action).log --ext-mount-map auto
* --enable-external-sharing --enable-external-masters
* --enable-fs hugetlbfs --enable-fs tracefs --ext-mount-map
console:/dev/pts/n
From 36662416441b051bf9d3bb0e1b5d08a61b3a6420 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Wed, 14 Sep 2016 14:38:46 +0000
Subject: [PATCH 2/6] cgroup: add new functions for interacting with hierachies
N.B. that these are only implemented in cgfsng, but,
15:28:28 tych0 | do we still use cgfs anywhere? or the cgm backend?
15:29:19 stgraber | not anywhere we care about
...I think that's okay.
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/cgroups/cgfs.c | 14 ++++++++++++++
src/lxc/cgroups/cgfsng.c | 27 +++++++++++++++++++++++++++
src/lxc/cgroups/cgmanager.c | 14 ++++++++++++++
src/lxc/cgroups/cgroup.c | 16 ++++++++++++++++
src/lxc/cgroups/cgroup.h | 4 ++++
5 files changed, 75 insertions(+)
diff --git a/src/lxc/cgroups/cgfs.c b/src/lxc/cgroups/cgfs.c
index 2d0de0c..80a336d 100644
--- a/src/lxc/cgroups/cgfs.c
+++ b/src/lxc/cgroups/cgfs.c
@@ -2434,6 +2434,18 @@ static bool cgfs_escape(void *hdata)
return ret;
}
+static int cgfs_num_hierarchies(void)
+{
+ /* not implemented */
+ return -1;
+}
+
+static bool cgfs_get_hierarchies(int i, char ***out)
+{
+ /* not implemented */
+ return false;
+}
+
static bool cgfs_unfreeze(void *hdata)
{
struct cgfs_data *d = hdata;
@@ -2627,6 +2639,8 @@ static struct cgroup_ops cgfs_ops = {
.get_cgroup = cgfs_get_cgroup,
.canonical_path = cgfs_canonical_path,
.escape = cgfs_escape,
+ .num_hierarchies = cgfs_num_hierarchies,
+ .get_hierarchies = cgfs_get_hierarchies,
.get = lxc_cgroupfs_get,
.set = lxc_cgroupfs_set,
.unfreeze = cgfs_unfreeze,
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 95f29ca..5b61554 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1457,6 +1457,31 @@ static bool cgfsng_escape()
return ret;
}
+static int cgfsng_num_hierarchies(void)
+{
+ int i;
+
+ for (i = 0; hierarchies[i]; i++)
+ ;
+
+ return i;
+}
+
+static bool cgfsng_get_hierarchies(int n, char ***out)
+{
+ int i;
+
+ /* sanity check n */
+ for (i = 0; i < n; i++) {
+ if (!hierarchies[i])
+ return false;
+ }
+
+ *out = hierarchies[i]->controllers;
+
+ return true;
+}
+
#define THAWED "THAWED"
#define THAWED_LEN (strlen(THAWED))
@@ -1674,6 +1699,8 @@ static struct cgroup_ops cgfsng_ops = {
.enter = cgfsng_enter,
.canonical_path = cgfsng_canonical_path,
.escape = cgfsng_escape,
+ .num_hierarchies = cgfsng_num_hierarchies,
+ .get_hierarchies = cgfsng_get_hierarchies,
.get_cgroup = cgfsng_get_cgroup,
.get = cgfsng_get,
.set = cgfsng_set,
diff --git a/src/lxc/cgroups/cgmanager.c b/src/lxc/cgroups/cgmanager.c
index 4da891d..f14eb17 100644
--- a/src/lxc/cgroups/cgmanager.c
+++ b/src/lxc/cgroups/cgmanager.c
@@ -337,6 +337,18 @@ static bool cgm_escape(void *hdata)
return ret;
}
+static int cgm_num_hierarchies(void)
+{
+ /* not implemented */
+ return -1;
+}
+
+static bool cgm_get_hierarchies(int i, char ***out)
+{
+ /* not implemented */
+ return false;
+}
+
struct chown_data {
const char *cgroup_path;
uid_t origuid;
@@ -1657,6 +1669,8 @@ static struct cgroup_ops cgmanager_ops = {
.get_cgroup = cgm_get_cgroup,
.canonical_path = cgm_canonical_path,
.escape = cgm_escape,
+ .num_hierarchies = cgm_num_hierarchies,
+ .get_hierarchies = cgm_get_hierarchies,
.get = cgm_get,
.set = cgm_set,
.unfreeze = cgm_unfreeze,
diff --git a/src/lxc/cgroups/cgroup.c b/src/lxc/cgroups/cgroup.c
index 91ef359..48cd403 100644
--- a/src/lxc/cgroups/cgroup.c
+++ b/src/lxc/cgroups/cgroup.c
@@ -132,6 +132,22 @@ const char *cgroup_canonical_path(struct lxc_handler
*handler)
return NULL;
}
+int cgroup_num_hierarchies(void)
+{
+ if (!ops)
+ return -1;
+
+ return ops->num_hierarchies();
+}
+
+bool cgroup_get_hierarchies(int n, char ***out)
+{
+ if (!ops)
+ return false;
+
+ return ops->get_hierarchies(n, out);
+}
+
bool cgroup_unfreeze(struct lxc_handler *handler)
{
if (ops)
diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index e56a115..f65cbfe 100644
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -49,6 +49,8 @@ struct cgroup_ops {
const char *(*get_cgroup)(void *hdata, const char *subsystem);
const char *(*canonical_path)(void *hdata);
bool (*escape)();
+ int (*num_hierarchies)();
+ bool (*get_hierarchies)(int n, char ***out);
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);
@@ -74,6 +76,8 @@ 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);
extern bool cgroup_escape();
+extern int cgroup_num_hierarchies();
+extern bool cgroup_get_hierarchies(int i, char ***out);
/*
* Currently, this call only makes sense for privileged containers.
From aeb3682ff631e6afaa9c0b5d8a32654e8ebd2771 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Wed, 14 Sep 2016 14:46:47 +0000
Subject: [PATCH 3/6] utils: add lxc_deslashify
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/utils.c | 18 ++++++++++++++++++
src/lxc/utils.h | 2 ++
2 files changed, 20 insertions(+)
diff --git a/src/lxc/utils.c b/src/lxc/utils.c
index 27362da..9a6ef4b 100644
--- a/src/lxc/utils.c
+++ b/src/lxc/utils.c
@@ -716,6 +716,24 @@ char **lxc_normalize_path(const char *path)
return components;
}
+bool lxc_deslashify(char *path)
+{
+ char **parts = NULL, *path2;
+
+ parts = lxc_normalize_path(path);
+ if (!parts)
+ return false;
+
+ path2 = lxc_string_join("/", (const char **) parts, *path == '/');
+ lxc_free_array((void **) parts, free);
+ if (!path2)
+ return false;
+
+ strncpy(path, path2, strlen(path));
+ free(path2);
+ return true;
+}
+
char *lxc_append_paths(const char *first, const char *second)
{
size_t len = strlen(first) + strlen(second) + 1;
diff --git a/src/lxc/utils.h b/src/lxc/utils.h
index 98b4e13..c2eef50 100644
--- a/src/lxc/utils.h
+++ b/src/lxc/utils.h
@@ -248,6 +248,8 @@ extern char *lxc_string_join(const char *sep, const char
**parts, bool use_as_pr
* foo//bar -> { foo, bar, NULL }
*/
extern char **lxc_normalize_path(const char *path);
+/* remove multiple slashes from the path, e.g. ///foo//bar -> /foo/bar */
+extern bool lxc_deslashify(char *path);
extern char *lxc_append_paths(const char *first, const char *second);
/* Note: the following two functions use strtok(), so they will never
* consider an empty element, even if two delimiters are next to
From 0ab5703fcff8aafc3512c8b3cb446780b1c6974c Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Wed, 14 Sep 2016 14:47:38 +0000
Subject: [PATCH 4/6] c/r: pass --cgroup-roots on checkpoint
CRIU has added support for passing --cgroup-root on dump, which we should
use (see the criu commit 07d259f365f224b32914de26ea0fd59fc6db0001 for
details). Note that we don't have to do any version checking or anything,
because CRIU just ignored --cgroup-root on checkpoint before, so passing it
is safe, and will result in correct behavior when a sufficient version of
CRIU is present.
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/criu.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 73 insertions(+), 6 deletions(-)
diff --git a/src/lxc/criu.c b/src/lxc/criu.c
index 2799102..0702ad2 100644
--- a/src/lxc/criu.c
+++ b/src/lxc/criu.c
@@ -69,7 +69,7 @@ struct criu_opts {
char tty_id[32]; /* the criu tty id for /dev/console, i.e.
"tty[${rdev}:${dev}]" */
/* restore: the file to write the init process' pid into */
- const char *cgroup_path;
+ struct lxc_handler *handler;
int console_fd;
/* The path that is bind mounted from /dev/console, if any. We don't
* want to use `--ext-mount-map auto`'s result here because the pts
@@ -175,10 +175,10 @@ static void exec_criu(struct criu_opts *opts)
static_args += 2;
} else if (strcmp(opts->action, "restore") == 0) {
/* --root $(lxc_mount_point) --restore-detached
- * --restore-sibling --cgroup-root $foo
+ * --restore-sibling
* --lsm-profile apparmor:whatever
*/
- static_args += 8;
+ static_args += 6;
tty_info[0] = 0;
if (load_tty_major_minor(opts->user->directory, tty_info,
sizeof(tty_info)))
@@ -191,6 +191,8 @@ static void exec_criu(struct criu_opts *opts)
return;
}
+ static_args += 2 * cgroup_num_hierarchies();
+
if (opts->user->verbose)
static_args++;
@@ -244,6 +246,66 @@ static void exec_criu(struct criu_opts *opts)
DECLARE_ARG("-o");
DECLARE_ARG(log);
+ for (i = 0; i < cgroup_num_hierarchies(); i++) {
+ char **controllers = NULL, *fullname;
+ char *path;
+
+ if (!cgroup_get_hierarchies(i, &controllers)) {
+ ERROR("failed to get hierarchy %d", i);
+ goto err;
+ }
+
+ /* if we are in a dump, we have to ask the monitor process what
+ * the right cgroup is. if this is a restore, we can just use
+ * the handler the restore task created.
+ */
+ if (!strcmp(opts->action, "dump") || !strcmp(opts->action,
"pre-dump")) {
+ path = lxc_cmd_get_cgroup_path(opts->c->name,
opts->c->config_path, controllers[0]);
+ if (!path) {
+ ERROR("failed to get cgroup path for %s",
controllers[0]);
+ goto err;
+ }
+ } else {
+ const char *p;
+
+ p = cgroup_get_cgroup(opts->handler, controllers[0]);
+ if (!p) {
+ ERROR("failed to get cgroup path for %s",
controllers[0]);
+ goto err;
+ }
+
+ path = strdup(p);
+ if (!path) {
+ ERROR("strdup failed");
+ goto err;
+ }
+ }
+
+ if (!lxc_deslashify(path)) {
+ ERROR("failed to deslashify %s", path);
+ free(path);
+ goto err;
+ }
+
+ fullname = lxc_string_join(",", (const char **) controllers,
false);
+ if (!fullname) {
+ ERROR("failed to join controllers");
+ free(path);
+ goto err;
+ }
+
+ ret = sprintf(buf, "%s:%s", fullname, path);
+ free(path);
+ free(fullname);
+ if (ret < 0 || ret >= sizeof(buf)) {
+ ERROR("sprintf of cgroup root arg failed");
+ goto err;
+ }
+
+ DECLARE_ARG("--cgroup-root");
+ DECLARE_ARG(buf);
+ }
+
if (opts->user->verbose)
DECLARE_ARG("-vvvvvv");
@@ -329,8 +391,6 @@ static void exec_criu(struct criu_opts *opts)
DECLARE_ARG(opts->c->lxc_conf->rootfs.mount);
DECLARE_ARG("--restore-detached");
DECLARE_ARG("--restore-sibling");
- DECLARE_ARG("--cgroup-root");
- DECLARE_ARG(opts->cgroup_path);
if (tty_info[0]) {
if (opts->console_fd < 0) {
@@ -682,9 +742,9 @@ static void do_restore(struct lxc_container *c, int
status_pipe, struct migrate_
os.action = "restore";
os.user = opts;
os.c = c;
- os.cgroup_path = cgroup_canonical_path(handler);
os.console_fd = c->lxc_conf->console.slave;
os.criu_version = criu_version;
+ os.handler = handler;
if (os.console_fd >= 0) {
/* Twiddle the FD_CLOEXEC bit. We want to pass this FD
to criu
@@ -891,6 +951,13 @@ static bool do_dump(struct lxc_container *c, char *mode,
struct migrate_opts *op
if (pid == 0) {
struct criu_opts os;
+ struct lxc_handler h;
+
+ h.name = c->name;
+ if (!cgroup_init(&h)) {
+ ERROR("failed to cgroup_init()");
+ exit(1);
+ }
os.action = mode;
os.user = opts;
From 6df334d1588dfcf99a405fb700971ac27cac438c Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Wed, 14 Sep 2016 14:53:21 +0000
Subject: [PATCH 5/6] cgroup: get rid of weird hack in cgfsng_escape
We initialized cgfsng in a strange way inside of its implementation of
escape so we could use it during checkpoint. Instead, the previous patch
does a hacky initialization in criu.c, and we can get rid of the hacks
elsewhere :)
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/cgroups/cgfsng.c | 15 ++-------------
1 file changed, 2 insertions(+), 13 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 5b61554..0777bf3 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1426,19 +1426,11 @@ static int cgfsng_nrtasks(void *hdata) {
/* Only root needs to escape to the cgroup of its init */
static bool cgfsng_escape()
{
- struct cgfsng_handler_data *d;
int i;
- bool ret = false;
if (geteuid())
return true;
- d = cgfsng_init("criu-temp-cgfsng");
- if (!d) {
- ERROR("cgfsng_init failed");
- return false;
- }
-
for (i = 0; hierarchies[i]; i++) {
char *fullpath = must_make_path(hierarchies[i]->mountpoint,
hierarchies[i]->base_cgroup,
@@ -1446,15 +1438,12 @@ static bool cgfsng_escape()
if (lxc_write_to_file(fullpath, "0", 2, false) != 0) {
SYSERROR("Failed to escape to %s", fullpath);
free(fullpath);
- goto out;
+ return false;
}
free(fullpath);
}
- ret = true;
-out:
- free_handler_data(d);
- return ret;
+ return true;
}
static int cgfsng_num_hierarchies(void)
From a0c91fccd9a1ae9c68a10880d4660d63a3b411e8 Mon Sep 17 00:00:00 2001
From: Tycho Andersen <tycho.ander...@canonical.com>
Date: Wed, 14 Sep 2016 14:58:38 +0000
Subject: [PATCH 6/6] cgroup: drop cgroup_canonical_path
This is almost never the right thing to use, and we don't use it any more
anyway.
Signed-off-by: Tycho Andersen <tycho.ander...@canonical.com>
---
src/lxc/cgroups/cgfs.c | 23 -----------------------
src/lxc/cgroups/cgfsng.c | 8 --------
src/lxc/cgroups/cgmanager.c | 10 ----------
src/lxc/cgroups/cgroup.c | 13 -------------
src/lxc/cgroups/cgroup.h | 6 ------
5 files changed, 60 deletions(-)
diff --git a/src/lxc/cgroups/cgfs.c b/src/lxc/cgroups/cgfs.c
index 80a336d..c5ba765 100644
--- a/src/lxc/cgroups/cgfs.c
+++ b/src/lxc/cgroups/cgfs.c
@@ -2363,28 +2363,6 @@ 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_escape(void *hdata)
{
struct cgroup_meta_data *md;
@@ -2637,7 +2615,6 @@ static struct cgroup_ops cgfs_ops = {
.enter = cgfs_enter,
.create_legacy = cgfs_create_legacy,
.get_cgroup = cgfs_get_cgroup,
- .canonical_path = cgfs_canonical_path,
.escape = cgfs_escape,
.num_hierarchies = cgfs_num_hierarchies,
.get_hierarchies = cgfs_get_hierarchies,
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 0777bf3..ec94099 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1087,13 +1087,6 @@ static inline bool cgfsng_create(void *hdata)
return false;
}
-static const char *cgfsng_canonical_path(void *hdata)
-{
- struct cgfsng_handler_data *d = hdata;
-
- return d->container_cgroup;
-}
-
static bool cgfsng_enter(void *hdata, pid_t pid)
{
char pidstr[25];
@@ -1686,7 +1679,6 @@ static struct cgroup_ops cgfsng_ops = {
.destroy = cgfsng_destroy,
.create = cgfsng_create,
.enter = cgfsng_enter,
- .canonical_path = cgfsng_canonical_path,
.escape = cgfsng_escape,
.num_hierarchies = cgfsng_num_hierarchies,
.get_hierarchies = cgfsng_get_hierarchies,
diff --git a/src/lxc/cgroups/cgmanager.c b/src/lxc/cgroups/cgmanager.c
index f14eb17..f2756b0 100644
--- a/src/lxc/cgroups/cgmanager.c
+++ b/src/lxc/cgroups/cgmanager.c
@@ -746,15 +746,6 @@ 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;
@@ -1667,7 +1658,6 @@ static struct cgroup_ops cgmanager_ops = {
.enter = cgm_enter,
.create_legacy = NULL,
.get_cgroup = cgm_get_cgroup,
- .canonical_path = cgm_canonical_path,
.escape = cgm_escape,
.num_hierarchies = cgm_num_hierarchies,
.get_hierarchies = cgm_get_hierarchies,
diff --git a/src/lxc/cgroups/cgroup.c b/src/lxc/cgroups/cgroup.c
index 48cd403..78472d4 100644
--- a/src/lxc/cgroups/cgroup.c
+++ b/src/lxc/cgroups/cgroup.c
@@ -119,19 +119,6 @@ bool cgroup_escape(struct lxc_handler *handler)
return false;
}
-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;
-}
-
int cgroup_num_hierarchies(void)
{
if (!ops)
diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h
index f65cbfe..11b251e 100644
--- a/src/lxc/cgroups/cgroup.h
+++ b/src/lxc/cgroups/cgroup.h
@@ -47,7 +47,6 @@ 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);
bool (*escape)();
int (*num_hierarchies)();
bool (*get_hierarchies)(int n, char ***out);
@@ -78,11 +77,6 @@ extern const char *cgroup_get_cgroup(struct lxc_handler
*handler, const char *su
extern bool cgroup_escape();
extern int cgroup_num_hierarchies();
extern bool cgroup_get_hierarchies(int i, char ***out);
-
-/*
- * 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);
extern cgroup_driver_t cgroup_driver(void);
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
