| Message ID | 65378a46dd8e00ddc6a485335a5ac43cfe7aba3b.1729764240.git.wqu@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | fstests: btrfs/330: enable the test case for both new and old APIs | expand |
On 24/10/24 18:05, Qu Wenruo wrote: > [BUG] > If the mount tool is utilizing the new fs-based API > (e.g. util-linux 2.40.2 from Archlinux), btrfs' per-subvolume RO/RW mount > is broken again: > > # mount -o subvol=subv1,ro /dev/test/scratch1 /mnt/test > # mount -o rw,subvol=subv2 /dev/test/scratch1 /mnt/scratch > # mount | grep mnt > /dev/mapper/test-scratch1 on /mnt/test type btrfs (ro,relatime,discard=async,space_cache=v2,subvolid=256,subvol=/subv1) > /dev/mapper/test-scratch1 on /mnt/scratch type btrfs (ro,relatime,discard=async,space_cache=v2,subvolid=257,subvol=/subv2) > # touch /mnt/scratch/foobar > touch: cannot touch '/mnt/scratch/foobar': Read-only file system > > [CAUSE] > Btrfs has an extra remount hack to handle above case, which will > re-configure the super block to be RW on the first RW mount. > > The initial promise is, the new fd-based API will not set ro FLAG, but > only MOUNT_ATTR_RDONLY, so that btrfs will skip the remount hack for new > API based mount request. > > However it's not the case, the first RO subvolume mount will set ro flag > at fsconfig(), and also set MOUNT_ATTR_RDONLY attribute for the mount > point: > > # strace mount -o subvol=subv1,ro /dev/test/scratch1 /mnt/test/ > ... > fsconfig(3, FSCONFIG_SET_STRING, "source", "/dev/mapper/test-scratch1", 0) = 0 > fsconfig(3, FSCONFIG_SET_STRING, "subvol", "subv1", 0) = 0 > fsconfig(3, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0 > fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = 0 > fsmount(3, FSMOUNT_CLOEXEC, 0) = 4 > mount_setattr(4, "", AT_EMPTY_PATH, {attr_set=MOUNT_ATTR_RDONLY, attr_clr=0, propagation=0 /* MS_??? */, userns_fd=0}, 32) = 0 > move_mount(4, "", AT_FDCWD, "/mnt/test", MOVE_MOUNT_F_EMPTY_PATH) = 0 > > This will result exactly the same behavior, no matter if it's the new > API or not. > > Furthermore we can even have corner cases like mounting the initial RO > subvolume using the old API, then mount the RW subvolume using the new > API. > > So even using the new API, there is no guarantee to keep the > per-subvolume RO/RW mount feature. > We have to do the reconfigure anyway. > > [FIX] > The kernel fix is already submitted, but for the test case part, we > should enable btrfs/330 for all mount tools, no matter the API it > utilizes. > > The only difference for the new API based mount is the new > _fixed_by_kernel_commit call, to show the proper fix. > > Now it can properly detects the broken feature. > > Signed-off-by: Qu Wenruo <wqu@suse.com> > --- > tests/btrfs/330 | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/tests/btrfs/330 b/tests/btrfs/330 > index 3545a116ecea..92cc498f2350 100755 > --- a/tests/btrfs/330 > +++ b/tests/btrfs/330 > @@ -17,10 +17,13 @@ _cleanup() > # Import common functions. > . ./common/filter.btrfs > > -_require_scratch > - > $MOUNT_PROG -V | grep -q 'fd-based-mount' > -[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" > +if [ "$?" -eq 0 ]; then > + _fixed_by_kernel_commit xxxxxxxxxxxx \ > + "btrfs: fix per-subvolume RO/RW flags with new mount API" > +fi > + > +_require_scratch > > _scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" > _scratch_mount Reviewed-by: Anand Jain <anand.jain@oracle.com> Thx. Anand
diff --git a/tests/btrfs/330 b/tests/btrfs/330 index 3545a116ecea..92cc498f2350 100755 --- a/tests/btrfs/330 +++ b/tests/btrfs/330 @@ -17,10 +17,13 @@ _cleanup() # Import common functions. . ./common/filter.btrfs -_require_scratch - $MOUNT_PROG -V | grep -q 'fd-based-mount' -[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" +if [ "$?" -eq 0 ]; then + _fixed_by_kernel_commit xxxxxxxxxxxx \ + "btrfs: fix per-subvolume RO/RW flags with new mount API" +fi + +_require_scratch _scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" _scratch_mount
[BUG] If the mount tool is utilizing the new fs-based API (e.g. util-linux 2.40.2 from Archlinux), btrfs' per-subvolume RO/RW mount is broken again: # mount -o subvol=subv1,ro /dev/test/scratch1 /mnt/test # mount -o rw,subvol=subv2 /dev/test/scratch1 /mnt/scratch # mount | grep mnt /dev/mapper/test-scratch1 on /mnt/test type btrfs (ro,relatime,discard=async,space_cache=v2,subvolid=256,subvol=/subv1) /dev/mapper/test-scratch1 on /mnt/scratch type btrfs (ro,relatime,discard=async,space_cache=v2,subvolid=257,subvol=/subv2) # touch /mnt/scratch/foobar touch: cannot touch '/mnt/scratch/foobar': Read-only file system [CAUSE] Btrfs has an extra remount hack to handle above case, which will re-configure the super block to be RW on the first RW mount. The initial promise is, the new fd-based API will not set ro FLAG, but only MOUNT_ATTR_RDONLY, so that btrfs will skip the remount hack for new API based mount request. However it's not the case, the first RO subvolume mount will set ro flag at fsconfig(), and also set MOUNT_ATTR_RDONLY attribute for the mount point: # strace mount -o subvol=subv1,ro /dev/test/scratch1 /mnt/test/ ... fsconfig(3, FSCONFIG_SET_STRING, "source", "/dev/mapper/test-scratch1", 0) = 0 fsconfig(3, FSCONFIG_SET_STRING, "subvol", "subv1", 0) = 0 fsconfig(3, FSCONFIG_SET_FLAG, "ro", NULL, 0) = 0 fsconfig(3, FSCONFIG_CMD_CREATE, NULL, NULL, 0) = 0 fsmount(3, FSMOUNT_CLOEXEC, 0) = 4 mount_setattr(4, "", AT_EMPTY_PATH, {attr_set=MOUNT_ATTR_RDONLY, attr_clr=0, propagation=0 /* MS_??? */, userns_fd=0}, 32) = 0 move_mount(4, "", AT_FDCWD, "/mnt/test", MOVE_MOUNT_F_EMPTY_PATH) = 0 This will result exactly the same behavior, no matter if it's the new API or not. Furthermore we can even have corner cases like mounting the initial RO subvolume using the old API, then mount the RW subvolume using the new API. So even using the new API, there is no guarantee to keep the per-subvolume RO/RW mount feature. We have to do the reconfigure anyway. [FIX] The kernel fix is already submitted, but for the test case part, we should enable btrfs/330 for all mount tools, no matter the API it utilizes. The only difference for the new API based mount is the new _fixed_by_kernel_commit call, to show the proper fix. Now it can properly detects the broken feature. Signed-off-by: Qu Wenruo <wqu@suse.com> --- tests/btrfs/330 | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-)