Skip to content

Add the --use_cgroupv2 flag when relevant#121

Merged
HassanAbouelela merged 2 commits into
mainfrom
cgroupsv2
Dec 7, 2021
Merged

Add the --use_cgroupv2 flag when relevant#121
HassanAbouelela merged 2 commits into
mainfrom
cgroupsv2

Conversation

@Akarys42
Copy link
Copy Markdown
Contributor

@Akarys42 Akarys42 commented Dec 7, 2021

According to google/nsjail#119, the flag should be passed for NsJail to try to use cgroupv2. This commit will use the /sys/fs/cgroup structure to guess the installed version, and depending on the version add that flag.

I couldn't confirm this actually solves the issue with v2 but at least it doesn't break with v1.

@Akarys42 Akarys42 added type: bug Something isn't working area: nsjail Related to NsJail and its configuration priority: 0 - critical Needs to be addressed ASAP labels Dec 7, 2021
According to google/nsjail#119, the  flag should be passed for NsJail to try to use cgroupv2. This commit will use the /sys/fs/cgroup structure to guess the installed version, and depending on the version add that flag.
@coveralls
Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.4%) to 90.661% when pulling 80c7318 on cgroupsv2 into df70f19 on main.

Copy link
Copy Markdown
Contributor

@HassanAbouelela HassanAbouelela left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed working on cgroups 1

@HassanAbouelela HassanAbouelela merged commit 94ba860 into main Dec 7, 2021
@HassanAbouelela HassanAbouelela deleted the cgroupsv2 branch December 7, 2021 08:34
Copy link
Copy Markdown
Contributor

@MarkKoz MarkKoz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose I am late but I'll review this anyway.

Comment thread snekbox/nsjail.py
Comment on lines +56 to +57
if DEBUG:
log.info(f"Guessed cgroups version: {version}")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just use log.debug.

Comment thread snekbox/nsjail.py
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
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally this would use statfs to get the filesystem type, but Python doesn't make it easy to perform this syscall unfortunately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: nsjail Related to NsJail and its configuration priority: 0 - critical Needs to be addressed ASAP type: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants