common/dump: do not override test cleanup trap

PATCH In review Amir Goldstein 2 participants 4 messages Jan 27, 2019 – Feb 3, 2019

Share & tools

Amir Goldstein

author

2019-01-27 07:51 UTC #0 lore raw

PATCH v2 1/2 common/dump: do not override test cleanup trap

Instead, call _cleanup_dump explicitly from a private _cleanup. Remove the generic cleanup bits (rm $tmp.*) from _cleanup_dump.

The only xfs/dump test that had anything other than rm $tmp.* in _cleanup in xfs/287, but that was _scratch_unmount, which is not needed anyway.

Signed-off-by Amir Goldstein <[email protected]>

---
 common/dump   | 6 +-----
 tests/xfs/022 | 9 ++++++++-
 tests/xfs/023 | 9 ++++++++-
 tests/xfs/024 | 9 ++++++++-
 tests/xfs/025 | 9 ++++++++-
 tests/xfs/026 | 9 ++++++++-
 tests/xfs/027 | 9 ++++++++-
 tests/xfs/028 | 9 ++++++++-
 tests/xfs/035 | 9 ++++++++-
 tests/xfs/036 | 9 ++++++++-
 tests/xfs/037 | 9 ++++++++-
 tests/xfs/038 | 9 ++++++++-
 tests/xfs/039 | 9 ++++++++-
 tests/xfs/043 | 9 ++++++++-
 tests/xfs/046 | 9 ++++++++-
 tests/xfs/047 | 9 ++++++++-
 tests/xfs/055 | 9 ++++++++-
 tests/xfs/056 | 9 ++++++++-
 tests/xfs/059 | 9 ++++++++-
 tests/xfs/060 | 9 ++++++++-
 tests/xfs/061 | 9 ++++++++-
 tests/xfs/063 | 9 ++++++++-
 tests/xfs/064 | 9 ++++++++-
 tests/xfs/065 | 9 ++++++++-
 tests/xfs/066 | 1 +
 tests/xfs/068 | 9 ++++++++-
 tests/xfs/266 | 9 ++++++++-
 tests/xfs/267 | 9 ++++++++-
 tests/xfs/268 | 9 ++++++++-
 tests/xfs/281 | 9 ++++++++-
 tests/xfs/282 | 9 ++++++++-
 tests/xfs/283 | 9 ++++++++-
 tests/xfs/287 | 2 +-
 tests/xfs/296 | 1 +
 tests/xfs/301 | 1 +
 tests/xfs/302 | 1 +
 36 files changed, 246 insertions(+), 36 deletions(-)

common/dump +1 -5

index 4d1a1607..89fa0391 100644
@@ -45,9 +45,6 @@ session_label="stress_$seq"
 nobody=4 # define this uid/gid as a number
 do_quota_check=true # do quota check if quotas enabled
 
-# install our cleaner
-trap "_cleanup; exit \$status" 0 1 2 3 15
-
 # start inventory from a known base - move it aside for test
 for dir in /var/xfsdump/inventory /var/lib/xfsdump/inventory; do
     if [ -d $dir ]; then

@@ -227,7 +224,7 @@ _wipe_fs()
 # Cleanup created dirs and files
 # Called by trap
 #
-_cleanup()
+_cleanup_dump()
 {
     # Some tests include this before checking _supported_fs xfs
     # and the sleeps & checks here get annoying

@@ -236,7 +233,6 @@ _cleanup()
     fi
 
     cd $here
-    rm -f $tmp.*
 
     if [ -n "$DEBUGDUMP" ]; then
 	# save it for inspection

tests/xfs/022 +8 -1

index e1162798..e668a6fc 100755
@@ -17,7 +17,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -rf $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 . ./common/rc
 . ./common/dump

tests/xfs/023 +8 -1

index b0763402..f7805b85 100755
@@ -15,7 +15,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -rf $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/024 +8 -1

index bf222150..b4ecd790 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/025 +8 -1

index 5ce844e6..74039062 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/026 +8 -1

index d4fd636c..96f71293 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/027 +8 -1

index 8459c5ad..7a501f03 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/028 +8 -1

index b71039b9..bedfa3d3 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/035 +8 -1

index 1874173f..2e09c35c 100755
@@ -14,7 +14,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/036 +8 -1

index 8d61a719..a7e7d572 100755
@@ -14,7 +14,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/037 +8 -1

index 478157e4..aea557fc 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/038 +8 -1

index 43fdc2c3..8ae93dad 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/039 +8 -1

index 79284afd..7b213e33 100755
@@ -14,7 +14,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/043 +8 -1

index b60c2870..5db2d6f9 100755
@@ -16,7 +16,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/046 +8 -1

index c5358c29..0955e453 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/047 +8 -1

index ddf871f8..2fdfef14 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/055 +8 -1

index 8a30d4b3..58144e98 100755
@@ -14,7 +14,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/056 +8 -1

index 533fef2e..b0f70e90 100755
@@ -14,7 +14,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/059 +8 -1

index 949dbe81..0121872a 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/060 +8 -1

index ca709983..a7048a66 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/061 +8 -1

index fcea0ee8..f8a639fc 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/063 +8 -1

index f4c65807..b6d4c03a 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/064 +8 -1

index f8fc382a..8adb406a 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/065 +8 -1

index 3fcdb604..c3472486 100755
@@ -16,7 +16,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/066 +1 -0

index b7da7966..90e1251c 100755
@@ -22,6 +22,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }

tests/xfs/068 +8 -1

index c755bc3e..7f5900fc 100755
@@ -18,7 +18,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -rf $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 . ./common/rc
 . ./common/dump

tests/xfs/266 +8 -1

index 9084f5b9..73c1096d 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 #
 # Add a new file and append a subset of the fill'ed files

tests/xfs/267 +8 -1

index 8887f4c1..d13ec19a 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 #
 # create a 40 MiB file with an extended attr.

tests/xfs/268 +8 -1

index c2686302..fa5b9283 100755
@@ -15,7 +15,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=0	# success is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 #
 # create two 12 MiB files with extended attrs.

tests/xfs/281 +8 -1

index 9df6154c..43534f10 100755
@@ -13,7 +13,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/282 +8 -1

index bb4975c4..3f0d8e59 100755
@@ -15,7 +15,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/283 +8 -1

index e647cef9..eab9f96b 100755
@@ -15,7 +15,14 @@ echo "QA output created by $seq"
 here=`pwd`
 tmp=/tmp/$$
 status=1	# failure is the default!
-trap "rm -f $tmp.*; exit \$status" 0 1 2 3 15
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+_cleanup()
+{
+	_cleanup_dump
+	cd /
+	rm -f $tmp.*
+}
 
 # get standard environment, filters and checks
 . ./common/rc

tests/xfs/287 +1 -1

index bde15e94..8dc754a5 100755
@@ -23,8 +23,8 @@ rm -f $seqres.full
 
 _cleanup()
 {
+	_cleanup_dump
 	cd /
-	_scratch_unmount 2>/dev/null
 	rm -rf $tmp.*
 }

tests/xfs/296 +1 -0

index bc190faf..e17adaa1 100755
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }

tests/xfs/301 +1 -0

index 440d314b..231e4a18 100755
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }

tests/xfs/302 +1 -0

index de619874..92806a23 100755
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }

Signature · 1 line

2.17.1

Amir Goldstein

author

2019-01-27 07:51 UTC #1 lore raw

PATCH v2 2/2 xfs/068: Verify actual file count instead of reported file count

This test has the number of files/dirs created by xfsrestore hardcoded in golden output.

When fsstress is added new ops, the number of files/dirs created with the same random seed changes and this regularly breaks this test, so when new fsstress ops are added they should be either added to the dump test blacklist or golden output of this test needs to be ammended to reflect the change.

The golden output includes only the file count reported by xfsrestore and test does not even verify that this is the correct file count. Instead, leave the golden output nuetral and explicitly verify that file count before and after the test are the same.

With this change, the test becomes agnostic to fsstress ops and we could also stop blacklisting clone/dedup/copy ops if we want.

Signed-off-by Amir Goldstein <[email protected]>

---
 common/dump       |  7 +++++++
 tests/xfs/068     | 14 +++++++++++++-
 tests/xfs/068.out |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

common/dump +7 -0

index 89fa0391..f112fc37 100644
@@ -1514,6 +1514,13 @@ _check_quota_file()
    _check_quota 'xfsdump_quotas' 'xfsdump_quotas_group' 'xfsdump_quotas_proj'
 }
 
+_count_dumpdir_files()
+{
+	local ndirs=$(find $dump_dir -type d | wc -l)
+	local nents=$(find $dump_dir | wc -l)
+
+	echo "$ndirs directories and $nents entries"
+}
 
 # make sure this script returns success
 /bin/true

tests/xfs/068 +13 -1

index 7f5900fc..264a9e96 100755
@@ -30,12 +30,24 @@ _cleanup()
 . ./common/rc
 . ./common/dump
 
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
 
 _create_dumpdir_stress_num 4096
-_do_dump_restore
+
+echo -n "Before: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.before >> $seqres.full
+
+# filter out the file count, it changes as fsstress adds new operations
+_do_dump_restore | sed -e "/entries processed$/s/[0-9][0-9]*/NUM/g"
+
+echo -n "After: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.after >> $seqres.full
+diff -u $tmp.before $tmp.after
 
 # success, all done
 exit

tests/xfs/068.out +1 -1

index fa3a5523..2b276b77 100644
@@ -22,7 +22,7 @@ xfsrestore: session id: ID
 xfsrestore: media ID: ID
 xfsrestore: searching media for directory dump
 xfsrestore: reading directories
-xfsrestore: 383 directories and 1335 entries processed
+xfsrestore: NUM directories and NUM entries processed
 xfsrestore: directory post-processing
 xfsrestore: restoring non-directory files
 xfsrestore: restore complete: SECS seconds elapsed

Signature · 1 line

2.17.1

Eryu Guan

2019-02-03 12:40 UTC #2 lore raw

Re: [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count

Quoted 14 lines · Amir Goldstein

This test has the number of files/dirs created by xfsrestore hardcoded in golden output.
When fsstress is added new ops, the number of files/dirs created with the same random seed changes and this regularly breaks this test, so when new fsstress ops are added they should be either added to the dump test blacklist or golden output of this test needs to be ammended to reflect the change.
The golden output includes only the file count reported by xfsrestore and test does not even verify that this is the correct file count. Instead, leave the golden output nuetral and explicitly verify that file count before and after the test are the same.

Quoted 4 lines

With this change, the test becomes agnostic to fsstress ops and we could also stop blacklisting clone/dedup/copy ops if we want.
Signed-off-by Amir Goldstein <[email protected]>

Looks fine to me overall, thanks a lot for the fix! But I'd like an ACK from XFS folks :)

Quoted 40 lines

---
 common/dump       |  7 +++++++
 tests/xfs/068     | 14 +++++++++++++-
 tests/xfs/068.out |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

common/dump +7 -0

index 89fa0391..f112fc37 100644
@@ -1514,6 +1514,13 @@ _check_quota_file()
    _check_quota 'xfsdump_quotas' 'xfsdump_quotas_group' 'xfsdump_quotas_proj'
 }
 
+_count_dumpdir_files()
+{
+	local ndirs=$(find $dump_dir -type d | wc -l)
+	local nents=$(find $dump_dir | wc -l)
+
+	echo "$ndirs directories and $nents entries"
+}
 
 # make sure this script returns success
 /bin/true

tests/xfs/068 +3 -0

index 7f5900fc..264a9e96 100755
@@ -30,12 +30,24 @@ _cleanup()
 . ./common/rc
 . ./common/dump
 
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
 
 _create_dumpdir_stress_num 4096

tests/xfs/068 _cleanup() · line 49

Quoted 10 lines

tests/xfs/068 +9 -1

-_do_dump_restore
+
+echo -n "Before: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.before >> $seqres.full
+
+# filter out the file count, it changes as fsstress adds new operations
+_do_dump_restore | sed -e "/entries processed$/s/[0-9][0-9]*/NUM/g"
+
+echo -n "After: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.after >> $seqres.full

The "Before" and "After" both count the file entries in $dump_dir, so they should always be the same, we should count $restore_dir at "After"?

Thanks,
Eryu

Quoted 20 lines

tests/xfs/068 +1 -0

+diff -u $tmp.before $tmp.after
 
 # success, all done
 exit

tests/xfs/068.out +1 -1

index fa3a5523..2b276b77 100644
@@ -22,7 +22,7 @@ xfsrestore: session id: ID
 xfsrestore: media ID: ID
 xfsrestore: searching media for directory dump
 xfsrestore: reading directories
-xfsrestore: 383 directories and 1335 entries processed
+xfsrestore: NUM directories and NUM entries processed
 xfsrestore: directory post-processing
 xfsrestore: restoring non-directory files
 xfsrestore: restore complete: SECS seconds elapsed

Signature · 1 line

2.17.1

Amir Goldstein

author

2019-02-03 13:07 UTC #3 lore raw

Re: [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count

Quoted 63 lines · Eryu Guan

Quoted 14 lines · Amir Goldstein
This test has the number of files/dirs created by xfsrestore hardcoded in golden output.
When fsstress is added new ops, the number of files/dirs created with the same random seed changes and this regularly breaks this test, so when new fsstress ops are added they should be either added to the dump test blacklist or golden output of this test needs to be ammended to reflect the change.
The golden output includes only the file count reported by xfsrestore and test does not even verify that this is the correct file count. Instead, leave the golden output nuetral and explicitly verify that file count before and after the test are the same.
Quoted 4 lines
With this change, the test becomes agnostic to fsstress ops and we could also stop blacklisting clone/dedup/copy ops if we want.
Signed-off-by Amir Goldstein <[email protected]>
Looks fine to me overall, thanks a lot for the fix! But I'd like an ACK from XFS folks :)
Quoted 39 lines
---
 common/dump       |  7 +++++++
 tests/xfs/068     | 14 +++++++++++++-
 tests/xfs/068.out |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)
common/dump +7 -0
index 89fa0391..f112fc37 100644
@@ -1514,6 +1514,13 @@ _check_quota_file()
    _check_quota 'xfsdump_quotas' 'xfsdump_quotas_group' 'xfsdump_quotas_proj'
 }
 
+_count_dumpdir_files()
+{
+     local ndirs=$(find $dump_dir -type d | wc -l)
+     local nents=$(find $dump_dir | wc -l)
+
+     echo "$ndirs directories and $nents entries"
+}
 
 # make sure this script returns success
 /bin/true
tests/xfs/068 +3 -0
index 7f5900fc..264a9e96 100755
@@ -30,12 +30,24 @@ _cleanup()
 . ./common/rc
 . ./common/dump
 
+# remove previous $seqres.full before test
+rm -f $seqres.full
+
 # real QA test starts here
 _supported_fs xfs
 _supported_os Linux
 

Quoted 15 lines

tests/xfs/068 _cleanup() · line 49
Quoted 11 lines
tests/xfs/068 +9 -1
 _create_dumpdir_stress_num 4096
-_do_dump_restore
+
+echo -n "Before: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.before >> $seqres.full
+
+# filter out the file count, it changes as fsstress adds new operations
+_do_dump_restore | sed -e "/entries processed$/s/[0-9][0-9]*/NUM/g"
+
+echo -n "After: " >> $seqres.full
+_count_dumpdir_files | tee $tmp.after >> $seqres.full
The "Before" and "After" both count the file entries in $dump_dir, so they should always be the same, we should count $restore_dir at "After"?

Certainly. We should count $restore_dir/$dump_sdir. I there is an ACK to concept, I will send a fix.

Thanks,
Amir.