cgroups/cgfsng: rework legacy cpuset handling

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;
+	bool 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 @@ copy_parent:
 }
 
 /* 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,10 +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;
-	size_t len;
+	int fret = -1;
 	int ret;
 	char v;
 	char *leaf, *slash;
@@ -586,33 +553,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 +590,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;
 }