overlay2: use index=off if possible (fix EBUSY on mount) by kolyshkin · Pull Request #37993 · moby/moby

kolyshkin · 2018-10-08T23:10:05Z

As pointed out in #37970,
Docker overlay driver can't work with index=on feature of
the Linux kernel "overlay" filesystem. In case the global
default is set to "yes", Docker will fail with EBUSY when
trying to mount, like this:

error creating overlay mount to ...../merged: device or resource busy

and the kernel log should contain something like:

overlayfs: upperdir is in-use by another mount, mount with '-o index=off' to override exclusive upperdir protection.

A workaround is to set index=off in overlay kernel module
parameters, or even recompile the kernel with
CONFIG_OVERLAY_FS_INDEX=n in .config. Surely this is not
always practical or even possible.

The solution, as pointed out my Amir Goldstein (as well as
the above kernel message 😄) is to use index=off option
when mounting.

NOTE since older (< 4.13rc1) kernels do not support index=
overlayfs parameter, try to figure out whether the option
is supported by the running kernel. In case it's not possible
to figure it out, assume the kernel is new enough to support it.

NOTE the default can be changed anytime (by writing to
/sys/module/overlay/parameters/index) so we need to always
use index=off.

Fixes: #37970

kolyshkin · 2018-10-08T23:10:55Z

@amir73il @thaJeztah @dmcgowan @cpuguy83 PTAL

dmcgowan · 2018-10-08T23:15:12Z

No need to convert this from fmt.Sprint

To me, it looks more readable that way (as the option arguments are right there next to options), and Sprintf() does nothing but string concatenating here.

Using fmt.Sprintf() in Golang to merely concatenate a few strings (without doing anything else like padding, quoting etc.) looks like a coding pattern that came straight from C, where you can't concatenate more than 2 strings at once, and should be discouraged unless necessary.

Concatenation is also quite faster (not that it matters in this particular case).

@dmcgowan please let me know what you think about ^^^ , and I'll surrender and obey :)

This change is fine, neither has great readability but not worth making it an array and joining

kolyshkin · 2018-10-09T01:52:00Z

CI failures due to my careless typos ⌨️ should now be fixed

codecov · 2018-10-09T05:17:46Z

Codecov Report

❗ No coverage uploaded for pull request base (master@1f48759). Click here to learn what that means.
The diff coverage is 34.78%.

@@            Coverage Diff            @@
##             master   #37993   +/-   ##
=========================================
  Coverage          ?   36.12%           
=========================================
  Files             ?      610           
  Lines             ?    45170           
  Branches          ?        0           
=========================================
  Hits              ?    16318           
  Misses            ?    26614           
  Partials          ?     2238

kolyshkin · 2018-10-09T05:18:29Z

v2: moved the detection code to driver's Init() where it belongs.

thaJeztah · 2018-10-09T10:55:07Z

What's the result if it's not supported and we set this option? Will it fail?

Yes, it will most probably result in kernel returning EINVAL from mount() syscall. This will only happen if:

kernel is > 4.13rc1 (and the relevant feature is not backported);
and

for some reason /sys/module/overlay/parameters/index is present but stat() returns an error.

The former is quite possible, say on older distros.

The latter is highly unlikely to happen, from the top of my head I can only think of a severe kernel memory shortage, or using any security mechanisms (say selinux) to disallow dockerd access to e.g. /sys/module/ directory.

Right, so I was thinking if we should take the safe approach, and only add index=off if we're successfully able to detect that it's supported (i.e. os.Stat() was successfull), so:

os.Stat() was successful; add index=off

os.Stat() produces an IsNotExist: don't add index=off, and continue silently (as this is an expected situation)

