Correct file name in cgroupsv2 #122
Conversation
Akarys42
added
type: bug
Cgroupsv2 renamed the limit file structure. This commit should fix that.
| @staticmethod | ||
| def _probe_cgroup_version() -> int: | ||
| """Poll the filesystem and return the guessed cgroup version.""" | ||
| # Right now we check whenever the controller path exists | ||
| version = 2 if CGROUPV2_PROBE_PATH.exists() else 1 | ||
|
|
||
| if DEBUG: | ||
| log.info(f"Guessed cgroups version: {version}") | ||
|
|
||
| return version | ||
| return 2 if CGROUPV2_PROBE_PATH.exists() else 1 |
There was a problem hiding this comment.
This doesn't even need to be a function. Just do self.cgroups_version = 2 if CGROUPV2_PROBE_PATH.exists() else 1.
| pids.mkdir(parents=True, exist_ok=True) | ||
| mem.mkdir(parents=True, exist_ok=True) |
There was a problem hiding this comment.
This also needs to be adjusted, since v2 doesn't have subdirectories for each controller. NsJail doesn't even use cgroup_pids_mount and cgroup_mem_mount for v2. Instead, it's simply https://github.com/google/nsjail/blob/bf93e8a25d1b521797a319bc1f6182e894597224/cgroup2.cc#L42-L44
| @@ -206,7 +210,7 @@ def python3( | |||
| cgroup = self._create_dynamic_cgroups() | |||
|
|
|||
| with NamedTemporaryFile() as nsj_log: | |||
| if self._probe_cgroup_version() == 2: | |||
| if self.cgroups_version == 2: | |||
| nsjail_args = (["--use_cgroupv2"]).extend(nsjail_args) | |||
There was a problem hiding this comment.
What happens if use_cgroupv2 is already in the NsJail config?
| # Swap limit is specified as the sum of the memory and swap limits. | ||
| # Therefore, setting it equal to the memory limit effectively disables swapping. |
There was a problem hiding this comment.
This description is for v1's behaviour. Does memory.swap.max in v2 also behave the same way?
|
@Akarys42 Are you still interested in working on this? |
|
@MarkKoz It seems they won't be able to work on this anymore.
From #dev-contrib, at this message. I'm a bit late to updating these PRs, sorry about that. |
I'm fine with the PR being re-opened using a different branch if Akarys wants to. Otherwise, I will try to finish the PR. |
|
I'm going to finish this PR. |
|
Closing in favour of #102. The approach taken here is inherently incorrect, and there wasn't any way to salvage it. |
Cgroupsv2 renamed the limit file structure. This commit should fix that.