* [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).