The following pull request was submitted through Github.
It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/3225

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) ===
Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
From f990d3bfde5cf50c393f4c4220e0a7a0e22dc2e6 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Tue, 10 Dec 2019 18:07:47 +0100
Subject: [PATCH 1/2] cgroupfs/cgfsng: pass cgroup to
 cg_legacy_handle_cpuset_hierarchy() as const char *

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 src/lxc/cgroups/cgfsng.c | 42 ++++++++++++++++++++++++----------------
 1 file changed, 25 insertions(+), 17 deletions(-)

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index 0469a101b6..fa00dbf44e 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -553,7 +553,8 @@ static bool is_unified_hierarchy(const struct hierarchy *h)
        return h->version == CGROUP2_SUPER_MAGIC;
 }
 
-/* Initialize the cpuset hierarchy in first directory of @gname and set
+/*
+ * Initialize the cpuset hierarchy in first directory of @cgroup_leaf and set
  * cgroup.clone_children so that children inherit settings. Since the
  * h->base_path is populated by init or ourselves, we know it is already
  * initialized.
@@ -561,14 +562,16 @@ static bool is_unified_hierarchy(const struct hierarchy 
*h)
  * returns -1 on error, 0 when we didn't created a cgroup, 1 if we created a
  * cgroup.
  */
-static int cg_legacy_handle_cpuset_hierarchy(struct hierarchy *h, char *cgname)
+static int cg_legacy_handle_cpuset_hierarchy(struct hierarchy *h,
+                                            const char *cgroup_leaf)
 {
        int fret = -1;
-       __do_free char *cgpath = NULL;
+       __do_free char *parent_or_self = NULL, *dup = NULL;
        __do_close_prot_errno int cgroup_fd = -EBADF;
+       size_t len;
        int ret;
        char v;
-       char *slash;
+       char *leaf, *slash;
 
        if (is_unified_hierarchy(h))
                return 0;
@@ -576,35 +579,40 @@ static int cg_legacy_handle_cpuset_hierarchy(struct 
hierarchy *h, char *cgname)
        if (!string_in_list(h->controllers, "cpuset"))
                return 0;
 
-       if (*cgname == '/')
-               cgname++;
-       slash = strchr(cgname, '/');
+       if (!cgroup_leaf)
+               return ret_set_errno(-1, EINVAL);
+
+       dup = strdup(cgroup_leaf);
+       if (!dup)
+               return ret_set_errno(-1, ENOMEM);
+
+       leaf += strspn(leaf, "/");
+       slash = strchr(leaf, '/');
        if (slash)
                *slash = '\0';
-
-       cgpath = must_make_path(h->mountpoint, h->container_base_path, cgname, 
NULL);
+       parent_or_self = must_make_path(h->mountpoint, h->container_base_path, 
leaf, NULL);
        if (slash)
                *slash = '/';
 
        fret = 1;
-       ret = mkdir(cgpath, 0755);
+       ret = mkdir(parent_or_self, 0755);
        if (ret < 0) {
                if (errno != EEXIST)
-                       return log_error_errno(-1, errno, "Failed to create 
directory \"%s\"", cgpath);
+                       return log_error_errno(-1, errno, "Failed to create 
directory \"%s\"", parent_or_self);
 
                fret = 0;
        }
 
-       cgroup_fd = lxc_open_dirfd(cgpath);
+       cgroup_fd = lxc_open_dirfd(parent_or_self);
        if (cgroup_fd < 0)
                return -1;
 
        ret = lxc_readat(cgroup_fd, "cgroup.clone_children", &v, 1);
        if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to read file 
\"%s/cgroup.clone_children\"", cgpath);
+               return log_error_errno(-1, errno, "Failed to read file 
\"%s/cgroup.clone_children\"", parent_or_self);
 
        /* Make sure any isolated cpus are removed from cpuset.cpus. */
-       if (!cg_legacy_filter_and_set_cpus(cgpath, v == '1'))
+       if (!cg_legacy_filter_and_set_cpus(parent_or_self, v == '1'))
                return log_error_errno(-1, errno, "Failed to remove isolated 
cpus");
 
        /* Already set for us by someone else. */
@@ -612,13 +620,13 @@ static int cg_legacy_handle_cpuset_hierarchy(struct 
hierarchy *h, char *cgname)
                TRACE("\"cgroup.clone_children\" was already set to \"1\"");
 
        /* copy parent's settings */
-       if (!copy_parent_file(cgpath, "cpuset.mems"))
+       if (!copy_parent_file(parent_or_self, "cpuset.mems"))
                return log_error_errno(-1, errno, "Failed to copy 
\"cpuset.mems\" settings");
 
        /* Set clone_children so children inherit our settings */
        ret = lxc_writeat(cgroup_fd, "cgroup.clone_children", "1", 1);
        if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to write 1 to 
\"%s/cgroup.clone_children\"", cgpath);
+               return log_error_errno(-1, errno, "Failed to write 1 to 
\"%s/cgroup.clone_children\"", parent_or_self);
 
        return fret;
 }
