From 55d5c99ca755a1dd915af0accd7bc90b3095679f Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Thu, 23 Apr 2020 16:04:18 -0700 Subject: [PATCH] libct/mountToRootfs: rm useless code To make a bind mount read-only, it needs to be remounted. This is what the code removed does, but it is not needed here. We have to deal with three cases here: 1. cgroup v2 unified mode. In this case the mount is real mount with fstype=cgroup2, and there is no need to have a bind mount on top, as we pass readonly flag to the mount as is. 2. cgroup v1 + cgroupns (enableCgroupns == true). In this case the "mount" is in fact a set of real mounts with fstype=cgroup, and they are all performed in mountCgroupV1, with readonly flag added if needed. 3. cgroup v1 as is (enableCgroupns == false). In this case mountCgroupV1() calls mountToRootfs() again with an argument from the list obtained from getCgroupMounts(), i.e. a bind mount with the same flags as the original mount has (plus unix.MS_BIND | unix.MS_REC), and mountToRootfs() does remounting (under the case "bind":). So, the code which this patch is removing is not needed -- it essentially does nothing in case 3 above (since the bind mount is already remounted readonly), and in cases 1 and 2 it creates an unneeded extra bind mount on top of a real one (or set of real ones). Signed-off-by: Kir Kolyshkin --- libcontainer/rootfs_linux.go | 22 ++-------------------- 1 file changed, 2 insertions(+), 20 deletions(-) diff --git a/libcontainer/rootfs_linux.go b/libcontainer/rootfs_linux.go index e6cc08785..df2fa03d9 100644 --- a/libcontainer/rootfs_linux.go +++ b/libcontainer/rootfs_linux.go @@ -402,27 +402,9 @@ func mountToRootfs(m *configs.Mount, rootfs, mountLabel string, enableCgroupns b } case "cgroup": if cgroups.IsCgroup2UnifiedMode() { - if err := mountCgroupV2(m, rootfs, mountLabel, enableCgroupns); err != nil { - return err - } - } else { - - if err := mountCgroupV1(m, rootfs, mountLabel, enableCgroupns); err != nil { - return err - } - } - if m.Flags&unix.MS_RDONLY != 0 { - // remount cgroup root as readonly - mcgrouproot := &configs.Mount{ - Source: m.Destination, - Device: "bind", - Destination: m.Destination, - Flags: defaultMountFlags | unix.MS_RDONLY | unix.MS_BIND, - } - if err := remount(mcgrouproot, rootfs); err != nil { - return err - } + return mountCgroupV2(m, rootfs, mountLabel, enableCgroupns) } + return mountCgroupV1(m, rootfs, mountLabel, enableCgroupns) default: // ensure that the destination of the mount is resolved of symlinks at mount time because // any previous mounts can invalidate the next mount's destination.