Re: [REGRESSION] Return change in 6.12.80+ with volatile mounting
Amir Goldstein <[email protected]>
| Newsgroups | org.kernel.vger.linux-unionfs,dev.linux.lists.regressions,org.kernel.vger.stable |
|---|---|
| Message-ID | <CAOQ4uxhCNhGinePrnkSfT9Mtf4o5FmBX7mTA2m4miCMOt3mJqA@mail.gmail.com> |
On Mon, Apr 20, 2026 at 8:31 AM Chenglong Tang <[email protected]> wrote: > > Hi, Amir, > > Thanks for looking into this! To answer your questions: > > 1. Production vs. Test Suite Impact > > The immediate failure we encountered is in containerd's integration > test suite (TestImageVolumeCheckVolatileOption). The test explicitly > reads /proc/mounts and expects the exact string "volatile". > > In default production, containerd passes the legacy "volatile" string > to the mount syscall, which your patch correctly handles under the > hood. So the standard "happy path" is not broken in production. > > 2. The purpose of WithTempMount() / RemoveVolatileOption > > Containerd regularly makes temporary overlay mounts (e.g., for > unpacking layers). Because overlayfs rejects reusing upper/work dirs > from a volatile mount, containerd uses RemoveVolatileOption to strip > the volatile flag before these temporary mounts. > > Currently, containerd's RemoveVolatileOption does an exact string > match for "volatile". While it works for the default path, there is a > production edge case: if a user explicitly configures their container > runtime to use the new "fsync=volatile" option, older containerd > binaries will fail to strip it, and the temporary mounts will be > rejected by the kernel. I need to challenge this specific argument because I do not agree that this edge case could be considered a regression at all. An admin from the past could not have set an explicit "fsync=volatile" mount option. Though experiment - overlayfs adds a new mode fsync=off which is more loose than "volatile" because "volatile" can actually return error on fsync in some cases. Overlayfs would also not allow to reuse workdir from such a mount. So would you then claim that adding the new mount option "fsync=off" is a regression because of the edge case that an admin decided to explicitly add "fsync=off" and RemoveVolatileOption() does not handle it. I don't buy it. There is a more correct way to handle this situation and it is documented in overlayfs.rst: "When overlay is mounted with "volatile" option, the directory "$workdir/work/incompat/volatile" is created. During next mount, overlay checks for this directory and refuses to mount if present. This is a strong indicator that the user should discard upper and work directories and create fresh ones. In very limited cases where the user knows that the system has not crashed and contents of upperdir are intact, the "volatile" directory can be removed." containerd unpacking layers falls under this very limited case. containerd can syncfs() workdir after unpack & unmount of temp overlayfs and remove the "incompat/volatile" directory and then the upperdir/workdir are are free to be remounted. > > Conclusion > > While containerd could theoretically patch their code to accept > strings.Contains() or fsync=volatile going forward, there are many Following the documentation advise would be better. > existing containerd binaries in the wild. Given that this patch breaks > containerd's CI tests and introduces an edge case for > RemoveVolatileOption, it might be safest to fix ovl_show_options in > the kernel to continue outputting the legacy "volatile" string to > strictly guarantee backwards compatibility with userspace. > Considering my comments above, I think we have not yet reached the point where the backward compat "volatile" string is called for, but with other reports from production workloads, we could get there. Thanks for the clear and honest explanations of the containerd situation! Amir.