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