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

config: Require the runtime to mount Spec.Mounts in order #142

Merged
merged 1 commit into from
Oct 5, 2015

Conversation

wking
Copy link
Contributor

@wking wking commented Sep 3, 2015

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:

$ 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.

Spun off from #136.

@wking wking force-pushed the explicitly-define-mount-order branch from 760b6b0 to 19c3bce Compare September 3, 2015 17:05
@wking
Copy link
Contributor Author

wking commented Sep 3, 2015

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, …).
Copy link
Member

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"

Copy link
Contributor Author

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.

@wking
Copy link
Contributor Author

wking commented Sep 10, 2015

#157 landed with similar phrasing but no parenthetical. I've dropped the parenthetical (and rebased onto master) with 19c3bceb051802 in case that help get this merged.

@laijs
Copy link
Contributor

laijs commented Sep 11, 2015

LGTM

I was wondering if you could add "Suggested-by: Lai Jiangshan jiangshanlai@gmail.com"

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 <jiangshanlai@gmail.com>
Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking force-pushed the explicitly-define-mount-order branch from b051802 to d282a32 Compare September 11, 2015 03:57
@wking
Copy link
Contributor Author

wking commented Sep 11, 2015

On Thu, Sep 10, 2015 at 05:31:12PM -0700, Lai Jiangshan wrote:

I was wondering if you could add "Suggested-by: Lai Jiangshan
jiangshanlai@gmail.com"

Done with b051802d282a32, and I cited your initial comment as a
reference for the possible alternative interpretation that this PR
closes off.

@mrunalp
Copy link
Contributor

mrunalp commented Oct 5, 2015

LGTM

1 similar comment
@vbatts
Copy link
Member

vbatts commented Oct 5, 2015

LGTM

mrunalp pushed a commit that referenced this pull request Oct 5, 2015
config: Require the runtime to mount Spec.Mounts in order
@mrunalp mrunalp merged commit f75f23f into opencontainers:master Oct 5, 2015
@wking wking deleted the explicitly-define-mount-order branch October 5, 2015 20:24
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