FSTests Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/3] fstests: btrfs/022 fixes
@ 2020-02-07  1:59 Qu Wenruo
  2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
                   ` (2 more replies)
  0 siblings, 3 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07  1:59 UTC (permalink / raw)
  To: fstests, linux-btrfs

Since now fsstress can create new subvolumes, it causes new problem for
btrfs/022.

First two patches will fix the problem by:
- Enhance _btrfs_get_subvolid()
  So it can catch correct subvolid

- Use proper qgroupid for grep
  This solves the random failure even after _btrfs_get_subvolid
  enhancement.

The last patch will add extra debug output for btrfs/022 to make our
lives easier.

Qu Wenruo (3):
  fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  fstests: btrfs/022: Match qgroup id more correctly
  fstests: btrfs/022: Add debug output

 common/btrfs    |  2 +-
 tests/btrfs/022 | 19 +++++++++++++------
 2 files changed, 14 insertions(+), 7 deletions(-)

-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  2020-02-07  1:59 [PATCH 0/3] fstests: btrfs/022 fixes Qu Wenruo
@ 2020-02-07  1:59 ` Qu Wenruo
  2020-02-07  2:09   ` Josef Bacik
  2020-02-07  9:41   ` Nikolay Borisov
  2020-02-07  1:59 ` [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly Qu Wenruo
  2020-02-07  1:59 ` [PATCH 3/3] fstests: btrfs/022: Add debug output Qu Wenruo
  2 siblings, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07  1:59 UTC (permalink / raw)
  To: fstests, linux-btrfs; +Cc: Josef Bacik

Current _btrfs_get_subvolid() can't handle the following case at all:
  # btrfs subvol list $SCRATCH_MNT
  ID 256 gen 9 top level 5 path subv1
  ID 257 gen 7 top level 256 path subv1/subv2
  ID 258 gen 8 top level 256 path subv1/subv3
  ID 259 gen 9 top level 256 path subv1/subv4

If we call "_btrfs_get_subvolid $SCRATCH_MNT subv1" we will get a list
of all subvolumes, not the subvolid of subv1.

To address this problem, we go egrep to match $name which starts with a
space, and at the end of a line.
So that all other subvolumes won't hit.

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 common/btrfs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/btrfs b/common/btrfs
index 19ac7cc4..85b33e4c 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -7,7 +7,7 @@ _btrfs_get_subvolid()
 	mnt=$1
 	name=$2
 
-	$BTRFS_UTIL_PROG sub list $mnt | grep $name | awk '{ print $2 }'
+	$BTRFS_UTIL_PROG sub list $mnt | egrep "\s$name$" | awk '{ print $2 }'
 }
 
 # _require_btrfs_command <command> [<subcommand>|<option>]
-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly
  2020-02-07  1:59 [PATCH 0/3] fstests: btrfs/022 fixes Qu Wenruo
  2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
@ 2020-02-07  1:59 ` Qu Wenruo
  2020-02-07  2:10   ` Josef Bacik
  2020-02-07  9:46   ` Nikolay Borisov
  2020-02-07  1:59 ` [PATCH 3/3] fstests: btrfs/022: Add debug output Qu Wenruo
  2 siblings, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07  1:59 UTC (permalink / raw)
  To: fstests, linux-btrfs; +Cc: Josef Bacik

[BUG]
Btrfs/022 sometimes fails with snapshot's reference mismatch with its
source.

[CAUSE]
Since commit fd0830929573 ("fsstress: add the ability to create
snapshots") adds the ability for fsstress to create/delete snapshot and
subvolumes, fsstress will create new subvolumes under test dir.

For example, we could have the following subvolumes created by fsstress:
subvol a id=256
subvol b id=306
qgroupid         rfer         excl
--------         ----         ----
0/5             16384        16384
0/256        13914112        16384
...
0/263         3080192      2306048 		<< 2 *306* 048
...
0/306        13914112        16384 		<< 0/ *306

So when we're greping for subvolid 306, it matches qgroup 0/263 first,
which has difference size, and caused false alert.

[FIX]
Instead of greping "$subvolid" blindly, now grep "0/$subvolid" to catch
qgroupid correctly, without hitting rfer/excl values.

Suggested-by: Josef Bacik <josef@toxicpanda.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/022 | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/btrfs/022 b/tests/btrfs/022
index 5348d3ed..3e729852 100755
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -49,10 +49,10 @@ _basic_test()
 
 	# the shared values of both the original subvol and snapshot should
 	# match
-	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
+	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	a_shared=$(echo $a_shared | awk '{ print $2 }')
 	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
-	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
+	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	b_shared=$(echo $b_shared | awk '{ print $2 }')
 	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
 }
