[PATCH v2 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_command

[PATCH v2 1/2] fstests: common: rename and enhance _require_btrfs to _require_btrfs_command

[Qu Wenruo]

Rename _require_btrfs() to _require_btrfs_command() to avoid
confusion, as all other _require_btrfs_* has a quite clear suffix, like
_require_btrfs_mkfs_feature() or _require_btrfs_fs_feature().

Also enhance _require_btrfs_command() to accept 2nd level commands or
options.
Options will be determined by the first "-" char.
This is quite useful for case like "btrfs inspect-internal dump-tree"
and "btrfs check --qgroup-report".

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
v2:
  Replace _subcommand with _command.
  Rebase to latest master
  Use grep -w and -q to make it safer and less noisy
  Update the _notrun string
  Fix typo _not_run
---
 common/btrfs    | 22 ++++++++++++++++++++--
 tests/btrfs/004 |  3 ++-
 tests/btrfs/048 |  2 +-
 tests/btrfs/059 |  2 +-
 tests/btrfs/131 |  2 +-
 5 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/common/btrfs b/common/btrfs
index ab6497d..302edc6 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -10,16 +10,34 @@ _btrfs_get_subvolid()
 	$BTRFS_UTIL_PROG sub list $mnt | grep $name | awk '{ print $2 }'
 }
 
+# _require_btrfs_command <command> [<subcommand>|<option>]
 # We check for btrfs and (optionally) features of the btrfs command
-_require_btrfs()
+# It can both subfunction like "inspect-internal dump-tree" and
+# options like "check --qgroup-report"
+_require_btrfs_command()
 {
 	cmd=$1
+	param=$2
+
 	_require_command "$BTRFS_UTIL_PROG" btrfs
 	if [ -z "$1" ]; then
 		return 1;
 	fi
-	$BTRFS_UTIL_PROG $cmd --help >/dev/null 2>&1
+	$BTRFS_UTIL_PROG $cmd --help &> /dev/null
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd)"
+
+	test -z "$param" && return
+
+	# If $param is an option, replace leading "-"s for grep
+	if [ ${param:0:1} == "-" ]; then
+		safe_param=$(echo $param | sed 's/^-*//')
+		$BTRFS_UTIL_PROG $cmd --help | grep -wq $safe_param || \
+			_notrun "$BTRFS_UTIL_PROG too old (must support $cmd $param)"
+		return
+	fi
+
+	$BTRFS_UTIL_PROG $cmd $param --help &> /dev/null
+	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd $param)"
 }
 
 _run_btrfs_util_prog()
diff --git a/tests/btrfs/004 b/tests/btrfs/004
index 905770a..3f8330f 100755
--- a/tests/btrfs/004
+++ b/tests/btrfs/004
@@ -51,7 +51,8 @@ _supported_fs btrfs
 _supported_os Linux
 _require_scratch
 _require_no_large_scratch_dev
-_require_btrfs inspect-internal
+_require_btrfs_command inspect-internal logical-resolve
+_require_btrfs_command inspect-internal inode-resolve
 _require_command "/usr/sbin/filefrag" filefrag
 
 rm -f $seqres.full
diff --git a/tests/btrfs/048 b/tests/btrfs/048
index 0b907b0..e03b3c5 100755
--- a/tests/btrfs/048
+++ b/tests/btrfs/048
@@ -48,7 +48,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_test
 _require_scratch
-_require_btrfs "property"
+_require_btrfs_command "property"
 
 send_files_dir=$TEST_DIR/btrfs-test-$seq
 
diff --git a/tests/btrfs/059 b/tests/btrfs/059
index 8f106d2..2d1ec23 100755
--- a/tests/btrfs/059
+++ b/tests/btrfs/059
@@ -51,7 +51,7 @@ _supported_fs btrfs
 _supported_os Linux
 _require_test
 _require_scratch
-_require_btrfs "property"
+_require_btrfs_command "property"
 
 rm -f $seqres.full
 
