diff mbox series

[3/5] btrfs/330: add test to validate ro/rw subvol mounting

Message ID d61b8bd818a8f9403982f532ac21586f6c96269c.1710871719.git.dsterba@suse.com (mailing list archive)
State New, archived
Headers show
Series Btrfs fstests fixups and updates | expand

Commit Message

David Sterba March 19, 2024, 6:12 p.m. UTC 
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>
---
 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

Comments

Anand Jain March 20, 2024, 11:33 a.m. UTC | #1 
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
Filipe Manana March 20, 2024, 5:01 p.m. UTC | #2 
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
>
>
Anand Jain March 21, 2024, 3:51 a.m. UTC | #3 
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 mbox series

Patch

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