* [PATCH] btrfs/022: Add qgroup assign test
@ 2020-09-20 8:57 Sidong Yang
2020-09-20 17:27 ` Eryu Guan
2020-09-21 1:21 ` Qu Wenruo
0 siblings, 2 replies; 7+ messages in thread
From: Sidong Yang @ 2020-09-20 8:57 UTC (permalink / raw)
To: linux-btrfs, fstests; +Cc: Sidong Yang, Qu Wenruo, Josef Bacik
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.
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
+
+ _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"
+}
+
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"
--
2.25.1
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-20 8:57 [PATCH] btrfs/022: Add qgroup assign test Sidong Yang
@ 2020-09-20 17:27 ` Eryu Guan
2020-09-21 0:55 ` Sidong Yang
2020-09-21 1:21 ` Qu Wenruo
1 sibling, 1 reply; 7+ messages in thread
From: Eryu Guan @ 2020-09-20 17:27 UTC (permalink / raw)
To: Sidong Yang; +Cc: linux-btrfs, fstests, Qu Wenruo, Josef Bacik
On Sun, Sep 20, 2020 at 08:57:53AM +0000, 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.
>
> Signed-off-by: Sidong Yang <realwakka@gmail.com>
We usually don't add new test case to existing tests, as that may make a
PASSed test starting to FAIL, which looks like a regression.
> ---
> 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()
Local function names don't need to be prefixed with "_".
> +{
> + echo "=== parent assign test ===" >> $seqres.full
> + _run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
The helpers based on run_check are not recommended, please just
open-coded btrfs subvolume command and filter the output when necessary.
j
> + _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
> +
> + _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
Just fsync the individule files if possible.
> +
> + 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')
$AWK_PROG
> +
> + 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"
Print the actual number and expected number as well?
Thanks,
Eryu
> +}
> +
> 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"
> --
> 2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-20 17:27 ` Eryu Guan
@ 2020-09-21 0:55 ` Sidong Yang
0 siblings, 0 replies; 7+ messages in thread
From: Sidong Yang @ 2020-09-21 0:55 UTC (permalink / raw)
To: Eryu Guan; +Cc: linux-btrfs, fstests, Qu Wenruo, Josef Bacik
On Mon, Sep 21, 2020 at 01:27:40AM +0800, Eryu Guan wrote:
> On Sun, Sep 20, 2020 at 08:57:53AM +0000, 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.
> >
> > Signed-off-by: Sidong Yang <realwakka@gmail.com>
>
Hi Eryu.
Thanks for review!
> We usually don't add new test case to existing tests, as that may make a
> PASSed test starting to FAIL, which looks like a regression.
Okay, If then, is it okay for writing a new script for this?
>
> > ---
> > 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()
>
> Local function names don't need to be prefixed with "_".
okay, thanks.
>
> > +{
> > + echo "=== parent assign test ===" >> $seqres.full
> > + _run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
>
> The helpers based on run_check are not recommended, please just
> open-coded btrfs subvolume command and filter the output when necessary.
> j
> > + _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
> > +
> > + _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
>
> Just fsync the individule files if possible.
Thanks. I'll fix it like this.
sync $SCRATCH_MNT/b/file
>
>
> > +
> > + 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')
>
> $AWK_PROG
Okay! awk -> $AWK_PROG
>
> > +
> > + 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"
>
> Print the actual number and expected number as well?
Yes, you mean writing like this?
echo "a_shared = $a_shared" >> $seqres.full
echo "b_shared = $b_shared" >> $seqres.full
echo "sum = $sum" >> $seqres.full
echo "q_shared = $q_shared" >> $seqres.full
and I'm just curious that we don't need to cleanup qgroup environment after testing finished?
Thanks,
Sidong
>
> Thanks,
> Eryu
>
> > +}
> > +
> > 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"
> > --
> > 2.25.1
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-20 8:57 [PATCH] btrfs/022: Add qgroup assign test Sidong Yang
2020-09-20 17:27 ` Eryu Guan
@ 2020-09-21 1:21 ` Qu Wenruo
2020-09-21 15:16 ` Sidong Yang
1 sibling, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2020-09-21 1:21 UTC (permalink / raw)
To: Sidong Yang, linux-btrfs, fstests; +Cc: Josef Bacik
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.
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")
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)
> +
> + _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.
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"
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-21 1:21 ` Qu Wenruo
@ 2020-09-21 15:16 ` Sidong Yang
2020-09-21 23:49 ` Qu Wenruo
0 siblings, 1 reply; 7+ messages in thread
From: Sidong Yang @ 2020-09-21 15:16 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, fstests, Josef Bacik
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?
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"
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-21 15:16 ` Sidong Yang
@ 2020-09-21 23:49 ` Qu Wenruo
2020-09-23 0:28 ` Sidong Yang
0 siblings, 1 reply; 7+ messages in thread
From: Qu Wenruo @ 2020-09-21 23:49 UTC (permalink / raw)
To: Sidong Yang, Qu Wenruo; +Cc: linux-btrfs, fstests, Josef Bacik
[-- 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 --]
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] btrfs/022: Add qgroup assign test
2020-09-21 23:49 ` Qu Wenruo
@ 2020-09-23 0:28 ` Sidong Yang
0 siblings, 0 replies; 7+ messages in thread
From: Sidong Yang @ 2020-09-23 0:28 UTC (permalink / raw)
To: Qu Wenruo; +Cc: Qu Wenruo, linux-btrfs, fstests, Josef Bacik
On Tue, Sep 22, 2020 at 07:49:19AM +0800, Qu Wenruo wrote:
>
>
> 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.
I've written a new patch v2 for this. Could you review it?
Thanks,
Sidong
>
> 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"
> >>>
> >>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-09-23 0:28 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-20 8:57 [PATCH] btrfs/022: Add qgroup assign test Sidong Yang
2020-09-20 17:27 ` Eryu Guan
2020-09-21 0:55 ` Sidong Yang
2020-09-21 1:21 ` Qu Wenruo
2020-09-21 15:16 ` Sidong Yang
2020-09-21 23:49 ` Qu Wenruo
2020-09-23 0:28 ` Sidong Yang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).