| Message ID | d61b8bd818a8f9403982f532ac21586f6c96269c.1710871719.git.dsterba@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Btrfs fstests fixups and updates | expand |
On 3/19/24 23:42, David Sterba wrote: > From: Josef Bacik <josef@toxicpanda.com> > > Btrfs has had the ability for almost a decade to allow ro and rw > mounting of subvols. This behavior specifically > > mount -o subvol=foo,ro /some/dir > mount -o subvol=bar,rw /some/other/dir > > This seems simple, but because of the limitations of how we did mounting > in ye olde days we would mark the super block as RO and the mount if we > mounted RO first. In the case above /some/dir would instantiate the > super block as read only and the mount point. So the second mount > command under the covers would convert the super block to RW, and then > allow the mount to continue. > > The results were still consistent, /some/dir was still read only because > the mount was marked read only, but /some/other/dir could be written to. > > This is a test to make sure we maintain this behavior, as I almost > regressed this behavior while converting us to the new mount API. > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> looks good. Reviewed-by: Anand Jain <anand.jain@oracle.com> Nits below. > --- > tests/btrfs/330 | 54 +++++++++++++++++++++++++++++++++++++++++++++ > tests/btrfs/330.out | 6 +++++ > 2 files changed, 60 insertions(+) > create mode 100755 tests/btrfs/330 > create mode 100644 tests/btrfs/330.out > > diff --git a/tests/btrfs/330 b/tests/btrfs/330 > new file mode 100755 > index 00000000000000..3ce9840e76d028 > --- /dev/null > +++ b/tests/btrfs/330 > @@ -0,0 +1,54 @@ > +#! /bin/bash > +# SPDX-License-Identifier: GPL-2.0 > +# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved. > +# > +# FS QA Test No. btrfs/330 > +# > +# Test mounting one subvolume as ro and another as rw > +# > +. ./common/preamble > +_begin_fstest auto quick subvol > + > +_cleanup() > +{ > + rm -rf $TEST_DIR/$seq > +} > + > +# Import common functions. > +. ./common/filter This can be deleted, as the filter.btrfs also calls the filter. > +. ./common/filter.btrfs > + > +# real QA test starts here > +_supported_fs btrfs > +_require_scratch > + > +$MOUNT_PROG -V | grep -q 'fd-based-mount' > +[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" > + > +_scratch_mkfs > /dev/null 2>&1 _scratch_mkfs >> $seqres.full Errors, if any, go to stdout. Thanks, Anand > +_scratch_mount > + > +# Create our subvolumes to mount > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/foo | _filter_scratch > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/bar | _filter_scratch > + > +_scratch_unmount > + > +mkdir -p $TEST_DIR/$seq/foo > +mkdir -p $TEST_DIR/$seq/bar > + > +_mount -t btrfs -o subvol=foo,ro $SCRATCH_DEV $TEST_DIR/$seq/foo > +_mount -t btrfs -o subvol=bar,rw $SCRATCH_DEV $TEST_DIR/$seq/bar > + > +echo "making sure foo is read only" > +touch $TEST_DIR/$seq/foo/baz > /dev/null 2&>1 > +ls $TEST_DIR/$seq/foo > + > +echo "making sure bar allows writes" > +touch $TEST_DIR/$seq/bar/qux > +ls $TEST_DIR/$seq/bar > + > +$UMOUNT_PROG $TEST_DIR/$seq/foo > +$UMOUNT_PROG $TEST_DIR/$seq/bar > + > +status=0 ; exit > diff --git a/tests/btrfs/330.out b/tests/btrfs/330.out > new file mode 100644 > index 00000000000000..4795a2ccc8cb62 > --- /dev/null > +++ b/tests/btrfs/330.out > @@ -0,0 +1,6 @@ > +QA output created by 330 > +Create subvolume 'SCRATCH_MNT/foo' > +Create subvolume 'SCRATCH_MNT/bar' > +making sure foo is read only > +making sure bar allows writes > +qux
On Wed, Mar 20, 2024 at 11:34 AM Anand Jain <anand.jain@oracle.com> wrote: > > On 3/19/24 23:42, David Sterba wrote: > > From: Josef Bacik <josef@toxicpanda.com> > > > > Btrfs has had the ability for almost a decade to allow ro and rw > > mounting of subvols. This behavior specifically > > > > mount -o subvol=foo,ro /some/dir > > mount -o subvol=bar,rw /some/other/dir > > > > This seems simple, but because of the limitations of how we did mounting > > in ye olde days we would mark the super block as RO and the mount if we > > mounted RO first. In the case above /some/dir would instantiate the > > super block as read only and the mount point. So the second mount > > command under the covers would convert the super block to RW, and then > > allow the mount to continue. > > > > The results were still consistent, /some/dir was still read only because > > the mount was marked read only, but /some/other/dir could be written to. > > > > This is a test to make sure we maintain this behavior, as I almost > > regressed this behavior while converting us to the new mount API. > > > > Signed-off-by: Josef Bacik <josef@toxicpanda.com> > > looks good. > > Reviewed-by: Anand Jain <anand.jain@oracle.com> > > Nits below. > > > --- > > tests/btrfs/330 | 54 +++++++++++++++++++++++++++++++++++++++++++++ > > tests/btrfs/330.out | 6 +++++ > > 2 files changed, 60 insertions(+) > > create mode 100755 tests/btrfs/330 > > create mode 100644 tests/btrfs/330.out > > > > diff --git a/tests/btrfs/330 b/tests/btrfs/330 > > new file mode 100755 > > index 00000000000000..3ce9840e76d028 > > --- /dev/null > > +++ b/tests/btrfs/330 > > @@ -0,0 +1,54 @@ > > +#! /bin/bash > > +# SPDX-License-Identifier: GPL-2.0 > > +# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved. > > +# > > +# FS QA Test No. btrfs/330 > > +# > > +# Test mounting one subvolume as ro and another as rw > > +# > > +. ./common/preamble > > +_begin_fstest auto quick subvol > > + > > +_cleanup() > > +{ > > + rm -rf $TEST_DIR/$seq > > +} > > + > > +# Import common functions. > > > +. ./common/filter > > This can be deleted, as the filter.btrfs also calls the filter. > > > +. ./common/filter.btrfs > > > > + > > +# real QA test starts here > > +_supported_fs btrfs > > +_require_scratch > > + > > +$MOUNT_PROG -V | grep -q 'fd-based-mount' > > +[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" > > + > > +_scratch_mkfs > /dev/null 2>&1 > > _scratch_mkfs >> $seqres.full > > Errors, if any, go to stdout. We typically redirect stderr to stdout because in the past mkfs.btrfs used to output to stderr a message when it performed trim. See this old commit: https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=afadb6e5958c5acf2425d6a8a9372b63afcb4f2a And nowadays we're encouraged to do: _scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" So in case mkfs fails the test doesn't continue and silently passes by using the filesystem SCRATCH_MNT belongs to. This has been discussed and pointed out several times, but I don't have a link to such discussion. > > Thanks, Anand > > > +_scratch_mount > > + > > +# Create our subvolumes to mount > > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/foo | _filter_scratch > > +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/bar | _filter_scratch > > + > > +_scratch_unmount > > + > > +mkdir -p $TEST_DIR/$seq/foo > > +mkdir -p $TEST_DIR/$seq/bar > > + > > +_mount -t btrfs -o subvol=foo,ro $SCRATCH_DEV $TEST_DIR/$seq/foo > > +_mount -t btrfs -o subvol=bar,rw $SCRATCH_DEV $TEST_DIR/$seq/bar > > + > > +echo "making sure foo is read only" > > +touch $TEST_DIR/$seq/foo/baz > /dev/null 2&>1 > > +ls $TEST_DIR/$seq/foo > > + > > +echo "making sure bar allows writes" > > +touch $TEST_DIR/$seq/bar/qux > > +ls $TEST_DIR/$seq/bar > > + > > +$UMOUNT_PROG $TEST_DIR/$seq/foo > > +$UMOUNT_PROG $TEST_DIR/$seq/bar > > + > > +status=0 ; exit > > diff --git a/tests/btrfs/330.out b/tests/btrfs/330.out > > new file mode 100644 > > index 00000000000000..4795a2ccc8cb62 > > --- /dev/null > > +++ b/tests/btrfs/330.out > > @@ -0,0 +1,6 @@ > > +QA output created by 330 > > +Create subvolume 'SCRATCH_MNT/foo' > > +Create subvolume 'SCRATCH_MNT/bar' > > +making sure foo is read only > > +making sure bar allows writes > > +qux > >
On 3/20/24 22:31, Filipe Manana wrote: > On Wed, Mar 20, 2024 at 11:34 AM Anand Jain <anand.jain@oracle.com> wrote: >> >> On 3/19/24 23:42, David Sterba wrote: >>> From: Josef Bacik <josef@toxicpanda.com> >>> >>> Btrfs has had the ability for almost a decade to allow ro and rw >>> mounting of subvols. This behavior specifically >>> >>> mount -o subvol=foo,ro /some/dir >>> mount -o subvol=bar,rw /some/other/dir >>> >>> This seems simple, but because of the limitations of how we did mounting >>> in ye olde days we would mark the super block as RO and the mount if we >>> mounted RO first. In the case above /some/dir would instantiate the >>> super block as read only and the mount point. So the second mount >>> command under the covers would convert the super block to RW, and then >>> allow the mount to continue. >>> >>> The results were still consistent, /some/dir was still read only because >>> the mount was marked read only, but /some/other/dir could be written to. >>> >>> This is a test to make sure we maintain this behavior, as I almost >>> regressed this behavior while converting us to the new mount API. >>> >>> Signed-off-by: Josef Bacik <josef@toxicpanda.com> >> >> looks good. >> >> Reviewed-by: Anand Jain <anand.jain@oracle.com> >> >> Nits below. >> >>> --- >>> tests/btrfs/330 | 54 +++++++++++++++++++++++++++++++++++++++++++++ >>> tests/btrfs/330.out | 6 +++++ >>> 2 files changed, 60 insertions(+) >>> create mode 100755 tests/btrfs/330 >>> create mode 100644 tests/btrfs/330.out >>> >>> diff --git a/tests/btrfs/330 b/tests/btrfs/330 >>> new file mode 100755 >>> index 00000000000000..3ce9840e76d028 >>> --- /dev/null >>> +++ b/tests/btrfs/330 >>> @@ -0,0 +1,54 @@ >>> +#! /bin/bash >>> +# SPDX-License-Identifier: GPL-2.0 >>> +# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved. >>> +# >>> +# FS QA Test No. btrfs/330 >>> +# >>> +# Test mounting one subvolume as ro and another as rw >>> +# >>> +. ./common/preamble >>> +_begin_fstest auto quick subvol >>> + >>> +_cleanup() >>> +{ >>> + rm -rf $TEST_DIR/$seq >>> +} >>> + >>> +# Import common functions. >> >>> +. ./common/filter >> >> This can be deleted, as the filter.btrfs also calls the filter. >> >>> +. ./common/filter.btrfs >> >> >>> + >>> +# real QA test starts here >>> +_supported_fs btrfs >>> +_require_scratch >>> + >>> +$MOUNT_PROG -V | grep -q 'fd-based-mount' >>> +[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" >>> + >>> +_scratch_mkfs > /dev/null 2>&1 >> >> _scratch_mkfs >> $seqres.full >> >> Errors, if any, go to stdout. > > We typically redirect stderr to stdout because in the past mkfs.btrfs > used to output to stderr a message when it performed trim. > See this old commit: > > https://git.kernel.org/pub/scm/fs/xfs/xfstests-dev.git/commit/?id=afadb6e5958c5acf2425d6a8a9372b63afcb4f2a > > And nowadays we're encouraged to do: > > _scratch_mkfs >> $seqres.full 2>&1 || _fail "mkfs failed" > > So in case mkfs fails the test doesn't continue and silently passes by > using the filesystem SCRATCH_MNT belongs to. Darn it, I keep forgetting about the trimmed message history that went to stderr. My bad. And I did search mkfs for what went to stderr, but it wasn't an error. Btrfs/303 fixed. Thanks, Anand
diff --git a/tests/btrfs/330 b/tests/btrfs/330 new file mode 100755 index 00000000000000..3ce9840e76d028 --- /dev/null +++ b/tests/btrfs/330 @@ -0,0 +1,54 @@ +#! /bin/bash +# SPDX-License-Identifier: GPL-2.0 +# Copyright (c) 2023 Meta Platforms, Inc. All Rights Reserved. +# +# FS QA Test No. btrfs/330 +# +# Test mounting one subvolume as ro and another as rw +# +. ./common/preamble +_begin_fstest auto quick subvol + +_cleanup() +{ + rm -rf $TEST_DIR/$seq +} + +# Import common functions. +. ./common/filter +. ./common/filter.btrfs + +# real QA test starts here +_supported_fs btrfs +_require_scratch + +$MOUNT_PROG -V | grep -q 'fd-based-mount' +[ "$?" -eq 0 ] && _notrun "mount uses the new mount api" + +_scratch_mkfs > /dev/null 2>&1 +_scratch_mount + +# Create our subvolumes to mount +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/foo | _filter_scratch +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/bar | _filter_scratch + +_scratch_unmount + +mkdir -p $TEST_DIR/$seq/foo +mkdir -p $TEST_DIR/$seq/bar + +_mount -t btrfs -o subvol=foo,ro $SCRATCH_DEV $TEST_DIR/$seq/foo +_mount -t btrfs -o subvol=bar,rw $SCRATCH_DEV $TEST_DIR/$seq/bar + +echo "making sure foo is read only" +touch $TEST_DIR/$seq/foo/baz > /dev/null 2&>1 +ls $TEST_DIR/$seq/foo + +echo "making sure bar allows writes" +touch $TEST_DIR/$seq/bar/qux +ls $TEST_DIR/$seq/bar + +$UMOUNT_PROG $TEST_DIR/$seq/foo +$UMOUNT_PROG $TEST_DIR/$seq/bar + +status=0 ; exit diff --git a/tests/btrfs/330.out b/tests/btrfs/330.out new file mode 100644 index 00000000000000..4795a2ccc8cb62 --- /dev/null +++ b/tests/btrfs/330.out @@ -0,0 +1,6 @@ +QA output created by 330 +Create subvolume 'SCRATCH_MNT/foo' +Create subvolume 'SCRATCH_MNT/bar' +making sure foo is read only +making sure bar allows writes +qux