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

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

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