| Message ID | 20200920085753.277590-1-realwakka@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs/022: Add qgroup assign test | expand |
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
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
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" >
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" > > >
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" >>> >>
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" > >>> > >> >
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"
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(+)