Allow negative value for some resource fields #648

Merged
merged 1 commit into from Jan 10, 2017

Projects

None yet

4 participants

@hqhq
Contributor
hqhq commented Jan 5, 2017

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

@hqhq hqhq Allow negative value for some resource fields
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>
082e93a
@@ -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
@wking
wking Jan 5, 2017 Contributor

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.

@tianon
Member
tianon commented Jan 5, 2017 edited

LGTM

IMO the additional clarity as to why this is an int64 rather than a uint64 that @wking points out would be excellent as well, but I don't personally think it's a blocker for this PR.

Approved with PullApprove

@crosbymichael
Member
crosbymichael commented Jan 9, 2017 edited

LGTM

Approved with PullApprove

@hqhq hqhq merged commit 4b42ec4 into opencontainers:master Jan 10, 2017

2 checks passed

code-review/pullapprove Approved by crosbymichael, tianon
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@hqhq hqhq deleted the hqhq:all_negative_value branch Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment