This repository has been archived by the owner on Dec 13, 2018. It is now read-only.
cgroups: fs: fix issues with .Apply() #652
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If a configuration option is set for a cgroup that isn't mounted, an error will be emitted. This is an issue, because newer cgroups aren't present in older kernels, and we can't break backcompat where a new config can't run on an older kernel. The issue arises because .Set() does no checks for whether the given path actually points to a mountpoint, so fix the code by just bailing if the cgroup mountpoint doesn't exist. Fixes: fc3981e Fixes: 606d906 Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
|
I found the relevant PR #474. However, I don't agree that we should fail if we have a configuration option that applies to a cgroup which the host kernel doesn't support and also isn't a hard requirement. As it currently stands, the |
|
/cc @crosbymichael |
|
I don't quite understand the problem here, can't #476 fix your problem? |
|
The logic is:
|
|
The point was that the fact we fail on options set on non-existant cgroups didn't really make sense to me. But really, I don't like the idea of us silently ignoring the config. I'll close this PR. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
If a configuration option is set for a cgroup that isn't mounted, an
error will be emitted. This is an issue, because newer cgroups aren't
present in older kernels, and we can't break backcompat where a new
config can't run on an older kernel.
The issue arises because .Set() does no checks for whether the given
path actually points to a mountpoint, so fix the code by just bailing if
the cgroup mountpoint doesn't exist.
Fixes: fc3981e
Fixes: 606d906
Signed-off-by: Aleksa Sarai cyphar@cyphar.com
This was already done in 606d906, and then reverted in fc3981e.
I could not find a related PR to explain why the reversion was done, but this is an issue I'm facing when testing a newcgroup/fsI'm writing, where the kernel doesn't support it but the config is also non-default.