config: Document 'recursive' and 'bind' mount options extensions

[wking]

The are not filesystem types, so they don't belong in type. The specs claim mount(2) as inspiration for this modeling (which makes sense, since that's the syscall Linux runtimes will make to implement it), and there (recursive) bind is represented by mountflags (MS_REC | MS_BIND). Currently the options property handles both mount(2)'s mountflags and data, so options is a good spot for these two settings.

We may eventually add entries for other mount(8) command-line options (e.g. --move, --make-shared, …), but I've left those off until someone pitches a motivational use case for them.

The example I'm touching used:

"type": "bind",
…
"options": ["rbind", ...]

but I don't see a point to putting bind in type when it's not a filesystem type and you already have rbind in options. We could have stuck closer to that example with an unset type and:

"options": ["rbind", ...]

and that would have been closer to mount(8)'s --rbind API, but I've gone with recursive here to stay closer to mount(2). This will scale better if/when we eventually add options for MS_SLAVE, etc.

Since there are existing consumers using the old example format and similar things like:

$ git log --oneline -1 | cat
03e8b89 Merge pull request #176 from hmeng-19/set_oom_score_adj
$ ./ocitools generate --template <(echo '{}') --bind ab:cd:ro | jq '.mounts[0]'
{
  "destination": "cd",
  "type": "bind",
  "source": "ab",
  "options": [
    "bind",
    "ro"
  ]
}

this may be a breaking change for some spec consumers (although that ocitools example will still work, because options contains bind, which means the type will be ignored). But even if there are broken consumers, we're still pre-1.0, the spec never explained what bind/rbind meant before this commit, and consolidating the Linux mount spec around mount(2) now will make life less confusing in the future.

Spun off from here and here.

[hqhq]

LGTM

[cyphar]

Quote missing on bind.

[wking]

I've pushed 1ef5938 → 05b3277 which:

• Fixed a missing quote around ‘bind’ 1.
• Removed a stale ‘rbind’ reference 2.

[crosbymichael]

NOT LGTM

recursive is not even an option, @wking just made it up and added it to a doc. MS_REC makes no sense what-so-ever outside of a bind mount and that is why we, mount, and everyone else in the world supports rbind and bind not recursive and recursive,bind.

Checking the mount options for bind or rbind is fine if the type is empty and is a good idea. But this does not require a spec change but can be updated in the runtime code. Its not critical to spec the change because passing bind as the fstype/device in the mount syscall does not cause any issues and is ignored.

This is getting old, maintainers, please watch what you approve.

[wking]

On Fri, Oct 28, 2016 at 2:06PM -0700, Michael Crosby wrote:

MS_REC makes no sense what-so-ever outside of a bind mount and that
is why we, mount, and everyone else in the world supports rbind and
bind not recursive and recursive,bind.

And mount(8) also has --make-rshared, --make-rslave, --make-rprivate,
and --make-runbindable, all of which use MS_REC [1](and I pointed out
the MS_SLAVE case in my topic post). So I still think a separate
‘recursive’ option is better.

However, if the maintainer consensus is that separate recursive
versions (rbind now and rshared, rslave, … later) are the way to go,
I'll reroll this PR to make that happen. Were there any other
concerns with this PR? Should I open a new one that goes down the r*
path? What I want to avoid is widespread use (including in
runtime-tools, opencontainers/runtime-tools#119) of an undefined
(in the nasal demons sense) mount option.