@@ -1228,7 +1236,7 @@ static int mkdir_eexist_on_last(const char *dir, mode_t 
mode)
 }
 
 static bool create_cgroup_tree(struct hierarchy *h, const char *cgroup_tree,
-                              char *cgroup_leaf, bool payload)
+                              const char *cgroup_leaf, bool payload)
 {
        __do_free char *path = NULL;
        int ret, ret_cpuset;

From 55b560980d3771aa7c99282e98f7508fcf3c1169 Mon Sep 17 00:00:00 2001
From: Christian Brauner <christian.brau...@ubuntu.com>
Date: Tue, 10 Dec 2019 18:15:30 +0100
Subject: [PATCH 2/2] cgroups/cgfsng: rework legacy cpuset handling

Signed-off-by: Christian Brauner <christian.brau...@ubuntu.com>
---
 src/lxc/cgroups/cgfsng.c | 131 +++++++++++++++------------------------
 1 file changed, 51 insertions(+), 80 deletions(-)

diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c
index fa00dbf44e..e7e3ec3995 100644
--- a/src/lxc/cgroups/cgfsng.c
+++ b/src/lxc/cgroups/cgfsng.c
@@ -271,12 +271,12 @@ static uint32_t *lxc_cpumask(char *buf, size_t nbits)
 {
        char *token;
        size_t arrlen;
-       uint32_t *bitarr;
+       __do_free uint32_t *bitarr = NULL;
 
        arrlen = BITS_TO_LONGS(nbits);
        bitarr = calloc(arrlen, sizeof(uint32_t));
        if (!bitarr)
-               return NULL;
+               return ret_set_errno(NULL, ENOMEM);
 
        lxc_iterate_parts(token, buf, ",") {
                errno = 0;
@@ -289,21 +289,17 @@ static uint32_t *lxc_cpumask(char *buf, size_t nbits)
                if (range)
                        end = strtoul(range + 1, NULL, 0);
 
-               if (!(start <= end)) {
-                       free(bitarr);
-                       return NULL;
-               }
+               if (!(start <= end))
+                       return ret_set_errno(NULL, EINVAL);
 
-               if (end >= nbits) {
-                       free(bitarr);
-                       return NULL;
-               }
+               if (end >= nbits)
+                       return ret_set_errno(NULL, EINVAL);
 
                while (start <= end)
                        set_bit(start++, bitarr);
        }
 
-       return bitarr;
+       return move_ptr(bitarr);
 }
 
 /* Turn cpumask into simple, comma-separated cpulist. */
@@ -328,12 +324,12 @@ static char *lxc_cpumask_to_cpulist(uint32_t *bitarr, 
size_t nbits)
                ret = lxc_append_string(&cpulist, numstr);
                if (ret < 0) {
                        lxc_free_array((void **)cpulist, free);
-                       return NULL;
+                       return ret_set_errno(NULL, ENOMEM);
                }
        }
 
        if (!cpulist)
-               return NULL;
+               return ret_set_errno(NULL, ENOMEM);
 
        tmp = lxc_string_join(",", (const char **)cpulist, false);
        lxc_free_array((void **)cpulist, free);
@@ -374,7 +370,8 @@ static ssize_t get_max_cpus(char *cpulist)
 
 #define __ISOL_CPUS "/sys/devices/system/cpu/isolated"
 #define __OFFLINE_CPUS "/sys/devices/system/cpu/offline"
