diff mbox series

[v2,4/4] common/log: Fix *_dump_log routines for ext4

Message ID 20211007002641.714906-5-catherine.hoang@oracle.com (mailing list archive)
State New, archived
Headers show
Series Dump log cleanups | expand

Commit Message

Catherine Hoang Oct. 7, 2021, 12:26 a.m. UTC
dumpe2fs -h displays the superblock contents, not the journal contents.
Use the logdump utility to dump the contents of the journal.

Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
---
 common/log | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Darrick J. Wong Oct. 7, 2021, 4:15 p.m. UTC | #1
On Thu, Oct 07, 2021 at 12:26:41AM +0000, Catherine Hoang wrote:
> dumpe2fs -h displays the superblock contents, not the journal contents.
> Use the logdump utility to dump the contents of the journal.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> ---
>  common/log | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/log b/common/log
> index 0a9aaa7f..154f3959 100644
> --- a/common/log
> +++ b/common/log
> @@ -229,7 +229,7 @@ _scratch_dump_log()
>  		$DUMP_F2FS_PROG $SCRATCH_DEV
>  		;;
>  	ext4)
> -		$DUMPE2FS_PROG -h $SCRATCH_DEV
> +		$DEBUGFS_PROG -R "logdump -a" $SCRATCH_DEV

Hmm.  Some of the tests call _require_command on various e2fsprogs
programs.  However, debugfs has been a part of e2fsprogs since forever
and e2fsprogs is a required fstests dependency, so I guess those
callsites are unnecessary (but otherwise benign).  For that matter, I
think e2fsprogs is an 'essential' package on Debian and almost always
installed by Linux distros.  I think that means it's safe to assume that
debugfs is present.

Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D


>  		;;
>  	*)
>  		;;
> @@ -246,7 +246,7 @@ _test_dump_log()
>  		$DUMP_F2FS_PROG $TEST_DEV
>  		;;
>  	ext4)
> -		$DUMPE2FS_PROG -h $TEST_DEV
> +		$DEBUGFS_PROG -R "logdump -a" $TEST_DEV
>  		;;
>  	*)
>  		;;
> -- 
> 2.25.1
>
Catherine Hoang Oct. 7, 2021, 6:24 p.m. UTC | #2
> On Oct 7, 2021, at 9:15 AM, Darrick J. Wong <djwong@kernel.org> wrote:
> 
> On Thu, Oct 07, 2021 at 12:26:41AM +0000, Catherine Hoang wrote:
>> dumpe2fs -h displays the superblock contents, not the journal contents.
>> Use the logdump utility to dump the contents of the journal.
>> 
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
>> ---
>> common/log | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>> 
>> diff --git a/common/log b/common/log
>> index 0a9aaa7f..154f3959 100644
>> --- a/common/log
>> +++ b/common/log
>> @@ -229,7 +229,7 @@ _scratch_dump_log()
>> 		$DUMP_F2FS_PROG $SCRATCH_DEV
>> 		;;
>> 	ext4)
>> -		$DUMPE2FS_PROG -h $SCRATCH_DEV
>> +		$DEBUGFS_PROG -R "logdump -a" $SCRATCH_DEV
> 
> Hmm.  Some of the tests call _require_command on various e2fsprogs
> programs.  However, debugfs has been a part of e2fsprogs since forever
> and e2fsprogs is a required fstests dependency, so I guess those
> callsites are unnecessary (but otherwise benign).  For that matter, I
> think e2fsprogs is an 'essential' package on Debian and almost always
> installed by Linux distros.  I think that means it's safe to assume that
> debugfs is present.
> 
> Reviewed-by: Darrick J. Wong <djwong@kernel.org>
> 
> —D
Thanks for the reviews Darrick!
Catherine
> 
> 
>> 		;;
>> 	*)
>> 		;;
>> @@ -246,7 +246,7 @@ _test_dump_log()
>> 		$DUMP_F2FS_PROG $TEST_DEV
>> 		;;
>> 	ext4)
>> -		$DUMPE2FS_PROG -h $TEST_DEV
>> +		$DEBUGFS_PROG -R "logdump -a" $TEST_DEV
>> 		;;
>> 	*)
>> 		;;
>> -- 
>> 2.25.1
>>
Allison Henderson Oct. 8, 2021, 5:49 p.m. UTC | #3
On 10/6/21 5:26 PM, Catherine Hoang wrote:
> dumpe2fs -h displays the superblock contents, not the journal contents.
> Use the logdump utility to dump the contents of the journal.
> 
> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
Looks good to me.  Thanks for the fix
Reviewed-by: Allison Henderson <allison.henderson@oracle.com>

> ---
>   common/log | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/common/log b/common/log
> index 0a9aaa7f..154f3959 100644
> --- a/common/log
> +++ b/common/log
> @@ -229,7 +229,7 @@ _scratch_dump_log()
>   		$DUMP_F2FS_PROG $SCRATCH_DEV
>   		;;
>   	ext4)
> -		$DUMPE2FS_PROG -h $SCRATCH_DEV
> +		$DEBUGFS_PROG -R "logdump -a" $SCRATCH_DEV
>   		;;
>   	*)
>   		;;
> @@ -246,7 +246,7 @@ _test_dump_log()
>   		$DUMP_F2FS_PROG $TEST_DEV
>   		;;
>   	ext4)
> -		$DUMPE2FS_PROG -h $TEST_DEV
> +		$DEBUGFS_PROG -R "logdump -a" $TEST_DEV
>   		;;
>   	*)
>   		;;
>
Catherine Hoang Oct. 8, 2021, 7:06 p.m. UTC | #4
> On Oct 8, 2021, at 10:49 AM, Allison Henderson <allison.henderson@oracle.com> wrote:
> 
> 
> 
> On 10/6/21 5:26 PM, Catherine Hoang wrote:
>> dumpe2fs -h displays the superblock contents, not the journal contents.
>> Use the logdump utility to dump the contents of the journal.
>> Signed-off-by: Catherine Hoang <catherine.hoang@oracle.com>
> Looks good to me.  Thanks for the fix
> Reviewed-by: Allison Henderson <allison.henderson@oracle.com>
Thanks for the review!
Catherine
> 
>> ---
>>  common/log | 4 ++--
>>  1 file changed, 2 insertions(+), 2 deletions(-)
>> diff --git a/common/log b/common/log
>> index 0a9aaa7f..154f3959 100644
>> --- a/common/log
>> +++ b/common/log
>> @@ -229,7 +229,7 @@ _scratch_dump_log()
>>  		$DUMP_F2FS_PROG $SCRATCH_DEV
>>  		;;
>>  	ext4)
>> -		$DUMPE2FS_PROG -h $SCRATCH_DEV
>> +		$DEBUGFS_PROG -R "logdump -a" $SCRATCH_DEV
>>  		;;
>>  	*)
>>  		;;
>> @@ -246,7 +246,7 @@ _test_dump_log()
>>  		$DUMP_F2FS_PROG $TEST_DEV
>>  		;;
>>  	ext4)
>> -		$DUMPE2FS_PROG -h $TEST_DEV
>> +		$DEBUGFS_PROG -R "logdump -a" $TEST_DEV
>>  		;;
>>  	*)
>>  		;;
diff mbox series

Patch

diff --git a/common/log b/common/log
index 0a9aaa7f..154f3959 100644
--- a/common/log
+++ b/common/log
@@ -229,7 +229,7 @@  _scratch_dump_log()
 		$DUMP_F2FS_PROG $SCRATCH_DEV
 		;;
 	ext4)
-		$DUMPE2FS_PROG -h $SCRATCH_DEV
+		$DEBUGFS_PROG -R "logdump -a" $SCRATCH_DEV
 		;;
 	*)
 		;;
@@ -246,7 +246,7 @@  _test_dump_log()
 		$DUMP_F2FS_PROG $TEST_DEV
 		;;
 	ext4)
-		$DUMPE2FS_PROG -h $TEST_DEV
+		$DEBUGFS_PROG -R "logdump -a" $TEST_DEV
 		;;
 	*)
 		;;