All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 1/2] common/dump: do not override test cleanup trap
@ 2019-01-27  7:50 Amir Goldstein
  2019-01-27  7:50 ` [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2019-01-27  7:50 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Dave Chinner, Zorro Lang, Eric Sandeen, Darrick J . Wong,
	fstests, linux-xfs

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 <amir73il@gmail.com>
---
 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(-)

diff --git a/common/dump b/common/dump
index 4d1a1607..89fa0391 100644
--- a/common/dump
+++ b/common/dump
@@ -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
diff --git a/tests/xfs/022 b/tests/xfs/022
index e1162798..e668a6fc 100755
--- a/tests/xfs/022
+++ b/tests/xfs/022
@@ -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
diff --git a/tests/xfs/023 b/tests/xfs/023
index b0763402..f7805b85 100755
--- a/tests/xfs/023
+++ b/tests/xfs/023
@@ -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
diff --git a/tests/xfs/024 b/tests/xfs/024
index bf222150..b4ecd790 100755
--- a/tests/xfs/024
+++ b/tests/xfs/024
@@ -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
diff --git a/tests/xfs/025 b/tests/xfs/025
index 5ce844e6..74039062 100755
--- a/tests/xfs/025
+++ b/tests/xfs/025
@@ -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
diff --git a/tests/xfs/026 b/tests/xfs/026
index d4fd636c..96f71293 100755
--- a/tests/xfs/026
+++ b/tests/xfs/026
@@ -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
diff --git a/tests/xfs/027 b/tests/xfs/027
index 8459c5ad..7a501f03 100755
--- a/tests/xfs/027
+++ b/tests/xfs/027
@@ -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
diff --git a/tests/xfs/028 b/tests/xfs/028
index b71039b9..bedfa3d3 100755
--- a/tests/xfs/028
+++ b/tests/xfs/028
@@ -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
diff --git a/tests/xfs/035 b/tests/xfs/035
index 1874173f..2e09c35c 100755
--- a/tests/xfs/035
+++ b/tests/xfs/035
@@ -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
diff --git a/tests/xfs/036 b/tests/xfs/036
index 8d61a719..a7e7d572 100755
--- a/tests/xfs/036
+++ b/tests/xfs/036
@@ -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
diff --git a/tests/xfs/037 b/tests/xfs/037
index 478157e4..aea557fc 100755
--- a/tests/xfs/037
+++ b/tests/xfs/037
@@ -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
diff --git a/tests/xfs/038 b/tests/xfs/038
index 43fdc2c3..8ae93dad 100755
--- a/tests/xfs/038
+++ b/tests/xfs/038
@@ -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
diff --git a/tests/xfs/039 b/tests/xfs/039
index 79284afd..7b213e33 100755
--- a/tests/xfs/039
+++ b/tests/xfs/039
@@ -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
diff --git a/tests/xfs/043 b/tests/xfs/043
index b60c2870..5db2d6f9 100755
--- a/tests/xfs/043
+++ b/tests/xfs/043
@@ -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
diff --git a/tests/xfs/046 b/tests/xfs/046
index c5358c29..0955e453 100755
--- a/tests/xfs/046
+++ b/tests/xfs/046
@@ -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
diff --git a/tests/xfs/047 b/tests/xfs/047
index ddf871f8..2fdfef14 100755
--- a/tests/xfs/047
+++ b/tests/xfs/047
@@ -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
diff --git a/tests/xfs/055 b/tests/xfs/055
index 8a30d4b3..58144e98 100755
--- a/tests/xfs/055
+++ b/tests/xfs/055
@@ -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
diff --git a/tests/xfs/056 b/tests/xfs/056
index 533fef2e..b0f70e90 100755
--- a/tests/xfs/056
+++ b/tests/xfs/056
@@ -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
diff --git a/tests/xfs/059 b/tests/xfs/059
index 949dbe81..0121872a 100755
--- a/tests/xfs/059
+++ b/tests/xfs/059
@@ -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
diff --git a/tests/xfs/060 b/tests/xfs/060
index ca709983..a7048a66 100755
--- a/tests/xfs/060
+++ b/tests/xfs/060
@@ -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
diff --git a/tests/xfs/061 b/tests/xfs/061
index fcea0ee8..f8a639fc 100755
--- a/tests/xfs/061
+++ b/tests/xfs/061
@@ -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
diff --git a/tests/xfs/063 b/tests/xfs/063
index f4c65807..b6d4c03a 100755
--- a/tests/xfs/063
+++ b/tests/xfs/063
@@ -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
diff --git a/tests/xfs/064 b/tests/xfs/064
index f8fc382a..8adb406a 100755
--- a/tests/xfs/064
+++ b/tests/xfs/064
@@ -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
diff --git a/tests/xfs/065 b/tests/xfs/065
index 3fcdb604..c3472486 100755
--- a/tests/xfs/065
+++ b/tests/xfs/065
@@ -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
diff --git a/tests/xfs/066 b/tests/xfs/066
index b7da7966..90e1251c 100755
--- a/tests/xfs/066
+++ b/tests/xfs/066
@@ -22,6 +22,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }
diff --git a/tests/xfs/068 b/tests/xfs/068
index c755bc3e..7f5900fc 100755
--- a/tests/xfs/068
+++ b/tests/xfs/068
@@ -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
diff --git a/tests/xfs/266 b/tests/xfs/266
index 9084f5b9..73c1096d 100755
--- a/tests/xfs/266
+++ b/tests/xfs/266
@@ -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
diff --git a/tests/xfs/267 b/tests/xfs/267
index 8887f4c1..d13ec19a 100755
--- a/tests/xfs/267
+++ b/tests/xfs/267
@@ -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.
diff --git a/tests/xfs/268 b/tests/xfs/268
index c2686302..fa5b9283 100755
--- a/tests/xfs/268
+++ b/tests/xfs/268
@@ -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.
diff --git a/tests/xfs/281 b/tests/xfs/281
index 9df6154c..43534f10 100755
--- a/tests/xfs/281
+++ b/tests/xfs/281
@@ -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
diff --git a/tests/xfs/282 b/tests/xfs/282
index bb4975c4..3f0d8e59 100755
--- a/tests/xfs/282
+++ b/tests/xfs/282
@@ -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
diff --git a/tests/xfs/283 b/tests/xfs/283
index e647cef9..eab9f96b 100755
--- a/tests/xfs/283
+++ b/tests/xfs/283
@@ -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
diff --git a/tests/xfs/287 b/tests/xfs/287
index bde15e94..8dc754a5 100755
--- a/tests/xfs/287
+++ b/tests/xfs/287
@@ -23,8 +23,8 @@ rm -f $seqres.full
 
 _cleanup()
 {
+	_cleanup_dump
 	cd /
-	_scratch_unmount 2>/dev/null
 	rm -rf $tmp.*
 }
 
diff --git a/tests/xfs/296 b/tests/xfs/296
index bc190faf..e17adaa1 100755
--- a/tests/xfs/296
+++ b/tests/xfs/296
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }
diff --git a/tests/xfs/301 b/tests/xfs/301
index 440d314b..231e4a18 100755
--- a/tests/xfs/301
+++ b/tests/xfs/301
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }
diff --git a/tests/xfs/302 b/tests/xfs/302
index de619874..92806a23 100755
--- a/tests/xfs/302
+++ b/tests/xfs/302
@@ -17,6 +17,7 @@ trap "_cleanup; exit \$status" 0 1 2 3 15
 
 _cleanup()
 {
+    _cleanup_dump
     cd /
     rm -f $tmp.*
 }
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count
  2019-01-27  7:50 [PATCH v2 1/2] common/dump: do not override test cleanup trap Amir Goldstein
@ 2019-01-27  7:50 ` Amir Goldstein
  2019-02-03 12:40   ` Eryu Guan
  0 siblings, 1 reply; 4+ messages in thread
From: Amir Goldstein @ 2019-01-27  7:50 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Dave Chinner, Zorro Lang, Eric Sandeen, Darrick J . Wong,
	fstests, linux-xfs

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 <amir73il@gmail.com>
---
 common/dump       |  7 +++++++
 tests/xfs/068     | 14 +++++++++++++-
 tests/xfs/068.out |  2 +-
 3 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/common/dump b/common/dump
index 89fa0391..f112fc37 100644
--- a/common/dump
+++ b/common/dump
@@ -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
diff --git a/tests/xfs/068 b/tests/xfs/068
index 7f5900fc..264a9e96 100755
--- a/tests/xfs/068
+++ b/tests/xfs/068
@@ -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
diff --git a/tests/xfs/068.out b/tests/xfs/068.out
index fa3a5523..2b276b77 100644
--- a/tests/xfs/068.out
+++ b/tests/xfs/068.out
@@ -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
-- 
2.17.1

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count
  2019-01-27  7:50 ` [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count Amir Goldstein
@ 2019-02-03 12:40   ` Eryu Guan
  2019-02-03 13:07     ` Amir Goldstein
  0 siblings, 1 reply; 4+ messages in thread
From: Eryu Guan @ 2019-02-03 12:40 UTC (permalink / raw)
  To: Amir Goldstein
  Cc: Dave Chinner, Zorro Lang, Eric Sandeen, Darrick J . Wong,
	fstests, linux-xfs

On Sun, Jan 27, 2019 at 09:50:57AM +0200, Amir Goldstein wrote:
> 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 <amir73il@gmail.com>

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

> ---
>  common/dump       |  7 +++++++
>  tests/xfs/068     | 14 +++++++++++++-
>  tests/xfs/068.out |  2 +-
>  3 files changed, 21 insertions(+), 2 deletions(-)
> 
> diff --git a/common/dump b/common/dump
> index 89fa0391..f112fc37 100644
> --- a/common/dump
> +++ b/common/dump
> @@ -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
> diff --git a/tests/xfs/068 b/tests/xfs/068
> index 7f5900fc..264a9e96 100755
> --- a/tests/xfs/068
> +++ b/tests/xfs/068
> @@ -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

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

> +diff -u $tmp.before $tmp.after
>  
>  # success, all done
>  exit
> diff --git a/tests/xfs/068.out b/tests/xfs/068.out
> index fa3a5523..2b276b77 100644
> --- a/tests/xfs/068.out
> +++ b/tests/xfs/068.out
> @@ -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
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count
  2019-02-03 12:40   ` Eryu Guan
@ 2019-02-03 13:07     ` Amir Goldstein
  0 siblings, 0 replies; 4+ messages in thread
From: Amir Goldstein @ 2019-02-03 13:07 UTC (permalink / raw)
  To: Eryu Guan
  Cc: Dave Chinner, Zorro Lang, Eric Sandeen, Darrick J . Wong,
	fstests, linux-xfs

On Sun, Feb 3, 2019 at 2:40 PM Eryu Guan <guaneryu@gmail.com> wrote:
>
> On Sun, Jan 27, 2019 at 09:50:57AM +0200, Amir Goldstein wrote:
> > 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 <amir73il@gmail.com>
>
> Looks fine to me overall, thanks a lot for the fix! But I'd like an ACK
> from XFS folks :)
>
> > ---
> >  common/dump       |  7 +++++++
> >  tests/xfs/068     | 14 +++++++++++++-
> >  tests/xfs/068.out |  2 +-
> >  3 files changed, 21 insertions(+), 2 deletions(-)
> >
> > diff --git a/common/dump b/common/dump
> > index 89fa0391..f112fc37 100644
> > --- a/common/dump
> > +++ b/common/dump
> > @@ -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
> > diff --git a/tests/xfs/068 b/tests/xfs/068
> > index 7f5900fc..264a9e96 100755
> > --- a/tests/xfs/068
> > +++ b/tests/xfs/068
> > @@ -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
>
> 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.

^ permalink raw reply	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-02-03 13:07 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-27  7:50 [PATCH v2 1/2] common/dump: do not override test cleanup trap Amir Goldstein
2019-01-27  7:50 ` [PATCH v2 2/2] xfs/068: Verify actual file count instead of reported file count Amir Goldstein
2019-02-03 12:40   ` Eryu Guan
2019-02-03 13:07     ` Amir Goldstein

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.