[lxc-devel] [lxc/master] cgroups/cgfsng: fixes, features, and improved cgroup2 handling
brauner on Github
lxc-bot at linuxcontainers.org
Tue Oct 31 16:09:30 UTC 2017
A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 364 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20171031/7a1ca34c/attachment.bin>
-------------- next part --------------
From 5eac91bca9b12c583aad7e6426ec99c75d4eee1c Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 11:15:07 +0100
Subject: [PATCH 1/9] cgroups/cgfsng: keep mountpoint intact
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index ec6440c13..41b8a0ed0 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -760,9 +760,10 @@ static char **get_controllers(char **klist, char **nlist, char *line)
{
/* the fourth field is /sys/fs/cgroup/comma-delimited-controller-list */
int i;
- char *p = line, *p2, *tok, *saveptr = NULL;
- char **aret = NULL;
+ char *dup, *p2, *tok;
bool is_cgroup_v2;
+ char *p = line, *saveptr = NULL;
+ char **aret = NULL;
/* handle cgroup v2 */
is_cgroup_v2 = is_cgroupfs_v2(line);
@@ -792,10 +793,19 @@ static char **get_controllers(char **klist, char **nlist, char *line)
/* cgroup v2 does not have separate mountpoints for controllers */
if (is_cgroup_v2) {
must_append_controller(klist, nlist, &aret, "cgroup2");
- return aret;
+ return NULL;
+ }
+
+ /* strdup() here for v1 hierarchies. Otherwise strtok_r() will destroy
+ * mountpoints such as "/sys/fs/cgroup/cpu,cpuacct".
+ */
+ dup = strdup(p);
+ if (!dup) {
+ SYSERROR("Failed to duplicate string");
+ return NULL;
}
- for (tok = strtok_r(p, ",", &saveptr); tok;
+ for (tok = strtok_r(dup, ",", &saveptr); tok;
tok = strtok_r(NULL, ",", &saveptr)) {
must_append_controller(klist, nlist, &aret, tok);
}
From eab2343f5a11b4d8afdf62d7d80a65f1ec70a0b9 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 11:55:23 +0100
Subject: [PATCH 2/9] cgroups/cgfsng: make sure pointer is NULL
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 41b8a0ed0..5cfcea185 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1186,6 +1186,8 @@ static void *cgfsng_init(struct lxc_handler *handler)
d->name = must_copy_string(handler->name);
/* copy per-container cgroup information */
+ d->cgroup_meta.dir = NULL;
+ d->cgroup_meta.controllers = NULL;
if (handler->conf) {
d->cgroup_meta.dir = must_copy_string(handler->conf->cgroup_meta.dir);
d->cgroup_meta.controllers = must_copy_string(handler->conf->cgroup_meta.controllers);
From 15665dffd92df004742427d5cfa2e8c7c7fbd560 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 11:16:46 +0100
Subject: [PATCH 3/9] cgroups/cgfsng: cgfsns_chown() -> cgfsng_chown()
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 5cfcea185..e303e6f1c 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1519,7 +1519,7 @@ static int chown_cgroup_wrapper(void *data)
return 0;
}
-static bool cgfsns_chown(void *hdata, struct lxc_conf *conf)
+static bool cgfsng_chown(void *hdata, struct lxc_conf *conf)
{
struct cgfsng_handler_data *d = hdata;
struct chown_data wrap;
@@ -2164,7 +2164,7 @@ static struct cgroup_ops cgfsng_ops = {
.setup_limits = cgfsng_setup_limits,
.name = "cgroupfs-ng",
.attach = cgfsng_attach,
- .chown = cgfsns_chown,
+ .chown = cgfsng_chown,
.mount_cgroup = cgfsng_mount,
.nrtasks = cgfsng_nrtasks,
.driver = CGFSNG,
From 03a043c20dfcb7a29b90b8fc7fe07dc4d1f685b0 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 12:00:49 +0100
Subject: [PATCH 4/9] cgroups/cgfsng: record all hierarchies
Before we only recorded writable hierarchies. But for unprivileged containers
that lack CAP_SYS_ADMIN in their user namespace we likely want to mount all
controllers including ones that are not writable.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 67 +++++++++++++++++++++++++-----------------------
1 file changed, 35 insertions(+), 32 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index e303e6f1c..0ee6ab04c 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -79,6 +79,7 @@ struct hierarchy {
char *base_cgroup;
char *fullcgpath;
bool is_cgroup_v2;
+ bool writable;
};
/*
@@ -243,7 +244,7 @@ struct hierarchy *get_hierarchy(const char *c)
if (!hierarchies)
return NULL;
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
if (string_in_list(hierarchies[i]->controllers, c))
return hierarchies[i];
}
@@ -708,7 +709,7 @@ static bool controller_found(struct hierarchy **hlist, char *entry)
if (!hlist)
return false;
- for (i = 0; hlist[i]; i++)
+ for (i = 0; hlist[i] && hlist[i]->writable; i++)
if (string_in_list(hlist[i]->controllers, entry))
return true;
return false;
@@ -831,6 +832,11 @@ static void add_controller(char **clist, char *mountpoint, char *base_cgroup)
else
new->is_cgroup_v2 = false;
+ if (new->is_cgroup_v2)
+ new->writable = test_writeable_v2(mountpoint, base_cgroup);
+ else
+ new->writable = test_writeable_v1(mountpoint, base_cgroup);
+
newentry = append_null_to_list((void ***)&hierarchies);
hierarchies[newentry] = new;
}
@@ -1027,8 +1033,9 @@ static void lxc_cgfsng_print_hierarchies()
for (i = 0, it = hierarchies; it && *it; it++, i++) {
char **cit;
int j;
- printf(" %d: base_cgroup %s\n", i, (*it)->base_cgroup ? (*it)->base_cgroup : "(null)");
- printf(" mountpoint %s\n", (*it)->mountpoint ? (*it)->mountpoint : "(null)");
+ printf(" %d: base_cgroup: %s\n", i, (*it)->base_cgroup ? (*it)->base_cgroup : "(null)");
+ printf(" mountpoint: %s\n", (*it)->mountpoint ? (*it)->mountpoint : "(null)");
+ printf(" writable %s:\n", (*it)->writable ? "yes" : "no");
printf(" controllers:\n");
for (j = 0, cit = (*it)->controllers; cit && *cit; cit++, j++)
printf(" %d: %s\n", j, *cit);
@@ -1091,7 +1098,7 @@ static bool parse_hierarchies(void)
while (getline(&line, &len, f) != -1) {
char **controller_list = NULL;
char *mountpoint, *base_cgroup;
- bool is_cgroup_v2, writeable;
+ bool is_cgroup_v2;
is_cgroup_v2 = is_cgroupfs_v2(line);
if (!is_lxcfs(line) && !is_cgroupfs_v1(line) && !is_cgroup_v2)
@@ -1123,16 +1130,6 @@ static bool parse_hierarchies(void)
trim(base_cgroup);
prune_init_scope(base_cgroup);
- if (is_cgroup_v2)
- writeable = test_writeable_v2(mountpoint, base_cgroup);
- else
- writeable = test_writeable_v1(mountpoint, base_cgroup);
- if (!writeable) {
- free_string_list(controller_list);
- free(mountpoint);
- free(base_cgroup);
- continue;
- }
add_controller(controller_list, mountpoint, base_cgroup);
}
@@ -1144,10 +1141,8 @@ static bool parse_hierarchies(void)
fclose(f);
free(line);
- if (lxc_cgfsng_debug) {
- printf("writeable subsystems:\n");
+ if (lxc_cgfsng_debug)
lxc_cgfsng_print_hierarchies();
- }
/* verify that all controllers in cgroup.use and all crucial
* controllers are accounted for
@@ -1308,7 +1303,7 @@ static void cgfsng_destroy(void *hdata, struct lxc_conf *conf)
if (d->container_cgroup && hierarchies) {
int i;
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
struct hierarchy *h = hierarchies[i];
if (h->fullcgpath) {
recursive_destroy(h->fullcgpath, conf);
@@ -1407,7 +1402,7 @@ static inline bool cgfsng_create(void *hdata)
}
}
}
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
if (!create_path_for_hierarchy(hierarchies[i], cgname)) {
int j;
ERROR("Failed to create \"%s\"", hierarchies[i]->fullcgpath);
@@ -1437,7 +1432,7 @@ static bool cgfsng_enter(void *hdata, pid_t pid)
if (len < 0 || len > 25)
return false;
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
char *fullpath = must_make_path(hierarchies[i]->fullcgpath,
"cgroup.procs", NULL);
if (lxc_write_to_file(fullpath, pidstr, len, false) != 0) {
@@ -1479,7 +1474,7 @@ static int chown_cgroup_wrapper(void *data)
destuid = get_ns_uid(arg->origuid);
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
char *fullpath, *path = hierarchies[i]->fullcgpath;
if (chown(path, destuid, 0) < 0) {
@@ -1689,6 +1684,9 @@ static bool cgfsng_mount(void *hdata, const char *root, int type)
goto bad;
for (i = 0; hierarchies[i]; i++) {
+ if (!has_cgns && !hierarchies[i]->writable)
+ continue;
+
char *controllerpath, *path2;
struct hierarchy *h = hierarchies[i];
char *controller = strrchr(h->mountpoint, '/');
@@ -1794,15 +1792,20 @@ static int recursive_count_nrtasks(char *dirname)
}
static int cgfsng_nrtasks(void *hdata) {
- struct cgfsng_handler_data *d = hdata;
+ int i;
char *path;
- int count;
+ struct cgfsng_handler_data *d = hdata;
+ int count = -1;
if (!d || !d->container_cgroup || !hierarchies)
- return -1;
- path = must_make_path(hierarchies[0]->fullcgpath, NULL);
- count = recursive_count_nrtasks(path);
- free(path);
+ return count;
+
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
+ path = must_make_path(hierarchies[i]->fullcgpath, NULL);
+ count = recursive_count_nrtasks(path);
+ free(path);
+ break;
+ }
return count;
}
@@ -1814,7 +1817,7 @@ static bool cgfsng_escape()
if (geteuid())
return true;
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
char *fullpath = must_make_path(hierarchies[i]->mountpoint,
hierarchies[i]->base_cgroup,
"cgroup.procs", NULL);
@@ -1833,7 +1836,7 @@ static int cgfsng_num_hierarchies(void)
{
int i;
- for (i = 0; hierarchies[i]; i++)
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++)
;
return i;
@@ -1845,7 +1848,7 @@ static bool cgfsng_get_hierarchies(int n, char ***out)
/* sanity check n */
for (i = 0; i < n; i++) {
- if (!hierarchies[i])
+ if (!hierarchies[i] || !hierarchies[i]->writable)
return false;
}
@@ -1902,7 +1905,7 @@ static bool cgfsng_attach(const char *name, const char *lxcpath, pid_t pid)
if (len < 0 || len > 25)
return false;
- for (i = 0; hierarchies[i]; i++) {
+ for (i = 0; hierarchies[i] && hierarchies[i]->writable; i++) {
char *path, *fullpath;
struct hierarchy *h = hierarchies[i];
From 546db863a21a9e75b24478f9bba52895e59c8f5d Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 12:01:29 +0100
Subject: [PATCH 5/9] cgroups/cgfsng: support MS_READONLY with cgroup ns
If we lack CAP_SYS_ADMIN this is really useful.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 31 ++++++++++++++++++++-----------
1 file changed, 20 insertions(+), 11 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 0ee6ab04c..b52033f1f 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -1624,27 +1624,36 @@ do_secondstage_mounts_if_needed(int type, struct hierarchy *h,
return 0;
}
-static int mount_cgroup_cgns_supported(struct hierarchy *h, const char *controllerpath)
+static int mount_cgroup_cgns_supported(int type, struct hierarchy *h, const char *controllerpath)
{
int ret;
char *controllers = NULL;
- char *type = "cgroup2";
+ char *fstype = "cgroup2";
+ unsigned long flags = 0;
- if (!h->is_cgroup_v2) {
- controllers = lxc_string_join(",", (const char **)h->controllers, false);
- if (!controllers)
- return -ENOMEM;
- type = "cgroup";
+ flags |= MS_NOSUID;
+ flags |= MS_NOEXEC;
+ flags |= MS_NODEV;
+ flags |= MS_RELATIME;
+
+ if (type == LXC_AUTO_CGROUP_RO || type == LXC_AUTO_CGROUP_FULL_RO)
+ flags |= MS_RDONLY;
+
+ if (!h->is_cgroup_v2) {
+ controllers = lxc_string_join(",", (const char **)h->controllers, false);
+ if (!controllers)
+ return -ENOMEM;
+ fstype = "cgroup";
}
- ret = mount("cgroup", controllerpath, type, MS_NOSUID|MS_NOEXEC|MS_NODEV|MS_RELATIME, controllers);
+ ret = mount("cgroup", controllerpath, fstype, flags, controllers);
free(controllers);
if (ret < 0) {
- SYSERROR("Failed to mount %s with cgroup filesystem type %s", controllerpath, type);
+ SYSERROR("Failed to mount %s with cgroup filesystem type %s", controllerpath, fstype);
return -1;
}
- DEBUG("Mounted %s with cgroup filesystem type %s", controllerpath, type);
+ DEBUG("Mounted %s with cgroup filesystem type %s", controllerpath, fstype);
return 0;
}
@@ -1711,7 +1720,7 @@ static bool cgfsng_mount(void *hdata, const char *root, int type)
* will not have CAP_SYS_ADMIN after it has started we
* need to mount the cgroups manually.
*/
- r = mount_cgroup_cgns_supported(h, controllerpath);
+ r = mount_cgroup_cgns_supported(type, h, controllerpath);
free(controllerpath);
if (r < 0)
goto bad;
From c2b1ee98dc54d5ffd5293c71097dcb7889229d10 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 12:05:13 +0100
Subject: [PATCH 6/9] log: check for i/o error with vsnprintf()
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/log.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/lxc/log.c b/src/lxc/log.c
index 16b79c09d..351b191fb 100644
--- a/src/lxc/log.c
+++ b/src/lxc/log.c
@@ -264,7 +264,7 @@ static int log_append_logfile(const struct lxc_log_appender *appender,
{
char buffer[LXC_LOG_BUFFER_SIZE];
char date_time[LXC_LOG_TIME_SIZE];
- int n;
+ int n, ret;
int fd_to_use = -1;
#ifndef NO_LXC_CONF
@@ -295,8 +295,13 @@ static int log_append_logfile(const struct lxc_log_appender *appender,
if (n < 0)
return n;
- if ((size_t)n < (sizeof(buffer) - 1))
- n += vsnprintf(buffer + n, sizeof(buffer) - n, event->fmt, *event->vap);
+ if ((size_t)n < (sizeof(buffer) - 1)) {
+ ret = vsnprintf(buffer + n, sizeof(buffer) - n, event->fmt, *event->vap);
+ if (ret < 0)
+ return 0;
+
+ n += ret;
+ }
if ((size_t)n >= sizeof(buffer))
n = sizeof(buffer) - 1;
From 795153310877211bc2e4b3db845f2cc756574b10 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 17:05:22 +0100
Subject: [PATCH 7/9] cgroupfs/cgfsng: introduce LOG_CONSTRUCTOR()
Logging in the construct is a bad idea since a constructor is guaranteed to run
before anything else so logging in the wild is really a bad idea.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 47 +++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index b52033f1f..62d25f7cc 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -61,6 +61,12 @@
lxc_log_define(lxc_cgfsng, lxc);
+#define LOG_CONSTRUCTOR(format, ...) \
+ do { \
+ fprintf(stderr, "%s: %d: %s: " format, __FILE__, __LINE__, \
+ __func__, __VA_ARGS__); \
+ } while (false)
+
static struct cgroup_ops cgfsng_ops;
/*
@@ -205,8 +211,8 @@ static void must_append_controller(char **klist, char **nlist, char ***clist, ch
char *copy;
if (string_in_list(klist, entry) && string_in_list(nlist, entry)) {
- ERROR("Refusing to use ambiguous controller '%s'", entry);
- ERROR("It is both a named and kernel subsystem");
+ LOG_CONSTRUCTOR("Refusing to use ambiguous controller \"%s\"", entry);
+ LOG_CONSTRUCTOR("%s", "It is both a named and kernel subsystem\n");
return;
}
@@ -779,14 +785,14 @@ static char **get_controllers(char **klist, char **nlist, char *line)
return NULL;
/* note - if we change how mountinfo works, then our caller
* will need to verify /sys/fs/cgroup/ in this field */
- if (strncmp(p, "/sys/fs/cgroup/", 15) != 0) {
- INFO("cgfsng: found hierarchy not under /sys/fs/cgroup: \"%s\"", p);
+ if (strncmp(p, "/sys/fs/cgroup/", 15)) {
+ LOG_CONSTRUCTOR("Found hierarchy not under /sys/fs/cgroup: \"%s\"\n", p);
return NULL;
}
p += 15;
p2 = strchr(p, ' ');
if (!p2) {
- ERROR("corrupt mountinfo");
+ LOG_CONSTRUCTOR("%s", "Corrupt mountinfo\n");
return NULL;
}
*p2 = '\0';
@@ -802,7 +808,7 @@ static char **get_controllers(char **klist, char **nlist, char *line)
*/
dup = strdup(p);
if (!dup) {
- SYSERROR("Failed to duplicate string");
+ LOG_CONSTRUCTOR("%s", "Failed to duplicate string\n");
return NULL;
}
@@ -1026,17 +1032,17 @@ static void lxc_cgfsng_print_hierarchies()
int i;
if (!hierarchies) {
- printf(" No hierarchies found.");
+ LOG_CONSTRUCTOR("%s", " No hierarchies found\n");
return;
}
printf(" Hierarchies:\n");
for (i = 0, it = hierarchies; it && *it; it++, i++) {
char **cit;
int j;
- printf(" %d: base_cgroup: %s\n", i, (*it)->base_cgroup ? (*it)->base_cgroup : "(null)");
- printf(" mountpoint: %s\n", (*it)->mountpoint ? (*it)->mountpoint : "(null)");
- printf(" writable %s:\n", (*it)->writable ? "yes" : "no");
- printf(" controllers:\n");
+ LOG_CONSTRUCTOR(" %d: base_cgroup: %s\n", i, (*it)->base_cgroup ? (*it)->base_cgroup : "(null)");
+ LOG_CONSTRUCTOR(" mountpoint: %s\n", (*it)->mountpoint ? (*it)->mountpoint : "(null)");
+ LOG_CONSTRUCTOR(" writable %s:\n", (*it)->writable ? "yes" : "no");
+ LOG_CONSTRUCTOR("%s", " controllers:\n");
for (j = 0, cit = (*it)->controllers; cit && *cit; cit++, j++)
printf(" %d: %s\n", j, *cit);
}
@@ -1047,13 +1053,13 @@ static void lxc_cgfsng_print_basecg_debuginfo(char *basecginfo, char **klist, ch
int k;
char **it;
- printf("basecginfo is:\n");
- printf("%s\n", basecginfo);
+ LOG_CONSTRUCTOR("%s", "basecginfo is:\n");
+ LOG_CONSTRUCTOR("%s\n", basecginfo);
for (k = 0, it = klist; it && *it; it++, k++)
- printf("kernel subsystem %d: %s\n", k, *it);
+ LOG_CONSTRUCTOR("kernel subsystem %d: %s\n", k, *it);
for (k = 0, it = nlist; it && *it; it++, k++)
- printf("named subsystem %d: %s\n", k, *it);
+ LOG_CONSTRUCTOR("named subsystem %d: %s\n", k, *it);
}
static void lxc_cgfsng_print_debuginfo(const struct cgfsng_handler_data *d)
@@ -1085,7 +1091,7 @@ static bool parse_hierarchies(void)
return false;
if ((f = fopen("/proc/self/mountinfo", "r")) == NULL) {
- SYSERROR("Failed opening /proc/self/mountinfo");
+ LOG_CONSTRUCTOR("%s", "Failed to open \"/proc/self/mountinfo\"\n");
return false;
}
@@ -1115,14 +1121,14 @@ static bool parse_hierarchies(void)
mountpoint = get_mountpoint(line);
if (!mountpoint) {
- ERROR("Error reading mountinfo: bad line '%s'", line);
+ LOG_CONSTRUCTOR("Failed reading \"/proc/self/mountinfo\": %s\n", line);
free_string_list(controller_list);
continue;
}
base_cgroup = get_current_cgroup(basecginfo, controller_list[0]);
if (!base_cgroup) {
- ERROR("Failed to find current cgroup for controller '%s'", controller_list[0]);
+ LOG_CONSTRUCTOR("Failed to find current cgroup for controller \"%s\"\n", controller_list[0]);
free_string_list(controller_list);
free(mountpoint);
continue;
@@ -1148,7 +1154,8 @@ static bool parse_hierarchies(void)
* controllers are accounted for
*/
if (!all_controllers_found()) {
- INFO("cgfsng: not all controllers were find, deferring to cgfs driver");
+ LOG_CONSTRUCTOR("%s", "Did not find all controllers. Deferring "
+ "to the cgfs driver\n");
return false;
}
@@ -1161,7 +1168,7 @@ static bool collect_hierarchy_info(void)
errno = 0;
tmp = lxc_global_config_value("lxc.cgroup.use");
if (!cgroup_use && errno != 0) { /* lxc.cgroup.use can be NULL */
- SYSERROR("cgfsng: error reading list of cgroups to use");
+ LOG_CONSTRUCTOR("%s", "cgfsng - Failed to retrieve list of cgroups to use\n");
return false;
}
cgroup_use = must_copy_string(tmp);
From c6acee5064f2072af59a9ef23271e27a39a1ab2f Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 17:06:58 +0100
Subject: [PATCH 8/9] cgroups/cgfsng: fix get_controllers() for cgroup2
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 62d25f7cc..3f3e4b34d 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -800,7 +800,7 @@ static char **get_controllers(char **klist, char **nlist, char *line)
/* cgroup v2 does not have separate mountpoints for controllers */
if (is_cgroup_v2) {
must_append_controller(klist, nlist, &aret, "cgroup2");
- return NULL;
+ return aret;
}
/* strdup() here for v1 hierarchies. Otherwise strtok_r() will destroy
From 6849a0365cfa96345ac233aaea6546be808850c1 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brauner at ubuntu.com>
Date: Tue, 31 Oct 2017 17:07:57 +0100
Subject: [PATCH 9/9] cgroupfs/cgfsng: improve cgroup2 handling
This fixes a bunch of bugs.
Signed-off-by: Christian Brauner <christian.brauner at ubuntu.com>
---
src/lxc/cgroups/cgfsng.c | 30 ++++++++++--------------------
src/lxc/cgroups/cgroup_utils.c | 41 ++++++++++++++++++++++++++++++++++++++++-
src/lxc/cgroups/cgroup_utils.h | 10 ++++++++++
3 files changed, 60 insertions(+), 21 deletions(-)
diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 3f3e4b34d..6680ecbf5 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -222,6 +222,8 @@ static void must_append_controller(char **klist, char **nlist, char ***clist, ch
copy = must_copy_string(entry);
else if (string_in_list(klist, entry))
copy = must_copy_string(entry);
+ else if (!strcmp(entry, "cgroup2"))
+ copy = must_copy_string(entry);
else
copy = must_prefix_named(entry);
@@ -747,15 +749,6 @@ static bool all_controllers_found(void)
return true;
}
-/* Return true if the fs type is fuse.lxcfs */
-static bool is_lxcfs(const char *line)
-{
- char *p = strstr(line, " - ");
- if (!p)
- return false;
- return strncmp(p, " - fuse.lxcfs ", 14) == 0;
-}
-
/*
* Get the controllers from a mountinfo line
* There are other ways we could get this info. For lxcfs, field 3
@@ -763,18 +756,14 @@ static bool is_lxcfs(const char *line)
* options. But we simply assume that the mountpoint must be
* /sys/fs/cgroup/controller-list
*/
-static char **get_controllers(char **klist, char **nlist, char *line)
+static char **get_controllers(char **klist, char **nlist, char *line, int type)
{
/* the fourth field is /sys/fs/cgroup/comma-delimited-controller-list */
int i;
char *dup, *p2, *tok;
- bool is_cgroup_v2;
char *p = line, *saveptr = NULL;
char **aret = NULL;
- /* handle cgroup v2 */
- is_cgroup_v2 = is_cgroupfs_v2(line);
-
for (i = 0; i < 4; i++) {
p = strchr(p, ' ');
if (!p)
@@ -786,7 +775,8 @@ static char **get_controllers(char **klist, char **nlist, char *line)
/* note - if we change how mountinfo works, then our caller
* will need to verify /sys/fs/cgroup/ in this field */
if (strncmp(p, "/sys/fs/cgroup/", 15)) {
- LOG_CONSTRUCTOR("Found hierarchy not under /sys/fs/cgroup: \"%s\"\n", p);
+ if (type != LXCFS_CGROUP)
+ LOG_CONSTRUCTOR("Found hierarchy not under /sys/fs/cgroup: \"%s\"\n", p);
return NULL;
}
p += 15;
@@ -798,7 +788,7 @@ static char **get_controllers(char **klist, char **nlist, char *line)
*p2 = '\0';
/* cgroup v2 does not have separate mountpoints for controllers */
- if (is_cgroup_v2) {
+ if (type == CGROUP_V2) {
must_append_controller(klist, nlist, &aret, "cgroup2");
return aret;
}
@@ -1104,13 +1094,13 @@ static bool parse_hierarchies(void)
while (getline(&line, &len, f) != -1) {
char **controller_list = NULL;
char *mountpoint, *base_cgroup;
- bool is_cgroup_v2;
+ int type;
- is_cgroup_v2 = is_cgroupfs_v2(line);
- if (!is_lxcfs(line) && !is_cgroupfs_v1(line) && !is_cgroup_v2)
+ type = get_cgroup_version(line);
+ if (type < 0)
continue;
- controller_list = get_controllers(klist, nlist, line);
+ controller_list = get_controllers(klist, nlist, line, type);
if (!controller_list)
continue;
diff --git a/src/lxc/cgroups/cgroup_utils.c b/src/lxc/cgroups/cgroup_utils.c
index c09ba1688..b935cb460 100644
--- a/src/lxc/cgroups/cgroup_utils.c
+++ b/src/lxc/cgroups/cgroup_utils.c
@@ -32,6 +32,29 @@
#include "cgroup_utils.h"
#include "utils.h"
+int get_cgroup_version(char *line)
+{
+ if (is_cgroupfs_v1(line))
+ return CGROUP_V1;
+
+ if (is_cgroupfs_v2(line))
+ return CGROUP_V2;
+
+ if (is_lxcfs(line))
+ return LXCFS_CGROUP;
+
+ return -1;
+}
+
+/* Return true if the fs type is fuse.lxcfs */
+bool is_lxcfs(const char *line)
+{
+ char *p = strstr(line, " - ");
+ if (!p)
+ return false;
+ return strncmp(p, " - fuse.lxcfs ", 14) == 0;
+}
+
bool is_cgroupfs_v1(char *line)
{
char *p = strstr(line, " - ");
@@ -67,7 +90,7 @@ bool test_writeable_v2(char *mountpoint, char *path)
* file.
*/
int ret;
- char *cgroup_path, *cgroup_procs_file;
+ char *cgroup_path, *cgroup_procs_file, *cgroup_threads_file;
cgroup_path = must_make_path(mountpoint, path, NULL);
cgroup_procs_file = must_make_path(cgroup_path, "cgroup.procs", NULL);
@@ -81,6 +104,22 @@ bool test_writeable_v2(char *mountpoint, char *path)
ret = access(cgroup_procs_file, W_OK);
free(cgroup_procs_file);
+ if (ret < 0)
+ return false;
+
+ /* Newer versions of cgroup2 now also require write access to the
+ * "cgroup.threads" file.
+ */
+ cgroup_threads_file = must_make_path(cgroup_path, "cgroup.threads", NULL);
+ if (!file_exists(cgroup_threads_file)) {
+ free(cgroup_threads_file);
+ return true;
+ }
+
+ ret = access(cgroup_threads_file, W_OK);
+ free(cgroup_threads_file);
+ if (ret < 0)
+ return false;
return ret == 0;
}
diff --git a/src/lxc/cgroups/cgroup_utils.h b/src/lxc/cgroups/cgroup_utils.h
index 49aae8567..b05fd7041 100644
--- a/src/lxc/cgroups/cgroup_utils.h
+++ b/src/lxc/cgroups/cgroup_utils.h
@@ -28,12 +28,22 @@
#include <stdbool.h>
#include <stdio.h>
+#define CGROUP_V1 0
+#define CGROUP_V2 1
+#define LXCFS_CGROUP 2
+
+/* Retrieve the cgroup version of a given entry from /proc/<pid>/mountinfo. */
+extern int get_cgroup_version(char *line);
+
/* Check if given entry from /proc/<pid>/mountinfo is a cgroupfs v1 mount. */
extern bool is_cgroupfs_v1(char *line);
/* Check if given entry from /proc/<pid>/mountinfo is a cgroupfs v2 mount. */
extern bool is_cgroupfs_v2(char *line);
+/* Check if given entry from /proc/<pid>/mountinfo is a lxcfs mount. */
+extern bool is_lxcfs(const char *line);
+
/* Given a v1 hierarchy @mountpoint and base @path, verify that we can create
* directories underneath it.
*/
More information about the lxc-devel
mailing list