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

bind is not a valid mount type #967

Closed
wants to merge 1 commit into from

Conversation

binchenX
Copy link

@binchenX binchenX commented May 9, 2018

Per spec, "bind" is not a valid mount type. To use bind mount,
the spec compatible way is to set either "bind" or "rbind"
in the mount options.

Although in practice, setting the type as "bind" won't actually
cause an error (at the moment using runc, as well as the system
call mount(7) itself) as long as either "bind" or "rbind" is set
as well, it provide an incorrect example that "bind" is avalid
type, and make people think following config is a valid one for
bind mount, which in fact will throw an error in runc.

{
"destination": "/data",
"type": "bind"
"source": "/volumes/testing",
}

[1] tried to automatically add a MS_BIND flag when the type is "bind"
but was rejected on the ground it "encourage" the incorrect usage of
bind as the mount type, which I agree. Hence, here I change the example
in the spec in the hope making it clearer that for bind mount to work,
we should use the correct mount options, instead of the "bind" mount type.

[1]opencontainers/runc#1799

Signed-off-by: Bin Chen <Bin Chen nk@devicu.com>

@binchenX binchenX force-pushed the master branch 2 times, most recently from 94dd6a9 to 06db44c Compare May 9, 2018 06:10
Per spec, "bind" is not a valid mount type. To use bind mount,
the spec compatible way is to set either "bind" or "rbind"
in the mount options.

Although in practice, setting the type as "bind" won't actually
cause an error (at the moment using runc, as well as the system
call mount(7) itself) as long as either "bind" or "rbind" is set
as well, it provide an incorrect example that "bind" is avalid
type, and make people think following config is a valid one for
bind mount, which in fact will throw an error in runc.

{
 "destination": "/data",
 "type": "bind"
 "source": "/volumes/testing",
}

[1] tried to automatically add a MS_BIND flag when the type is "bind"
but was rejected on the ground it "encourage" the incorrect usage of
bind as the mount type, which I agree. Hence, here I change the example
in the spec in the hope making it clearer that for bind mount to work,
we should use the correct mount options, instead of the "bind" mount type.

[1]opencontainers/runc#1799

Signed-off-by: Bin Chen <nk@devicu.com>
@@ -115,7 +115,6 @@ For POSIX platforms the `mounts` structure has the following fields:
},
{
"destination": "/data",
"type": "bind",
Copy link
Member

Choose a reason for hiding this comment

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

Might as well change this to "type": "none" to make this more clear it's not an accidental omission.

@binchenX
Copy link
Author

binchenX commented May 9, 2018 via email

@cyphar
Copy link
Member

cyphar commented May 9, 2018

Woo, I don't know "none" is a valid type. That's definitely better.

Well, it's less that none is a valid type, it's more that the kernel doesn't care what you shove in the type field when doing a mount(2) of type MS_BIND (you could make the type foobar if you wanted 😉). But none is a fairly common convention when doing mounts to make it clearer that it's not a normal mount.

@wking
Copy link
Contributor

wking commented May 9, 2018

This is a dup of #954 (which I still think we want). Also in this space are #878 (still in flight) and #771 (which I'd like to revisit).

@dongsupark
Copy link
Contributor

+1
I've just stumbled upon it, and it makes sense. bind (or rbind) should be an option instead of a type.
Since opencontainers/runc#1753 was merged, how about updating the somehow misleading example in the spec?
Both this PR and #954 look good to me.
Describing more about it like #878 looks also good.

alban added a commit to kinvolk/runtime-spec that referenced this pull request Jul 27, 2018
- The 'source' of a bind mount can either be a file or a directory.

- The 'type' field is a dummy  in case of bind mounts, often left to
"none" (in that case it does not have to be something listed in
/proc/filesystems).

- 'Relative' paths can only be used for bind mounts, not for other
mounts. That's how runc manages this.

Replaces: opencontainers#967

Signed-off-by: Alban Crequy <alban@kinvolk.io>
@vbatts
Copy link
Member

vbatts commented Sep 13, 2018

obsoleted by #981

@vbatts vbatts closed this Sep 13, 2018
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.

5 participants