Skip to content

Disallow overlay/overlay2 on top of NFS #35483

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

Merged
merged 1 commit into from
Nov 30, 2017

Conversation

thaJeztah
Copy link
Member

@thaJeztah thaJeztah commented Nov 13, 2017

From https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt:

The lower filesystem can be any filesystem supported by Linux and does
not need to be writable. The lower filesystem can even be another
overlayfs. The upper filesystem will normally be writable and if it
is it must support the creation of trusted.* extended attributes, and
must provide valid d_type in readdir responses, so NFS is not suitable.

fixes #35476

- Description for the changelog

* Disallow running overlay and overlay2 on top of NFS, as it is not supported by overlayfs [moby/moby#35483](https://github.com/moby/moby/pull/35483)

- A picture of a cute animal (not mandatory but encouraged)

From https://www.kernel.org/doc/Documentation/filesystems/overlayfs.txt:

> The lower filesystem can be any filesystem supported by Linux and does
> not need to be writable. The lower filesystem can even be another
> overlayfs. The upper filesystem will normally be writable and if it
> is it must support the creation of trusted.* extended attributes, and
> must provide valid d_type in readdir responses, so NFS is not suitable.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Copy link
Member

@dmcgowan dmcgowan left a comment

Choose a reason for hiding this comment

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

LGTM

@mdlinville
Copy link
Contributor

What about other network-backed FS? SMB? sshfs? HDFS?

@thaJeztah
Copy link
Member Author

Another option (possibly as a follow-up) would be to reverse the list; and only allow running on backing-filesystems that we know to work; other filesystems will error-out, but we can add a --storage-opt overlay.allow-unsupported-filesystems (e.g.) that allows people to override that restriction (in which case we should both log a big-red-warning, and print a warning on docker info as well)

@dmcgowan
Copy link
Member

dmcgowan commented Nov 14, 2017

What about other network-backed FS? SMB? sshfs? HDFS?

SSHFS and HDFS are fuse based, we can't really detect those specifically. We would have to switch to a whitelist approach and imo that is too opinionated. NFS is a common setup so users can easily walk into it and expect it to work, not sure anyone expects the same over FUSE or SMB.

@mdlinville
Copy link
Contributor

SMB is only FUSE on Linux hosts -- on Windows hosts it is about as native as it gets.

@thaJeztah
Copy link
Member Author

Oh, but on Windows we don't support the overlay drivers, only the "Windows" storage driver

@kazh000
Copy link

kazh000 commented Nov 14, 2017

As far as I understand recently NFS become suitable for overlayfs https://lwn.net/Articles/736677/ but it requires very fresh kernel and corresponding NFS options. So I suggest to check a kernel version or/and NFS options.

@thaJeztah
Copy link
Member Author

Ah, thanks for that information; I'd not be against doing that check. If possible through feature detection (instead of just comparing kernel versions); perhaps it should be in a follow up.

ping @amir73il any suggestions on how to detect if overlay is supported on NFS?

@amir73il
Copy link
Contributor

There is a lot of confusion and inaccurate statements in this thread. Quite understandable with the current state of overlayfs documentation. I'll try to add clarity.

@kazh000 the mentioned article has nothing to do with supporting an NFS mount as upper layer for overlayfs. It is WIP, not anywhere near mainline, to support exporting an overlayfs mount by NFS server.

@MistyHacks the reason that network backed file systems and FUSE are not supported as upper layer is because those file systems can invalidate VFS cache entries (when files are changed remotely) and overlayfs doesn't like things to change under its feet. Incidently, overlayfs doesn't like it when remote file systems change files remotely even when those are used as lower layer, but lower layer files are not supposed to be change at all, while overlay is moutned.

@thaJeztah the text you quoted in the bug from overlayfs.txt has mislead you. Overlayfs will throw a warning to dmesg complaining if d_type is not supported ("overlayfs: upper fs needs to support d_type.").
However, Overlayfs will NOT fail the mount on no d_type support, nor on no xattr support, but it will perform sub optimally (exposing whiteouts and returning EIO in some cases). So w.r.t feature detection, you SHOULD to check d_type support and xattr support before using overlayfs driver, but this is not the reason that overlayfs over NFS is failing to mount.
You do not really need "feature detection" for this case, you just need to try to mount overlay and see that it fails. Anyway, I don't know of an API to check if a file system is a "remote" file system, but FYI, here is the list of such file systems in the kernel, found by 'git grep -lw d_revalidate fs/':
9p
afs
ceph
cifs
coda
ecryptfs
fat
fuse
gfs2
hfs
jfs
kernfs
ncpfs
nfs
ocfs2
orangefs
overlayfs
proc

@thaJeztah
Copy link
Member Author

Thanks for your input @amir73il - detection of d_type support is already in our code base (as running without leads to "interesting side effects"); I'll check/discuss the other options.

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah
Copy link
Member Author

We discussed this change, and for now just disallow overlay on NFS, but contributions are welcome if we can implement this using actual detection if its supported

@tonistiigi
Copy link
Member

LGTM

@tonistiigi tonistiigi merged commit bdd9668 into moby:master Nov 30, 2017
@thaJeztah thaJeztah deleted the disallow-nfs-backing-for-overlay branch November 30, 2017 03:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment