Allow negative value for some resource fields #648
Merged
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
Carry opencontainers#499 For these values, cgroup kernal APIs accept -1 to set them as unlimited, as docker and runc all support update resources, we should not set drawbacks in spec. Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
This was referenced Jan 5, 2017
wking
reviewed
Jan 5, 2017
| @@ -283,15 +283,15 @@ For more information, see [the memory cgroup man page][cgroup-v1-memory]. | |||
|
|
|||
| The following parameters can be specified to setup the controller: | |||
|
|
|||
| * **`limit`** *(uint64, OPTIONAL)* - sets limit of memory usage in bytes | |||
| * **`limit`** *(int64, OPTIONAL)* - sets limit of memory usage in bytes | |||
There was a problem hiding this comment.
These properties are not tied directly to their backing cgroup property. I think we should either:
a. Be explicit about the cgroup property which MUST be used to satisfy the configuration (in which case the kernel docs will explain the 0 and -1 cases) or
b. Explicitly define the 0 and -1 cases here.
|
LGTM IMO the additional clarity as to why this is an |
|
LGTM |
This was referenced Jan 12, 2017
hqhq
added a commit
to hqhq/runtime-spec
that referenced
this pull request
Mar 1, 2017
This partially revert opencontainers#648 , after a second thought, I think we should use specs value the same as kernel API input, see: opencontainers#692 (comment) For memory and hugetlb limits *.limit_in_bytes, cgroup APIs take the values as string, but the parsed values are unsigned long, see: https://github.com/torvalds/linux/blob/v4.10/mm/page_counter.c#L175-L193 For `cpu.cfs_quota_us` and `cpu.rt_runtime_us`, cgroup APIs take the input value as signed long long, while `cpu.cfs_period_us` and `cpu.rt_periof_us` take the input value as unsigned long long. Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Carry #499
For these values, cgroup kernal APIs accept -1 to set
them as unlimited, as docker and runc all support
update resources, we should not set drawbacks in spec.
Signed-off-by: Qiang Huang h.huangqiang@huawei.com