diff --git a/tests/btrfs/131 b/tests/btrfs/131
index d1a11d2..ce486e6 100755
--- a/tests/btrfs/131
+++ b/tests/btrfs/131
@@ -48,7 +48,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
-_require_btrfs inspect-internal
+_require_btrfs_command inspect-internal dump-super
 
 mkfs_v1()
 {
-- 
2.7.4

[PATCH v2 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code

[Qu Wenruo]

Introduce new _require_btrfs_qgroup_report function, which will check
the accessibility to "btrfs check --qgroup-report", then set a global
flag to info _check_scratch_fs() to do extra qgroup check.

Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
---
v2:
  Use "${RESULT_DIR}/require_scratch.require_qgroup_report" instead of
  global variant
  Rebased to latest master
  Replace btrfsck with $BTRFS_UTIL_PROG check.
---
 check           |  4 ++--
 common/btrfs    | 22 +++++++++++++++++++++-
 common/rc       |  5 +++--
 tests/btrfs/022 |  4 ++++
 tests/btrfs/028 |  5 ++---
 tests/btrfs/042 |  6 ++----
 tests/btrfs/099 |  1 +
 tests/btrfs/104 | 20 +++++---------------
 tests/btrfs/122 |  9 +++------
 tests/btrfs/123 |  5 ++---
 tests/btrfs/126 |  1 +
 11 files changed, 46 insertions(+), 36 deletions(-)

diff --git a/check b/check
index 8f2a1bb..76eb054 100755
--- a/check
+++ b/check
@@ -405,11 +405,11 @@ _check_filesystems()
 {
 	if [ -f ${RESULT_DIR}/require_test ]; then
 		_check_test_fs || err=true
-		rm -f ${RESULT_DIR}/require_test
+		rm -f ${RESULT_DIR}/require_test*
 	fi
 	if [ -f ${RESULT_DIR}/require_scratch ]; then
 		_check_scratch_fs || err=true
-		rm -f ${RESULT_DIR}/require_scratch
+		rm -f ${RESULT_DIR}/require_scratch*
 	fi
 }
 
diff --git a/common/btrfs b/common/btrfs
index 302edc6..a860228 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -40,6 +40,13 @@ _require_btrfs_command()
 	[ $? -eq 0 ] || _notrun "$BTRFS_UTIL_PROG too old (must support $cmd $param)"
 }
 
+# Require extra check on btrfs qgroup numbers
+_require_btrfs_qgroup_report()
+{
+	_require_btrfs_command check --qgroup-report
+	touch ${RESULT_DIR}/require_scratch.require_qgroup_report
+}
+
 _run_btrfs_util_prog()
 {
 	run_check $BTRFS_UTIL_PROG $*
@@ -97,7 +104,20 @@ _check_btrfs_filesystem()
 		mountpoint=`_umount_or_remount_ro $device`
 	fi
 
-	btrfsck $device >$tmp.fsck 2>&1
+	if [ -f ${RESULT_DIR}/require_scratch.require_qgroup_report ]; then
+		$BTRFS_UTIL_PROG check $device --qgroup-report > $tmp.qgroup_report 2>&1
+		if grep -qE "Counts for qgroup.*are different" $tmp.qgroup_report ; then
+			echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers (see $seqres.full)"
+			echo "_check_btrfs_filesystem: filesystem on $device has wrong qgroup numbers" \
+				>> $seqres.full
+			echo "*** qgroup_report.$FSTYP output ***"	>>$seqres.full
+			cat $tmp.qgroup_report				>>$seqres.full
+			echo "*** qgroup_report.$FSTYP output ***"	>>$seqres.full
+		fi
+		rm -f $tmp.qgroup_report
+	fi
+
+	$BTRFS_UTIL_PROG check $device >$tmp.fsck 2>&1
 	if [ $? -ne 0 ]; then
 		echo "_check_btrfs_filesystem: filesystem on $device is inconsistent (see $seqres.full)"
 
diff --git a/common/rc b/common/rc
index 2719b23..b50283c 100644
--- a/common/rc
+++ b/common/rc
@@ -1222,8 +1222,9 @@ _notrun()
 {
     echo "$*" > $seqres.notrun
     echo "$seq not run: $*"
-    rm -f ${RESULT_DIR}/require_test
-    rm -f ${RESULT_DIR}/require_scratch
+    rm -f ${RESULT_DIR}/require_test*
+    rm -f ${RESULT_DIR}/require_scratch*
+
     status=0
     exit
 }
diff --git a/tests/btrfs/022 b/tests/btrfs/022
index 56d4f3d..9abad6c 100755
--- a/tests/btrfs/022
+++ b/tests/btrfs/022
@@ -43,6 +43,7 @@ _cleanup()
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 rm -f $seqres.full
 
@@ -125,16 +126,19 @@ _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 _basic_test
 _scratch_unmount
+_check_scratch_fs
 
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 _rescan_test
 _scratch_unmount
+_check_scratch_fs
 
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
 _limit_test_exceed
 _scratch_unmount
+_check_scratch_fs
 
 _scratch_mkfs > /dev/null 2>&1
 _scratch_mount
diff --git a/tests/btrfs/028 b/tests/btrfs/028
index 1425609..a3d9a27 100755
--- a/tests/btrfs/028
+++ b/tests/btrfs/028
@@ -51,6 +51,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 _scratch_mkfs
 _scratch_mount
@@ -86,9 +87,7 @@ _run_btrfs_util_prog filesystem sync $SCRATCH_MNT
 
 _scratch_unmount
 
-# generate a qgroup report and look for inconsistent groups
-$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
-	grep -E "Counts for qgroup.*are different"
+# qgroup will be checked at _check_scratch_fs() by fstest.
 echo "Silence is golden"
 status=0
 
diff --git a/tests/btrfs/042 b/tests/btrfs/042
index 498ccc9..dc9b762 100755
--- a/tests/btrfs/042
+++ b/tests/btrfs/042
@@ -43,6 +43,7 @@ _cleanup()
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 rm -f $seqres.full
 
@@ -84,10 +85,7 @@ for i in `seq 10 -1 1`; do
 	total_written=$(($total_written+$filesize))
 done
 
-#check if total written exceeds limit
-if [ $total_written -gt $LIMIT_SIZE ];then
-	_fail "total written should be less than $LIMIT_SIZE"
-fi
+# qgroup will be checked automatically at _check_scratch_fs() by fstest
 
 # success, all done
 echo "Silence is golden"
diff --git a/tests/btrfs/099 b/tests/btrfs/099
index 70f07b5..65ea79b 100755
--- a/tests/btrfs/099
+++ b/tests/btrfs/099
@@ -46,6 +46,7 @@ _cleanup()
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 # Use big blocksize to ensure there is still enough space left for metadata
 # space reserve.
diff --git a/tests/btrfs/104 b/tests/btrfs/104
index 6afaa02..e6a6d3b 100755
--- a/tests/btrfs/104
+++ b/tests/btrfs/104
@@ -58,6 +58,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 rm -f $seqres.full
 
@@ -145,21 +146,10 @@ _scratch_cycle_mount
 # referenced above.
 _run_btrfs_util_prog subvolume delete $SCRATCH_MNT/snap1
 
-# There is no way from userspace to force btrfs_drop_snapshot to run
-# at a given time (even via mount/unmount). We must wait for it to
-# start and complete. This is the shortest time on my tests systems I
-# have found which always allows drop_snapshot to run to completion.
-sleep 45
+# "btrfs filesystem sync" will trigger subvolume deletion
+_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
 
-_scratch_unmount
-
-# generate a qgroup report and look for inconsistent groups
-#  - don't use _run_btrfs_util_prog here as it captures the output and
-#    we need to grep it.
-$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
-			grep -E -q "Counts for qgroup.*are different"
-if [ $? -ne 0 ]; then
-	status=0
-fi
+# Qgroup will be checked by fstest at _check_scratch_fs()
 
+status=0
 exit
diff --git a/tests/btrfs/122 b/tests/btrfs/122
index 82252ab..6a7f17f 100755
--- a/tests/btrfs/122
+++ b/tests/btrfs/122
@@ -49,6 +49,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 rm -f $seqres.full
 
@@ -78,11 +79,7 @@ _run_btrfs_util_prog subvolume snapshot $SCRATCH_MNT "$SCRATCH_MNT/snaps/snap2"
 
 _scratch_unmount
 
-# generate a qgroup report and look for inconsistent groups
-$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
-			grep -q -E "Counts for qgroup.*are different"
-if [ $? -ne 0 ]; then
-	status=0
-fi
 
+# qgroup will be checked by fstest at _check_scratch_fs()
+status=0
 exit
diff --git a/tests/btrfs/123 b/tests/btrfs/123
index e89d541..52d87fb 100755
--- a/tests/btrfs/123
+++ b/tests/btrfs/123
@@ -53,6 +53,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 _scratch_mkfs
 # Need to use inline extents to fill metadata rapidly
@@ -76,9 +77,7 @@ _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 _run_btrfs_util_prog balance start -d $SCRATCH_MNT
 
 _scratch_unmount
-# generate a qgroup report and look for inconsistent groups
-$BTRFS_UTIL_PROG check --qgroup-report $SCRATCH_DEV 2>&1 | \
-		grep -E "Counts for qgroup.*are different"
+# qgroup will be check at _check_scratch_fs() by fstest
 
 # success, all done
 status=0
diff --git a/tests/btrfs/126 b/tests/btrfs/126
index cc51f4a..c06c00b 100755
--- a/tests/btrfs/126
+++ b/tests/btrfs/126
@@ -49,6 +49,7 @@ rm -f $seqres.full
 _supported_fs btrfs
 _supported_os Linux
 _require_scratch
+_require_btrfs_qgroup_report
 
 _scratch_mkfs
 # Use enospc_debug mount option to trigger restrict space info check
-- 
2.7.4

Re: [PATCH v2 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code

[Eryu Guan]

On Thu, Dec 08, 2016 at 04:12:13PM +0800, Qu Wenruo wrote:
> Introduce new _require_btrfs_qgroup_report function, which will check
> the accessibility to "btrfs check --qgroup-report", then set a global
> flag to info _check_scratch_fs() to do extra qgroup check.
> 
> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> ---
> v2:
>   Use "${RESULT_DIR}/require_scratch.require_qgroup_report" instead of
>   global variant
>   Rebased to latest master
>   Replace btrfsck with $BTRFS_UTIL_PROG check.
[snip]
> diff --git a/tests/btrfs/042 b/tests/btrfs/042
> index 498ccc9..dc9b762 100755
> --- a/tests/btrfs/042
> +++ b/tests/btrfs/042
> @@ -43,6 +43,7 @@ _cleanup()
>  _supported_fs btrfs
>  _supported_os Linux
>  _require_scratch
> +_require_btrfs_qgroup_report
>  
>  rm -f $seqres.full
>  
> @@ -84,10 +85,7 @@ for i in `seq 10 -1 1`; do
>  	total_written=$(($total_written+$filesize))
>  done
>  
> -#check if total written exceeds limit
> -if [ $total_written -gt $LIMIT_SIZE ];then
> -	_fail "total written should be less than $LIMIT_SIZE"
> -fi
> +# qgroup will be checked automatically at _check_scratch_fs() by fstest

This doesn't look like an equivalent replacement, and btrfs/042 fails
for me after this update (wrong qgroup numbers) on 4.9-rc4 kernel. Is
this change intentional?

Thanks,
Eryu

Re: [PATCH v2 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code

[Qu Wenruo]

At 12/08/2016 04:47 PM, Eryu Guan wrote:
> On Thu, Dec 08, 2016 at 04:12:13PM +0800, Qu Wenruo wrote:
>> Introduce new _require_btrfs_qgroup_report function, which will check
>> the accessibility to "btrfs check --qgroup-report", then set a global
>> flag to info _check_scratch_fs() to do extra qgroup check.
>>
>> Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
>> ---
>> v2:
>>   Use "${RESULT_DIR}/require_scratch.require_qgroup_report" instead of
>>   global variant
>>   Rebased to latest master
>>   Replace btrfsck with $BTRFS_UTIL_PROG check.
> [snip]
>> diff --git a/tests/btrfs/042 b/tests/btrfs/042
>> index 498ccc9..dc9b762 100755
>> --- a/tests/btrfs/042
>> +++ b/tests/btrfs/042
>> @@ -43,6 +43,7 @@ _cleanup()
>>  _supported_fs btrfs
>>  _supported_os Linux
>>  _require_scratch
>> +_require_btrfs_qgroup_report
>>
>>  rm -f $seqres.full
>>
>> @@ -84,10 +85,7 @@ for i in `seq 10 -1 1`; do
>>  	total_written=$(($total_written+$filesize))
>>  done
>>
>> -#check if total written exceeds limit
>> -if [ $total_written -gt $LIMIT_SIZE ];then
>> -	_fail "total written should be less than $LIMIT_SIZE"
>> -fi
>> +# qgroup will be checked automatically at _check_scratch_fs() by fstest
>
> This doesn't look like an equivalent replacement, and btrfs/042 fails
> for me after this update (wrong qgroup numbers) on 4.9-rc4 kernel. Is
> this change intentional?
>
> Thanks,
> Eryu
>
>
Sorry, just a typo.For 042 only the _require_btrfs_qgroup_report line is 
need.

I also ran the test with v4.9-rc6 with some new btrfs patches (David's 
for-next branch), it ran without problem.

So the error seems to be a fixed btrfs qgroup problem.

Thanks,
Qu

Re: [PATCH v2 2/2] fstests: btrfs: Use _require_btrfs_qgroup_report to replace open code

[Eryu Guan]

On Thu, Dec 08, 2016 at 05:20:49PM +0800, Qu Wenruo wrote:
> 
> 
> At 12/08/2016 04:47 PM, Eryu Guan wrote:
> > On Thu, Dec 08, 2016 at 04:12:13PM +0800, Qu Wenruo wrote:
> > > Introduce new _require_btrfs_qgroup_report function, which will check
> > > the accessibility to "btrfs check --qgroup-report", then set a global
> > > flag to info _check_scratch_fs() to do extra qgroup check.
> > > 
> > > Signed-off-by: Qu Wenruo <quwenruo@cn.fujitsu.com>
> > > ---
> > > v2:
> > >   Use "${RESULT_DIR}/require_scratch.require_qgroup_report" instead of
> > >   global variant
> > >   Rebased to latest master
> > >   Replace btrfsck with $BTRFS_UTIL_PROG check.
> > [snip]
> > > diff --git a/tests/btrfs/042 b/tests/btrfs/042
> > > index 498ccc9..dc9b762 100755
> > > --- a/tests/btrfs/042
> > > +++ b/tests/btrfs/042
> > > @@ -43,6 +43,7 @@ _cleanup()
> > >  _supported_fs btrfs
> > >  _supported_os Linux
> > >  _require_scratch
> > > +_require_btrfs_qgroup_report
> > > 
> > >  rm -f $seqres.full
> > > 
> > > @@ -84,10 +85,7 @@ for i in `seq 10 -1 1`; do
> > >  	total_written=$(($total_written+$filesize))
> > >  done
> > > 
> > > -#check if total written exceeds limit
> > > -if [ $total_written -gt $LIMIT_SIZE ];then
> > > -	_fail "total written should be less than $LIMIT_SIZE"
> > > -fi
> > > +# qgroup will be checked automatically at _check_scratch_fs() by fstest
> > 
> > This doesn't look like an equivalent replacement, and btrfs/042 fails
> > for me after this update (wrong qgroup numbers) on 4.9-rc4 kernel. Is
> > this change intentional?
> > 
> > Thanks,
> > Eryu
> > 
> > 
> Sorry, just a typo.For 042 only the _require_btrfs_qgroup_report line is
> need.
> 
> I also ran the test with v4.9-rc6 with some new btrfs patches (David's
> for-next branch), it ran without problem.
> 
> So the error seems to be a fixed btrfs qgroup problem.

Ok, good to know. Can you please send an updated patch?

Thanks,
Eryu