os.Stat() produces any other error: log a warning ("unable to detect if index=off is supported: <original error message>"), but continue as usual (but don't add index=off)

Logging a warning at least makes it visible that we were unable to detect, and may assist in troubleshooting why we failed to detect.

Makes sense; please see updated commit

This simplifies the code a lot. Signed-off-by: Kir Kolyshkin <[email protected]>

As pointed out in moby#37970, Docker overlay driver can't work with index=on feature of the Linux kernel "overlay" filesystem. In case the global default is set to "yes", Docker will fail with EBUSY when trying to mount, like this: > error creating overlay mount to ...../merged: device or resource busy and the kernel log should contain something like: > overlayfs: upperdir is in-use by another mount, mount with > '-o index=off' to override exclusive upperdir protection. A workaround is to set index=off in overlay kernel module parameters, or even recompile the kernel with CONFIG_OVERLAY_FS_INDEX=n in .config. Surely this is not always practical or even possible. The solution, as pointed out my Amir Goldstein (as well as the above kernel message:) is to use 'index=off' option when mounting. NOTE since older (< 4.13rc1) kernels do not support "index=" overlayfs parameter, try to figure out whether the option is supported. In case it's not possible to figure out, assume it is not. NOTE the default can be changed anytime (by writing to /sys/module/overlay/parameters/index) so we need to always use index=off. [v2: move the detection code to Init()] [v3: don't set index=off if stat() failed] Signed-off-by: Kir Kolyshkin <[email protected]>

thaJeztah

LGTM, thanks!

thaJeztah · 2018-10-11T20:04:43Z

 	defer func() {
 		if err := os.RemoveAll(td); err != nil {
-			logrus.WithField("storage-driver", "overlay2").Warnf("Failed to remove check directory %v: %v", td, err)
+			logger.Warnf("Failed to remove check directory %v: %v", td, err)


We should also start using .WithError() for these;

logger.WithError(err).Warnf("Failed to remove check directory %v", td)

right (also in this case there's no need to print filename, as it should be a part of error message returned from os.RemoveAll())

tonistiigi

@dmcgowan

dmcgowan

LGTM

kernel version > 4.13rc1 support index=on feature, it will be failed with EBUSY when trying to mount. Related: moby/moby#37993 Signed-off-by: Rudy Zhang <[email protected]>

cody-c-lewis · 2022-02-22T21:32:13Z

It looks like this change blocks a docker image from setting up an NFS export of a directory in the root file system (ie, what's enabled by this overlay Linux kernel module addition). Is there no way to export the filesystem with the overlay driver in Docker?

In the past I've used the AUFS driver, but that's deprecated and going away bit by bit.

amir73il · 2022-02-23T15:42:11Z

@cody-c-lewis how were you planing to enable nfs_export in overlayfs? Without changing docker?
IMO, enabling NFS export should be controlled by a docker image config and/or runtime parameter.
When NFS export is requested, docker should not use index=off and add nfs_export=on mount option instead.
This will have at least two implications:

If there are sill unfixed mount leaks in docker, that could lead to EBUSY errors on overlayfs mount
Mounting overlayfs with index=on,nfs_export=on is recommended when upper layer is empty - it is not recommended to mount the same overlay with index=off, make changes and mount with index=on,nfs_export=on again
At best case, NFS client may get ESTALE errors, at worst case, even local application may get EIO error when trying to access files in such an overlay mount

cody-c-lewis · 2022-02-23T18:29:42Z

@cody-c-lewis how were you planing to enable nfs_export in overlayfs? Without changing docker?

Since there isn't any docker setting for this then my next thought was to set it via the kernel module parameters (such as /sys/module/overlay/parameters). But I'm searching in the dark here and open to suggestions and working backwards from a desired end state (being able to run an NFS server inside of a docker container that serves a static part of the image).

IMO, enabling NFS export should be controlled by a docker image config and/or runtime parameter. When NFS export is requested, docker should not use index=off and add nfs_export=on mount option instead. This will have at least two implications...

It sounds like these two issues were the motivation behind this PR so I guess the real question is if they've been resolved in the intervening 3 years. If that's the case then maybe undoing this PR would be a solution, or adding first class support to docker as you describe.

rdlarkins · 2022-02-23T19:27:40Z

Since there isn't any docker setting for this then my next thought was to set it via the kernel module parameters (such as /sys/module/overlay/parameters).

The kernel module config gets overridden by the mount options set here. I rebuilt the kernel module to ignore these two mount options and just use the default parameters set, and starting a single container with a nfs export does seem to work at least superficially.

cpuguy83 · 2022-02-23T19:54:46Z

Just trying to understand, you are trying to create an nfs export on a path inside the container which is just part of the normal container rootfs (ie. overlay) and not a bind-mount from the host?

Have you considered bind-mounting a dir here (or just an anonymous volume) so that it bypasses overlay entirely? You can still create the nfs export in the container, just now it doesn't have to depend on the details of the container rootfs.

rdlarkins · 2022-02-23T20:39:48Z

That is correct. I originally posted about this on the docker forums, you might find the details there more interesting: https://forums.docker.com/t/nfs-export-disabled-with-overlay2-as-storage-driver/121325

I have considered other options for the source of the files in the nfs export, but as I would like the files to be versioned alongside the rest of the container content, updating them as a single unit is logistically easier.

rdlarkins · 2022-02-28T15:16:41Z

I've had some more time to work with this over the past couple days. So far it seems that for my specific use case, there aren't any issues with enabling index and nfs_export. I've created an issue so that discussion can happen there rather than a 4 year old PR. #43299

kolyshkin requested a review from dmcgowan as a code owner October 8, 2018 23:10

GordonTheTurtle added the status/0-triage label Oct 8, 2018

dmcgowan reviewed Oct 8, 2018

View reviewed changes

kolyshkin force-pushed the ovr2-index branch 2 times, most recently from 780ea19 to c685bdb Compare October 9, 2018 01:50

kolyshkin force-pushed the ovr2-index branch from c685bdb to 57be456 Compare October 9, 2018 05:17

thaJeztah mentioned this pull request Oct 9, 2018

Getting error with overlayfs: device or resource busy #37970

Closed

thaJeztah reviewed Oct 9, 2018

View reviewed changes

thaJeztah added area/storage/overlay status/2-code-review and removed status/0-triage labels Oct 9, 2018

kolyshkin added 2 commits October 11, 2018 12:50

overlay2: use global logger instance

a55d325

This simplifies the code a lot. Signed-off-by: Kir Kolyshkin <[email protected]>

kolyshkin force-pushed the ovr2-index branch from 57be456 to 8422d85 Compare October 11, 2018 19:56

thaJeztah approved these changes Oct 11, 2018

View reviewed changes

tonistiigi approved these changes Oct 11, 2018

View reviewed changes

thaJeztah mentioned this pull request Oct 11, 2018

[18.09 backport] overlay2: use index=off if possible (fix EBUSY on mount) docker-archive/engine#84

Merged

dmcgowan approved these changes Oct 12, 2018

View reviewed changes

yongtang merged commit ee6fc90 into moby:master Oct 13, 2018

kolyshkin mentioned this pull request Nov 29, 2018

Unable to docker build on Linux kernel 4.19 docker/for-linux#480

Open

3 tasks

tianon mentioned this pull request Apr 23, 2019

error creating overlay mount boot2docker/boot2docker#1388

Open

AkihiroSuda mentioned this pull request Jul 9, 2019

overlayfs failure weber-software/diuid#7

Closed

dliappis mentioned this pull request Oct 24, 2019

:test:fixtures:krb5kdc-fixture:composeUp failure elastic/elasticsearch#48027

Closed

rudyfly mentioned this pull request Jun 8, 2020

overlay: use index=off to fix EBUSY on mount containerd/containerd#4311

Merged

rdlarkins mentioned this pull request Feb 28, 2022

Allow NFS Export in Container Filesystem with overlay2 #43299

Open

corhere mentioned this pull request Jun 25, 2024

Rootless docker cannot pull images with user.overlay.* xattrs on Linux <= 6.6 - lsetxattr operation not supported #47962

Open

Conversation

kolyshkin commented Oct 8, 2018

Uh oh!

kolyshkin commented Oct 8, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kolyshkin commented Oct 9, 2018

Uh oh!

codecov Bot commented Oct 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

kolyshkin commented Oct 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

thaJeztah left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tonistiigi left a comment

Choose a reason for hiding this comment

Uh oh!

dmcgowan left a comment

Choose a reason for hiding this comment

Uh oh!

cody-c-lewis commented Feb 22, 2022

Uh oh!

amir73il commented Feb 23, 2022

Uh oh!

cody-c-lewis commented Feb 23, 2022

Uh oh!

rdlarkins commented Feb 23, 2022

Uh oh!

cpuguy83 commented Feb 23, 2022

Uh oh!

rdlarkins commented Feb 23, 2022

Uh oh!

rdlarkins commented Feb 28, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

codecov Bot commented Oct 9, 2018 •

edited

Loading