From f160352682f4518c8356ba394e5f147822a1eb74 Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Wed, 13 May 2020 16:18:46 -0700 Subject: [PATCH] libct/cgroup: prep to rm GetClosestMountpointAncestor This function is not very efficient, does not really belong to cgroup package, and is only used once (from fs/cpuset.go). Prepare to remove it by replacing with the implementation based on the parser from github.com/moby/sys/mountinfo parser. This commit is here to make sure the proposed replacement passes the unit test. Funny, but the unit test need to be slightly modified since it supplies the wrong mountinfo (space as the first character, empty line at the end). Validated by $ go test -v -run Ance === RUN TestGetClosestMountpointAncestor --- PASS: TestGetClosestMountpointAncestor (0.00s) PASS ok github.com/opencontainers/runc/libcontainer/cgroups 0.002s Signed-off-by: Kir Kolyshkin --- libcontainer/cgroups/fs/cpuset.go | 6 +++++- libcontainer/cgroups/utils.go | 29 ++++++++++++++++++----------- libcontainer/cgroups/utils_test.go | 11 +++++++---- 3 files changed, 30 insertions(+), 16 deletions(-) diff --git a/libcontainer/cgroups/fs/cpuset.go b/libcontainer/cgroups/fs/cpuset.go index bfc900e3f..a0c3c5f68 100644 --- a/libcontainer/cgroups/fs/cpuset.go +++ b/libcontainer/cgroups/fs/cpuset.go @@ -62,7 +62,11 @@ func (s *CpusetGroup) ApplyDir(dir string, cgroup *configs.Cgroup, pid int) erro if err != nil { return err } - root := filepath.Dir(cgroups.GetClosestMountpointAncestor(dir, string(mountInfo))) + root, err := cgroups.GetClosestMountpointAncestor(dir, string(mountInfo)) + if err != nil { + return err + } + root = filepath.Dir(root) // 'ensureParent' start with parent because we don't want to // explicitly inherit from parent, it could conflict with // 'cpuset.cpu_exclusive'. diff --git a/libcontainer/cgroups/utils.go b/libcontainer/cgroups/utils.go index 8cdd6b7ed..9859557a7 100644 --- a/libcontainer/cgroups/utils.go +++ b/libcontainer/cgroups/utils.go @@ -16,6 +16,7 @@ import ( "time" units "github.com/docker/go-units" + "github.com/moby/sys/mountinfo" "golang.org/x/sys/unix" ) @@ -126,19 +127,25 @@ func isSubsystemAvailable(subsystem string) bool { return avail } -func GetClosestMountpointAncestor(dir, mountinfo string) string { - deepestMountPoint := "" - for _, mountInfoEntry := range strings.Split(mountinfo, "\n") { - mountInfoParts := strings.Fields(mountInfoEntry) - if len(mountInfoParts) < 5 { - continue - } - mountPoint := mountInfoParts[4] - if strings.HasPrefix(mountPoint, deepestMountPoint) && strings.HasPrefix(dir, mountPoint) { - deepestMountPoint = mountPoint +func GetClosestMountpointAncestor(dir, data string) (string, error) { + mi, err := mountinfo.GetMountsFromReader(strings.NewReader(data), mountinfo.ParentsFilter(dir)) + if err != nil { + return "", err + } + if len(mi) < 1 { + return "", err + } + + // find the longest mount point + var idx, maxlen int + for i := range mi { + if len(mi[i].Mountpoint) > maxlen { + maxlen = len(mi[i].Mountpoint) + idx = i } } - return deepestMountPoint + + return mi[idx].Mountpoint, nil } func FindCgroupMountpointDir() (string, error) { diff --git a/libcontainer/cgroups/utils_test.go b/libcontainer/cgroups/utils_test.go index adf266858..a3be72443 100644 --- a/libcontainer/cgroups/utils_test.go +++ b/libcontainer/cgroups/utils_test.go @@ -374,7 +374,7 @@ func TestIgnoreCgroup2Mount(t *testing.T) { } func TestGetClosestMountpointAncestor(t *testing.T) { - fakeMountInfo := ` 18 24 0:17 / /sys rw,nosuid,nodev,noexec,relatime - sysfs sysfs rw + const fakeMountInfo = `18 24 0:17 / /sys rw,nosuid,nodev,noexec,relatime - sysfs sysfs rw 100 99 1:31 / /foo/bar rw,relatime - fake fake rw,fake 100 99 1:31 / /foo/bar/baz2 rw,relatime - fake fake rw,fake 100 99 1:31 / /foo/bar/baz rw,relatime - fake fake rw,fake @@ -382,8 +382,8 @@ func TestGetClosestMountpointAncestor(t *testing.T) { 100 99 1:31 / /foo/bar/baz3 rw,relatime - fake fake rw,fake 100 99 1:31 / /foo rw,relatime - fake fake rw,fake 100 99 1:31 / /unrelated rw,relatime - fake fake rw,fake -100 99 1:31 / / rw,relatime - fake fake rw,fake -` +100 99 1:31 / / rw,relatime - fake fake rw,fake` + testCases := []struct { input string output string @@ -395,7 +395,10 @@ func TestGetClosestMountpointAncestor(t *testing.T) { } for _, c := range testCases { - mountpoint := GetClosestMountpointAncestor(c.input, fakeMountInfo) + mountpoint, err := GetClosestMountpointAncestor(c.input, fakeMountInfo) + if err != nil { + t.Fatal(err) + } if mountpoint != c.output { t.Errorf("expected %s, got %s", c.output, mountpoint) }