Skip to content

Conversation

@sargun
Copy link

@sargun sargun commented Jan 27, 2018

This adds support for cgroup namespaces, which were
added in Linux v4.6. The primary benefit is that it makes
/proc/self/cgroup's paths consistent with what's seen
under /sys/fs/cgroup.

Signed-off-by: Sargun Dhillon [email protected]

This adds support for cgroup namespaces, which were
added in Linux v4.6. The primary benefit is that it makes
/proc/self/cgroup's paths consistent with what's seen
under /sys/fs/cgroup.

Signed-off-by: Sargun Dhillon <[email protected]>
@cyphar
Copy link
Member

cyphar commented Jan 28, 2018

As discussed in #781 and #1184, it's a bit more complicated than this to add cgroup namespaces support. You need to unshare the cgroup namespace after you've joined the relevant cgroups, otherwise you're going to end up with broken /proc/self/cgroup paths (since the root of the namespace is determined by the cgroup that you were in at the time).

The main thing stalling #1184 is that we need to do a unix.Unshare after setting up cgroups (as well as quite a bit of special casing all over the place) which is pretty ugly (and given Go's already quite broken multi-threading scheme, the Unshare is potentially dangerous). LXC does this by doing the unshare after cgroup setup, but since they're written in C they don't need to worry about the Go runtime changing threads underneath them (LockOSThread used to be completely broken, but from memory they fixed some of the bugs with it -- so it might be safer now).

@cyphar
Copy link
Member

cyphar commented Jan 28, 2018

Sorry, correction to the above:

We eventually decided on having cgroup unsharing be delayed based on communication between the top-level runc process and the still-running-in-C runc init process. The code was almost done in #1184, so I'm going to carry that patch in the coming week. Would that work for you?

@sargun
Copy link
Author

sargun commented Jan 28, 2018

@cyphar Thanks. I'll go ahead and close this PR, and wait for yours.

@sargun sargun closed this Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants