Skip to content
This repository has been archived by the owner on Dec 13, 2018. It is now read-only.

Revert "cgroups: only return path when subsystem really mounted #474

Merged

Conversation

crosbymichael
Copy link
Contributor

We need to do this so that when a user specifies an option and we are unable to fulfill the request we should fail hard instead of ignoring the error and keep going. We also need to fail hard for cgroups like devices because of security reasons.

@LK4D4
Copy link
Contributor

LK4D4 commented Mar 23, 2015

@crosbymichael first commit is not signed

@jessfraz
Copy link
Contributor

Probably because it was done through the UI ;)

On Monday, March 23, 2015, Alexander Morozov notifications@github.com
wrote:

@crosbymichael /~https://github.com/crosbymichael first commit is not
signed


Reply to this email directly or view it on GitHub
#474 (comment).

@tianon
Copy link
Contributor

tianon commented Mar 23, 2015

Shouldn't these instead only fail hard like this if both the feature was requested and the join failed with NotFound?

Otherwise, any cgroup we add support for becomes required, like blkio did in #337 (and then revised via #437).

This reverts commit 606d906.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
Adding this check here allows a nice error displaying that the specified
cgroup subsystem is not mounted.

Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
@crosbymichael crosbymichael force-pushed the revert-cgroups-change branch from b7034a0 to c5eef90 Compare March 23, 2015 20:32
@jessfraz
Copy link
Contributor

this is signed now :)

@LK4D4
Copy link
Contributor

LK4D4 commented Mar 23, 2015

LGTM

crosbymichael added a commit that referenced this pull request Mar 23, 2015
Revert "cgroups: only return path when subsystem really mounted
@crosbymichael crosbymichael merged commit fd0087d into docker-archive:master Mar 23, 2015
@crosbymichael crosbymichael deleted the revert-cgroups-change branch March 23, 2015 21:09
@tianon
Copy link
Contributor

tianon commented Mar 23, 2015

c5eef90 thanks @crosbymichael ❤️

@hqhq
Copy link
Contributor

hqhq commented Mar 24, 2015

Hi, the problem @tianon pointed out seems still exist. This revert also revert the fix that we don't fail when some subsystems are not mounted, I'm doing this another way in #476, so we also don't ignoring the error when a user specifies an option and we are unable to fulfill the request.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants