Skip to contentSkip to conversation

fstests: btrfs/057: Fix false alerts due to orphan files

PATCH In review Qu Wenruo 3 participants 4 messages Oct 29, 2018 Btrfs file system

lore
Share & tools
lore

Qu Wenruo

author
#0 lore raw

PATCH 1/1 fstests: btrfs/057: Fix false alerts due to orphan files

For latest kernel, there is a chance that btrfs/057 reports false errors.

The false error would look like:
btrfs/057 4s ... - output mismatch (see /home/adam/xfstests-dev/results//btrfs/057.out.bad)

      --- tests/btrfs/057.out	2017-08-21 09:25:33.166666666 +0800
      +++ /home/adam/xfstests-dev/results//btrfs/057.out.bad	2018-10-29 14:07:28.443651293 +0800
      @@ -1,3 +1,3 @@
       QA output created by 057
       4096 4096
      -4096 4096
      +28672 28672

This is related to the fact that "btrfs subvolume sync" (or vanilla sync) will not ensure orphan (unlinked but still exist) files to be removed.

In fact, for that false error case, if inspecting the fs after umount, its qgroup number is correct and btrfs check won't report qgroup error.

To fix the false alerts, just skip any manual qgroup number comparison, and let fsck done after the test case to detect problem.

This also elimiate the necessary of using specified mount and mkfs option, allowing us to improve coverage.

---
 tests/btrfs/057     | 17 ++++-------------
 tests/btrfs/057.out |  3 +--
 2 files changed, 5 insertions(+), 15 deletions(-)
tests/btrfs/057 +4 -13
index b019f4e1..0b5a36d3 100755
@@ -33,12 +33,9 @@ _require_scratch
 rm -f $seqres.full
 
 # use small leaf size to get higher btree height.
-run_check _scratch_mkfs "-b 1g --nodesize 4096"
+run_check _scratch_mkfs "-b 1g"
 
-# inode cache is saved in the FS tree itself for every
-# individual FS tree,that affects the sizes reported by qgroup show
-# so we need to explicitly turn it off to get consistent values.
-_scratch_mount "-o noinode_cache"
+_scratch_mount
 
 # -w ensures that the only ops are ones which cause write I/O
 run_check $FSSTRESS_PROG -d $SCRATCH_MNT -w -p 5 -n 1000 \
@@ -53,14 +50,8 @@ run_check $FSSTRESS_PROG -d $SCRATCH_MNT/snap1 -w -p 5 -n 1000 \
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 
-# remove all file/dir other than subvolume
-rm -rf $SCRATCH_MNT/snap1/* >& /dev/null
-rm -rf $SCRATCH_MNT/p* >& /dev/null
-
-_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
-units=`_btrfs_qgroup_units`
-$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | $SED_PROG -n '/[0-9]/p' \
-	| $AWK_PROG '{print $2" "$3}'
+echo "Silence is golden"
+# btrfs check will detect any qgroup number mismatch.
 
 status=0
 exit
tests/btrfs/057.out +1 -2
index 60cb92d0..185023c7 100644
@@ -1,3 +1,2 @@
 QA output created by 057
-4096 4096
-4096 4096
+Silence is golden
Signature · 1 line
2.18.0
#1 lore raw
Quoted 3 lines · Qu Wenruo

For latest kernel, there is a chance that btrfs/057 reports false errors.

By latest kernel you mean 4.20?

Quoted 11 lines

The false error would look like:
btrfs/057 4s ... - output mismatch (see /home/adam/xfstests-dev/results//btrfs/057.out.bad)

      --- tests/btrfs/057.out   2017-08-21 09:25:33.166666666 +0800
      +++ /home/adam/xfstests-dev/results//btrfs/057.out.bad    2018-10-29 14:07:28.443651293 +0800
      @@ -1,3 +1,3 @@
       QA output created by 057
       4096 4096
      -4096 4096
      +28672 28672
Quoted 3 lines

This is related to the fact that "btrfs subvolume sync" (or vanilla sync) will not ensure orphan (unlinked but still exist) files to be removed.

So when did that happen, which commit introduced the behaviour change?

Quoted 65 lines

In fact, for that false error case, if inspecting the fs after umount, its qgroup number is correct and btrfs check won't report qgroup error.

To fix the false alerts, just skip any manual qgroup number comparison, and let fsck done after the test case to detect problem.

This also elimiate the necessary of using specified mount and mkfs option, allowing us to improve coverage.

---
 tests/btrfs/057     | 17 ++++-------------
 tests/btrfs/057.out |  3 +--
 2 files changed, 5 insertions(+), 15 deletions(-)
tests/btrfs/057 +4 -13
index b019f4e1..0b5a36d3 100755
@@ -33,12 +33,9 @@ _require_scratch
 rm -f $seqres.full
 
 # use small leaf size to get higher btree height.
-run_check _scratch_mkfs "-b 1g --nodesize 4096"
+run_check _scratch_mkfs "-b 1g"
 
-# inode cache is saved in the FS tree itself for every
-# individual FS tree,that affects the sizes reported by qgroup show
-# so we need to explicitly turn it off to get consistent values.
-_scratch_mount "-o noinode_cache"
+_scratch_mount
 
 # -w ensures that the only ops are ones which cause write I/O
 run_check $FSSTRESS_PROG -d $SCRATCH_MNT -w -p 5 -n 1000 \
@@ -53,14 +50,8 @@ run_check $FSSTRESS_PROG -d $SCRATCH_MNT/snap1 -w -p 5 -n 1000 \
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 
-# remove all file/dir other than subvolume
-rm -rf $SCRATCH_MNT/snap1/* >& /dev/null
-rm -rf $SCRATCH_MNT/p* >& /dev/null
-
-_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
-units=`_btrfs_qgroup_units`
-$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | $SED_PROG -n '/[0-9]/p' \
-       | $AWK_PROG '{print $2" "$3}'
+echo "Silence is golden"
+# btrfs check will detect any qgroup number mismatch.
 
 status=0
 exit
tests/btrfs/057.out +1 -3
index 60cb92d0..185023c7 100644
@@ -1,3 +1,2 @@
 QA output created by 057
-4096 4096
-4096 4096
+Silence is golden
--

2.18.0

Signature · 3 lines
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”
#2 lore raw
Quoted 6 lines · Filipe Manana
Quoted 3 lines · Qu Wenruo

For latest kernel, there is a chance that btrfs/057 reports false errors.

By latest kernel you mean 4.20?

I mean almost all kernels.

Quoted 12 lines
Quoted 12 lines

The false error would look like:
btrfs/057 4s ... - output mismatch (see /home/adam/xfstests-dev/results//btrfs/057.out.bad)

      --- tests/btrfs/057.out   2017-08-21 09:25:33.166666666 +0800
      +++ /home/adam/xfstests-dev/results//btrfs/057.out.bad    2018-10-29 14:07:28.443651293 +0800
      @@ -1,3 +1,3 @@
       QA output created by 057
       4096 4096
      -4096 4096
      +28672 28672
Quoted 5 lines
Quoted 3 lines

This is related to the fact that "btrfs subvolume sync" (or vanilla sync) will not ensure orphan (unlinked but still exist) files to be removed.

So when did that happen, which commit introduced the behaviour change?

No behavior change, it's always the case.
Just not that easy to hit.

Thanks,
Qu

Quoted 68 lines
Quoted 66 lines

In fact, for that false error case, if inspecting the fs after umount, its qgroup number is correct and btrfs check won't report qgroup error.

To fix the false alerts, just skip any manual qgroup number comparison, and let fsck done after the test case to detect problem.

This also elimiate the necessary of using specified mount and mkfs option, allowing us to improve coverage.

---
 tests/btrfs/057     | 17 ++++-------------
 tests/btrfs/057.out |  3 +--
 2 files changed, 5 insertions(+), 15 deletions(-)
tests/btrfs/057 +4 -13
index b019f4e1..0b5a36d3 100755
@@ -33,12 +33,9 @@ _require_scratch
 rm -f $seqres.full
 
 # use small leaf size to get higher btree height.
-run_check _scratch_mkfs "-b 1g --nodesize 4096"
+run_check _scratch_mkfs "-b 1g"
 
-# inode cache is saved in the FS tree itself for every
-# individual FS tree,that affects the sizes reported by qgroup show
-# so we need to explicitly turn it off to get consistent values.
-_scratch_mount "-o noinode_cache"
+_scratch_mount
 
 # -w ensures that the only ops are ones which cause write I/O
 run_check $FSSTRESS_PROG -d $SCRATCH_MNT -w -p 5 -n 1000 \
@@ -53,14 +50,8 @@ run_check $FSSTRESS_PROG -d $SCRATCH_MNT/snap1 -w -p 5 -n 1000 \
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 
-# remove all file/dir other than subvolume
-rm -rf $SCRATCH_MNT/snap1/* >& /dev/null
-rm -rf $SCRATCH_MNT/p* >& /dev/null
-
-_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
-units=`_btrfs_qgroup_units`
-$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | $SED_PROG -n '/[0-9]/p' \
-       | $AWK_PROG '{print $2" "$3}'
+echo "Silence is golden"
+# btrfs check will detect any qgroup number mismatch.
 
 status=0
 exit
tests/btrfs/057.out +1 -3
index 60cb92d0..185023c7 100644
@@ -1,3 +1,2 @@
 QA output created by 057
-4096 4096
-4096 4096
+Silence is golden
--

2.18.0

Reviewed-by
#3 lore raw
Quoted 4 lines · Qu Wenruo

On 2018/10/29 下午5:52, Filipe Manana wrote:

Quoted 8 lines
Quoted 6 lines · Qu Wenruo
Quoted 3 lines

For latest kernel, there is a chance that btrfs/057 reports false errors.

By latest kernel you mean 4.20?

I mean almost all kernels.

So s/For latest kernel/For any recent kernel/ or something like that which isn't singular.

Quoted 17 lines
Quoted 17 lines
Quoted 16 lines

The false error would look like:
btrfs/057 4s ... - output mismatch (see /home/adam/xfstests-dev/results//btrfs/057.out.bad)

      --- tests/btrfs/057.out   2017-08-21 09:25:33.166666666 +0800
      +++ /home/adam/xfstests-dev/results//btrfs/057.out.bad    2018-10-29 14:07:28.443651293 +0800
      @@ -1,3 +1,3 @@
       QA output created by 057
       4096 4096
      -4096 4096
      +28672 28672

This is related to the fact that "btrfs subvolume sync" (or vanilla sync) will not ensure orphan (unlinked but still exist) files to be removed.

Quoted 21 lines
Quoted 1 line

So when did that happen, which commit introduced the behaviour change?

No behavior change, it's always the case.
Just not that easy to hit.

Thanks,
Qu

Quoted 13 lines
Quoted 13 lines

In fact, for that false error case, if inspecting the fs after umount, its qgroup number is correct and btrfs check won't report qgroup error.

To fix the false alerts, just skip any manual qgroup number comparison, and let fsck done after the test case to detect problem.

This also elimiate the necessary of using specified mount and mkfs option, allowing us to improve coverage.

Anyway, looks good to me.

Quoted 56 lines
Quoted 55 lines
Quoted 53 lines
---
 tests/btrfs/057     | 17 ++++-------------
 tests/btrfs/057.out |  3 +--
 2 files changed, 5 insertions(+), 15 deletions(-)
tests/btrfs/057 +4 -13
index b019f4e1..0b5a36d3 100755
@@ -33,12 +33,9 @@ _require_scratch
 rm -f $seqres.full
 
 # use small leaf size to get higher btree height.
-run_check _scratch_mkfs "-b 1g --nodesize 4096"
+run_check _scratch_mkfs "-b 1g"
 
-# inode cache is saved in the FS tree itself for every
-# individual FS tree,that affects the sizes reported by qgroup show
-# so we need to explicitly turn it off to get consistent values.
-_scratch_mount "-o noinode_cache"
+_scratch_mount
 
 # -w ensures that the only ops are ones which cause write I/O
 run_check $FSSTRESS_PROG -d $SCRATCH_MNT -w -p 5 -n 1000 \
@@ -53,14 +50,8 @@ run_check $FSSTRESS_PROG -d $SCRATCH_MNT/snap1 -w -p 5 -n 1000 \
 _run_btrfs_util_prog quota enable $SCRATCH_MNT
 _run_btrfs_util_prog quota rescan -w $SCRATCH_MNT
 
-# remove all file/dir other than subvolume
-rm -rf $SCRATCH_MNT/snap1/* >& /dev/null
-rm -rf $SCRATCH_MNT/p* >& /dev/null
-
-_run_btrfs_util_prog filesystem sync $SCRATCH_MNT
-units=`_btrfs_qgroup_units`
-$BTRFS_UTIL_PROG qgroup show $units $SCRATCH_MNT | $SED_PROG -n '/[0-9]/p' \
-       | $AWK_PROG '{print $2" "$3}'
+echo "Silence is golden"
+# btrfs check will detect any qgroup number mismatch.
 
 status=0
 exit
tests/btrfs/057.out +1 -3
index 60cb92d0..185023c7 100644
@@ -1,3 +1,2 @@
 QA output created by 057
-4096 4096
-4096 4096
+Silence is golden
--

2.18.0

Signature · 3 lines
Filipe David Manana,

“Whether you think you can, or you think you can't — you're right.”