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@xxxxxxxxx>
> > ---
> > 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?
It's okay that write some file and make a snapshot from it?
>
> > +
> > + _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"
> >
>