@@ -68,12 +68,12 @@ _rescan_test()
 	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
 		$FSSTRESS_AVOID
 	sync
-	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
+	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	echo $output >> $seqres.full
 	refer=$(echo $output | awk '{ print $2 }')
 	excl=$(echo $output | awk '{ print $3 }')
 	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
-	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
+	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	echo $output >> $seqres.full
 	[ $refer -eq $(echo $output | awk '{ print $2 }') ] || \
 		_fail "reference values don't match after rescan"
-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/3] fstests: btrfs/022: Add debug output
  2020-02-07  1:59 [PATCH 0/3] fstests: btrfs/022 fixes Qu Wenruo
  2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
  2020-02-07  1:59 ` [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly Qu Wenruo
@ 2020-02-07  1:59 ` Qu Wenruo
  2020-02-07  2:10   ` Josef Bacik
  2020-02-07  9:48   ` Nikolay Borisov
  2 siblings, 2 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07  1:59 UTC (permalink / raw)
  To: fstests, linux-btrfs

When btrfs/022 fails, its $seqres.full doesn't contain much useful info
to debug.

This patch will add extra debug, including subvolid and full "btrfs
qgroup show" output.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 tests/btrfs/022 | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/tests/btrfs/022 b/tests/btrfs/022
index 3e729852..aaa27aaa 100755
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -35,6 +35,7 @@ rm -f $seqres.full
 # Test to make sure we can actually turn it on and it makes sense
 _basic_test()
 {
+	echo "=== basic test ===" >> $seqres.full
 	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
 	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
 	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
@@ -51,9 +52,12 @@ _basic_test()
 	# match
 	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	a_shared=$(echo $a_shared | awk '{ print $2 }')
+	echo "subvol a id=$subvolid" >> $seqres.full
 	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
+	echo "subvol b id=$subvolid" >> $seqres.full
 	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
 	b_shared=$(echo $b_shared | awk '{ print $2 }')
+	$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT >> $seqres.full
 	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
 }
 
