-
Notifications
You must be signed in to change notification settings - Fork 557
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
config: Require the runtime to mount Spec.Mounts in order #142
config: Require the runtime to mount Spec.Mounts in order #142
Conversation
760b6b0
to
19c3bce
Compare
Ping @laijs. |
@@ -38,6 +38,7 @@ Each container has exactly one *root filesystem*, specified in the *root* object | |||
|
|||
You can add array of mount points inside container as `mounts`. | |||
Each record in this array must have configuration in [runtime config](runtime-config.md#mount-configuration). | |||
The runtime MUST mount entries in the listed order (`mounts[0]` first, `mounts[1]` second, …). |
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.
Perhaps simpler to say "order matters"
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.
On Fri, Sep 04, 2015 at 12:02:02PM -0700, Vincent Batts wrote:
+The runtime MUST mount entries in the listed order (
mounts[0]
first,mounts[1]
second, …).Perhaps simpler to say "order matters"
That wouldn't say how the order matters ;). I think it's hard to
have too much clarity, but I'm fine dropping the parenthetical if it
feels too wordy. The sentence through “… listed order” should be
sufficient to stand alone.
19c3bce
to
b051802
Compare
#157 landed with similar phrasing but no parenthetical. I've dropped the parenthetical (and rebased onto master) with 19c3bce → b051802 in case that help get this merged. |
LGTM I was wondering if you could add "Suggested-by: Lai Jiangshan [email protected]" |
If we don't specify this, some bundle-authors or runtime-implementers might expect the runtime to intelligently order mounts to get the "right" order [1]. But that's not possible because: $ mkdir -p a/b/c d/e/f h # mount --bind a/b h # mount --bind d a/b $ tree --charset=ascii h h `-- c But in the other order: # umount a/b # umount h # mount --bind d a/b # mount --bind a/b h $ tree --charset=ascii h h `-- e `-- f So there's no "right" order. Allowing the bundle-author to specify their intended order is both easy to implement and unambiguous. [1]: opencontainers#136 (comment) Suggested-by: Lai Jiangshan <[email protected]> Signed-off-by: W. Trevor King <[email protected]>
b051802
to
d282a32
Compare
On Thu, Sep 10, 2015 at 05:31:12PM -0700, Lai Jiangshan wrote:
Done with b051802 → d282a32, and I cited your initial comment as a |
LGTM |
1 similar comment
LGTM |
config: Require the runtime to mount Spec.Mounts in order
If we don't specify this, some bundle-authors or runtime-implementers
might expect the runtime to intelligently order mounts to get the
"right" order. But that's not possible because:
But in the other order:
So there's no "right" order. Allowing the bundle-author to specify
their intended order is both easy to implement and unambiguous.
Spun off from #136.