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

schema: Add enumeration to rootfsPropagation values #775

Merged
merged 1 commit into from
Apr 26, 2017

Conversation

zhouhao3
Copy link

Signed-off-by: zhouhao [email protected]

@zhouhao3
Copy link
Author

@wking I think it should be in the config.go file to increase the corresponding value, but I do not know how to name it, is it should be made such a style:
const (
ArchX86 Arch = "SCMP_ARCH_X86"

@wking
Copy link
Contributor

wking commented Apr 21, 2017 via email

"private",
"shared",
"slave",
"unbindable"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You probably want to drop unbindable until #770 lands.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#770 has landed, so there's no need to drop unbindable anymore.

@tianon
Copy link
Member

tianon commented Apr 26, 2017

I'm +1 on this (as discussed on our maintainer call just now) -- for values like "CPU Quota", the value in our spec is the value as defined in the kernel, so it makes a lot of sense to punt on "valid values" to the kernel docs/implementation.

For values like rootfsPropagation, the value in our spec is a custom string which we've created for the purposes of this spec that maps to some other value (defined by a kernel enum), so in my mind we must enumerate the expected values (and possibly even the kernel mappings unless they're already pretty obvious, which IMO they are in this case).

@tianon
Copy link
Member

tianon commented Apr 26, 2017

LGTM

(in other words)

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Apr 26, 2017

LGTM

Approved with PullApprove

@mrunalp mrunalp merged commit 13895d6 into opencontainers:master Apr 26, 2017
@zhouhao3 zhouhao3 deleted the rootfs-enum branch April 27, 2017 01:56
@vbatts vbatts mentioned this pull request Jul 5, 2017
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.

4 participants