don't enable threaded mode by default

lifubang · lifubang · commit fe0669b26dee · 2020-05-13T16:27:36.000+08:00
Because in threaded mode, we can't enable the memory controller -- it isn't thread-aware.

Signed-off-by: lifubang &lt;lifubang@acmcoder.com&gt;
diff --git a/libcontainer/cgroups/fs2/create.go b/libcontainer/cgroups/fs2/create.go
@@ -63,6 +63,13 @@ func neededControllers(cgroup *configs.Cgroup) ([]string, error) {
 	return list, nil
 }
 
+// containsDomainController returns whether the current config contains domain controller or not.
+// Refer to: http://man7.org/linux/man-pages/man7/cgroups.7.html
+// As at Linux 4.19, the following controllers are threaded: cpu, perf_event, and pids.
+func containsDomainController(cg *configs.Cgroup) bool {
+	return isMemorySet(cg) || isIoSet(cg) || isCpuSet(cg) || isHugeTlbSet(cg)
+}
+
 // CreateCgroupPath creates cgroupv2 path, enabling all the
 // needed controllers in the process.
 func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
@@ -95,10 +102,34 @@ func CreateCgroupPath(path string, c *configs.Cgroup) (Err error) {
 					}
 				}()
 			}
-			// Write cgroup.type explicitly.
-			// Otherwise ENOTSUP may happen.
-			cgType := filepath.Join(current, "cgroup.type")
-			_ = ioutil.WriteFile(cgType, []byte("threaded"), 0644)
+			cgTypeFile := filepath.Join(current, "cgroup.type")
+			cgType, _ := ioutil.ReadFile(cgTypeFile)
+			switch strings.TrimSpace(string(cgType)) {
+			// If the cgroup is in an invalid mode (usually this means there's an internal
+			// process in the cgroup tree, because we created a cgroup under an
+			// already-populated-by-other-processes cgroup), then we have to error out if
+			// the user requested controllers which are not thread-aware. However, if all
+			// the controllers requested are thread-aware we can simply put the cgroup into
+			// threaded mode.
+			case "domain invalid":
+				if containsDomainController(c) {
+					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in an invalid state", current)
+				} else {
+					// Not entirely correct (in theory we'd always want to be a domain --
+					// since that means we're a properly delegated cgroup subtree) but in
+					// this case there's not much we can do and it's better than giving an
+					// error.
+					_ = ioutil.WriteFile(cgTypeFile, []byte("threaded"), 0644)
+				}
+			// If the cgroup is in (threaded) or (domain threaded) mode, we can only use thread-aware controllers
+			// (and you cannot usually take a cgroup out of threaded mode).
+			case "domain threaded":
+				fallthrough
+			case "threaded":
+				if containsDomainController(c) {
+					return fmt.Errorf("cannot enter cgroupv2 %q with domain controllers -- it is in %s mode", current, strings.TrimSpace(string(cgType)))
+				}
+			}
 		}
 		// enable needed controllers
 		if i < len(elements)-1 {
diff --git a/tests/rootless.sh b/tests/rootless.sh
@@ -99,13 +99,6 @@ function enable_cgroup() {
 		chown root:rootless "$CGROUP_MOUNT/$CGROUP_PATH" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.subtree_control" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs"
 		chmod g+rwx "$CGROUP_MOUNT/$CGROUP_PATH"
 		chmod g+rw "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.subtree_control" "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.procs" "$CGROUP_MOUNT/cgroup.procs"
-		# Fix up cgroup.type.
-		echo threaded > "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.type"
-		# Make sure cgroup.type doesn't contain "invalid". Otherwise write ops will fail with ENOTSUP.
-		# See http://man7.org/linux/man-pages/man7/cgroups.7.html
-		if grep -qw invalid "$CGROUP_MOUNT/$CGROUP_PATH/cgroup.type"; then
-			exit 1
-		fi
 	fi
 }
 
