On Wed, Oct 30, 2024 at 10:48:15PM -0500, Eric Sandeen wrote:
> On 10/28/24 7:22 AM, Christian Brauner wrote:
> > On Sun, Oct 27, 2024 at 02:07:41AM +0800, Zorro Lang wrote:
> >> Hi,
> >>
> >> Recently, I hit lots of fstests cases fail on overlayfs (xfs underlying, no
> >> specific mount options), e.g.
> >>
> >> FSTYP -- overlay
> >> PLATFORM -- Linux/s390x s390x-xxxx 6.12.0-rc4+ #1 SMP Fri Oct 25 14:29:18 EDT 2024
> >> MKFS_OPTIONS -- -m crc=1,finobt=1,rmapbt=0,reflink=1,inobtcount=1,bigtime=1 /mnt/fstests/SCRATCH_DIR
> >> MOUNT_OPTIONS -- -o context=system_u:object_r:root_t:s0 /mnt/fstests/SCRATCH_DIR /mnt/fstests/SCRATCH_DIR/ovl-mnt
> >>
> >> generic/294 [failed, exit status 1]- output mismatch (see /var/lib/xfstests/results//generic/294.out.bad)
> >> --- tests/generic/294.out 2024-10-25 14:38:32.098692473 -0400
> >> +++ /var/lib/xfstests/results//generic/294.out.bad 2024-10-25 15:02:34.698605062 -0400
> >> @@ -1,5 +1,5 @@
> >> QA output created by 294
> >> -mknod: SCRATCH_MNT/294.test/testnode: File exists
> >> -mkdir: cannot create directory 'SCRATCH_MNT/294.test/testdir': File exists
> >> -touch: cannot touch 'SCRATCH_MNT/294.test/testtarget': Read-only file system
> >> -ln: creating symbolic link 'SCRATCH_MNT/294.test/testlink': File exists
> >> +mount: /mnt/fstests/SCRATCH_DIR/ovl-mnt: fsconfig system call failed: overlay: No changes allowed in reconfigure.
> >> + dmesg(1) may have more information after failed mount system call.
> >
> > In the new mount api overlayfs has been changed to reject invalid mount
> > option on remount whereas in the old mount api we just igorned them.
> > If this a big problem then we need to change overlayfs to continue
> > ignoring garbage mount options passed to it during remount.
> >
>
> It fails on /any/ overlayfs-specific options during reconfigure, invalid or
> not, right?
Yes.
>
> if (fc->purpose == FS_CONTEXT_FOR_RECONFIGURE) {
> /*
> * On remount overlayfs has always ignored all mount
> * options no matter if malformed or not so for
> * backwards compatibility we do the same here.
> */
> if (fc->oldapi)
> return 0;
>
> /*
> * Give us the freedom to allow changing mount options
> * with the new mount api in the future. So instead of
> * silently ignoring everything we report a proper
> * error. This is only visible for users of the new
> * mount api.
> */
> return invalfc(fc, "No changes allowed in reconfigure");
> }
>
> opt = fs_parse(fc, ovl_parameter_spec, param, &result);
> if (opt < 0)
> return opt;
>
> And because today mount(8) will re-specify everything it finds in
> /proc/mounts during remount, presumably that's why all these tests are
> failing - even a simple remount,ro will fail:
The easiest way is to just fallback to the old behavior and continue to
ignore unknown mount options. IOW, ignore my change.
>
> # mount -t overlay overlay -o lowerdir=lower,upperdir=upper,workdir=work merged
> # strace -e fsconfig mount -o remount,ro merged
> fsconfig(4, FSCONFIG_SET_FLAG, "seclabel", NULL, 0) = 0
> fsconfig(4, FSCONFIG_SET_STRING, "lowerdir", "lower", 0) = -1 EINVAL (Invalid argument)
> ...
>
> Surely mount -o remount,ro should continue to work for overlayfs when the new
> API is used.
>
> Maybe there's a third way: accept remount options as long as they match
> current options, but fail if they try to modify anything? Not sure how tricky
> that would be.
>
> (side note: it's a bit worrisome that there is probably no consistency at
> all across filesystems, here.)
One option would be to add a fsconfig() flag that enforces strict
remount behavior if the filesystem supports it. So it's would become an
opt-in thing.
