Re: [PATCH] ovl: Allow mount options to be parsed on remount

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Em 22/05/2025 06:52, Amir Goldstein escreveu:
On Thu, May 22, 2025 at 8:20 AM André Almeida <andrealmeid@xxxxxxxxxx> wrote:

Hi Christian, Amir,

Thanks for the feedback :)

Em 21/05/2025 08:20, Christian Brauner escreveu:
On Wed, May 21, 2025 at 12:35:57PM +0200, Amir Goldstein wrote:
On Wed, May 21, 2025 at 8:45 AM André Almeida <andrealmeid@xxxxxxxxxx> wrote:


[...]


I see the test generic/623 failure - this test needs to be fixed for overlay
or not run on overlayfs.

I do not see those other 5 failures although before running the test I did:
export LIBMOUNT_FORCE_MOUNT2=always

Not sure what I am doing differently.


I have created a smaller reproducer for this, have a look:

   mkdir -p ovl/lower ovl/upper ovl/merge ovl/work ovl/mnt
   sudo mount -t overlay overlay -o lowerdir=ovl/lower,upperdir=ovl/
upper,workdir=ovl/work ovl/mnt
   sudo mount ovl/mnt -o remount,ro


Why would you use this command?
Why would you want to re-specify the lower/upperdir when remounting ro?
And more specifically, fstests does not use this command in the tests
that you mention that they fail, so what am I missing?


I've added "set -x" to tests/generic/294 to see exactly which mount parameters were being used and I got this from the output:

+ _try_scratch_mount -o remount,ro
+ local mount_ret
+ '[' overlay == overlay ']'
+ _overlay_scratch_mount -o remount,ro
+ echo '-o remount,ro'
+ grep -q remount
+ /usr/bin/mount /tmp/dir2/ovl-mnt -o remount,ro
mount: /tmp/dir2/ovl-mnt: fsconfig() failed: ...

So, from what I can see, fstests is using this command. Not sure if I did something wrong when setting up fstests.

And this returns:

   mount: /tmp/ovl/mnt: fsconfig() failed: overlay: No changes allowed in
   reconfigure.
         dmesg(1) may have more information after failed mount system call.

However, when I use mount like this:

   sudo mount -t overlay overlay -o remount,ro ovl/mnt

mount succeeds. Having a look at strace, I found out that the first
mount command tries to set lowerdir to "ovl/lower" again, which will to
return -EINVAL from ovl_parse_param():

     fspick(3, "", FSPICK_NO_AUTOMOUNT|FSPICK_EMPTY_PATH) = 4
     fsconfig(4, FSCONFIG_SET_STRING, "lowerdir", "/tmp/ovl/lower", 0) =
-1 EINVAL (Invalid argument)

Now, the second mount command sets just the "ro" flag, which will return
after vfs_parse_sb_flag(), before getting to ovl_parse_param():

     fspick(3, "", FSPICK_NO_AUTOMOUNT|FSPICK_EMPTY_PATH) = 4
     fsconfig(4, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0

After applying my patch and running the first mount command again, we
can set that this flag is set only after setting all the strings:

     fsconfig(4, FSCONFIG_SET_STRING, "lowerdir", "/tmp/ovl/lower", 0) = 0
     fsconfig(4, FSCONFIG_SET_STRING, "upperdir", "/tmp/ovl/upper", 0) = 0
     fsconfig(4, FSCONFIG_SET_STRING, "workdir", "/tmp/ovl/work", 0) = 0
     fsconfig(4, FSCONFIG_SET_STRING, "uuid", "on", 0) = 0
     fsconfig(4, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0

I understood that the patch that I proposed is wrong, and now I wonder
if the kernel needs to be fixed at all, or if the bug is how mount is
using fsconfig() in the first mount command?

Maybe I am not reading your report correctly, but as this commands works:

mount -t overlay overlay -o remount,ro ovl/mnt

and the fstests that call _scratch_remount() work
I don't think there is anything to fix and I do not understand
what is the complaint.

Thanks,
Amir.





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux