Re: [PATCH] btrfs/022: Add qgroup assign test

[Qu Wenruo <quwenruo.btrfs@gmx.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4555 bytes --]



On 2020/9/21 下午11:16, Sidong Yang wrote:
> On Mon, Sep 21, 2020 at 09:21:33AM +0800, Qu Wenruo wrote:
>>
>>
>> On 2020/9/20 下午4:57, Sidong Yang wrote:
>>> The btrfs/022 test is basic test about qgroup. but it doesn't have
>>> test with qgroup assign function. This patch adds parent assign
>>> test. parent assign test make two subvolumes and a qgroup for assign.
>>> Assign two subvolumes with a qgroup and check that quota of group
>>> has same value with sum of two subvolumes.
> 
> Hi Qu!
> Thanks for review!
> 
>>
>> A little surprised that I haven't submitted such test case, especially
>> we had a fix in kernel.
>>
>> cbab8ade585a ("btrfs: qgroup: mark qgroup inconsistent if we're
>> inherting snapshot to a new qgroup")
> 
> Yeah, there was no test code for qgroup.
> 
>>
>> Despite the comment from Eryu, some btrfs specific comment inlined below.
>>
>>>
>>> Signed-off-by: Sidong Yang <realwakka@gmail.com>
>>> ---
>>>  tests/btrfs/022 | 40 ++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 40 insertions(+)
>>>
>>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>>> index aaa27aaa..cafaa8b2 100755
>>> --- a/tests/btrfs/022
>>> +++ b/tests/btrfs/022
>>> @@ -110,6 +110,40 @@ _limit_test_noexceed()
>>>  	[ $? -eq 0 ] || _fail "should have been allowed to write"
>>>  }
>>>  
>>> +#basic assign testing
>>> +_parent_assign_test()
>>> +{
>>> +	echo "=== parent assign test ===" >> $seqres.full
>>> +	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
>>> +	_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>> +	subvolid_a=$(_btrfs_get_subvolid $SCRATCH_MNT a)
>>> +
>>> +	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/b
>>> +	_run_btrfs_util_prog quota enable $SCRATCH_MNT
>>> +	subvolid_b=$(_btrfs_get_subvolid $SCRATCH_MNT b)
>>> +
>>> +	_run_btrfs_util_prog qgroup create 1/100 $SCRATCH_MNT
>>> +
>>> +	_run_btrfs_util_prog qgroup assign 0/$subvolid_a 1/100 $SCRATCH_MNT
>>> +	_run_btrfs_util_prog qgroup assign 0/$subvolid_b 1/100 $SCRATCH_MNT
>>
>> The coverage is not good enough.
>>
>> Qgroup assign (or inherit) happens not only during "btrfs qgroup assign"
>> but also "btrfs subvolume snapshot -i".
>>
>> And we also need to consider cases like shared extents between two
>> subvolumes (either caused by snapshot or reflink).
>>
>> That means we have two factors, assign or snapshot -i, subvolumes with
>> shared extents or not.
>> That means we need at least 3 combinations:
>>
>> - assign, no shared extents
>> - assign, shared extents
>> - snapshot -i, shared extents
>>
>> (snapshot -i, no shared extents is invalid, as snapshot will always
>> cause shared extents)
> 
> Thanks for good example!
> but there is a question. How can I make shared extents in test code?

Reflink is the most simple solution.

> It's okay that write some file and make a snapshot from it?

If you only want some shared extents, reflink is easier than snapshot.

Thanks,
Qu

> 
>>
>>> +
>>> +	_ddt of=$SCRATCH_MNT/a/file bs=4M count=1 >> $seqres.full 2>&1
>>> +	_ddt of=$SCRATCH_MNT/b/file bs=4M count=1 >> $seqres.full 2>&1
>>> +	sync
>>> +
>>> +	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid_a")
>>> +	a_shared=$(echo $a_shared | awk '{ print $2 }' | tr -dc '0-9')
>>> +
>>> +	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid_b")
>>> +	b_shared=$(echo $b_shared | awk '{ print $2 }' | tr -dc '0-9')
>>> +	sum=$(expr $b_shared  + $a_shared)
>>> +
>>> +	q_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "1/100")
>>> +	q_shared=$(echo $q_shared | awk '{ print $2 }' | tr -dc '0-9')
>>> +
>>> +	[ $sum -eq $q_shared ] || _fail "shared values don't match"
>>
>> Nope, we don't need to do such complex checking all by ourselves.
>>
>> Just let "btrfs check" to handle it, as it will also check the qgroup
>> numbers.
> 
> It's very easy way to check! Thanks.
> 
> Taken together, I'll work for new test case that tests qgroup cases.
> 
> Sincerely,
> Sidong
> 
>>
>> Thanks,
>> Qu
>>
>>> +}
>>> +
>>>  units=`_btrfs_qgroup_units`
>>>  
>>>  _scratch_mkfs > /dev/null 2>&1
>>> @@ -133,6 +167,12 @@ _check_scratch_fs
>>>  _scratch_mkfs > /dev/null 2>&1
>>>  _scratch_mount
>>>  _limit_test_noexceed
>>> +_scratch_unmount
>>> +_check_scratch_fs
>>> +
>>> +_scratch_mkfs > /dev/null 2>&1
>>> +_scratch_mount
>>> +_parent_assign_test
>>>  
>>>  # success, all done
>>>  echo "Silence is golden"
>>>
>>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]