config: linux: update description of PidsLimit #234
Conversation
|
preciously LGTM |
| // Maximum number of PIDs. A value < 0 implies "no limit". | ||
| // Maximum number of PIDs. A value <= 0 implies "no limit". | ||
| // While a limit of 0 is technically valid, it is precisely identical to | ||
| // a limit of 1, so there is no distinction. |
There was a problem hiding this comment.
Do you want both “≤ 0 means no limit” and “0 is the same as 1”? I'd expect either “≤ 0 means no limit and 0 is the same as -1” or “< 0 means no limit and 0 is the same as 1”. Or does 1 really mean “no limit”?
There was a problem hiding this comment.
Oh, sorry. What I meant was that "<= 0 means no limit within the spec, but a limit of 0 is identical to a limit of 1 in the PIDs controller". I guess I'm mixing specification and justification, based on the discussion in opencontainers/runc#58. I'll fix up the wording to only mention the actual specification and only leave the justification in the commit message.
There was a problem hiding this comment.
On Wed, Oct 28, 2015 at 09:28:23AM -0700, Aleksa Sarai wrote:
Oh, sorry. What I meant was that "<= 0 means no limit within the
spec, but a limit of 0 is identical to a limit of 1 in the PIDs
controller". I guess I'm mixing specification and justification,
based on the discussion in opencontainers/runc#58.
Oh, I see. In that case, my personal preference would be to just use
a uint pointer with “not set in the JSON means the runtime should
ignore” (see discussion in #233). But however this lands, any
semantic rules (like what values the runtime should ignore) should
also land in the runtime-config-linux.md docs where bundle authors
will find them.
Fix a misleading comment for how PidsLimit works when given a limit of 0. In the PIDs controller, a limit of 0 is identical to a limit of 1, since it is not possible to impose a limit on 0 processes. As such, it makes no sense to distinguish the two values, rather the value 0 (which is also the default value of an integer in Go) should instead indicate no limit (which is the default for all new PIDs controllers). Signed-off-by: Aleksa Sarai <cyphar@cyphar.com>
|
LGTM |
config: linux: update description of PidsLimit
Fix a misleading comment for how PidsLimit works when given a limit of
0. A limit of 0 is identical to a limit of 1, and so there is no reason
to distinguish the two. Since the default value for integers in Go is 0,
it makes more sense to make the default value mean that there is no
limit.
Signed-off-by: Aleksa Sarai cyphar@cyphar.com (github: cyphar)