-static bool cg_legacy_filter_and_set_cpus(char *path, bool am_initialized)
+static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup,
+                                         char *child_cgroup, bool 
am_initialized)
 {
        __do_free char *cpulist = NULL, *fpath = NULL, *isolcpus = NULL,
                       *offlinecpus = NULL, *posscpus = NULL;
@@ -382,23 +379,14 @@ static bool cg_legacy_filter_and_set_cpus(char *path, 
bool am_initialized)
                           *possmask = NULL;
        int ret;
        ssize_t i;
-       char *lastslash;
        ssize_t maxisol = 0, maxoffline = 0, maxposs = 0;
        bool bret = false, flipped_bit = false;
 
-       lastslash = strrchr(path, '/');
-       if (!lastslash) {
-               ERROR("Failed to detect \"/\" in \"%s\"", path);
-               return bret;
-       }
-       *lastslash = '\0';
-       fpath = must_make_path(path, "cpuset.cpus", NULL);
-       *lastslash = '/';
+       SYSERROR("AAAA: %s | %s", parent_cgroup, child_cgroup);
+       fpath = must_make_path(parent_cgroup, "cpuset.cpus", NULL);
        posscpus = read_file(fpath);
-       if (!posscpus) {
-               SYSERROR("Failed to read file \"%s\"", fpath);
-               return false;
-       }
+       if (!posscpus)
+               return log_error_errno(false, errno, "Failed to read file 
\"%s\"", fpath);
 
        /* Get maximum number of cpus found in possible cpuset. */
        maxposs = get_max_cpus(posscpus);
@@ -407,10 +395,8 @@ static bool cg_legacy_filter_and_set_cpus(char *path, bool 
am_initialized)
 
        if (file_exists(__ISOL_CPUS)) {
                isolcpus = read_file(__ISOL_CPUS);
-               if (!isolcpus) {
-                       SYSERROR("Failed to read file \"%s\"", __ISOL_CPUS);
-                       return false;
-               }
+               if (!isolcpus)
+                       return log_error_errno(false, errno, "Failed to read 
file \"%s\"", __ISOL_CPUS);
 
                if (isdigit(isolcpus[0])) {
                        /* Get maximum number of cpus found in isolated cpuset. 
*/
@@ -428,10 +414,8 @@ static bool cg_legacy_filter_and_set_cpus(char *path, bool 
am_initialized)
 
        if (file_exists(__OFFLINE_CPUS)) {
                offlinecpus = read_file(__OFFLINE_CPUS);
-               if (!offlinecpus) {
-                       SYSERROR("Failed to read file \"%s\"", __OFFLINE_CPUS);
-                       return false;
-               }
+               if (!offlinecpus)
+                       return log_error_errno(false, errno, "Failed to read 
file \"%s\"", __OFFLINE_CPUS);
 
                if (isdigit(offlinecpus[0])) {
                        /* Get maximum number of cpus found in offline cpuset. 
*/
@@ -453,25 +437,19 @@ static bool cg_legacy_filter_and_set_cpus(char *path, 
bool am_initialized)
        }
 
        possmask = lxc_cpumask(posscpus, maxposs);
-       if (!possmask) {
-               ERROR("Failed to create cpumask for possible cpus");
-               return false;
-       }
+       if (!possmask)
+               return log_error_errno(false, errno, "Failed to create cpumask 
for possible cpus");
 
        if (maxisol > 0) {
                isolmask = lxc_cpumask(isolcpus, maxposs);
-               if (!isolmask) {
-                       ERROR("Failed to create cpumask for isolated cpus");
-                       return false;
-               }
+               if (!isolmask)
+                       return log_error_errno(false, errno, "Failed to create 
cpumask for isolated cpus");
        }
 
        if (maxoffline > 0) {
                offlinemask = lxc_cpumask(offlinecpus, maxposs);
-               if (!offlinemask) {
-                       ERROR("Failed to create cpumask for offline cpus");
-                       return false;
-               }
+               if (!offlinemask)
+                       return log_error_errno(false, errno, "Failed to create 
cpumask for offline cpus");
        }
 
        for (i = 0; i <= maxposs; i++) {
@@ -491,18 +469,16 @@ static bool cg_legacy_filter_and_set_cpus(char *path, 
bool am_initialized)
                cpulist = move_ptr(posscpus);
                TRACE("Removed isolated or offline cpus from cpuset");
        }
-       if (!cpulist) {
-               ERROR("Failed to create cpu list");
-               return false;
-       }
+       if (!cpulist)
+               return log_error_errno(false, errno, "Failed to create cpu 
list");
 
 copy_parent:
        if (!am_initialized) {
-               ret = lxc_write_openat(path, "cpuset.cpus", cpulist, 
strlen(cpulist));
+               ret = lxc_write_openat(child_cgroup, "cpuset.cpus", cpulist, 
strlen(cpulist));
                if (ret < 0)
                        return log_error_errno(false,
                                               errno, "Failed to write cpu list 
to \"%s/cpuset.cpus\"",
-                                              path);
+                                              child_cgroup);
 
                TRACE("Copied cpu settings of parent cgroup");
        }
@@ -511,40 +487,32 @@ static bool cg_legacy_filter_and_set_cpus(char *path, 
bool am_initialized)
 }
 
 /* Copy contents of parent(@path)/@file to @path/@file */
-static bool copy_parent_file(char *path, char *file)
+static bool copy_parent_file(const char *parent_cgroup,
+                            const char *child_cgroup, const char *file)
 {
-       __do_free char *parent_path = NULL, *value = NULL;
+       __do_free char *parent_file = NULL, *value = NULL;
        int len = 0;
-       char *lastslash = NULL;
        int ret;
 
-       lastslash = strrchr(path, '/');
-       if (!lastslash)
-               return log_error_errno(false, ENOENT,
-                                      "Failed to detect \"/\" in \"%s\"", 
path);
-
-       *lastslash = '\0';
-       parent_path = must_make_path(path, file, NULL);
-       *lastslash = '/';
-
-       len = lxc_read_from_file(parent_path, NULL, 0);
+       parent_file = must_make_path(parent_cgroup, file, NULL);
+       len = lxc_read_from_file(parent_file, NULL, 0);
        if (len <= 0)
                return log_error_errno(false, errno,
                                       "Failed to determine buffer size");
 
        value = must_realloc(NULL, len + 1);
        value[len] = '\0';
-       ret = lxc_read_from_file(parent_path, value, len);
+       ret = lxc_read_from_file(parent_file, value, len);
        if (ret != len)
                return log_error_errno(false, errno,
                                       "Failed to read from parent file \"%s\"",
-                                      parent_path);
+                                      parent_file);
 
-       ret = lxc_write_openat(path, file, value, len);
+       ret = lxc_write_openat(child_cgroup, file, value, len);
        if (ret < 0 && errno != EACCES)
                return log_error_errno(false,
                                       errno, "Failed to write \"%s\" to file 
\"%s/%s\"",
-                                      value, path, file);
+                                      value, child_cgroup, file);
        return true;
 }
 
@@ -565,9 +533,9 @@ static bool is_unified_hierarchy(const struct hierarchy *h)
 static int cg_legacy_handle_cpuset_hierarchy(struct hierarchy *h,
                                             const char *cgroup_leaf)
 {
-       int fret = -1;
-       __do_free char *parent_or_self = NULL, *dup = NULL;
+       __do_free char *parent_cgroup = NULL, *child_cgroup = NULL, *dup = NULL;
        __do_close_prot_errno int cgroup_fd = -EBADF;
+       int fret = -1;
        size_t len;
        int ret;
        char v;
@@ -586,33 +554,36 @@ static int cg_legacy_handle_cpuset_hierarchy(struct 
hierarchy *h,
        if (!dup)
                return ret_set_errno(-1, ENOMEM);
 
+       parent_cgroup = must_make_path(h->mountpoint, h->container_base_path, 
NULL);
+
+       leaf = dup;
        leaf += strspn(leaf, "/");
        slash = strchr(leaf, '/');
        if (slash)
                *slash = '\0';
-       parent_or_self = must_make_path(h->mountpoint, h->container_base_path, 
leaf, NULL);
+       child_cgroup = must_make_path(parent_cgroup, leaf, NULL);
        if (slash)
                *slash = '/';
 
        fret = 1;
-       ret = mkdir(parent_or_self, 0755);
+       ret = mkdir(child_cgroup, 0755);
        if (ret < 0) {
                if (errno != EEXIST)
-                       return log_error_errno(-1, errno, "Failed to create 
directory \"%s\"", parent_or_self);
+                       return log_error_errno(-1, errno, "Failed to create 
directory \"%s\"", child_cgroup);
 
                fret = 0;
        }
 
-       cgroup_fd = lxc_open_dirfd(parent_or_self);
+       cgroup_fd = lxc_open_dirfd(child_cgroup);
        if (cgroup_fd < 0)
                return -1;
 
        ret = lxc_readat(cgroup_fd, "cgroup.clone_children", &v, 1);
        if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to read file 
\"%s/cgroup.clone_children\"", parent_or_self);
+               return log_error_errno(-1, errno, "Failed to read file 
\"%s/cgroup.clone_children\"", child_cgroup);
 
        /* Make sure any isolated cpus are removed from cpuset.cpus. */
-       if (!cg_legacy_filter_and_set_cpus(parent_or_self, v == '1'))
+       if (!cg_legacy_filter_and_set_cpus(parent_cgroup, child_cgroup, v == 
'1'))
                return log_error_errno(-1, errno, "Failed to remove isolated 
cpus");
 
        /* Already set for us by someone else. */
@@ -620,13 +591,13 @@ static int cg_legacy_handle_cpuset_hierarchy(struct 
hierarchy *h,
                TRACE("\"cgroup.clone_children\" was already set to \"1\"");
 
        /* copy parent's settings */
-       if (!copy_parent_file(parent_or_self, "cpuset.mems"))
+       if (!copy_parent_file(parent_cgroup, child_cgroup, "cpuset.mems"))
                return log_error_errno(-1, errno, "Failed to copy 
\"cpuset.mems\" settings");
 
        /* Set clone_children so children inherit our settings */
        ret = lxc_writeat(cgroup_fd, "cgroup.clone_children", "1", 1);
        if (ret < 0)
-               return log_error_errno(-1, errno, "Failed to write 1 to 
\"%s/cgroup.clone_children\"", parent_or_self);
+               return log_error_errno(-1, errno, "Failed to write 1 to 
\"%s/cgroup.clone_children\"", child_cgroup);
 
        return fret;
 }
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Reply via email to