Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cgroup: v2: do not set subtree_control for new cg #37

Merged
merged 1 commit into from
Dec 14, 2022

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Dec 9, 2022

A cgroup can either have delegations (cgroup.subtree_control is populated) OR it can have processes in it, but not both. So we need to make sure that we don't add any controllers to the new cgroup's cgroup.subtree_control BUT we should continue to do so for any parents that we create.

Fixes #35

@cpuguy83 cpuguy83 requested a review from danbugs December 9, 2022 17:15
@cpuguy83
Copy link
Member Author

cpuguy83 commented Dec 9, 2022

Hit another issue I thought was due to some other changes I have in my working tree, guess it's not just that...

@cpuguy83 cpuguy83 force-pushed the cg2_ebusy branch 2 times, most recently from f777fa4 to c51cb4a Compare December 9, 2022 22:07
@cpuguy83
Copy link
Member Author

cpuguy83 commented Dec 9, 2022

Ok this is good to go.

@cpuguy83
Copy link
Member Author

cpuguy83 commented Dec 9, 2022

/cc @jsturtevant

Copy link
Contributor

@jsturtevant jsturtevant left a comment

Choose a reason for hiding this comment

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

lgtm

crates/containerd-shim-wasm/src/sandbox/exec.rs Outdated Show resolved Hide resolved
A cgroup can either have delegations (`cgroup.subtree_control` is
populated) OR it can have processes in it, but not both.
So we need to make sure that we don't add any controllers to the new
cgroup's `cgroup.subtree_control` BUT we should continue to do so for
any parents that we create.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83
Copy link
Member Author

Fixed comments

Copy link
Contributor

@devigned devigned left a comment

Choose a reason for hiding this comment

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

lgtm!

@cpuguy83 cpuguy83 merged commit 32cb0cd into containerd:main Dec 14, 2022
@cpuguy83 cpuguy83 deleted the cg2_ebusy branch December 14, 2022 18:31
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.

Others("Device or resource busy (os error 16)"): unknown
3 participants