runc icon indicating copy to clipboard operation
runc copied to clipboard

add a mount options check for type bind in linux

Open lifubang opened this issue 5 years ago • 5 comments

For a fresh man with runc/containerd, mount a directory to a container may be a common action to them. If there is a mount like this:

{
  "destination": "/context",
  "type": "bind",
  "source": "/root"
}

When run runc run -d redis8, the response will be: container_linux.go:348: starting container process caused "process_linux.go:402: container init caused "rootfs_linux.go:58: mounting \"/root\" to rootfs \"/opt/runctest/redis/rootfs\" at \"/opt/runctest/redis/rootfs/context\" caused \"no such device\"""

I think this is hard for lots of people to realize what has happened. So, maybe we can response some more clear messages.

Such issue is also in containerd: https://github.com/containerd/containerd/issues/2831

I don't know whether this is reasonable or not, @maintainers, if you have time, please check, thanks.

Signed-off-by: Lifubang [email protected]

lifubang avatar Nov 26 '18 10:11 lifubang

I understand the argument for a warning like this, but I'd prefer that we instead just had warnings for type: "bind" or type: "rbind" without a corresponding option entry. This would be more generic and would handle most mistakes.

cyphar avatar Nov 27 '18 12:11 cyphar

Thank you for your review. Updated.

lifubang avatar Nov 28 '18 02:11 lifubang

Any way we can get this in? This is pretty easy to mess up. It is not intuitive that type: bind also requires an option of bind or rbind. I often find myself forgetting that and seeing the unknown device error (or something along those lines).

Just an idea, couldn't the option be automatically injected based on the type? I guess I am failing to understand the significance of type: bind.

andrewrynhard avatar Jun 17 '20 09:06 andrewrynhard

@andrewrynhard

This might be worthwhile to have, but just to clarify -- it's not that type: "bind" additionally requires a "bind" argument. It's that type: "bind" does nothing and you must have a "bind" argument. This mirrors the Linux kernel API (bind mounts are created with the MS_BIND flag, not a filesystem type of bind).

The main concern is that it's adding extra semantics on top of the Linux API when really we should be staying as close as possible to the Linux API. Though admittedly the only reason why people think type: "bind" does something is because our examples include it, so this is kind of our own fault :confused:.

cyphar avatar Jan 18 '21 01:01 cyphar

@andrewrynhard

This might be worthwhile to have, but just to clarify -- it's not that type: "bind" additionally requires a "bind" argument. It's that type: "bind" does nothing and you must have a "bind" argument. This mirrors the Linux kernel API (bind mounts are created with the MS_BIND flag, not a filesystem type of bind).

The main concern is that it's adding extra semantics on top of the Linux API when really we should be staying as close as possible to the Linux API. Though admittedly the only reason why people think type: "bind" does something is because our examples include it, so this is kind of our own fault 😕.

I can understand this. I would opt for staying closer to the Linux semantics in that case.

andrewrynhard avatar Jan 18 '21 05:01 andrewrynhard