Use naive diff for overlay2 when opaque copy up bug present#28138
Use naive diff for overlay2 when opaque copy up bug present#28138cpuguy83 merged 1 commit intomoby:masterfrom
Conversation
tonistiigi
left a comment
tonistiigi
left a comment
There was a problem hiding this comment.
Would be good to add a test for #25244
There was a problem hiding this comment.
We could do this like https://github.com/docker/docker/blob/master/daemon/graphdriver/aufs/aufs.go#L643 with sync.Once and lazy loading. Kernel check could probably be skipped then.
There was a problem hiding this comment.
Why should we skip the kernel check though? The fix was merged before 4.8 was released so we can be confident that 4.8 or later have the fixed. Kernel tests were also added around this issue to prevent regression in later versions. Ideally users on newer Kernels won't have to incur the additional startup time required for this check.
There was a problem hiding this comment.
We can keep that check if you like it, although kernel versions are abused all the time and it is hard to predict who will decide to release something with a wrong version. With lazy loading, the additional time for full check should be very small.
There was a problem hiding this comment.
Updated to use sync.Once, not every workflow will need to diff so it makes sense to do lazily.
There was a problem hiding this comment.
How about printing something like "Please consider updating the kernel to 4.8.0 or later" here
There was a problem hiding this comment.
windows fails to compile during UT:
https://jenkins.dockerproject.org/job/Docker-PRs-WoW-RS1/5763/console
00:09:14.608 # github.com/docker/docker/daemon/graphdriver/overlay2
00:09:14.609 daemon\graphdriver\overlay2\check.go:53: undefined: syscall.Mount
00:09:14.609 daemon\graphdriver\overlay2\check.go:57: undefined: syscall.Unmount
Please add the build tag?
icecrime
added
the
status/failing-ci
dmcgowan
force-pushed
the
handle-overlay2-copy-up-bug
branch
from
38ee5c0 to
8985860
Compare
|
@AkihiroSuda added build tag, updated the error to mention updating to fix, and also added to the status output (if this merges first you will need to rebase your other PR). |
|
@dmcgowan Need to mark that test for linux as well. |
dmcgowan
force-pushed
the
handle-overlay2-copy-up-bug
branch
from
8985860 to
bc430aa
Compare
tonistiigi
removed
the
status/failing-ci
When running on a kernel which is not patched for the copy up bug overlay2 will use the naive diff driver. Signed-off-by: Derek McGowan <[email protected]> (github: dmcgowan)
dmcgowan
force-pushed
the
handle-overlay2-copy-up-bug
branch
from
bc430aa to
64b43ed
Compare
|
LGTM |
1 similar comment
|
LGTM |
|
Am I correct to read that this patch means that for kernels without backports yet (RHEL 7), overlay2 will now have same issues with inode exhaustion as overlay in 1.13? If so, the documentation (https://docs.docker.com/engine/userguide/storagedriver/overlayfs-driver/) should be updated to reflect that the overlay2 benefits do not apply to kernels older than 4.8 without backports, even though it will work. |
|
@tristanz No, this only changes the diff algorithm, making |
|
@tonistiigi @dmcgowan thanks! Reading #21555 it looks like this is no longer critical bug after #27209 mitigation. With this PR and #27209 are there any major issues with overlay2 on RHEL 7.2+ beyond SELinux not working yet? |
When running on a kernel which is not patched for the copy up bug overlay2 will use the naive diff driver. cherry-pick from moby#28138 and backport some code from moby#22641 Signed-off-by: Derek McGowan <[email protected]> (github: dmcgowan) Signed-off-by: Lei Jitang <[email protected]>
9 participants
- What I did
Update overlay2 driver to use naive diff when kernel has bug which copies up opaque directory flag
- How I did it
Added a check on kernels older than 4.8 for the bug.
- How to verify it
Run this Dockerfile on kernel without patched fix and should work
Closes #25244
... for all kernels which support overlay2
Ping @tonistiigi