allow bind mount w/o explicit "bind" opt but w/ explicit "bind" type

[AkihiroSuda]

Previously, {"type":"bind"} without {"options": ["bind"]} was failing with ENODEV.

See containers/podman#7652

[kolyshkin]

this will also match something like "ls: /tmp/bind/config.json: no such file or directory" which is not good.

I think we should list the directory instead, and check the file is present in the output.

[AkihiroSuda]

We check the status code, so it won't match ls: /tmp/bind/config.json: no such file or directory

[kolyshkin]

well in this case it does not make sense to check the output, since ls will definitely return the error if the file is not there, so this check is redundant

[kolyshkin]

So I guess either

• list the directory, check the exit code, check the output to contain file name

or

• list the file, check the exit code

[kolyshkin]

test case needs fixing

[AkihiroSuda]

The test should be ok, please see #2590 (comment)

[cyphar]

I would argue the config is wrong -- "type": "bind" doesn't mean anything in Linux (all that matters is the existence of MS_BIND in the mount options) -- though in fairness this was a bug in runc as well (#1753). Is there a reason that the config generator is not adding the bind option to mounts?

I do see why we'd want to add it, but it means that a hypothetical filesystem called bind couldn't be mounted with this change.

[AkihiroSuda]

Is there a reason that the config generator is not adding the bind option to mounts?

As crun doesn't require this option, projects using crun as the primary runtime are likely to forget adding this option. containers/podman#7652

a hypothetical filesystem called bind couldn't be mounted with this change.

This sounds too much hypothetical. When a new kind of "bind" filesystem is being added to the kernel, probably it will be called in another name like "bind2", to avoid breaking userspace mount(8) CLI

[cyphar]

That means crun is not implementing the runtime-spec correctly (or rather, is implementing an extension to the runtime-spec). To quote the spec:

A mount is a bind mount if it has either bind or rbind in the options. [...] For bind mounts (when options include either bind or rbind), the type is a dummy, often "none" (not listed in /proc/filesystems).

I'm not against adding something as simple as this workaround, but just because crun (or runc for that matter) does something doesn't make it the correct behaviour. Podman shouldn't be generating invalid OCI configurations.

When a new kind of "bind" filesystem is being added to the kernel, probably it will be called in another name like "bind2", to avoid breaking userspace mount(8) CLI

mount -t bind doesn't work, you have to do mount --bind.

[giuseppe]

As crun doesn't require this option, projects using crun as the primary runtime are likely to forget adding this option. containers/podman#7652

I'll fix it in crun to follow more closely the OCI specs.