Re: [PATCH] btrfs: regression test for subvol deletion after rename

[Nikolay Borisov <nborisov@suse.com>]

On 19.12.19 г. 23:16 ч., Josef Bacik wrote:
> On 12/19/19 11:12 AM, Nikolay Borisov wrote:
>>
>>
>> On 19.12.19 г. 16:28 ч., Josef Bacik wrote:
>>> Test removal of a subvolume via rmdir after it has been renamed into a
>>> snapshot of the volume that originally contained the subvolume
>>> reference.
>>>
>>> This currently fails on btrfs but is fixed by the patch with the title
>>>
>>>    "btrfs: fix invalid removal of root ref"
>>>
>>> Signed-off-by: Josef Bacik <josef@toxicpanda.com>
>>> ---
>>>   tests/btrfs/202     | 54 +++++++++++++++++++++++++++++++++++++++++++++
>>>   tests/btrfs/202.out |  4 ++++
>>>   tests/btrfs/group   |  1 +
>>>   3 files changed, 59 insertions(+)
>>>   create mode 100755 tests/btrfs/202
>>>   create mode 100644 tests/btrfs/202.out
>>>
>>> diff --git a/tests/btrfs/202 b/tests/btrfs/202
>>> new file mode 100755
>>> index 00000000..b02ee446
>>> --- /dev/null
>>> +++ b/tests/btrfs/202
>>> @@ -0,0 +1,54 @@
>>> +#! /bin/bash
>>> +# SPDX-License-Identifier: GPL-2.0
>>> +#
>>> +# FS QA Test 201
>>> +#
>>> +# Regression test for fix "btrfs: fix invalid removal of root ref"
>>> +#
>>> +seq=`basename $0`
>>> +seqres=$RESULT_DIR/$seq
>>> +echo "QA output created by $seq"
>>> +
>>> +here=`pwd`
>>> +tmp=/tmp/$$
>>> +status=1    # failure is the default!
>>> +trap "_cleanup; exit \$status" 0 1 2 3 15
>>> +
>>> +_cleanup()
>>> +{
>>> +    cd /
>>> +    rm -f $tmp.*
>>> +}
>>> +
>>> +. ./common/rc
>>> +. ./common/filter
>>> +
>>> +rm -f $seqres.full
>>> +
>>> +_supported_fs btrfs
>>> +_supported_os Linux
>>> +
>>> +_scratch_mkfs >> $seqres.full 2>&1
>>> +_scratch_mount
>>> +
>>> +# Create a subvol b under a and then snapshot a into c.  This 
>>> create's a stub
>>> +# entry in c for b because c doesn't have a reference for b.
>>> +#
>>> +# But when we rename b c/foo it creates a ref for b in c.  However 
>>> if we go to
>>> +# remove c/b btrfs used to depend on not finding the root ref to 
>>> handle the
>>> +# unlink properly, but we now have a ref for that root.  We also had 
>>> a bug that
>>> +# would allow us to remove mis-matched refs if the keys matched, so 
>>> we'd end up
>>> +# removing too many entries which would cause a transaction abort.
>>> +
>>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/a | _filter_scratch
>>> +$BTRFS_UTIL_PROG subvolume create $SCRATCH_MNT/a/b | _filter_scratch
>>> +$BTRFS_UTIL_PROG subvolume snapshot $SCRATCH_MNT/a $SCRATCH_MNT/c \
>>> +    | _filter_scratch
>>> +ls $SCRATCH_MNT/c/b
>>
>> Isn't this ls redundant?
>>
> 
> No we need the dummy entry in cache before we add the root ref during 
> the rename.  Thanks,

A comment stating this would be good.

> 
> Josef