@@ -61,6 +65,7 @@ _basic_test()
 #come up with the same answer
 _rescan_test()
 {
+	echo "=== rescan test ===" >> $seqres.full
 	# first with a blank subvol
 	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
 	_run_btrfs_util_prog quota enable $SCRATCH_MNT/a
@@ -69,12 +74,12 @@ _rescan_test()
 		$FSSTRESS_AVOID
 	sync
 	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	echo $output >> $seqres.full
+	echo "qgroup values before rescan: $output" >> $seqres.full
 	refer=$(echo $output | awk '{ print $2 }')
 	excl=$(echo $output | awk '{ print $3 }')
 	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
-	echo $output >> $seqres.full
+	echo "qgroup values after rescan: $output" >> $seqres.full
 	[ $refer -eq $(echo $output | awk '{ print $2 }') ] || \
 		_fail "reference values don't match after rescan"
 	[ $excl -eq $(echo $output | awk '{ print $3 }') ] || \
@@ -84,6 +89,7 @@ _rescan_test()
 #basic exceed limit testing
 _limit_test_exceed()
 {
+	echo "=== limit exceed test ===" >> $seqres.full
 	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
 	_run_btrfs_util_prog quota enable $SCRATCH_MNT
 	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
@@ -95,6 +101,7 @@ _limit_test_exceed()
 #basic noexceed limit testing
 _limit_test_noexceed()
 {
+	echo "=== limit not exceed test ===" >> $seqres.full
 	_run_btrfs_util_prog subvolume create $SCRATCH_MNT/a
 	_run_btrfs_util_prog quota enable $SCRATCH_MNT
 	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT a)
-- 
2.23.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
@ 2020-02-07  2:09   ` Josef Bacik
  2020-02-07  9:41   ` Nikolay Borisov
  1 sibling, 0 replies; 13+ messages in thread
From: Josef Bacik @ 2020-02-07  2:09 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs

On 2/6/20 8:59 PM, Qu Wenruo wrote:
> Current _btrfs_get_subvolid() can't handle the following case at all:
>    # btrfs subvol list $SCRATCH_MNT
>    ID 256 gen 9 top level 5 path subv1
>    ID 257 gen 7 top level 256 path subv1/subv2
>    ID 258 gen 8 top level 256 path subv1/subv3
>    ID 259 gen 9 top level 256 path subv1/subv4
> 
> If we call "_btrfs_get_subvolid $SCRATCH_MNT subv1" we will get a list
> of all subvolumes, not the subvolid of subv1.
> 
> To address this problem, we go egrep to match $name which starts with a
> space, and at the end of a line.
> So that all other subvolumes won't hit.
> 
> Suggested-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly
  2020-02-07  1:59 ` [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly Qu Wenruo
@ 2020-02-07  2:10   ` Josef Bacik
  2020-02-07  9:46   ` Nikolay Borisov
  1 sibling, 0 replies; 13+ messages in thread
From: Josef Bacik @ 2020-02-07  2:10 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs

On 2/6/20 8:59 PM, Qu Wenruo wrote:
> [BUG]
> Btrfs/022 sometimes fails with snapshot's reference mismatch with its
> source.
> 
> [CAUSE]
> Since commit fd0830929573 ("fsstress: add the ability to create
> snapshots") adds the ability for fsstress to create/delete snapshot and
> subvolumes, fsstress will create new subvolumes under test dir.
> 
> For example, we could have the following subvolumes created by fsstress:
> subvol a id=256
> subvol b id=306
> qgroupid         rfer         excl
> --------         ----         ----
> 0/5             16384        16384
> 0/256        13914112        16384
> ...
> 0/263         3080192      2306048 		<< 2 *306* 048
> ...
> 0/306        13914112        16384 		<< 0/ *306
> 
> So when we're greping for subvolid 306, it matches qgroup 0/263 first,
> which has difference size, and caused false alert.
> 
> [FIX]
> Instead of greping "$subvolid" blindly, now grep "0/$subvolid" to catch
> qgroupid correctly, without hitting rfer/excl values.
> 
> Suggested-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] fstests: btrfs/022: Add debug output
  2020-02-07  1:59 ` [PATCH 3/3] fstests: btrfs/022: Add debug output Qu Wenruo
@ 2020-02-07  2:10   ` Josef Bacik
  2020-02-07  9:48   ` Nikolay Borisov
  1 sibling, 0 replies; 13+ messages in thread
From: Josef Bacik @ 2020-02-07  2:10 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs

On 2/6/20 8:59 PM, Qu Wenruo wrote:
> When btrfs/022 fails, its $seqres.full doesn't contain much useful info
> to debug.
> 
> This patch will add extra debug, including subvolid and full "btrfs
> qgroup show" output.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Josef Bacik <josef@toxicpanda.com>

Thanks,

Josef

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
  2020-02-07  2:09   ` Josef Bacik
@ 2020-02-07  9:41   ` Nikolay Borisov
  2020-02-07 10:02     ` Qu Wenruo
  1 sibling, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2020-02-07  9:41 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs; +Cc: Josef Bacik



On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
> Current _btrfs_get_subvolid() can't handle the following case at all:
>   # btrfs subvol list $SCRATCH_MNT
>   ID 256 gen 9 top level 5 path subv1
>   ID 257 gen 7 top level 256 path subv1/subv2
>   ID 258 gen 8 top level 256 path subv1/subv3
>   ID 259 gen 9 top level 256 path subv1/subv4
> 
> If we call "_btrfs_get_subvolid $SCRATCH_MNT subv1" we will get a list
> of all subvolumes, not the subvolid of subv1.
> 
> To address this problem, we go egrep to match $name which starts with a
> space, and at the end of a line.
> So that all other subvolumes won't hit.
> 
> Suggested-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

> ---
>  common/btrfs | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/btrfs b/common/btrfs
> index 19ac7cc4..85b33e4c 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -7,7 +7,7 @@ _btrfs_get_subvolid()
>  	mnt=$1
>  	name=$2
>  
> -	$BTRFS_UTIL_PROG sub list $mnt | grep $name | awk '{ print $2 }'
> +	$BTRFS_UTIL_PROG sub list $mnt | egrep "\s$name$" | awk '{ print $2 }'

nit: But you don't even need egrep for this, you could have simply used
"grep $name$"

>  }
>  
>  # _require_btrfs_command <command> [<subcommand>|<option>]
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly
  2020-02-07  1:59 ` [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly Qu Wenruo
  2020-02-07  2:10   ` Josef Bacik
@ 2020-02-07  9:46   ` Nikolay Borisov
  2020-02-07 10:03     ` Qu Wenruo
  1 sibling, 1 reply; 13+ messages in thread
From: Nikolay Borisov @ 2020-02-07  9:46 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs; +Cc: Josef Bacik



On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
> [BUG]
> Btrfs/022 sometimes fails with snapshot's reference mismatch with its
> source.
> 
> [CAUSE]
> Since commit fd0830929573 ("fsstress: add the ability to create
> snapshots") adds the ability for fsstress to create/delete snapshot and
> subvolumes, fsstress will create new subvolumes under test dir.
> 
> For example, we could have the following subvolumes created by fsstress:
> subvol a id=256
> subvol b id=306
> qgroupid         rfer         excl
> --------         ----         ----
> 0/5             16384        16384
> 0/256        13914112        16384
> ...
> 0/263         3080192      2306048 		<< 2 *306* 048
> ...
> 0/306        13914112        16384 		<< 0/ *306
> 
> So when we're greping for subvolid 306, it matches qgroup 0/263 first,
> which has difference size, and caused false alert.
> 
> [FIX]
> Instead of greping "$subvolid" blindly, now grep "0/$subvolid" to catch
> qgroupid correctly, without hitting rfer/excl values.

That 0/ can it ever be a number different than 0, if so a more correct
regular expression should be:
grep "[[:digit:]]/306"  ?


> 
> Suggested-by: Josef Bacik <josef@toxicpanda.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>  tests/btrfs/022 | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/btrfs/022 b/tests/btrfs/022
> index 5348d3ed..3e729852 100755
> --- a/tests/btrfs/022
> +++ b/tests/btrfs/022
> @@ -49,10 +49,10 @@ _basic_test()
>  
>  	# the shared values of both the original subvol and snapshot should
>  	# match
> -	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
> +	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>  	a_shared=$(echo $a_shared | awk '{ print $2 }')
>  	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
> -	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
> +	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>  	b_shared=$(echo $b_shared | awk '{ print $2 }')
>  	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
>  }
> @@ -68,12 +68,12 @@ _rescan_test()
>  	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
>  		$FSSTRESS_AVOID
>  	sync
> -	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
> +	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>  	echo $output >> $seqres.full
>  	refer=$(echo $output | awk '{ print $2 }')
>  	excl=$(echo $output | awk '{ print $3 }')
>  	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
> -	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
> +	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>  	echo $output >> $seqres.full
>  	[ $refer -eq $(echo $output | awk '{ print $2 }') ] || \
>  		_fail "reference values don't match after rescan"
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 3/3] fstests: btrfs/022: Add debug output
  2020-02-07  1:59 ` [PATCH 3/3] fstests: btrfs/022: Add debug output Qu Wenruo
  2020-02-07  2:10   ` Josef Bacik
@ 2020-02-07  9:48   ` Nikolay Borisov
  1 sibling, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2020-02-07  9:48 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs



On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
> When btrfs/022 fails, its $seqres.full doesn't contain much useful info
> to debug.
> 
> This patch will add extra debug, including subvolid and full "btrfs
> qgroup show" output.
> 
> Signed-off-by: Qu Wenruo <wqu@suse.com>

Reviewed-by: Nikolay Borisov <nborisov@suse.com>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  2020-02-07  9:41   ` Nikolay Borisov
@ 2020-02-07 10:02     ` Qu Wenruo
  2020-02-07 11:07       ` Nikolay Borisov
  0 siblings, 1 reply; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07 10:02 UTC (permalink / raw)
  To: Nikolay Borisov, fstests, linux-btrfs; +Cc: Josef Bacik



On 2020/2/7 下午5:41, Nikolay Borisov wrote:
> 
> 
> On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
>> Current _btrfs_get_subvolid() can't handle the following case at all:
>>   # btrfs subvol list $SCRATCH_MNT
>>   ID 256 gen 9 top level 5 path subv1
>>   ID 257 gen 7 top level 256 path subv1/subv2
>>   ID 258 gen 8 top level 256 path subv1/subv3
>>   ID 259 gen 9 top level 256 path subv1/subv4
>>
>> If we call "_btrfs_get_subvolid $SCRATCH_MNT subv1" we will get a list
>> of all subvolumes, not the subvolid of subv1.
>>
>> To address this problem, we go egrep to match $name which starts with a
>> space, and at the end of a line.
>> So that all other subvolumes won't hit.
>>
>> Suggested-by: Josef Bacik <josef@toxicpanda.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
> 
> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
> 
>> ---
>>  common/btrfs | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/common/btrfs b/common/btrfs
>> index 19ac7cc4..85b33e4c 100644
>> --- a/common/btrfs
>> +++ b/common/btrfs
>> @@ -7,7 +7,7 @@ _btrfs_get_subvolid()
>>  	mnt=$1
>>  	name=$2
>>  
>> -	$BTRFS_UTIL_PROG sub list $mnt | grep $name | awk '{ print $2 }'
>> +	$BTRFS_UTIL_PROG sub list $mnt | egrep "\s$name$" | awk '{ print $2 }'
> 
> nit: But you don't even need egrep for this, you could have simply used
> "grep $name$"

That \s is needed. Or the following case can't be handled:

   ID 256 gen 9 top level 5 path subv1
   ID 257 gen 7 top level 256 path subv1/subv1

Thanks,
Qu

> 
>>  }
>>  
>>  # _require_btrfs_command <command> [<subcommand>|<option>]
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly
  2020-02-07  9:46   ` Nikolay Borisov
@ 2020-02-07 10:03     ` Qu Wenruo
  0 siblings, 0 replies; 13+ messages in thread
From: Qu Wenruo @ 2020-02-07 10:03 UTC (permalink / raw)
  To: Nikolay Borisov, fstests, linux-btrfs; +Cc: Josef Bacik



On 2020/2/7 下午5:46, Nikolay Borisov wrote:
> 
> 
> On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
>> [BUG]
>> Btrfs/022 sometimes fails with snapshot's reference mismatch with its
>> source.
>>
>> [CAUSE]
>> Since commit fd0830929573 ("fsstress: add the ability to create
>> snapshots") adds the ability for fsstress to create/delete snapshot and
>> subvolumes, fsstress will create new subvolumes under test dir.
>>
>> For example, we could have the following subvolumes created by fsstress:
>> subvol a id=256
>> subvol b id=306
>> qgroupid         rfer         excl
>> --------         ----         ----
>> 0/5             16384        16384
>> 0/256        13914112        16384
>> ...
>> 0/263         3080192      2306048 		<< 2 *306* 048
>> ...
>> 0/306        13914112        16384 		<< 0/ *306
>>
>> So when we're greping for subvolid 306, it matches qgroup 0/263 first,
>> which has difference size, and caused false alert.
>>
>> [FIX]
>> Instead of greping "$subvolid" blindly, now grep "0/$subvolid" to catch
>> qgroupid correctly, without hitting rfer/excl values.
> 
> That 0/ can it ever be a number different than 0, if so a more correct
> regular expression should be:
> grep "[[:digit:]]/306"  ?

In this particular case, we only care level 0 qgroup, thus it's enough.

If we're extracting it into a generic wrapper, then your
[[:digit:]]/$subvolid will be needed.

Thanks,
Qu
> 
> 
>>
>> Suggested-by: Josef Bacik <josef@toxicpanda.com>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>>  tests/btrfs/022 | 8 ++++----
>>  1 file changed, 4 insertions(+), 4 deletions(-)
>>
>> diff --git a/tests/btrfs/022 b/tests/btrfs/022
>> index 5348d3ed..3e729852 100755
>> --- a/tests/btrfs/022
>> +++ b/tests/btrfs/022
>> @@ -49,10 +49,10 @@ _basic_test()
>>  
>>  	# the shared values of both the original subvol and snapshot should
>>  	# match
>> -	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
>> +	a_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>>  	a_shared=$(echo $a_shared | awk '{ print $2 }')
>>  	subvolid=$(_btrfs_get_subvolid $SCRATCH_MNT b)
>> -	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
>> +	b_shared=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>>  	b_shared=$(echo $b_shared | awk '{ print $2 }')
>>  	[ $b_shared -eq $a_shared ] || _fail "shared values don't match"
>>  }
>> @@ -68,12 +68,12 @@ _rescan_test()
>>  	run_check $FSSTRESS_PROG -d $SCRATCH_MNT/a -w -p 1 -n 2000 \
>>  		$FSSTRESS_AVOID
>>  	sync
>> -	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
>> +	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>>  	echo $output >> $seqres.full
>>  	refer=$(echo $output | awk '{ print $2 }')
>>  	excl=$(echo $output | awk '{ print $3 }')
>>  	_run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
>> -	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep $subvolid)
>> +	output=$($BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | grep "0/$subvolid")
>>  	echo $output >> $seqres.full
>>  	[ $refer -eq $(echo $output | awk '{ print $2 }') ] || \
>>  		_fail "reference values don't match after rescan"
>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid()
  2020-02-07 10:02     ` Qu Wenruo
@ 2020-02-07 11:07       ` Nikolay Borisov
  0 siblings, 0 replies; 13+ messages in thread
From: Nikolay Borisov @ 2020-02-07 11:07 UTC (permalink / raw)
  To: Qu Wenruo, fstests, linux-btrfs; +Cc: Josef Bacik



On 7.02.20 г. 12:02 ч., Qu Wenruo wrote:
> 
> 
> On 2020/2/7 下午5:41, Nikolay Borisov wrote:
>>
>>
>> On 7.02.20 г. 3:59 ч., Qu Wenruo wrote:
>>> Current _btrfs_get_subvolid() can't handle the following case at all:
>>>   # btrfs subvol list $SCRATCH_MNT
>>>   ID 256 gen 9 top level 5 path subv1
>>>   ID 257 gen 7 top level 256 path subv1/subv2
>>>   ID 258 gen 8 top level 256 path subv1/subv3
>>>   ID 259 gen 9 top level 256 path subv1/subv4
>>>
>>> If we call "_btrfs_get_subvolid $SCRATCH_MNT subv1" we will get a list
>>> of all subvolumes, not the subvolid of subv1.
>>>
>>> To address this problem, we go egrep to match $name which starts with a
>>> space, and at the end of a line.
>>> So that all other subvolumes won't hit.
>>>
>>> Suggested-by: Josef Bacik <josef@toxicpanda.com>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>
>> Reviewed-by: Nikolay Borisov <nborisov@suse.com>
>>
>>> ---
>>>  common/btrfs | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/common/btrfs b/common/btrfs
>>> index 19ac7cc4..85b33e4c 100644
>>> --- a/common/btrfs
>>> +++ b/common/btrfs
>>> @@ -7,7 +7,7 @@ _btrfs_get_subvolid()
>>>  	mnt=$1
>>>  	name=$2
>>>  
>>> -	$BTRFS_UTIL_PROG sub list $mnt | grep $name | awk '{ print $2 }'
>>> +	$BTRFS_UTIL_PROG sub list $mnt | egrep "\s$name$" | awk '{ print $2 }'
>>
>> nit: But you don't even need egrep for this, you could have simply used
>> "grep $name$"
> 
> That \s is needed. Or the following case can't be handled:
> 
>    ID 256 gen 9 top level 5 path subv1
>    ID 257 gen 7 top level 256 path subv1/subv1


Good point.

> 
> Thanks,
> Qu
> 
>>
>>>  }
>>>  
>>>  # _require_btrfs_command <command> [<subcommand>|<option>]
>>>

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, back to index

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-02-07  1:59 [PATCH 0/3] fstests: btrfs/022 fixes Qu Wenruo
2020-02-07  1:59 ` [PATCH 1/3] fstests: btrfs: Use word mathcing for _btrfs_get_subvolid() Qu Wenruo
2020-02-07  2:09   ` Josef Bacik
2020-02-07  9:41   ` Nikolay Borisov
2020-02-07 10:02     ` Qu Wenruo
2020-02-07 11:07       ` Nikolay Borisov
2020-02-07  1:59 ` [PATCH 2/3] fstests: btrfs/022: Match qgroup id more correctly Qu Wenruo
2020-02-07  2:10   ` Josef Bacik
2020-02-07  9:46   ` Nikolay Borisov
2020-02-07 10:03     ` Qu Wenruo
2020-02-07  1:59 ` [PATCH 3/3] fstests: btrfs/022: Add debug output Qu Wenruo
2020-02-07  2:10   ` Josef Bacik
2020-02-07  9:48   ` Nikolay Borisov

FSTests Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/fstests/0 fstests/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 fstests fstests/ https://lore.kernel.org/fstests \
		fstests@vger.kernel.org
	public-inbox-index fstests

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.fstests


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git