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 <kolyshkin@gmail.com>
The change ensures that the passed in value of NoNewPrivileges under spec.Process
is reflected in the container config generated by specconv.CreateLibcontainerConfig
Closes#2397
Signed-off-by: Pradyumna Agrawal <pradyumnaa@vmware.com>
This unties the Gordian Knot of using GetPaths in cgroupv2 code.
The problem is, the current code uses GetPaths for three kinds of things:
1. Get all the paths to cgroup v1 controllers to save its state (see
(*linuxContainer).currentState(), (*LinuxFactory).loadState()
methods).
2. Get all the paths to cgroup v1 controllers to have the setns process
enter the proper cgroups in `(*setnsProcess).start()`.
3. Get the path to a specific controller (for example,
`m.GetPaths()["devices"]`).
Now, for cgroup v2 instead of a set of per-controller paths, we have only
one single unified path, and a dedicated function `GetUnifiedPath()` to get it.
This discrepancy between v1 and v2 cgroupManager API leads to the
following problems with the code:
- multiple if/else code blocks that have to treat v1 and v2 separately;
- backward-compatible GetPaths() methods in v2 controllers;
- - repeated writing of the PID into the same cgroup for v2;
Overall, it's hard to write the right code with all this, and the code
that is written is kinda hard to follow.
The solution is to slightly change the API to do the 3 things outlined
above in the same manner for v1 and v2:
1. Use `GetPaths()` for state saving and setns process cgroups entering.
2. Introduce and use Path(subsys string) to obtain a path to a
subsystem. For v2, the argument is ignored and the unified path is
returned.
This commit converts all the controllers to the new API, and modifies
all the users to use it.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
1. Prevent theoretical "concurrent map access" error to m.paths.
2. There is no need to call m.Paths -- we can access m.paths directly.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
c.cgroupManager.GetPaths() are called twice here: once in currentState()
and then in newSetnsProcess(). Reuse the result of the first call, which
is stored into state.CgroupPaths.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Instead of passing the whole map of paths, pass the path to the memory
controller which these functions actually require.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The test cases need to take into account the assembly modifications.
The instruction:
LdXMemH dst: r2 src: r1 off: 0 imm: 0
has been replaced with:
LdXMemW dst: r2 src: r1 off: 0 imm: 0
And32Imm dst: r2 imm: 65535
Signed-off-by: Alice Frosi <afrosi@de.ibm.com>
Load the full 32 bits word and take the lower 16 bits, instead of
reading just 16 bits.
Same fix as 07bae05e61
Signed-off-by: Alice Frosi <afrosi@de.ibm.com>
In cgroup v2, when memory and memorySwap set to the same value which is greater than zero,
runc should write zero in `memory.swap.max` to disable swap.
Signed-off-by: lifubang <lifubang@acmcoder.com>
The error message was including both the rootfs path, and the full
mount path, which also includes the path of the rootfs.
This patch removes the rootfs path from the error message, as it
was redundant, and made the error message overly verbose
Before this patch (errors wrapped for readability):
```
container_linux.go:348: starting container process caused: process_linux.go:438:
container init caused: rootfs_linux.go:58: mounting "/foo.txt"
to rootfs "/var/lib/docker/overlay2/de506d67da606b807009e23b548fec60d72359c77eec88785d8c7ecd54a6e4b2/merged"
at "/var/lib/docker/overlay2/de506d67da606b807009e23b548fec60d72359c77eec88785d8c7ecd54a6e4b2/merged/usr/share/nginx/html"
caused: not a directory: unknown
```
With this patch applied:
```
container_linux.go:348: starting container process caused: process_linux.go:438:
container init caused: rootfs_linux.go:58: mounting "/foo.txt"
to rootfs at "/var/lib/docker/overlay2/de506d67da606b807009e23b548fec60d72359c77eec88785d8c7ecd54a6e4b2/merged/usr/share/nginx/html"
caused: not a directory: unknown
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
genericError.Error() was formatting the underlying error using `%q`; as a
result, quotes in underlying errors were escaped multiple times, which
caused the output to become hard to read, for example (wrapped for readability):
```
container_linux.go:345: starting container process caused "process_linux.go:430:
container init caused \"rootfs_linux.go:58: mounting \\\"/foo.txt\\\"
to rootfs \\\"/var/lib/docker/overlay2/f49a0ae0ec6646c818dcf05dbcbbdd79fc7c42561f3684fbb1fc5d2b9d3ad192/merged\\\"
at \\\"/var/lib/docker/overlay2/f49a0ae0ec6646c818dcf05dbcbbdd79fc7c42561f3684fbb1fc5d2b9d3ad192/merged/usr/share/nginx/html\\\"
caused \\\"not a directory\\\"\"": unknown
```
With this patch applied:
```
container_linux.go:348: starting container process caused: process_linux.go:438:
container init caused: rootfs_linux.go:58: mounting "/foo.txt"
to rootfs "/var/lib/docker/overlay2/de506d67da606b807009e23b548fec60d72359c77eec88785d8c7ecd54a6e4b2/merged"
at "/var/lib/docker/overlay2/de506d67da606b807009e23b548fec60d72359c77eec88785d8c7ecd54a6e4b2/merged/usr/share/nginx/html"
caused: not a directory: unknown
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Function defaultPath always parses /proc/self/cgroup, but
the resulting value is not always used.
Avoid unnecessary reading/parsing by moving the code
to just before its use.
Modify the test case accordingly.
[v2: test: use UnifiedMountpoint, skip test if not on v2]
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
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 <kolyshkin@gmail.com>
In case of cgroupv2 unified hierarchy, the /sys/fs/cgroup mount
is the real mount with fstype of cgroup2 (rather than a set of
external bind mounts like for cgroupv1).
So, we should not add it to the list of "external bind mounts"
on both checkpoint and restore.
Without this fix, checkpoint integration tests fail on cgroup v2.
Also, same is true for cgroup v1 + cgroupns.
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Kir Kolyshkin
lifubang
Akihiro Suda
Pradyumna Agrawal
Alice Frosi
Sebastiaan van Stijn
Katarzyna Kujawa
Mrunal Patel
Ted Yu
Paweł Szulik
Kir Kolyshkin
Kenta Tada