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

Clarify cgroupsPath to be absolute #266

Closed
wants to merge 1 commit into from

Conversation

hqhq
Copy link
Contributor

@hqhq hqhq commented Dec 11, 2015

The former description is not accurate, it only says relative to the cgroups mount point,
if the path is absolute, it's easy to understand, but if it's relative, should it still be relative
to the cgroups mount point?

I think we can define it to be only absolute path, or we can also clarify if it's relative,
let implementation define what it would be relative to.

Signed-off-by: Qiang Huang h.huangqiang@huawei.com

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
@wking
Copy link
Contributor

wking commented Dec 11, 2015

On Thu, Dec 10, 2015 at 11:54:10PM -0800, Qiang Huang wrote:

The former description is not accurate, it only says relative to the cgroups mount point, if the path is absolute, it's easy to
understand, but if it's relative, should it still be relative to the
cgroups mount point?

I'd understood it to mean “relative to the cgroups mount point
(regardless of a leading slash)”.

I think we can define it to be only absolute path, or we can also
clarify if it's relative, let implementation define what it would be
relative to.

I don't think “relative to an implementation-defined thing” is
particularly useful. How about we adjust the example to:

"cgroupsPath": "myRuntime/myContainer"

and just error out if cgroupsPath starts with a slash? Then we
clearly meant “relative to the cgroups mount point” for all provided
cgroupsPaths.

@mrunalp
Copy link
Contributor

mrunalp commented Dec 16, 2015

I'd prefer it to be specified as an absolute path (internally relative to cgroups path). One reason is that it will be consistent with what one sees in /proc//cgroup.
For e.g.

memory:/user.slice/user-0.slice/session-1.scope

@wking
Copy link
Contributor

wking commented Dec 16, 2015

On Wed, Dec 16, 2015 at 10:10:26AM -0800, Mrunal Patel wrote:

I'd prefer it to be specified as an absolute path (internally
relative to cgroups path).

I think that “absolute but internally relative to the cgroups mount
point” is harder to explain than “relative to the cgroups mount
point”, but…

One reason is that it will be consistent with what one sees in
/proc//cgroup.

For e.g.

memory:/user.slice/user-0.slice/session-1.scope

+1 for consistency. If this is the consistency we're shooting for
(vs. consistency with “relative paths” more generally), I'd mention it
in the spec.

@hqhq
Copy link
Contributor Author

hqhq commented Dec 22, 2015

Close in favor of #270
They are fixing the same problem.

@hqhq hqhq closed this Dec 22, 2015
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.

3 participants