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

[Anand Jain <anand.jain@oracle.com>]

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