Forbid duplicated rlimits with same type #607

Merged
merged 1 commit into from Jan 11, 2017

Projects

None yet

5 participants

@hqhq
Contributor
hqhq commented Nov 3, 2016

Alternative of #583 , as what we do for namespaces
in #597 .

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

@hqhq hqhq Forbid duplicated rlimits with same type
Alternative of #583 , as what we do for `namespaces`
in #597 .

Signed-off-by: Qiang Huang <h.huangqiang@huawei.com>
6696846
@wking
Contributor
wking commented Nov 3, 2016

On Wed, Nov 02, 2016 at 07:15:57PM -0700, Qiang Huang wrote:

Alternative of #583 , as what we do for namespaces
in #597 .

The concern with using objects for namespaces was the path handling
[1](and I still think it would have been better to use an object
there [2]). But we require at least one limit for rlimit entries
(otherwise what's the point?), so I don't see a reason to not go
with an object for rlimits.

@wking
Contributor
wking commented Nov 3, 2016

On Wed, Nov 02, 2016 at 08:36:27PM -0700, W. Trevor King wrote:

But we require at least one limit for rlimit entries (otherwise
what's the point?)…

Actually, we don't. But an rlimit entry with neither hard nor soft is
a no-op, while in #598 we needed to distinguish between no-op and
“create a new namespace”. So I think my initial “so I don't see a
reason to not go with an object for rlimits” still stands.

@hqhq
Contributor
hqhq commented Nov 3, 2016

@wking I still don't see the problem of using array here after we fix the duplicated type issue.

@wking
Contributor
wking commented Nov 3, 2016

It's nothing insurmountable, but add-or-replace and removal are much easier with objects. In #597 I linked to this hoop jumping you need to support then for arrays.

@crosbymichael
Member
crosbymichael commented Nov 28, 2016 edited

LGTM

Approved with PullApprove

@dqminh
Contributor
dqminh commented Jan 11, 2017 edited

LGTM

Approved with PullApprove

@mrunalp
Contributor
mrunalp commented Jan 11, 2017 edited

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit 985b97a into opencontainers:master Jan 11, 2017

2 checks passed

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