On Wed, May 28, 2025 at 08:54:51AM +0200, Amir Goldstein wrote:
> On Tue, May 27, 2025 at 4:49 PM Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> >
> > On Mon, 26 May 2025 at 16:35, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
> > >
> > > libmount v1.41 calls several unneeded fsconfig() calls to reconfigure
> > > lowerdir/upperdir when user requests only -o remount,ro.
> >
> > Isn't this a libmount bug then?
It's not just "only -o remount,ro"; the mount manual is quite explicit about it:
The remount functionality follows the standard way the mount command works with
options from fstab. This means that mount does not read fstab (or mtab) only when
both device and dir are specified.
mount -o remount,rw /dev/foo /dir
After this call all old mount options are replaced and arbitrary stuff from fstab
(or mtab) is ignored, except the loop= option which is internally generated and
maintained by the mount command.
mount -o remount,rw /dir
After this call, mount reads fstab and merges these options with the options from
the command line (-o). If no mountpoint is found in fstab, then it defaults to
mount options from /proc/self/mountinfo.
...
--options-mode mode
Controls how to combine options from fstab/mtab with options from the command line.
mode can be one of ignore, append, prepend or replace. For example, append means
that options from fstab are appended to options from the command line. The default
value is prepend — it means command line options are evaluated after fstab options.
Note that the last option wins if there are conflicting ones.
Anyway, I agree that this semantics sucks, and from my point of view,
the best approach would be to introduce a new mount(8) command line
semantics to reflect the new kernel API, something like:
mount modify [--clear noexec] [--set nodev,ro] [--make-private] [--recursive] /mnt
mount reconfigure data=journal,errors=continue,foo,bar /mnt
and do not include options from fstab in this by default.
> > Working around it in xfstests just hides this, which seems counter productive.
> >
>
> Yes, to some extent, however, IMO the purpose of fstests is to test the
> filesystem and the vfs and for very fs-specific tests it also tests the
> *progs utils.
>
> I do not think we can afford fstest being a test for libc+libmount and the
> entire ecosystem. That's part of the reason that fstest implements
> some of its own utilities to exercise syscalls.
>
> If we leave the tests failing, we will loose test coverage from all
> the people that are running with not-cutting-edge distro.
> I don't think this is a desired outcome for us.
> Test coverage for remount,ro is pretty important IMO.
>
> FWIW, we already used LIBMOUNT_FORCE_MOUNT2 to workaround
> another libmount bug I believe you were in the loop when we did that
> (see blow).
>
> Any other idea how to address those libmount bugs in the test suite
> other than keeping the tests failing or not running for libmount >= v1.39?
Why do you think it's a libmount bug? Do we really expect that
userspace will parse filesystem-specific mount options and "if
(overlayfs)" then it will exclude some options from fsconfig()?
# LIBMOUNT_FORCE_MOUNT2=always strace -e mount mount -o remount,ro /mnt/merged
mount("overlay", "/mnt/merged", 0x561d36355030, MS_RDONLY|MS_REMOUNT, "lowerdir=/mnt/low,upperdir=/mnt/"... ) = 0
# strace -e fsconfig mount -o remount,ro /mnt/merged
fsconfig(4, FSCONFIG_SET_STRING, "lowerdir", "/mnt/low", 0) = -1 EINVAL (Invalid argument)
As you can see, mount(2) is absolutely fine with lowerdir=. Why not fsconfig()?
I think the way libmount uses fsconfig() is exactly the same as how it
uses the classic mount(2), just sending the mount options to the
kernel. The difference is only that for the new mount kernel API, we
differentiate between filesystem options (fsconfig()) and VFS
attributes (mount_setattr()).
Would it be better to ignore attempts to reconfigure the
lowerdir/upperdir in the kernel?
And example if you suppress fstab/mountinfo:
# strace -e fsconfig mount --options-mode ignore -o remount,ro /mnt/merged
fsconfig(4, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0
fsconfig(4, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0) = 0
or if you specify both, source and target:
# strace -e fsconfig mount -o remount,ro overlay /mnt/merged
fsconfig(4, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0
fsconfig(4, FSCONFIG_CMD_RECONFIGURE, NULL, NULL, 0) = 0
So, you do not need LIBMOUNT_FORCE_MOUNT2= workaround, use
"--options-mode ignore" or source and target ;-)
Karel
--
Karel Zak <kzak@xxxxxxxxxx>
http://karelzak.blogspot.com
