-
Notifications
You must be signed in to change notification settings - Fork 554
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
Windows: Clarify r/o root filesystem #819
Conversation
Signed-off-by: John Howard <jhoward@microsoft.com>
@@ -32,7 +32,7 @@ For example, if a configuration is compliant with version 1.1 of this specificat | |||
The path is either an absolute path or a relative path to the bundle. | |||
On Linux, for example, with a bundle at `/to/bundle` and a root filesystem at `/to/bundle/rootfs`, the `path` value can be either `/to/bundle/rootfs` or `rootfs`. | |||
A directory MUST exist at the path declared by the field. | |||
* **`readonly`** (bool, OPTIONAL) If true then the root filesystem MUST be read-only inside the container, defaults to false. | |||
* **`readonly`** (bool, OPTIONAL) If true then the root filesystem MUST be read-only inside the container, defaults to false. On Windows, this field must be omitted or false. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Previous discussion in #23 and #476.
An alternative to this would be to label the property as Linux/Solaris-specific, so it wouldn't be specified for Windows and Windows runtimes could silently ignore it (using this). That gets us to the runtime behavior proposed in #476, but avoids confusing config authors (like the spec wording in #476) because Windows authors would know that the property did not apply to their platform, and non-Windows authors would know that the property did apply (and runtimes would have to implement it) on their platforms.
LGTM Forcing this to be a known-supported value like this makes sense IMO -- that allows for it to be supported in a future update if the platform adds support. This also ends up mirroring other places in the spec where features requested in the configuration that are unsupported by the runtime or the underlying kernel MUST result in an error (which is essentially what this is codifying for Windows). |
To clarify my alternative proposal, what do you think a Linux runtime should do if a Linux config sets the Windows-only |
As a user, anything other than an error would be very surprising behavior.
|
LGTM |
Signed-off-by: John Howard jhoward@microsoft.com
Clarification that Windows does not support a read-only root filesystem.