Re: [REGRESSION] Return change in 6.12.80+ with volatile mounting

CC: Samnuel from containerd

On Mon, Apr 20, 2026 at 12:59 AM Amir Goldstein <[email protected]> wrote:
>
> 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.