fstests.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] Dump log cleanups
@ 2021-09-09 17:41 Catherine Hoang
  2021-09-09 17:41 ` [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log Catherine Hoang
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Catherine Hoang @ 2021-09-09 17:41 UTC (permalink / raw)
  To: linux-xfs, fstests

Hi all,

This cleanup set is a followup to the log attribute replay test that was
posted here:

https://lore.kernel.org/linux-xfs/20210901221006.125888-2-catherine.hoang@oracle.com/

This set renames the *_inject_logprint functions to *_remount_dump_log
and moves them to common/xfs.

Questions and feedback are appreciated!

Catherine

Catherine Hoang (3):
  xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log
  xfstests: Rename _test_inject_logprint to _test_remount_dump_log
  xfstests: Move *_dump_log routines to common/xfs

 common/inject | 26 --------------------------
 common/xfs    | 26 ++++++++++++++++++++++++++
 tests/xfs/312 |  2 +-
 tests/xfs/313 |  2 +-
 tests/xfs/314 |  2 +-
 tests/xfs/315 |  2 +-
 tests/xfs/316 |  2 +-
 tests/xfs/317 |  2 +-
 tests/xfs/318 |  2 +-
 tests/xfs/319 |  2 +-
 tests/xfs/320 |  2 +-
 tests/xfs/321 |  2 +-
 tests/xfs/322 |  2 +-
 tests/xfs/323 |  2 +-
 tests/xfs/324 |  2 +-
 tests/xfs/325 |  2 +-
 tests/xfs/326 |  2 +-
 tests/xfs/329 |  2 +-
 18 files changed, 42 insertions(+), 42 deletions(-)

-- 
2.25.1


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

* [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log
  2021-09-09 17:41 [PATCH 0/3] Dump log cleanups Catherine Hoang
@ 2021-09-09 17:41 ` Catherine Hoang
  2021-09-11 14:11   ` Allison Henderson
  2021-09-09 17:41 ` [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log Catherine Hoang
  2021-09-09 17:41 ` [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs Catherine Hoang
  2 siblings, 1 reply; 10+ messages in thread
From: Catherine Hoang @ 2021-09-09 17:41 UTC (permalink / raw)
  To: linux-xfs, fstests

Rename _scratch_inject_logprint to _scratch_remount_dump_log to better
describe what this function does. _scratch_remount_dump_log unmounts
and remounts the scratch device, dumping the log.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 common/inject | 2 +-
 tests/xfs/312 | 2 +-
 tests/xfs/313 | 2 +-
 tests/xfs/314 | 2 +-
 tests/xfs/315 | 2 +-
 tests/xfs/316 | 2 +-
 tests/xfs/317 | 2 +-
 tests/xfs/318 | 2 +-
 tests/xfs/319 | 2 +-
 tests/xfs/320 | 2 +-
 tests/xfs/321 | 2 +-
 tests/xfs/322 | 2 +-
 tests/xfs/323 | 2 +-
 tests/xfs/324 | 2 +-
 tests/xfs/325 | 2 +-
 tests/xfs/326 | 2 +-
 tests/xfs/329 | 2 +-
 17 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/common/inject b/common/inject
index 984ec209..3b731df7 100644
--- a/common/inject
+++ b/common/inject
@@ -113,7 +113,7 @@ _scratch_inject_error()
 }
 
 # Unmount and remount the scratch device, dumping the log
-_scratch_inject_logprint()
+_scratch_remount_dump_log()
 {
 	local opts="$1"
 
diff --git a/tests/xfs/312 b/tests/xfs/312
index 1fcf26ab..94f868fe 100755
--- a/tests/xfs/312
+++ b/tests/xfs/312
@@ -63,7 +63,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/313 b/tests/xfs/313
index 6d2f9fac..9c7cf5b9 100755
--- a/tests/xfs/313
+++ b/tests/xfs/313
@@ -63,7 +63,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/314 b/tests/xfs/314
index 5165393e..9ac311d0 100755
--- a/tests/xfs/314
+++ b/tests/xfs/314
@@ -64,7 +64,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/315 b/tests/xfs/315
index 958a8c99..105515ab 100755
--- a/tests/xfs/315
+++ b/tests/xfs/315
@@ -61,7 +61,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/316 b/tests/xfs/316
index cf0c5adc..f0af19d2 100755
--- a/tests/xfs/316
+++ b/tests/xfs/316
@@ -61,7 +61,7 @@ echo "CoW all the blocks"
 $XFS_IO_PROG -c "pwrite -W -S 0x67 -b $sz 0 $((blks * blksz))" $SCRATCH_MNT/file2 >> $seqres.full
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/317 b/tests/xfs/317
index 7eef67af..1ca2672d 100755
--- a/tests/xfs/317
+++ b/tests/xfs/317
@@ -54,7 +54,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "Check files"
 md5sum $SCRATCH_MNT/file0 | _filter_scratch
diff --git a/tests/xfs/318 b/tests/xfs/318
index d822e89a..38c7aa60 100755
--- a/tests/xfs/318
+++ b/tests/xfs/318
@@ -60,7 +60,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "Check files"
 md5sum $SCRATCH_MNT/file1 2>&1 | _filter_scratch
diff --git a/tests/xfs/319 b/tests/xfs/319
index 0f61c119..d64651fb 100755
--- a/tests/xfs/319
+++ b/tests/xfs/319
@@ -57,7 +57,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/320 b/tests/xfs/320
index f65f3ad1..d22d76d9 100755
--- a/tests/xfs/320
+++ b/tests/xfs/320
@@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "Check files"
 md5sum $SCRATCH_MNT/file1 | _filter_scratch
diff --git a/tests/xfs/321 b/tests/xfs/321
index daff4449..06a34347 100755
--- a/tests/xfs/321
+++ b/tests/xfs/321
@@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "Check files"
 md5sum $SCRATCH_MNT/file1 | _filter_scratch
diff --git a/tests/xfs/322 b/tests/xfs/322
index f36e54d8..89a2f741 100755
--- a/tests/xfs/322
+++ b/tests/xfs/322
@@ -56,7 +56,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "Check files"
 md5sum $SCRATCH_MNT/file1 | _filter_scratch
diff --git a/tests/xfs/323 b/tests/xfs/323
index f66a8ebf..66737da0 100755
--- a/tests/xfs/323
+++ b/tests/xfs/323
@@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/324 b/tests/xfs/324
index ca2f25ac..9909db62 100755
--- a/tests/xfs/324
+++ b/tests/xfs/324
@@ -61,7 +61,7 @@ echo "Reflink all the blocks"
 _cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file4
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/325 b/tests/xfs/325
index 3b98fd50..5b26b2b3 100755
--- a/tests/xfs/325
+++ b/tests/xfs/325
@@ -59,7 +59,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/326 b/tests/xfs/326
index bf5db08a..8b95a18a 100755
--- a/tests/xfs/326
+++ b/tests/xfs/326
@@ -71,7 +71,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log"
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 
 echo "FS should be online, touch should succeed"
 touch $SCRATCH_MNT/goodfs
diff --git a/tests/xfs/329 b/tests/xfs/329
index e57f6f7f..e9a30d05 100755
--- a/tests/xfs/329
+++ b/tests/xfs/329
@@ -52,7 +52,7 @@ echo "FS should be shut down, touch will fail"
 touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
 
 echo "Remount to replay log" | tee /dev/ttyprintk
-_scratch_inject_logprint >> $seqres.full
+_scratch_remount_dump_log >> $seqres.full
 new_nextents=$(_count_extents $testdir/file1)
 
 echo "Check extent count" | tee /dev/ttyprintk
-- 
2.25.1


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

* [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log
  2021-09-09 17:41 [PATCH 0/3] Dump log cleanups Catherine Hoang
  2021-09-09 17:41 ` [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log Catherine Hoang
@ 2021-09-09 17:41 ` Catherine Hoang
  2021-09-11 14:11   ` Allison Henderson
  2021-09-09 17:41 ` [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs Catherine Hoang
  2 siblings, 1 reply; 10+ messages in thread
From: Catherine Hoang @ 2021-09-09 17:41 UTC (permalink / raw)
  To: linux-xfs, fstests

Rename _test_inject_logprint to _test_remount_dump_log to better
describe what this function does. _test_remount_dump_log unmounts
and remounts the test device, dumping the log.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 common/inject | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/inject b/common/inject
index 3b731df7..b5334d4a 100644
--- a/common/inject
+++ b/common/inject
@@ -126,7 +126,7 @@ _scratch_remount_dump_log()
 }
 
 # Unmount and remount the test device, dumping the log
-_test_inject_logprint()
+_test_remount_dump_log()
 {
 	local opts="$1"
 
-- 
2.25.1


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

* [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs
  2021-09-09 17:41 [PATCH 0/3] Dump log cleanups Catherine Hoang
  2021-09-09 17:41 ` [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log Catherine Hoang
  2021-09-09 17:41 ` [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log Catherine Hoang
@ 2021-09-09 17:41 ` Catherine Hoang
  2021-09-11 14:11   ` Allison Henderson
  2021-09-12  8:13   ` Eryu Guan
  2 siblings, 2 replies; 10+ messages in thread
From: Catherine Hoang @ 2021-09-09 17:41 UTC (permalink / raw)
  To: linux-xfs, fstests

Move _scratch_remount_dump_log and _test_remount_dump_log from
common/inject to common/xfs. These routines do not inject errors and
should be placed with other xfs common functions.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 common/inject | 26 --------------------------
 common/xfs    | 26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/common/inject b/common/inject
index b5334d4a..6b590804 100644
--- a/common/inject
+++ b/common/inject
@@ -111,29 +111,3 @@ _scratch_inject_error()
 		_fail "Cannot inject error ${type} value ${value}."
 	fi
 }
-
-# Unmount and remount the scratch device, dumping the log
-_scratch_remount_dump_log()
-{
-	local opts="$1"
-
-	if test -n "$opts"; then
-		opts="-o $opts"
-	fi
-	_scratch_unmount
-	_scratch_dump_log
-	_scratch_mount "$opts"
-}
-
-# Unmount and remount the test device, dumping the log
-_test_remount_dump_log()
-{
-	local opts="$1"
-
-	if test -n "$opts"; then
-		opts="-o $opts"
-	fi
-	_test_unmount
-	_test_dump_log
-	_test_mount "$opts"
-}
diff --git a/common/xfs b/common/xfs
index bfb1bf1e..cda1f768 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1263,3 +1263,29 @@ _require_scratch_xfs_bigtime()
 		_notrun "bigtime feature not advertised on mount?"
 	_scratch_unmount
 }
+
+# Unmount and remount the scratch device, dumping the log
+_scratch_remount_dump_log()
+{
+	local opts="$1"
+
+	if test -n "$opts"; then
+		opts="-o $opts"
+	fi
+	_scratch_unmount
+	_scratch_dump_log
+	_scratch_mount "$opts"
+}
+
+# Unmount and remount the test device, dumping the log
+_test_remount_dump_log()
+{
+	local opts="$1"
+
+	if test -n "$opts"; then
+		opts="-o $opts"
+	fi
+	_test_unmount
+	_test_dump_log
+	_test_mount "$opts"
+}
-- 
2.25.1


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

* Re: [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log
  2021-09-09 17:41 ` [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log Catherine Hoang
@ 2021-09-11 14:11   ` Allison Henderson
  0 siblings, 0 replies; 10+ messages in thread
From: Allison Henderson @ 2021-09-11 14:11 UTC (permalink / raw)
  To: Catherine Hoang, linux-xfs, fstests



On 9/9/21 10:41 AM, Catherine Hoang wrote:
> Rename _scratch_inject_logprint to _scratch_remount_dump_log to better
> describe what this function does. _scratch_remount_dump_log unmounts
> and remounts the scratch device, dumping the log.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>

Looks good to me.
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/inject | 2 +-
>   tests/xfs/312 | 2 +-
>   tests/xfs/313 | 2 +-
>   tests/xfs/314 | 2 +-
>   tests/xfs/315 | 2 +-
>   tests/xfs/316 | 2 +-
>   tests/xfs/317 | 2 +-
>   tests/xfs/318 | 2 +-
>   tests/xfs/319 | 2 +-
>   tests/xfs/320 | 2 +-
>   tests/xfs/321 | 2 +-
>   tests/xfs/322 | 2 +-
>   tests/xfs/323 | 2 +-
>   tests/xfs/324 | 2 +-
>   tests/xfs/325 | 2 +-
>   tests/xfs/326 | 2 +-
>   tests/xfs/329 | 2 +-
>   17 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/common/inject b/common/inject
> index 984ec209..3b731df7 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -113,7 +113,7 @@ _scratch_inject_error()
>   }
>   
>   # Unmount and remount the scratch device, dumping the log
> -_scratch_inject_logprint()
> +_scratch_remount_dump_log()
>   {
>   	local opts="$1"
>   
> diff --git a/tests/xfs/312 b/tests/xfs/312
> index 1fcf26ab..94f868fe 100755
> --- a/tests/xfs/312
> +++ b/tests/xfs/312
> @@ -63,7 +63,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/313 b/tests/xfs/313
> index 6d2f9fac..9c7cf5b9 100755
> --- a/tests/xfs/313
> +++ b/tests/xfs/313
> @@ -63,7 +63,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/314 b/tests/xfs/314
> index 5165393e..9ac311d0 100755
> --- a/tests/xfs/314
> +++ b/tests/xfs/314
> @@ -64,7 +64,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/315 b/tests/xfs/315
> index 958a8c99..105515ab 100755
> --- a/tests/xfs/315
> +++ b/tests/xfs/315
> @@ -61,7 +61,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/316 b/tests/xfs/316
> index cf0c5adc..f0af19d2 100755
> --- a/tests/xfs/316
> +++ b/tests/xfs/316
> @@ -61,7 +61,7 @@ echo "CoW all the blocks"
>   $XFS_IO_PROG -c "pwrite -W -S 0x67 -b $sz 0 $((blks * blksz))" $SCRATCH_MNT/file2 >> $seqres.full
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/317 b/tests/xfs/317
> index 7eef67af..1ca2672d 100755
> --- a/tests/xfs/317
> +++ b/tests/xfs/317
> @@ -54,7 +54,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "Check files"
>   md5sum $SCRATCH_MNT/file0 | _filter_scratch
> diff --git a/tests/xfs/318 b/tests/xfs/318
> index d822e89a..38c7aa60 100755
> --- a/tests/xfs/318
> +++ b/tests/xfs/318
> @@ -60,7 +60,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "Check files"
>   md5sum $SCRATCH_MNT/file1 2>&1 | _filter_scratch
> diff --git a/tests/xfs/319 b/tests/xfs/319
> index 0f61c119..d64651fb 100755
> --- a/tests/xfs/319
> +++ b/tests/xfs/319
> @@ -57,7 +57,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/320 b/tests/xfs/320
> index f65f3ad1..d22d76d9 100755
> --- a/tests/xfs/320
> +++ b/tests/xfs/320
> @@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "Check files"
>   md5sum $SCRATCH_MNT/file1 | _filter_scratch
> diff --git a/tests/xfs/321 b/tests/xfs/321
> index daff4449..06a34347 100755
> --- a/tests/xfs/321
> +++ b/tests/xfs/321
> @@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "Check files"
>   md5sum $SCRATCH_MNT/file1 | _filter_scratch
> diff --git a/tests/xfs/322 b/tests/xfs/322
> index f36e54d8..89a2f741 100755
> --- a/tests/xfs/322
> +++ b/tests/xfs/322
> @@ -56,7 +56,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "Check files"
>   md5sum $SCRATCH_MNT/file1 | _filter_scratch
> diff --git a/tests/xfs/323 b/tests/xfs/323
> index f66a8ebf..66737da0 100755
> --- a/tests/xfs/323
> +++ b/tests/xfs/323
> @@ -55,7 +55,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/324 b/tests/xfs/324
> index ca2f25ac..9909db62 100755
> --- a/tests/xfs/324
> +++ b/tests/xfs/324
> @@ -61,7 +61,7 @@ echo "Reflink all the blocks"
>   _cp_reflink $SCRATCH_MNT/file1 $SCRATCH_MNT/file4
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/325 b/tests/xfs/325
> index 3b98fd50..5b26b2b3 100755
> --- a/tests/xfs/325
> +++ b/tests/xfs/325
> @@ -59,7 +59,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/326 b/tests/xfs/326
> index bf5db08a..8b95a18a 100755
> --- a/tests/xfs/326
> +++ b/tests/xfs/326
> @@ -71,7 +71,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log"
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   
>   echo "FS should be online, touch should succeed"
>   touch $SCRATCH_MNT/goodfs
> diff --git a/tests/xfs/329 b/tests/xfs/329
> index e57f6f7f..e9a30d05 100755
> --- a/tests/xfs/329
> +++ b/tests/xfs/329
> @@ -52,7 +52,7 @@ echo "FS should be shut down, touch will fail"
>   touch $SCRATCH_MNT/badfs 2>&1 | _filter_scratch
>   
>   echo "Remount to replay log" | tee /dev/ttyprintk
> -_scratch_inject_logprint >> $seqres.full
> +_scratch_remount_dump_log >> $seqres.full
>   new_nextents=$(_count_extents $testdir/file1)
>   
>   echo "Check extent count" | tee /dev/ttyprintk
> 

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

* Re: [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs
  2021-09-09 17:41 ` [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs Catherine Hoang
@ 2021-09-11 14:11   ` Allison Henderson
  2021-09-12  8:13   ` Eryu Guan
  1 sibling, 0 replies; 10+ messages in thread
From: Allison Henderson @ 2021-09-11 14:11 UTC (permalink / raw)
  To: Catherine Hoang, linux-xfs, fstests



On 9/9/21 10:41 AM, Catherine Hoang wrote:
> Move _scratch_remount_dump_log and _test_remount_dump_log from
> common/inject to common/xfs. These routines do not inject errors and
> should be placed with other xfs common functions.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>

Looks ok
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/inject | 26 --------------------------
>   common/xfs    | 26 ++++++++++++++++++++++++++
>   2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/common/inject b/common/inject
> index b5334d4a..6b590804 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -111,29 +111,3 @@ _scratch_inject_error()
>   		_fail "Cannot inject error ${type} value ${value}."
>   	fi
>   }
> -
> -# Unmount and remount the scratch device, dumping the log
> -_scratch_remount_dump_log()
> -{
> -	local opts="$1"
> -
> -	if test -n "$opts"; then
> -		opts="-o $opts"
> -	fi
> -	_scratch_unmount
> -	_scratch_dump_log
> -	_scratch_mount "$opts"
> -}
> -
> -# Unmount and remount the test device, dumping the log
> -_test_remount_dump_log()
> -{
> -	local opts="$1"
> -
> -	if test -n "$opts"; then
> -		opts="-o $opts"
> -	fi
> -	_test_unmount
> -	_test_dump_log
> -	_test_mount "$opts"
> -}
> diff --git a/common/xfs b/common/xfs
> index bfb1bf1e..cda1f768 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1263,3 +1263,29 @@ _require_scratch_xfs_bigtime()
>   		_notrun "bigtime feature not advertised on mount?"
>   	_scratch_unmount
>   }
> +
> +# Unmount and remount the scratch device, dumping the log
> +_scratch_remount_dump_log()
> +{
> +	local opts="$1"
> +
> +	if test -n "$opts"; then
> +		opts="-o $opts"
> +	fi
> +	_scratch_unmount
> +	_scratch_dump_log
> +	_scratch_mount "$opts"
> +}
> +
> +# Unmount and remount the test device, dumping the log
> +_test_remount_dump_log()
> +{
> +	local opts="$1"
> +
> +	if test -n "$opts"; then
> +		opts="-o $opts"
> +	fi
> +	_test_unmount
> +	_test_dump_log
> +	_test_mount "$opts"
> +}
> 

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

* Re: [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log
  2021-09-09 17:41 ` [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log Catherine Hoang
@ 2021-09-11 14:11   ` Allison Henderson
  0 siblings, 0 replies; 10+ messages in thread
From: Allison Henderson @ 2021-09-11 14:11 UTC (permalink / raw)
  To: Catherine Hoang, linux-xfs, fstests



On 9/9/21 10:41 AM, Catherine Hoang wrote:
> Rename _test_inject_logprint to _test_remount_dump_log to better
> describe what this function does. _test_remount_dump_log unmounts
> and remounts the test device, dumping the log.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>

I notice this function has no callers, but I think it's a good utility 
function to keep around.  I think it looks ok.

Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/inject | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/common/inject b/common/inject
> index 3b731df7..b5334d4a 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -126,7 +126,7 @@ _scratch_remount_dump_log()
>   }
>   
>   # Unmount and remount the test device, dumping the log
> -_test_inject_logprint()
> +_test_remount_dump_log()
>   {
>   	local opts="$1"
>   
> 

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

* Re: [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs
  2021-09-09 17:41 ` [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs Catherine Hoang
  2021-09-11 14:11   ` Allison Henderson
@ 2021-09-12  8:13   ` Eryu Guan
  2021-09-13 17:08     ` Darrick J. Wong
  2021-09-13 17:41     ` [External] : " Catherine Hoang
  1 sibling, 2 replies; 10+ messages in thread
From: Eryu Guan @ 2021-09-12  8:13 UTC (permalink / raw)
  To: Catherine Hoang; +Cc: linux-xfs, fstests

On Thu, Sep 09, 2021 at 05:41:42PM +0000, Catherine Hoang wrote:
> Move _scratch_remount_dump_log and _test_remount_dump_log from
> common/inject to common/xfs. These routines do not inject errors and
> should be placed with other xfs common functions.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  common/inject | 26 --------------------------
>  common/xfs    | 26 ++++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/common/inject b/common/inject
> index b5334d4a..6b590804 100644
> --- a/common/inject
> +++ b/common/inject
> @@ -111,29 +111,3 @@ _scratch_inject_error()
>  		_fail "Cannot inject error ${type} value ${value}."
>  	fi
>  }
> -
> -# Unmount and remount the scratch device, dumping the log
> -_scratch_remount_dump_log()
> -{
> -	local opts="$1"
> -
> -	if test -n "$opts"; then
> -		opts="-o $opts"
> -	fi
> -	_scratch_unmount
> -	_scratch_dump_log

This function is a common function that could handle multiple
filesystems, currently it supports xfs, ext4 and f2fs. So it's not a
xfs-specific function, and moving it to common/xfs doesn't seem correct.
Perhaps we should move it to common/log.

> -	_scratch_mount "$opts"
> -}
> -
> -# Unmount and remount the test device, dumping the log
> -_test_remount_dump_log()
> -{
> -	local opts="$1"
> -
> -	if test -n "$opts"; then
> -		opts="-o $opts"
> -	fi
> -	_test_unmount
> -	_test_dump_log

Same here.

Thanks,
Eryu

> -	_test_mount "$opts"
> -}
> diff --git a/common/xfs b/common/xfs
> index bfb1bf1e..cda1f768 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -1263,3 +1263,29 @@ _require_scratch_xfs_bigtime()
>  		_notrun "bigtime feature not advertised on mount?"
>  	_scratch_unmount
>  }
> +
> +# Unmount and remount the scratch device, dumping the log
> +_scratch_remount_dump_log()
> +{
> +	local opts="$1"
> +
> +	if test -n "$opts"; then
> +		opts="-o $opts"
> +	fi
> +	_scratch_unmount
> +	_scratch_dump_log
> +	_scratch_mount "$opts"
> +}
> +
> +# Unmount and remount the test device, dumping the log
> +_test_remount_dump_log()
> +{
> +	local opts="$1"
> +
> +	if test -n "$opts"; then
> +		opts="-o $opts"
> +	fi
> +	_test_unmount
> +	_test_dump_log
> +	_test_mount "$opts"
> +}
> -- 
> 2.25.1

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

* Re: [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs
  2021-09-12  8:13   ` Eryu Guan
@ 2021-09-13 17:08     ` Darrick J. Wong
  2021-09-13 17:41     ` [External] : " Catherine Hoang
  1 sibling, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2021-09-13 17:08 UTC (permalink / raw)
  To: Eryu Guan; +Cc: Catherine Hoang, linux-xfs, fstests, linux-ext4

[add ext4 list to cc]

On Sun, Sep 12, 2021 at 04:13:33PM +0800, Eryu Guan wrote:
> On Thu, Sep 09, 2021 at 05:41:42PM +0000, Catherine Hoang wrote:
> > Move _scratch_remount_dump_log and _test_remount_dump_log from
> > common/inject to common/xfs. These routines do not inject errors and
> > should be placed with other xfs common functions.
> > 
> > Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> > ---
> >  common/inject | 26 --------------------------
> >  common/xfs    | 26 ++++++++++++++++++++++++++
> >  2 files changed, 26 insertions(+), 26 deletions(-)
> > 
> > diff --git a/common/inject b/common/inject
> > index b5334d4a..6b590804 100644
> > --- a/common/inject
> > +++ b/common/inject
> > @@ -111,29 +111,3 @@ _scratch_inject_error()
> >  		_fail "Cannot inject error ${type} value ${value}."
> >  	fi
> >  }
> > -
> > -# Unmount and remount the scratch device, dumping the log
> > -_scratch_remount_dump_log()
> > -{
> > -	local opts="$1"
> > -
> > -	if test -n "$opts"; then
> > -		opts="-o $opts"
> > -	fi
> > -	_scratch_unmount
> > -	_scratch_dump_log
> 
> This function is a common function that could handle multiple
> filesystems, currently it supports xfs, ext4 and f2fs. So it's not a

I don't think it even works correctly for ext*.  *_dump_log() calls
dumpe2fs -h, which displays the superblock contents, not the journal
contents themselves, which is (I think) what this helper is supposed to
do.

It really ought to do something along the lines of:

	echo 'logdump -a' | debugfs $SCRATCH_DEV

Though fixing this is probably something the ext4 developers should look
at...

> xfs-specific function, and moving it to common/xfs doesn't seem correct.
> Perhaps we should move it to common/log.

Agreed. ;)

--D

> > -	_scratch_mount "$opts"
> > -}
> > -
> > -# Unmount and remount the test device, dumping the log
> > -_test_remount_dump_log()
> > -{
> > -	local opts="$1"
> > -
> > -	if test -n "$opts"; then
> > -		opts="-o $opts"
> > -	fi
> > -	_test_unmount
> > -	_test_dump_log
> 
> Same here.
> 
> Thanks,
> Eryu
> 
> > -	_test_mount "$opts"
> > -}
> > diff --git a/common/xfs b/common/xfs
> > index bfb1bf1e..cda1f768 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -1263,3 +1263,29 @@ _require_scratch_xfs_bigtime()
> >  		_notrun "bigtime feature not advertised on mount?"
> >  	_scratch_unmount
> >  }
> > +
> > +# Unmount and remount the scratch device, dumping the log
> > +_scratch_remount_dump_log()
> > +{
> > +	local opts="$1"
> > +
> > +	if test -n "$opts"; then
> > +		opts="-o $opts"
> > +	fi
> > +	_scratch_unmount
> > +	_scratch_dump_log
> > +	_scratch_mount "$opts"
> > +}
> > +
> > +# Unmount and remount the test device, dumping the log
> > +_test_remount_dump_log()
> > +{
> > +	local opts="$1"
> > +
> > +	if test -n "$opts"; then
> > +		opts="-o $opts"
> > +	fi
> > +	_test_unmount
> > +	_test_dump_log
> > +	_test_mount "$opts"
> > +}
> > -- 
> > 2.25.1

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

* Re: [External] : Re: [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs
  2021-09-12  8:13   ` Eryu Guan
  2021-09-13 17:08     ` Darrick J. Wong
@ 2021-09-13 17:41     ` Catherine Hoang
  1 sibling, 0 replies; 10+ messages in thread
From: Catherine Hoang @ 2021-09-13 17:41 UTC (permalink / raw)
  To: Eryu Guan; +Cc: linux-xfs, fstests


> On Sep 12, 2021, at 1:13 AM, Eryu Guan <guan@eryu.me> wrote:
> 
> On Thu, Sep 09, 2021 at 05:41:42PM +0000, Catherine Hoang wrote:
>> Move _scratch_remount_dump_log and _test_remount_dump_log from
>> common/inject to common/xfs. These routines do not inject errors and
>> should be placed with other xfs common functions.
>> 
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> ---
>> common/inject | 26 --------------------------
>> common/xfs    | 26 ++++++++++++++++++++++++++
>> 2 files changed, 26 insertions(+), 26 deletions(-)
>> 
>> diff --git a/common/inject b/common/inject
>> index b5334d4a..6b590804 100644
>> --- a/common/inject
>> +++ b/common/inject
>> @@ -111,29 +111,3 @@ _scratch_inject_error()
>> 		_fail "Cannot inject error ${type} value ${value}."
>> 	fi
>> }
>> -
>> -# Unmount and remount the scratch device, dumping the log
>> -_scratch_remount_dump_log()
>> -{
>> -	local opts="$1"
>> -
>> -	if test -n "$opts"; then
>> -		opts="-o $opts"
>> -	fi
>> -	_scratch_unmount
>> -	_scratch_dump_log
> 
> This function is a common function that could handle multiple
> filesystems, currently it supports xfs, ext4 and f2fs. So it's not a
> xfs-specific function, and moving it to common/xfs doesn't seem correct.
> Perhaps we should move it to common/log.
I see, I will move this to common/log on the next revision.
> 
>> -	_scratch_mount "$opts"
>> -}
>> -
>> -# Unmount and remount the test device, dumping the log
>> -_test_remount_dump_log()
>> -{
>> -	local opts="$1"
>> -
>> -	if test -n "$opts"; then
>> -		opts="-o $opts"
>> -	fi
>> -	_test_unmount
>> -	_test_dump_log
> 
> Same here.
> 
> Thanks,
> Eryu
Sure, will move this too. Thanks for the feedback!
Catherine
> 
>> -	_test_mount "$opts"
>> -}
>> diff --git a/common/xfs b/common/xfs
>> index bfb1bf1e..cda1f768 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -1263,3 +1263,29 @@ _require_scratch_xfs_bigtime()
>> 		_notrun "bigtime feature not advertised on mount?"
>> 	_scratch_unmount
>> }
>> +
>> +# Unmount and remount the scratch device, dumping the log
>> +_scratch_remount_dump_log()
>> +{
>> +	local opts="$1"
>> +
>> +	if test -n "$opts"; then
>> +		opts="-o $opts"
>> +	fi
>> +	_scratch_unmount
>> +	_scratch_dump_log
>> +	_scratch_mount "$opts"
>> +}
>> +
>> +# Unmount and remount the test device, dumping the log
>> +_test_remount_dump_log()
>> +{
>> +	local opts="$1"
>> +
>> +	if test -n "$opts"; then
>> +		opts="-o $opts"
>> +	fi
>> +	_test_unmount
>> +	_test_dump_log
>> +	_test_mount "$opts"
>> +}
>> -- 
>> 2.25.1


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

end of thread, other threads:[~2021-09-13 17:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-09 17:41 [PATCH 0/3] Dump log cleanups Catherine Hoang
2021-09-09 17:41 ` [PATCH 1/3] xfstests: Rename _scratch_inject_logprint to _scratch_remount_dump_log Catherine Hoang
2021-09-11 14:11   ` Allison Henderson
2021-09-09 17:41 ` [PATCH 2/3] xfstests: Rename _test_inject_logprint to _test_remount_dump_log Catherine Hoang
2021-09-11 14:11   ` Allison Henderson
2021-09-09 17:41 ` [PATCH 3/3] xfstests: Move *_dump_log routines to common/xfs Catherine Hoang
2021-09-11 14:11   ` Allison Henderson
2021-09-12  8:13   ` Eryu Guan
2021-09-13 17:08     ` Darrick J. Wong
2021-09-13 17:41     ` [External] : " Catherine Hoang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).