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

introduce lxc.cgroup.dir.{monitor,container,container.inner} #3353

Merged
merged 4 commits into from Apr 3, 2020

Conversation

Blub
Copy link
Member

@Blub Blub commented Apr 3, 2020

This is a new approach to #1302 with a container-side
configuration instead of a global boolean flag.

Contrary to the previous PR using an optional additional
parameter for the get-cgroup command, this introduces two
new additional commands to get the limiting cgroup path and
cgroup2 file descriptor. If the limiting option is not in
use, these behave identical to their full-path counterparts.

If these variables are used the payload will end up in the
concatenation of lxc.cgroup.dir.payload and
lxc.cgroup.dir.namespace (namespace may be empty), and the
monitor will end up in lxc.cgruop.dir.monitor. The
directories are fixed, no retry count logic is applied,
failing to create these directories will simply be a hard
error.

Signed-off-by: Wolfgang Bumiller w.bumiller@proxmox.com

Note that this is designed to be opt in, so if there are still some setup
issues with eg. the devices cgroup in v1 or with cgroupv2 (which I'll be
testing shortly), it still shouldn't affect existing configurations.
Posting this now for initial review.

@lxc-jenkins
Copy link

Testsuite passed

Copy link
Member

@brauner brauner left a comment

Choose a reason for hiding this comment

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

Cool, I like it a lot. Mostly nits!

src/lxc/cgroups/cgfsng.c Outdated Show resolved Hide resolved
src/lxc/cgroups/cgfsng.c Outdated Show resolved Hide resolved
src/lxc/cgroups/cgfsng.c Outdated Show resolved Hide resolved
src/lxc/cgroups/cgfsng.c Show resolved Hide resolved
src/lxc/cgroups/cgroup.h Show resolved Hide resolved
src/lxc/cgroups/cgroup.h Outdated Show resolved Hide resolved
src/lxc/confile.c Outdated Show resolved Hide resolved
src/lxc/confile.c Outdated Show resolved Hide resolved
src/lxc/confile.c Outdated Show resolved Hide resolved
src/lxc/confile.c Outdated Show resolved Hide resolved
@Blub Blub force-pushed the lxc.cgroup.dir-components branch from 8f1e561 to f16d703 Compare April 3, 2020 13:18
@Blub Blub changed the title introduce lxc.cgroup.dir.{payload,monitor,limit_prefix} introduce lxc.cgroup.dir.{payload,monitor,namespace} Apr 3, 2020
This is a new approach to lxc#1302 with a container-side
configuration instead of a global boolean flag.

Contrary to the previous PR using an optional additional
parameter for the get-cgroup command, this introduces two
new additional commands to get the limiting cgroup path and
cgroup2 file descriptor. If the limiting option is not in
use, these behave identical to their full-path counterparts.

If these variables are used the payload will end up in the
concatenation of lxc.cgroup.dir.container and
lxc.cgroup.dir.container.inner (which may be empty), and the
monitor will end up in lxc.cgruop.dir.monitor. The
directories are fixed, no retry count logic is applied,
failing to create these directories will simply be a hard
error.

Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
@Blub Blub force-pushed the lxc.cgroup.dir-components branch from f16d703 to a900cba Compare April 3, 2020 15:22
Christian Brauner added 3 commits April 3, 2020 20:07
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
@brauner brauner changed the title introduce lxc.cgroup.dir.{payload,monitor,namespace} introduce lxc.cgroup.dir.{monitor,container,container.inner} Apr 3, 2020
@brauner brauner merged commit a6e5687 into lxc:master Apr 3, 2020
@brauner
Copy link
Member

brauner commented Apr 3, 2020

Thank you very much for that! Should make privileged containers safer!

@hallyn
Copy link
Member

hallyn commented Apr 3, 2020

Nice.

raboof pushed a commit to raboof/pve-container that referenced this pull request May 16, 2020
See: lxc/lxc#3353
Signed-off-by: Wolfgang Bumiller <w.bumiller@proxmox.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants