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

Disallow overlay/overlay2 on top of NFS #35483

Merged
merged 1 commit into from Nov 30, 2017

Conversation

Projects
None yet
9 participants
@thaJeztah
Member

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)

Disallow overlay/overlay2 on top of NFS
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>
@dmcgowan

LGTM

@mistyhacks

This comment has been minimized.

Show comment
Hide comment
@mistyhacks

mistyhacks Nov 14, 2017

Contributor

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

Contributor

mistyhacks commented Nov 14, 2017

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

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 14, 2017

Member

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)

Member

thaJeztah commented Nov 14, 2017

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

This comment has been minimized.

Show comment
Hide comment
@dmcgowan

dmcgowan Nov 14, 2017

Member

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.

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.

@mistyhacks

This comment has been minimized.

Show comment
Hide comment
@mistyhacks

mistyhacks Nov 14, 2017

Contributor

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

Contributor

mistyhacks commented Nov 14, 2017

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

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 14, 2017

Member

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

Member

thaJeztah commented Nov 14, 2017

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

@kazh000

This comment has been minimized.

Show comment
Hide comment
@kazh000

kazh000 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.

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 14, 2017

Member

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?

Member

thaJeztah commented Nov 14, 2017

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

This comment has been minimized.

Show comment
Hide comment
@amir73il

amir73il Nov 14, 2017

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

Contributor

amir73il commented Nov 14, 2017

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

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 14, 2017

Member

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.

Member

thaJeztah commented Nov 14, 2017

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.

@cpuguy83

LGTM

@thaJeztah

This comment has been minimized.

Show comment
Hide comment
@thaJeztah

thaJeztah Nov 30, 2017

Member

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

Member

thaJeztah commented Nov 30, 2017

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

This comment has been minimized.

Show comment
Hide comment
@tonistiigi

tonistiigi Nov 30, 2017

Member

LGTM

Member

tonistiigi commented Nov 30, 2017

LGTM

@tonistiigi tonistiigi merged commit bdd9668 into moby:master Nov 30, 2017

6 checks passed

dco-signed All commits are signed
experimental Jenkins build Docker-PRs-experimental 37828 has succeeded
Details
janky Jenkins build Docker-PRs 46539 has succeeded
Details
powerpc Jenkins build Docker-PRs-powerpc 6956 has succeeded
Details
windowsRS1 Jenkins build Docker-PRs-WoW-RS1 18096 has succeeded
Details
z Jenkins build Docker-PRs-s390x 6756 has succeeded
Details

@thaJeztah thaJeztah deleted the thaJeztah:disallow-nfs-backing-for-overlay branch Nov 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment