[PATCH 0/5] Add support for testing XFS metadump v2

[PATCH 0/5] Add support for testing XFS metadump v2

[Chandan Babu R]

Hi,

This patchset modifies existing tests to add support for testing
Metadump v2 feature. Metadump v2 feature is already part of Xfsprogs'
for-next branch.

Chandan Babu R (5):
  common/xfs: Do not append -a and -o options to metadump
  common/xfs: Add function to detect support for metadump v2
  _scratch_xfs_mdrestore: Pass scratch log device when applicable
  xfs: Add support for testing metadump v2
  xfs: Check correctness of metadump/mdrestore's ability to work with
    dirty log

 common/populate   |   2 +-
 common/xfs        |  34 ++++++--
 tests/xfs/129     |  63 ++++++++++++---
 tests/xfs/129.out |   4 +-
 tests/xfs/234     |  63 ++++++++++++---
 tests/xfs/234.out |   4 +-
 tests/xfs/253     | 195 ++++++++++++++++++++++++++--------------------
 tests/xfs/291     |  25 +++++-
 tests/xfs/336     |   2 +-
 tests/xfs/432     |  29 +++++--
 tests/xfs/432.out |   3 +-
 tests/xfs/503     |  94 +++++++++++++---------
 tests/xfs/503.out |  12 +--
 tests/xfs/801     | 115 +++++++++++++++++++++++++++
 tests/xfs/801.out |  14 ++++
 15 files changed, 485 insertions(+), 174 deletions(-)
 create mode 100755 tests/xfs/801
 create mode 100644 tests/xfs/801.out

-- 
2.43.0

[PATCH 1/5] common/xfs: Do not append -a and -o options to metadump

[Chandan Babu R]

xfs/253 requires the metadump to be obfuscated. However _xfs_metadump() would
append the '-o' option causing the metadump to be unobfuscated.

This commit fixes the bug by modifying _xfs_metadump() to no longer append any
metadump options. The direct/indirect callers of this function now pass the
required options explicitly.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 common/populate | 2 +-
 common/xfs      | 7 +++----
 tests/xfs/291   | 2 +-
 tests/xfs/336   | 2 +-
 tests/xfs/432   | 2 +-
 tests/xfs/503   | 2 +-
 6 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/common/populate b/common/populate
index 3d233073..cfbfd88a 100644
--- a/common/populate
+++ b/common/populate
@@ -55,7 +55,7 @@ __populate_fail() {
 	case "$FSTYP" in
 	xfs)
 		_scratch_unmount
-		_scratch_xfs_metadump "$metadump"
+		_scratch_xfs_metadump "$metadump" -a -o
 		;;
 	ext4)
 		_scratch_unmount
diff --git a/common/xfs b/common/xfs
index f53b33fc..38094828 100644
--- a/common/xfs
+++ b/common/xfs
@@ -667,7 +667,6 @@ _xfs_metadump() {
 	local compressopt="$4"
 	shift; shift; shift; shift
 	local options="$@"
-	test -z "$options" && options="-a -o"
 
 	if [ "$logdev" != "none" ]; then
 		options="$options -l $logdev"
@@ -855,7 +854,7 @@ _check_xfs_filesystem()
 	if [ "$ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
 		local flatdev="$(basename "$device")"
 		_xfs_metadump "$seqres.$flatdev.check.md" "$device" "$logdev" \
-			compress >> $seqres.full
+			compress -a -o >> $seqres.full
 	fi
 
 	# Optionally test the index rebuilding behavior.
@@ -888,7 +887,7 @@ _check_xfs_filesystem()
 		if [ "$rebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
 			local flatdev="$(basename "$device")"
 			_xfs_metadump "$seqres.$flatdev.rebuild.md" "$device" \
-				"$logdev" compress >> $seqres.full
+				"$logdev" compress -a -o >> $seqres.full
 		fi
 	fi
 
@@ -972,7 +971,7 @@ _check_xfs_filesystem()
 		if [ "$orebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
 			local flatdev="$(basename "$device")"
 			_xfs_metadump "$seqres.$flatdev.orebuild.md" "$device" \
-				"$logdev" compress >> $seqres.full
+				"$logdev" compress -a -o >> $seqres.full
 		fi
 	fi
 
diff --git a/tests/xfs/291 b/tests/xfs/291
index 600dcb2e..54448497 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -92,7 +92,7 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
 
 # Yes they can!  Now...
 # Can xfs_metadump cope with this monster?
-_scratch_xfs_metadump $tmp.metadump || _fail "xfs_metadump failed"
+_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
 SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
 SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
 	_fail "xfs_repair of metadump failed"
diff --git a/tests/xfs/336 b/tests/xfs/336
index d7a074d9..43b3790c 100755
--- a/tests/xfs/336
+++ b/tests/xfs/336
@@ -62,7 +62,7 @@ _scratch_cycle_mount
 
 echo "Create metadump file"
 _scratch_unmount
-_scratch_xfs_metadump $metadump_file
+_scratch_xfs_metadump $metadump_file -a
 
 # Now restore the obfuscated one back and take a look around
 echo "Restore metadump"
diff --git a/tests/xfs/432 b/tests/xfs/432
index 66315b03..dae68fb2 100755
--- a/tests/xfs/432
+++ b/tests/xfs/432
@@ -86,7 +86,7 @@ echo "qualifying extent: $extlen blocks" >> $seqres.full
 test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
 
 echo "Try to metadump"
-_scratch_xfs_metadump $metadump_file -w
+_scratch_xfs_metadump $metadump_file -a -o -w
 SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
 
 echo "Check restored metadump image"
diff --git a/tests/xfs/503 b/tests/xfs/503
index f5710ece..8805632d 100755
--- a/tests/xfs/503
+++ b/tests/xfs/503
@@ -46,7 +46,7 @@ metadump_file_ag=${metadump_file}.ag
 copy_file=$testdir/copy.img
 
 echo metadump
-_scratch_xfs_metadump $metadump_file >> $seqres.full
+_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
 
 echo metadump a
 _scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
-- 
2.43.0

Re: [PATCH 1/5] common/xfs: Do not append -a and -o options to metadump

[Darrick J. Wong]

On Tue, Jan 02, 2024 at 02:13:48PM +0530, Chandan Babu R wrote:
> xfs/253 requires the metadump to be obfuscated. However _xfs_metadump() would
> append the '-o' option causing the metadump to be unobfuscated.
> 
> This commit fixes the bug by modifying _xfs_metadump() to no longer append any
> metadump options. The direct/indirect callers of this function now pass the
> required options explicitly.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  common/populate | 2 +-
>  common/xfs      | 7 +++----
>  tests/xfs/291   | 2 +-
>  tests/xfs/336   | 2 +-
>  tests/xfs/432   | 2 +-
>  tests/xfs/503   | 2 +-

Hmm.  Shouldn't "-a -o" be lowered into xfs/168?  It generates a
metadump if a post-shrinkfs repair fails and someone needs to debug the
resulting breakage.

I think that in general, tests should be using -a (copy entire fs
blocks) and -o (do not obfuscate) to reduce the amount of processing
that fstests has to do; and to make it easy for developers to examine
the fs metadata.

(Also given the number of bugs that we keep finding in the "zero unused
parts of blocks" code, I almost always use -a to reduce the number of
pieces that can fail.)

Obviously that does not apply to tests that are exercising metadump
itself.

--D

>  6 files changed, 8 insertions(+), 9 deletions(-)
> 
> diff --git a/common/populate b/common/populate
> index 3d233073..cfbfd88a 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -55,7 +55,7 @@ __populate_fail() {
>  	case "$FSTYP" in
>  	xfs)
>  		_scratch_unmount
> -		_scratch_xfs_metadump "$metadump"
> +		_scratch_xfs_metadump "$metadump" -a -o
>  		;;
>  	ext4)
>  		_scratch_unmount
> diff --git a/common/xfs b/common/xfs
> index f53b33fc..38094828 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -667,7 +667,6 @@ _xfs_metadump() {
>  	local compressopt="$4"
>  	shift; shift; shift; shift
>  	local options="$@"
> -	test -z "$options" && options="-a -o"
>  
>  	if [ "$logdev" != "none" ]; then
>  		options="$options -l $logdev"
> @@ -855,7 +854,7 @@ _check_xfs_filesystem()
>  	if [ "$ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>  		local flatdev="$(basename "$device")"
>  		_xfs_metadump "$seqres.$flatdev.check.md" "$device" "$logdev" \
> -			compress >> $seqres.full
> +			compress -a -o >> $seqres.full
>  	fi
>  
>  	# Optionally test the index rebuilding behavior.
> @@ -888,7 +887,7 @@ _check_xfs_filesystem()
>  		if [ "$rebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>  			local flatdev="$(basename "$device")"
>  			_xfs_metadump "$seqres.$flatdev.rebuild.md" "$device" \
> -				"$logdev" compress >> $seqres.full
> +				"$logdev" compress -a -o >> $seqres.full
>  		fi
>  	fi
>  
> @@ -972,7 +971,7 @@ _check_xfs_filesystem()
>  		if [ "$orebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>  			local flatdev="$(basename "$device")"
>  			_xfs_metadump "$seqres.$flatdev.orebuild.md" "$device" \
> -				"$logdev" compress >> $seqres.full
> +				"$logdev" compress -a -o >> $seqres.full
>  		fi
>  	fi
>  
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index 600dcb2e..54448497 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -92,7 +92,7 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
>  
>  # Yes they can!  Now...
>  # Can xfs_metadump cope with this monster?
> -_scratch_xfs_metadump $tmp.metadump || _fail "xfs_metadump failed"
> +_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
>  SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
>  SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
>  	_fail "xfs_repair of metadump failed"
> diff --git a/tests/xfs/336 b/tests/xfs/336
> index d7a074d9..43b3790c 100755
> --- a/tests/xfs/336
> +++ b/tests/xfs/336
> @@ -62,7 +62,7 @@ _scratch_cycle_mount
>  
>  echo "Create metadump file"
>  _scratch_unmount
> -_scratch_xfs_metadump $metadump_file
> +_scratch_xfs_metadump $metadump_file -a
>  
>  # Now restore the obfuscated one back and take a look around
>  echo "Restore metadump"
> diff --git a/tests/xfs/432 b/tests/xfs/432
> index 66315b03..dae68fb2 100755
> --- a/tests/xfs/432
> +++ b/tests/xfs/432
> @@ -86,7 +86,7 @@ echo "qualifying extent: $extlen blocks" >> $seqres.full
>  test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
>  
>  echo "Try to metadump"
> -_scratch_xfs_metadump $metadump_file -w
> +_scratch_xfs_metadump $metadump_file -a -o -w
>  SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
>  
>  echo "Check restored metadump image"
> diff --git a/tests/xfs/503 b/tests/xfs/503
> index f5710ece..8805632d 100755
> --- a/tests/xfs/503
> +++ b/tests/xfs/503
> @@ -46,7 +46,7 @@ metadump_file_ag=${metadump_file}.ag
>  copy_file=$testdir/copy.img
>  
>  echo metadump
> -_scratch_xfs_metadump $metadump_file >> $seqres.full
> +_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
>  
>  echo metadump a
>  _scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
> -- 
> 2.43.0
> 
> 

Re: [PATCH 1/5] common/xfs: Do not append -a and -o options to metadump

[Chandan Babu R]

On Tue, Jan 02, 2024 at 10:10:29 AM -0800, Darrick J. Wong wrote:
> On Tue, Jan 02, 2024 at 02:13:48PM +0530, Chandan Babu R wrote:
>> xfs/253 requires the metadump to be obfuscated. However _xfs_metadump() would
>> append the '-o' option causing the metadump to be unobfuscated.
>> 
>> This commit fixes the bug by modifying _xfs_metadump() to no longer append any
>> metadump options. The direct/indirect callers of this function now pass the
>> required options explicitly.
>> 
>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>  common/populate | 2 +-
>>  common/xfs      | 7 +++----
>>  tests/xfs/291   | 2 +-
>>  tests/xfs/336   | 2 +-
>>  tests/xfs/432   | 2 +-
>>  tests/xfs/503   | 2 +-
>
> Hmm.  Shouldn't "-a -o" be lowered into xfs/168?  It generates a
> metadump if a post-shrinkfs repair fails and someone needs to debug the
> resulting breakage.

Sorry, I can't find xfs_metadump being directly/indirectly executed by
xfs/168. The test invokes _scratch_xfs_repair() and that in turn executes
xfs_repair on the scratch filesystem. On failure, _fail() is invoked which
causes the script to exit after printing some messages on the terminal.

However, _check_xfs_filesystem() can generate a metadump and this patch
appends both '-a' and '-o' options to all instances of _xfs_metadump() command
lines inside _check_xfs_filesystem().

>
> I think that in general, tests should be using -a (copy entire fs
> blocks) and -o (do not obfuscate) to reduce the amount of processing
> that fstests has to do; and to make it easy for developers to examine
> the fs metadata.
>
> (Also given the number of bugs that we keep finding in the "zero unused
> parts of blocks" code, I almost always use -a to reduce the number of
> pieces that can fail.)
>
> Obviously that does not apply to tests that are exercising metadump
> itself.
>
> --D
>
>>  6 files changed, 8 insertions(+), 9 deletions(-)
>> 
>> diff --git a/common/populate b/common/populate
>> index 3d233073..cfbfd88a 100644
>> --- a/common/populate
>> +++ b/common/populate
>> @@ -55,7 +55,7 @@ __populate_fail() {
>>  	case "$FSTYP" in
>>  	xfs)
>>  		_scratch_unmount
>> -		_scratch_xfs_metadump "$metadump"
>> +		_scratch_xfs_metadump "$metadump" -a -o
>>  		;;
>>  	ext4)
>>  		_scratch_unmount
>> diff --git a/common/xfs b/common/xfs
>> index f53b33fc..38094828 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -667,7 +667,6 @@ _xfs_metadump() {
>>  	local compressopt="$4"
>>  	shift; shift; shift; shift
>>  	local options="$@"
>> -	test -z "$options" && options="-a -o"
>>  
>>  	if [ "$logdev" != "none" ]; then
>>  		options="$options -l $logdev"
>> @@ -855,7 +854,7 @@ _check_xfs_filesystem()
>>  	if [ "$ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>>  		local flatdev="$(basename "$device")"
>>  		_xfs_metadump "$seqres.$flatdev.check.md" "$device" "$logdev" \
>> -			compress >> $seqres.full
>> +			compress -a -o >> $seqres.full
>>  	fi
>>  
>>  	# Optionally test the index rebuilding behavior.
>> @@ -888,7 +887,7 @@ _check_xfs_filesystem()
>>  		if [ "$rebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>>  			local flatdev="$(basename "$device")"
>>  			_xfs_metadump "$seqres.$flatdev.rebuild.md" "$device" \
>> -				"$logdev" compress >> $seqres.full
>> +				"$logdev" compress -a -o >> $seqres.full
>>  		fi
>>  	fi
>>  
>> @@ -972,7 +971,7 @@ _check_xfs_filesystem()
>>  		if [ "$orebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
>>  			local flatdev="$(basename "$device")"
>>  			_xfs_metadump "$seqres.$flatdev.orebuild.md" "$device" \
>> -				"$logdev" compress >> $seqres.full
>> +				"$logdev" compress -a -o >> $seqres.full
>>  		fi
>>  	fi
>>  
>> diff --git a/tests/xfs/291 b/tests/xfs/291
>> index 600dcb2e..54448497 100755
>> --- a/tests/xfs/291
>> +++ b/tests/xfs/291
>> @@ -92,7 +92,7 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
>>  
>>  # Yes they can!  Now...
>>  # Can xfs_metadump cope with this monster?
>> -_scratch_xfs_metadump $tmp.metadump || _fail "xfs_metadump failed"
>> +_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
>>  SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
>>  SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
>>  	_fail "xfs_repair of metadump failed"
>> diff --git a/tests/xfs/336 b/tests/xfs/336
>> index d7a074d9..43b3790c 100755
>> --- a/tests/xfs/336
>> +++ b/tests/xfs/336
>> @@ -62,7 +62,7 @@ _scratch_cycle_mount
>>  
>>  echo "Create metadump file"
>>  _scratch_unmount
>> -_scratch_xfs_metadump $metadump_file
>> +_scratch_xfs_metadump $metadump_file -a
>>  
>>  # Now restore the obfuscated one back and take a look around
>>  echo "Restore metadump"
>> diff --git a/tests/xfs/432 b/tests/xfs/432
>> index 66315b03..dae68fb2 100755
>> --- a/tests/xfs/432
>> +++ b/tests/xfs/432
>> @@ -86,7 +86,7 @@ echo "qualifying extent: $extlen blocks" >> $seqres.full
>>  test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
>>  
>>  echo "Try to metadump"
>> -_scratch_xfs_metadump $metadump_file -w
>> +_scratch_xfs_metadump $metadump_file -a -o -w
>>  SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
>>  
>>  echo "Check restored metadump image"
>> diff --git a/tests/xfs/503 b/tests/xfs/503
>> index f5710ece..8805632d 100755
>> --- a/tests/xfs/503
>> +++ b/tests/xfs/503
>> @@ -46,7 +46,7 @@ metadump_file_ag=${metadump_file}.ag
>>  copy_file=$testdir/copy.img
>>  
>>  echo metadump
>> -_scratch_xfs_metadump $metadump_file >> $seqres.full
>> +_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
>>  
>>  echo metadump a
>>  _scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
>> -- 
>> 2.43.0
>> 
>> 


-- 
Chandan

Re: [PATCH 1/5] common/xfs: Do not append -a and -o options to metadump

[Darrick J. Wong]

On Fri, Jan 05, 2024 at 12:24:41PM +0530, Chandan Babu R wrote:
> On Tue, Jan 02, 2024 at 10:10:29 AM -0800, Darrick J. Wong wrote:
> > On Tue, Jan 02, 2024 at 02:13:48PM +0530, Chandan Babu R wrote:
> >> xfs/253 requires the metadump to be obfuscated. However _xfs_metadump() would
> >> append the '-o' option causing the metadump to be unobfuscated.
> >> 
> >> This commit fixes the bug by modifying _xfs_metadump() to no longer append any
> >> metadump options. The direct/indirect callers of this function now pass the
> >> required options explicitly.
> >> 
> >> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> >> ---
> >>  common/populate | 2 +-
> >>  common/xfs      | 7 +++----
> >>  tests/xfs/291   | 2 +-
> >>  tests/xfs/336   | 2 +-
> >>  tests/xfs/432   | 2 +-
> >>  tests/xfs/503   | 2 +-
> >
> > Hmm.  Shouldn't "-a -o" be lowered into xfs/168?  It generates a
> > metadump if a post-shrinkfs repair fails and someone needs to debug the
> > resulting breakage.
> 
> Sorry, I can't find xfs_metadump being directly/indirectly executed by
> xfs/168. The test invokes _scratch_xfs_repair() and that in turn executes
> xfs_repair on the scratch filesystem. On failure, _fail() is invoked which
> causes the script to exit after printing some messages on the terminal.

Oops, heh, I forgot that I had a debugging patch that adds metadumping
on failure in that test.  Please ignore my stupid message. :(

> However, _check_xfs_filesystem() can generate a metadump and this patch
> appends both '-a' and '-o' options to all instances of _xfs_metadump() command
> lines inside _check_xfs_filesystem().

Ok, good. :)

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

--D

> >
> > I think that in general, tests should be using -a (copy entire fs
> > blocks) and -o (do not obfuscate) to reduce the amount of processing
> > that fstests has to do; and to make it easy for developers to examine
> > the fs metadata.
> >
> > (Also given the number of bugs that we keep finding in the "zero unused
> > parts of blocks" code, I almost always use -a to reduce the number of
> > pieces that can fail.)
> >
> > Obviously that does not apply to tests that are exercising metadump
> > itself.
> >
> > --D
> >
> >>  6 files changed, 8 insertions(+), 9 deletions(-)
> >> 
> >> diff --git a/common/populate b/common/populate
> >> index 3d233073..cfbfd88a 100644
> >> --- a/common/populate
> >> +++ b/common/populate
> >> @@ -55,7 +55,7 @@ __populate_fail() {
> >>  	case "$FSTYP" in
> >>  	xfs)
> >>  		_scratch_unmount
> >> -		_scratch_xfs_metadump "$metadump"
> >> +		_scratch_xfs_metadump "$metadump" -a -o
> >>  		;;
> >>  	ext4)
> >>  		_scratch_unmount
> >> diff --git a/common/xfs b/common/xfs
> >> index f53b33fc..38094828 100644
> >> --- a/common/xfs
> >> +++ b/common/xfs
> >> @@ -667,7 +667,6 @@ _xfs_metadump() {
> >>  	local compressopt="$4"
> >>  	shift; shift; shift; shift
> >>  	local options="$@"
> >> -	test -z "$options" && options="-a -o"
> >>  
> >>  	if [ "$logdev" != "none" ]; then
> >>  		options="$options -l $logdev"
> >> @@ -855,7 +854,7 @@ _check_xfs_filesystem()
> >>  	if [ "$ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
> >>  		local flatdev="$(basename "$device")"
> >>  		_xfs_metadump "$seqres.$flatdev.check.md" "$device" "$logdev" \
> >> -			compress >> $seqres.full
> >> +			compress -a -o >> $seqres.full
> >>  	fi
> >>  
> >>  	# Optionally test the index rebuilding behavior.
> >> @@ -888,7 +887,7 @@ _check_xfs_filesystem()
> >>  		if [ "$rebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
> >>  			local flatdev="$(basename "$device")"
> >>  			_xfs_metadump "$seqres.$flatdev.rebuild.md" "$device" \
> >> -				"$logdev" compress >> $seqres.full
> >> +				"$logdev" compress -a -o >> $seqres.full
> >>  		fi
> >>  	fi
> >>  
> >> @@ -972,7 +971,7 @@ _check_xfs_filesystem()
> >>  		if [ "$orebuild_ok" -ne 1 ] && [ "$DUMP_CORRUPT_FS" = "1" ]; then
> >>  			local flatdev="$(basename "$device")"
> >>  			_xfs_metadump "$seqres.$flatdev.orebuild.md" "$device" \
> >> -				"$logdev" compress >> $seqres.full
> >> +				"$logdev" compress -a -o >> $seqres.full
> >>  		fi
> >>  	fi
> >>  
> >> diff --git a/tests/xfs/291 b/tests/xfs/291
> >> index 600dcb2e..54448497 100755
> >> --- a/tests/xfs/291
> >> +++ b/tests/xfs/291
> >> @@ -92,7 +92,7 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
> >>  
> >>  # Yes they can!  Now...
> >>  # Can xfs_metadump cope with this monster?
> >> -_scratch_xfs_metadump $tmp.metadump || _fail "xfs_metadump failed"
> >> +_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
> >>  SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
> >>  SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
> >>  	_fail "xfs_repair of metadump failed"
> >> diff --git a/tests/xfs/336 b/tests/xfs/336
> >> index d7a074d9..43b3790c 100755
> >> --- a/tests/xfs/336
> >> +++ b/tests/xfs/336
> >> @@ -62,7 +62,7 @@ _scratch_cycle_mount
> >>  
> >>  echo "Create metadump file"
> >>  _scratch_unmount
> >> -_scratch_xfs_metadump $metadump_file
> >> +_scratch_xfs_metadump $metadump_file -a
> >>  
> >>  # Now restore the obfuscated one back and take a look around
> >>  echo "Restore metadump"
> >> diff --git a/tests/xfs/432 b/tests/xfs/432
> >> index 66315b03..dae68fb2 100755
> >> --- a/tests/xfs/432
> >> +++ b/tests/xfs/432
> >> @@ -86,7 +86,7 @@ echo "qualifying extent: $extlen blocks" >> $seqres.full
> >>  test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
> >>  
> >>  echo "Try to metadump"
> >> -_scratch_xfs_metadump $metadump_file -w
> >> +_scratch_xfs_metadump $metadump_file -a -o -w
> >>  SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
> >>  
> >>  echo "Check restored metadump image"
> >> diff --git a/tests/xfs/503 b/tests/xfs/503
> >> index f5710ece..8805632d 100755
> >> --- a/tests/xfs/503
> >> +++ b/tests/xfs/503
> >> @@ -46,7 +46,7 @@ metadump_file_ag=${metadump_file}.ag
> >>  copy_file=$testdir/copy.img
> >>  
> >>  echo metadump
> >> -_scratch_xfs_metadump $metadump_file >> $seqres.full
> >> +_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
> >>  
> >>  echo metadump a
> >>  _scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
> >> -- 
> >> 2.43.0
> >> 
> >> 
> 
> 
> -- 
> Chandan
> 

[PATCH 2/5] common/xfs: Add function to detect support for metadump v2

[Chandan Babu R]

This commit defines a new function to help detect support for metadump v2.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 common/xfs | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/common/xfs b/common/xfs
index 38094828..558a6bb5 100644
--- a/common/xfs
+++ b/common/xfs
@@ -698,6 +698,14 @@ _xfs_mdrestore() {
 	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
 }
 
+_scratch_metadump_v2_supported()
+{
+	$XFS_DB_PROG -c "help metadump" $SCRATCH_DEV | \
+		grep -q "Metadump version to be used"
+
+	return $?
+}
+
 # Snapshot the metadata on the scratch device
 _scratch_xfs_metadump()
 {
-- 
2.43.0

Re: [PATCH 2/5] common/xfs: Add function to detect support for metadump v2

[Darrick J. Wong]

On Tue, Jan 02, 2024 at 02:13:49PM +0530, Chandan Babu R wrote:
> This commit defines a new function to help detect support for metadump v2.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  common/xfs | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/common/xfs b/common/xfs
> index 38094828..558a6bb5 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -698,6 +698,14 @@ _xfs_mdrestore() {
>  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
>  }
>  
> +_scratch_metadump_v2_supported()
> +{
> +	$XFS_DB_PROG -c "help metadump" $SCRATCH_DEV | \
> +		grep -q "Metadump version to be used"
> +
> +	return $?

You don't need this; bash will retain the status code of the last
process in the pipe as the result value.

(Looks good to me otherwise.)

--D

> +}
> +
>  # Snapshot the metadata on the scratch device
>  _scratch_xfs_metadump()
>  {
> -- 
> 2.43.0
> 
> 

Re: [PATCH 2/5] common/xfs: Add function to detect support for metadump v2

[Chandan Babu R]

On Tue, Jan 02, 2024 at 10:27:33 AM -0800, Darrick J. Wong wrote:
> On Tue, Jan 02, 2024 at 02:13:49PM +0530, Chandan Babu R wrote:
>> This commit defines a new function to help detect support for metadump v2.
>> 
>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>  common/xfs | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>> 
>> diff --git a/common/xfs b/common/xfs
>> index 38094828..558a6bb5 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -698,6 +698,14 @@ _xfs_mdrestore() {
>>  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
>>  }
>>  
>> +_scratch_metadump_v2_supported()
>> +{
>> +	$XFS_DB_PROG -c "help metadump" $SCRATCH_DEV | \
>> +		grep -q "Metadump version to be used"
>> +
>> +	return $?
>
> You don't need this; bash will retain the status code of the last
> process in the pipe as the result value.
>
> (Looks good to me otherwise.)
>

Ok. I will fix it up.

>> +}
>> +
>>  # Snapshot the metadata on the scratch device
>>  _scratch_xfs_metadump()
>>  {
>> -- 
>> 2.43.0
>> 
>> 


-- 
Chandan

[PATCH 3/5] _scratch_xfs_mdrestore: Pass scratch log device when applicable

[Chandan Babu R]

Metadump v2 supports dumping contents of an external log device. This commit
modifies _scratch_xfs_mdrestore() and _xfs_mdrestore() to be able to restore
metadump files which contain data from external log devices.

The callers of _scratch_xfs_mdrestore() must set the value of $SCRATCH_LOGDEV
only when all of the following conditions are met:
1. Metadump is in v2 format.
2. Metadump has contents dumped from an external log device.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 common/xfs | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/common/xfs b/common/xfs
index 558a6bb5..248c8361 100644
--- a/common/xfs
+++ b/common/xfs
@@ -682,7 +682,8 @@ _xfs_metadump() {
 _xfs_mdrestore() {
 	local metadump="$1"
 	local device="$2"
-	shift; shift
+	local logdev="$3"
+	shift; shift; shift
 	local options="$@"
 
 	# If we're configured for compressed dumps and there isn't already an
@@ -695,6 +696,10 @@ _xfs_mdrestore() {
 	fi
 	test -r "$metadump" || return 1
 
+	if [ "$logdev" != "none" ]; then
+		options="$options -l $logdev"
+	fi
+
 	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
 }
 
@@ -724,8 +729,18 @@ _scratch_xfs_mdrestore()
 {
 	local metadump=$1
 	shift
+	local logdev=none
+	local options="$@"
 
-	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
+	# $SCRATCH_LOGDEV should have a non-zero length value only when all of
+	# the following conditions are met.
+	# 1. Metadump is in v2 format.
+	# 2. Metadump has contents dumped from an external log device.
+	if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ]; then
+		logdev=$SCRATCH_LOGDEV
+	fi
+
+	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$logdev" "$@"
 }
 
 # Do not use xfs_repair (offline fsck) to rebuild the filesystem
-- 
2.43.0

Re: [PATCH 3/5] _scratch_xfs_mdrestore: Pass scratch log device when applicable

[Darrick J. Wong]

On Tue, Jan 02, 2024 at 02:13:50PM +0530, Chandan Babu R wrote:
> Metadump v2 supports dumping contents of an external log device. This commit
> modifies _scratch_xfs_mdrestore() and _xfs_mdrestore() to be able to restore
> metadump files which contain data from external log devices.
> 
> The callers of _scratch_xfs_mdrestore() must set the value of $SCRATCH_LOGDEV
> only when all of the following conditions are met:
> 1. Metadump is in v2 format.
> 2. Metadump has contents dumped from an external log device.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>

Looks very similar to my version,
Reviewed-by: Darrick J. Wong <djwong@kernel.org>

--D

> ---
>  common/xfs | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/common/xfs b/common/xfs
> index 558a6bb5..248c8361 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -682,7 +682,8 @@ _xfs_metadump() {
>  _xfs_mdrestore() {
>  	local metadump="$1"
>  	local device="$2"
> -	shift; shift
> +	local logdev="$3"
> +	shift; shift; shift
>  	local options="$@"
>  
>  	# If we're configured for compressed dumps and there isn't already an
> @@ -695,6 +696,10 @@ _xfs_mdrestore() {
>  	fi
>  	test -r "$metadump" || return 1
>  
> +	if [ "$logdev" != "none" ]; then
> +		options="$options -l $logdev"
> +	fi
> +
>  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
>  }
>  
> @@ -724,8 +729,18 @@ _scratch_xfs_mdrestore()
>  {
>  	local metadump=$1
>  	shift
> +	local logdev=none
> +	local options="$@"
>  
> -	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
> +	# $SCRATCH_LOGDEV should have a non-zero length value only when all of
> +	# the following conditions are met.
> +	# 1. Metadump is in v2 format.
> +	# 2. Metadump has contents dumped from an external log device.
> +	if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ]; then
> +		logdev=$SCRATCH_LOGDEV
> +	fi
> +
> +	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$logdev" "$@"
>  }
>  
>  # Do not use xfs_repair (offline fsck) to rebuild the filesystem
> -- 
> 2.43.0
> 
> 

[PATCH 4/5] xfs: Add support for testing metadump v2

[Chandan Babu R]

This commit adds the ability to test metadump v2 to existing metadump tests.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 tests/xfs/129     |  63 ++++++++++++---
 tests/xfs/129.out |   4 +-
 tests/xfs/234     |  63 ++++++++++++---
 tests/xfs/234.out |   4 +-
 tests/xfs/253     | 195 ++++++++++++++++++++++++++--------------------
 tests/xfs/291     |  25 +++++-
 tests/xfs/432     |  29 +++++--
 tests/xfs/432.out |   3 +-
 tests/xfs/503     |  94 +++++++++++++---------
 tests/xfs/503.out |  12 +--
 10 files changed, 326 insertions(+), 166 deletions(-)

diff --git a/tests/xfs/129 b/tests/xfs/129
index 6f2ef564..7226d57d 100755
--- a/tests/xfs/129
+++ b/tests/xfs/129
@@ -16,7 +16,11 @@ _cleanup()
 {
     cd /
     _scratch_unmount > /dev/null 2>&1
-    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image
+    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
+	    _destroy_loop_device $logdev
+    [[ -n $datadev ]] && _destroy_loop_device $datadev
+    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/data-image \
+       $TEST_DIR/log-image
 }
 
 # Import common functions.
@@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
 			$testdir/file2 $((nr * blksz)) $blksz >> $seqres.full
 done
 
-echo "Create metadump file"
 _scratch_unmount
-_scratch_xfs_metadump $metadump_file
 
-# Now restore the obfuscated one back and take a look around
-echo "Restore metadump"
-SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
-SCRATCH_DEV=$TEST_DIR/image _scratch_mount
-SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
 
-echo "Check restored fs"
-_check_generic_filesystem $metadump_file
+echo "Create metadump file, restore it and check restored fs"
+for md_version in $(seq 1 $max_md_version); do
+	# Determine the version to be passed to metadump/mdrestore
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file $version
+
+	# Now restore the obfuscated one back and take a look around
+
+	# Metadump v2 files can contain contents dumped from an external log
+	# device. Use a temporary file to hold the log device contents restored
+	# from such a metadump file.
+	slogdev=$TEST_DIR/log-image
+	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
+		slogdev=""
+	fi
+
+ 	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
+		   _scratch_xfs_mdrestore $metadump_file
+
+	datadev=$(_create_loop_device $TEST_DIR/data-image)
+
+	logdev=${SCRATCH_LOGDEV}
+	if [[ -s $TEST_DIR/log-image ]]; then
+		logdev=$(_create_loop_device $TEST_DIR/log-image)
+	fi
+
+	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
+	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
+
+	[[ -z $logdev ]] && logdev=none
+	_check_xfs_filesystem $datadev $logdev none
+
+	if [[ -s $TEST_DIR/log-image ]]; then
+		_destroy_loop_device $logdev
+		logdev=""
+		rm -f $TEST_DIR/log-image
+	fi
+
+	_destroy_loop_device $datadev
+	datadev=""
+	rm -f $TEST_DIR/data-image
+done
 
 # success, all done
 status=0
diff --git a/tests/xfs/129.out b/tests/xfs/129.out
index da6f43fd..0f24c431 100644
--- a/tests/xfs/129.out
+++ b/tests/xfs/129.out
@@ -1,6 +1,4 @@
 QA output created by 129
 Create the original file blocks
 Reflink every other block
-Create metadump file
-Restore metadump
-Check restored fs
+Create metadump file, restore it and check restored fs
diff --git a/tests/xfs/234 b/tests/xfs/234
index 57d447c0..2f6b1f65 100755
--- a/tests/xfs/234
+++ b/tests/xfs/234
@@ -16,7 +16,11 @@ _cleanup()
 {
     cd /
     _scratch_unmount > /dev/null 2>&1
-    rm -rf $tmp.* $metadump_file $TEST_DIR/image
+    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
+	    _destroy_loop_device $logdev
+    [[ -n $datadev ]] && _destroy_loop_device $datadev
+    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image \
+       $TEST_DIR/log-image
 }
 
 # Import common functions.
@@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
 	$XFS_IO_PROG -c "fpunch $((nr * blksz)) $blksz" $testdir/file1 >> $seqres.full
 done
 
-echo "Create metadump file"
 _scratch_unmount
-_scratch_xfs_metadump $metadump_file
 
-# Now restore the obfuscated one back and take a look around
-echo "Restore metadump"
-SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
-SCRATCH_DEV=$TEST_DIR/image _scratch_mount
-SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
 
-echo "Check restored fs"
-_check_generic_filesystem $metadump_file
+echo "Create metadump file, restore it and check restored fs"
+for md_version in $(seq 1 $max_md_version); do
+	# Determine the version to be passed to metadump/mdrestore
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file $version
+
+	# Now restore the obfuscated one back and take a look around
+
+	# Metadump v2 files can contain contents dumped from an external log
+	# device. Use a temporary file to hold the log device contents restored
+	# from such a metadump file.
+	slogdev=$TEST_DIR/log-image
+	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
+		slogdev=""
+	fi
+
+	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
+		   _scratch_xfs_mdrestore $metadump_file
+
+	datadev=$(_create_loop_device $TEST_DIR/data-image)
+
+	logdev=${SCRATCH_LOGDEV}
+	if [[ -s $TEST_DIR/log-image ]]; then
+		logdev=$(_create_loop_device $TEST_DIR/log-image)
+	fi
+
+	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
+	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
+
+	[[ -z $logdev ]] && logdev=none
+	_check_xfs_filesystem $datadev $logdev none
+
+	if [[ -s $TEST_DIR/log-image ]]; then
+		_destroy_loop_device $logdev
+		logdev=""
+		rm -f $TEST_DIR/log-image
+	fi
+
+	_destroy_loop_device $datadev
+	datadev=""
+	rm -f $TEST_DIR/data-image
+done
 
 # success, all done
 status=0
diff --git a/tests/xfs/234.out b/tests/xfs/234.out
index 463d4660..fc2ddd77 100644
--- a/tests/xfs/234.out
+++ b/tests/xfs/234.out
@@ -1,6 +1,4 @@
 QA output created by 234
 Create the original file blocks
 Punch every other block
-Create metadump file
-Restore metadump
-Check restored fs
+Create metadump file, restore it and check restored fs
diff --git a/tests/xfs/253 b/tests/xfs/253
index ce902477..b69a1faf 100755
--- a/tests/xfs/253
+++ b/tests/xfs/253
@@ -52,114 +52,139 @@ function create_file() {
 echo "Disciplyne of silence is goed."
 
 _scratch_mkfs >/dev/null 2>&1
-_scratch_mount
 
-# Initialize and mount the scratch filesystem, then create a bunch
-# of files that exercise the original problem.
-#
-# The problem arose when a file name produced a hash that contained
-# either 0x00 (string terminator) or 0x27 ('/' character) in a
-# spot used to determine a character in an obfuscated name.  This
-# occurred in one of 5 spots at the end of the name, at position
-# (last-4), (last-3), (last-2), (last-1), or (last).
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
 
-rm -f "${METADUMP_FILE}"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
 
-mkdir -p "${OUTPUT_DIR}"
+	cd $here
 
-cd "${OUTPUT_DIR}"
-# Start out with some basic test files
-create_file 'abcde'		# hash 0x1c58f263 ("normal" name)
+	# Initialize and mount the scratch filesystem, then create a bunch of
+	# files that exercise the original problem.
+	#
+	# The problem arose when a file name produced a hash that contained
+	# either 0x00 (string terminator) or 0x27 ('/' character) in a spot used
+	# to determine a character in an obfuscated name.  This occurred in one
+	# of 5 spots at the end of the name, at position (last-4), (last-3),
+	# (last-2), (last-1), or (last).
 
-create_file 'f'			# hash 0x00000066 (1-byte name)
-create_file 'gh'		# hash 0x000033e8 (2-byte name)
-create_file 'ijk'		# hash 0x001a756b (3-byte name)
-create_file 'lmno'		# hash 0x0d9b776f (4-byte name)
-create_file 'pqrstu'		# hash 0x1e5cf9f2 (6-byte name)
-create_file 'abcdefghijklmnopqrstuvwxyz' # a most remarkable word (0x55004ae3)
+	_scratch_unmount >> $seqres.full 2>&1
 
-# Create a short directory name; it won't be obfuscated.  Populate
-# it with some longer named-files.  The first part of the obfuscated
-# filenames should use printable characters.
-mkdir foo
-create_file 'foo/longer_file_name_1'	# hash 0xe83634ec
-create_file 'foo/longer_file_name_2'	# hash 0xe83634ef
-create_file 'foo/longer_file_name_3'	# hash 0xe83634ee
+	_scratch_mkfs >> $seqres.full 2>&1
 
-# Now create a longer directory name
-mkdir longer_directory_name
-create_file 'longer_directory_name/f1'	# directory hash 0x9c7accdd
-create_file 'longer_directory_name/f2'	# filenames are short, no hash
-create_file 'longer_directory_name/f3'
+	_scratch_mount
 
-# The problematic name originally reported by Arkadiusz Miśkiewicz
+	rm -f "${METADUMP_FILE}"
 
-create_file 'R\323\257NE'	# hash 0x3a4be740, forces  (last-3) = 0x2f
+	mkdir -p "${OUTPUT_DIR}"
 
-# Other names that force a 0x00 byte
-create_file 'Pbcde'		# hash 0x0c58f260, forces  (last-4) = 0x00
-create_file 'a\001\203de'	# hash 0x1000f263, forces  (last-3) = 0x00
-create_file 'ab\001\344e'	# hash 0x1c403263, forces  (last-2) = 0x00
-create_file 'abc\200e'		# hash 0x1c588063, forces  (last-1) = 0x00
-create_file 'abcd\006'		# hash 0x1c58f200, forces    (last) = 0x00
+	cd "${OUTPUT_DIR}"
+	# Start out with some basic test files
+	create_file 'abcde'		# hash 0x1c58f263 ("normal" name)
 
-# Names that force a 0x2f byte; note no name will ever force (last-4) = 0x2f
-create_file 'a.\343de'		# hash 0x15f8f263 forces   (last-3) = 0x00
-create_file 'ac\257de'		# hash 0x1c4bf263, forces  (last-2) = 0x2f
-create_file 'abe\257e'		# hash 0x1c5917e3, forces  (last-1) = 0x2f
-create_file 'abcd)'		# hash 0x1c58f22f, forces    (last) = 0x2f
+	create_file 'f'			# hash 0x00000066 (1-byte name)
+	create_file 'gh'		# hash 0x000033e8 (2-byte name)
+	create_file 'ijk'		# hash 0x001a756b (3-byte name)
+	create_file 'lmno'		# hash 0x0d9b776f (4-byte name)
+	create_file 'pqrstu'		# hash 0x1e5cf9f2 (6-byte name)
+	create_file 'abcdefghijklmnopqrstuvwxyz' # a most remarkable word (0x55004ae3)
 
-# The following names are possible results of obfuscating the name
-# "abcde".  Previously, xfs_metadump could get hung up trying to
-# obfuscate names when too many of the same length had the same hash
-# value.
-create_file '!bcda'		# essentially a dup of 'abcde'
-create_file 'Abcdg'		# essentially a dup of 'abcde'
-create_file 'qbcdd'		# essentially a dup of 'abcde'
-create_file '1bcd`'		# essentially a dup of 'abcde'
-create_file 'Qbcdf'		# essentially a dup of 'abcde'
-create_file '\001bcdc'		# essentially a dup of 'abcde'
-create_file 'Qbce\346'		# essentially a dup of 'abcde'
-create_file 'abb\344e'		# essentially a dup of 'abcde'
+	# Create a short directory name; it won't be obfuscated.  Populate it
+	# with some longer named-files.  The first part of the obfuscated
+	# filenames should use printable characters.
+	mkdir foo
+	create_file 'foo/longer_file_name_1'	# hash 0xe83634ec
+	create_file 'foo/longer_file_name_2'	# hash 0xe83634ef
+	create_file 'foo/longer_file_name_3'	# hash 0xe83634ee
 
-# The orphanage directory (lost+found) should not be obfuscated.
-# Files thereunder can be, but not if their name is the same as
-# their inode number.  Test this.
+	# Now create a longer directory name
+	mkdir longer_directory_name
+	create_file 'longer_directory_name/f1'	# directory hash 0x9c7accdd
+	create_file 'longer_directory_name/f2'	# filenames are short, no hash
+	create_file 'longer_directory_name/f3'
 
-cd "${SCRATCH_MNT}"
-mkdir -p "${ORPHANAGE}"
+	# The problematic name originally reported by Arkadiusz Miśkiewicz
 
-TEMP_ORPHAN="${ORPHANAGE}/__orphan__"
-NON_ORPHAN="${ORPHANAGE}/__should_be_obfuscated__"
+	create_file 'R\323\257NE'	# hash 0x3a4be740, forces  (last-3) = 0x2f
 
-# Create an orphan, whose name is the same as its inode number
-touch "${TEMP_ORPHAN}"
-INUM=$(ls -i "${TEMP_ORPHAN}" | awk '{ print $1; }')
-ORPHAN="${SCRATCH_MNT}/lost+found/${INUM}"
-mv "${TEMP_ORPHAN}" "${ORPHAN}"
+	# Other names that force a 0x00 byte
+	create_file 'Pbcde'		# hash 0x0c58f260, forces  (last-4) = 0x00
+	create_file 'a\001\203de'	# hash 0x1000f263, forces  (last-3) = 0x00
+	create_file 'ab\001\344e'	# hash 0x1c403263, forces  (last-2) = 0x00
+	create_file 'abc\200e'		# hash 0x1c588063, forces  (last-1) = 0x00
+	create_file 'abcd\006'		# hash 0x1c58f200, forces    (last) = 0x00
 
-# Create non-orphan, which *should* be obfuscated
-touch "${NON_ORPHAN}"
+	# Names that force a 0x2f byte; note no name will ever force (last-4) = 0x2f
+	create_file 'a.\343de'		# hash 0x15f8f263 forces   (last-3) = 0x00
+	create_file 'ac\257de'		# hash 0x1c4bf263, forces  (last-2) = 0x2f
+	create_file 'abe\257e'		# hash 0x1c5917e3, forces  (last-1) = 0x2f
+	create_file 'abcd)'		# hash 0x1c58f22f, forces    (last) = 0x2f
 
-# Get a listing of all the files before obfuscation
-ls -R >> $seqres.full
-ls -R | od -c >> $seqres.full
+	# The following names are possible results of obfuscating the name
+	# "abcde".  Previously, xfs_metadump could get hung up trying to
+	# obfuscate names when too many of the same length had the same hash
+	# value.
+	create_file '!bcda'		# essentially a dup of 'abcde'
+	create_file 'Abcdg'		# essentially a dup of 'abcde'
+	create_file 'qbcdd'		# essentially a dup of 'abcde'
+	create_file '1bcd`'		# essentially a dup of 'abcde'
+	create_file 'Qbcdf'		# essentially a dup of 'abcde'
+	create_file '\001bcdc'		# essentially a dup of 'abcde'
+	create_file 'Qbce\346'		# essentially a dup of 'abcde'
+	create_file 'abb\344e'		# essentially a dup of 'abcde'
 
-# Now unmount the filesystem and create a metadump file
-cd $here
+	# The orphanage directory (lost+found) should not be obfuscated.
+	# Files thereunder can be, but not if their name is the same as
+	# their inode number.  Test this.
 
-_scratch_unmount
-_scratch_xfs_metadump $METADUMP_FILE
+	cd "${SCRATCH_MNT}"
+	mkdir -p "${ORPHANAGE}"
 
-# Now restore the obfuscated one back and take a look around
-_scratch_xfs_mdrestore "$METADUMP_FILE"
+	TEMP_ORPHAN="${ORPHANAGE}/__orphan__"
+	NON_ORPHAN="${ORPHANAGE}/__should_be_obfuscated__"
 
-_scratch_mount
+	# Create an orphan, whose name is the same as its inode number
+	touch "${TEMP_ORPHAN}"
+	INUM=$(ls -i "${TEMP_ORPHAN}" | awk '{ print $1; }')
+	ORPHAN="${SCRATCH_MNT}/lost+found/${INUM}"
+	mv "${TEMP_ORPHAN}" "${ORPHAN}"
 
-# Get a listing of all the files after obfuscation
-cd ${SCRATCH_MNT}
-ls -R >> $seqres.full
-ls -R | od -c >> $seqres.full
+	# Create non-orphan, which *should* be obfuscated
+	touch "${NON_ORPHAN}"
+
+	# Get a listing of all the files before obfuscation
+	ls -R >> $seqres.full
+	ls -R | od -c >> $seqres.full
+
+	# Now unmount the filesystem and create a metadump file
+	cd $here
+
+	_scratch_unmount
+	
+	_scratch_xfs_metadump $METADUMP_FILE $version
+
+	# Now restore the obfuscated one back and take a look around
+
+	slogdev=$SCRATCH_LOGDEV
+	if [[ -z $version || $version == "-v 1" ]]; then
+		slogdev=""
+	fi
+
+	SCRATCH_LOGDEV=${slogdev} _scratch_xfs_mdrestore "$METADUMP_FILE"
+
+	_scratch_mount
+
+	cd "${SCRATCH_MNT}"
+
+	# Get a listing of all the files after obfuscation
+	ls -R >> $seqres.full
+	ls -R | od -c >> $seqres.full
+done
 
 # Finally, re-make the filesystem since to ensure we don't
 # leave a directory with duplicate entries lying around.
diff --git a/tests/xfs/291 b/tests/xfs/291
index 54448497..33193eb7 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -92,10 +92,27 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
 
 # Yes they can!  Now...
 # Can xfs_metadump cope with this monster?
-_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
-SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
-SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
-	_fail "xfs_repair of metadump failed"
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
+
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $tmp.metadump -a -o $version || \
+		_fail "xfs_metadump failed"
+
+	slogdev=$SCRATCH_LOGDEV
+	if [[ -z $version || $version == "-v 1" ]]; then
+		slogdev=""
+	fi
+	SCRATCH_DEV=$tmp.img SCRATCH_LOGDEV=$slogdev _scratch_xfs_mdrestore \
+		   $tmp.metadump || _fail "xfs_mdrestore failed"
+	SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
+		_fail "xfs_repair of metadump failed"
+done
 
 # Yes it can; success, all done
 status=0
diff --git a/tests/xfs/432 b/tests/xfs/432
index dae68fb2..a215d3ce 100755
--- a/tests/xfs/432
+++ b/tests/xfs/432
@@ -50,6 +50,7 @@ echo "Format and mount"
 # block.  8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
 # dablock.  33 dirblocks * 64k mean that we can expand a directory by
 # 2112k before we have to allocate another da btree block.
+
 _scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
 _scratch_mount >> "$seqres.full" 2>&1
 
@@ -85,13 +86,29 @@ extlen="$(check_for_long_extent $dir_inum)"
 echo "qualifying extent: $extlen blocks" >> $seqres.full
 test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
 
-echo "Try to metadump"
-_scratch_xfs_metadump $metadump_file -a -o -w
-SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
+echo "Try to metadump, restore and check restored metadump image"
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
 
-echo "Check restored metadump image"
-SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
-	echo "xfs_repair on restored fs returned $?"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -a -o -w $version
+
+	slogdev=$SCRATCH_LOGDEV
+	if [[ -z $version || $version == "-v 1" ]]; then
+		slogdev=""
+	fi
+
+	SCRATCH_DEV=$metadump_img SCRATCH_LOGDEV=$slogdev \
+		   _scratch_xfs_mdrestore $metadump_file
+
+	SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
+		echo "xfs_repair on restored fs returned $?"
+done
 
 # success, all done
 status=0
diff --git a/tests/xfs/432.out b/tests/xfs/432.out
index 1f135d16..37bac902 100644
--- a/tests/xfs/432.out
+++ b/tests/xfs/432.out
@@ -2,5 +2,4 @@ QA output created by 432
 Format and mount
 Create huge dir
 Check for > 1000 block extent?
-Try to metadump
-Check restored metadump image
+Try to metadump, restore and check restored metadump image
diff --git a/tests/xfs/503 b/tests/xfs/503
index 8805632d..a1479eb6 100755
--- a/tests/xfs/503
+++ b/tests/xfs/503
@@ -29,6 +29,7 @@ testdir=$TEST_DIR/test-$seq
 _supported_fs xfs
 
 _require_command "$XFS_MDRESTORE_PROG" "xfs_mdrestore"
+_require_loop
 _require_xfs_copy
 _require_scratch_nocheck
 _require_populate_commands
@@ -40,22 +41,69 @@ _scratch_populate_cached nofill > $seqres.full 2>&1
 
 mkdir -p $testdir
 metadump_file=$testdir/scratch.md
-metadump_file_a=${metadump_file}.a
-metadump_file_g=${metadump_file}.g
-metadump_file_ag=${metadump_file}.ag
 copy_file=$testdir/copy.img
 
-echo metadump
-_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
+check_restored_metadump_image()
+{
+	local image=$1
 
-echo metadump a
-_scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
+	loop_dev=$(_create_loop_device $image)
+	SCRATCH_DEV=$loop_dev _scratch_mount
+	SCRATCH_DEV=$loop_dev _check_scratch_fs
+	SCRATCH_DEV=$loop_dev _scratch_unmount
+	_destroy_loop_device $loop_dev
+}
 
-echo metadump g
-_scratch_xfs_metadump $metadump_file_g -g >> $seqres.full
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
 
-echo metadump ag
-_scratch_xfs_metadump $metadump_file_ag -a -g >> $seqres.full
+echo "metadump and mdrestore"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -a -o $version >> $seqres.full
+	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
+	check_restored_metadump_image $TEST_DIR/image
+done
+
+echo "metadump a and mdrestore"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -a $version >> $seqres.full
+	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
+	check_restored_metadump_image $TEST_DIR/image
+done
+
+echo "metadump g and mdrestore"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -g $version >> $seqres.full
+	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
+	check_restored_metadump_image $TEST_DIR/image
+done
+
+echo "metadump ag and mdrestore"
+for md_version in $(seq 1 $max_md_version); do
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -a -g $version >> $seqres.full
+	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
+	check_restored_metadump_image $TEST_DIR/image
+done
 
 echo copy
 $XFS_COPY_PROG $SCRATCH_DEV $copy_file >> $seqres.full
@@ -67,30 +115,6 @@ _scratch_mount
 _check_scratch_fs
 _scratch_unmount
 
-echo mdrestore
-_scratch_xfs_mdrestore $metadump_file
-_scratch_mount
-_check_scratch_fs
-_scratch_unmount
-
-echo mdrestore a
-_scratch_xfs_mdrestore $metadump_file_a
-_scratch_mount
-_check_scratch_fs
-_scratch_unmount
-
-echo mdrestore g
-_scratch_xfs_mdrestore $metadump_file_g
-_scratch_mount
-_check_scratch_fs
-_scratch_unmount
-
-echo mdrestore ag
-_scratch_xfs_mdrestore $metadump_file_ag
-_scratch_mount
-_check_scratch_fs
-_scratch_unmount
-
 # success, all done
 status=0
 exit
diff --git a/tests/xfs/503.out b/tests/xfs/503.out
index 8ef31dbe..496f2516 100644
--- a/tests/xfs/503.out
+++ b/tests/xfs/503.out
@@ -1,12 +1,8 @@
 QA output created by 503
 Format and populate
-metadump
-metadump a
-metadump g
-metadump ag
+metadump and mdrestore
+metadump a and mdrestore
+metadump g and mdrestore
+metadump ag and mdrestore
 copy
 recopy
-mdrestore
-mdrestore a
-mdrestore g
-mdrestore ag
-- 
2.43.0

Re: [PATCH 4/5] xfs: Add support for testing metadump v2

[Darrick J. Wong]

On Tue, Jan 02, 2024 at 02:13:51PM +0530, Chandan Babu R wrote:
> This commit adds the ability to test metadump v2 to existing metadump tests.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  tests/xfs/129     |  63 ++++++++++++---
>  tests/xfs/129.out |   4 +-
>  tests/xfs/234     |  63 ++++++++++++---
>  tests/xfs/234.out |   4 +-
>  tests/xfs/253     | 195 ++++++++++++++++++++++++++--------------------
>  tests/xfs/291     |  25 +++++-
>  tests/xfs/432     |  29 +++++--
>  tests/xfs/432.out |   3 +-
>  tests/xfs/503     |  94 +++++++++++++---------
>  tests/xfs/503.out |  12 +--
>  10 files changed, 326 insertions(+), 166 deletions(-)
> 
> diff --git a/tests/xfs/129 b/tests/xfs/129
> index 6f2ef564..7226d57d 100755
> --- a/tests/xfs/129
> +++ b/tests/xfs/129
> @@ -16,7 +16,11 @@ _cleanup()
>  {
>      cd /
>      _scratch_unmount > /dev/null 2>&1
> -    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image
> +    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
> +	    _destroy_loop_device $logdev
> +    [[ -n $datadev ]] && _destroy_loop_device $datadev
> +    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/data-image \
> +       $TEST_DIR/log-image
>  }
>  
>  # Import common functions.
> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>  			$testdir/file2 $((nr * blksz)) $blksz >> $seqres.full
>  done
>  
> -echo "Create metadump file"
>  _scratch_unmount
> -_scratch_xfs_metadump $metadump_file
>  
> -# Now restore the obfuscated one back and take a look around
> -echo "Restore metadump"
> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
>  
> -echo "Check restored fs"
> -_check_generic_filesystem $metadump_file
> +echo "Create metadump file, restore it and check restored fs"
> +for md_version in $(seq 1 $max_md_version); do
> +	# Determine the version to be passed to metadump/mdrestore
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file $version
> +
> +	# Now restore the obfuscated one back and take a look around
> +
> +	# Metadump v2 files can contain contents dumped from an external log
> +	# device. Use a temporary file to hold the log device contents restored
> +	# from such a metadump file.
> +	slogdev=$TEST_DIR/log-image
> +	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
> +		slogdev=""
> +	fi
> +
> + 	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \

   ^ space before tab

> +		   _scratch_xfs_mdrestore $metadump_file
> +
> +	datadev=$(_create_loop_device $TEST_DIR/data-image)
> +
> +	logdev=${SCRATCH_LOGDEV}
> +	if [[ -s $TEST_DIR/log-image ]]; then
> +		logdev=$(_create_loop_device $TEST_DIR/log-image)
> +	fi
> +
> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
> +
> +	[[ -z $logdev ]] && logdev=none
> +	_check_xfs_filesystem $datadev $logdev none
> +
> +	if [[ -s $TEST_DIR/log-image ]]; then
> +		_destroy_loop_device $logdev
> +		logdev=""
> +		rm -f $TEST_DIR/log-image
> +	fi
> +
> +	_destroy_loop_device $datadev
> +	datadev=""
> +	rm -f $TEST_DIR/data-image
> +done

Given the major differences between v1 and v2, would the two metadump
tests be easier to understand if this loop were unrolled?

I like that it mdrestores to a separate device to preserve the original
$SCRATCH_DEV metadata between metadump v1 and v2 testing.

>  # success, all done
>  status=0
> diff --git a/tests/xfs/129.out b/tests/xfs/129.out
> index da6f43fd..0f24c431 100644
> --- a/tests/xfs/129.out
> +++ b/tests/xfs/129.out
> @@ -1,6 +1,4 @@
>  QA output created by 129
>  Create the original file blocks
>  Reflink every other block
> -Create metadump file
> -Restore metadump
> -Check restored fs
> +Create metadump file, restore it and check restored fs
> diff --git a/tests/xfs/234 b/tests/xfs/234
> index 57d447c0..2f6b1f65 100755
> --- a/tests/xfs/234
> +++ b/tests/xfs/234
> @@ -16,7 +16,11 @@ _cleanup()
>  {
>      cd /
>      _scratch_unmount > /dev/null 2>&1
> -    rm -rf $tmp.* $metadump_file $TEST_DIR/image
> +    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
> +	    _destroy_loop_device $logdev
> +    [[ -n $datadev ]] && _destroy_loop_device $datadev
> +    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image \
> +       $TEST_DIR/log-image
>  }
>  
>  # Import common functions.
> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>  	$XFS_IO_PROG -c "fpunch $((nr * blksz)) $blksz" $testdir/file1 >> $seqres.full
>  done
>  
> -echo "Create metadump file"
>  _scratch_unmount
> -_scratch_xfs_metadump $metadump_file
>  
> -# Now restore the obfuscated one back and take a look around
> -echo "Restore metadump"
> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
>  
> -echo "Check restored fs"
> -_check_generic_filesystem $metadump_file
> +echo "Create metadump file, restore it and check restored fs"
> +for md_version in $(seq 1 $max_md_version); do
> +	# Determine the version to be passed to metadump/mdrestore
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file $version
> +
> +	# Now restore the obfuscated one back and take a look around
> +
> +	# Metadump v2 files can contain contents dumped from an external log
> +	# device. Use a temporary file to hold the log device contents restored
> +	# from such a metadump file.
> +	slogdev=$TEST_DIR/log-image
> +	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
> +		slogdev=""
> +	fi
> +
> +	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
> +		   _scratch_xfs_mdrestore $metadump_file
> +
> +	datadev=$(_create_loop_device $TEST_DIR/data-image)
> +
> +	logdev=${SCRATCH_LOGDEV}
> +	if [[ -s $TEST_DIR/log-image ]]; then
> +		logdev=$(_create_loop_device $TEST_DIR/log-image)
> +	fi
> +
> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
> +
> +	[[ -z $logdev ]] && logdev=none
> +	_check_xfs_filesystem $datadev $logdev none
> +
> +	if [[ -s $TEST_DIR/log-image ]]; then
> +		_destroy_loop_device $logdev
> +		logdev=""
> +		rm -f $TEST_DIR/log-image
> +	fi
> +
> +	_destroy_loop_device $datadev
> +	datadev=""
> +	rm -f $TEST_DIR/data-image
> +done
>  
>  # success, all done
>  status=0
> diff --git a/tests/xfs/234.out b/tests/xfs/234.out
> index 463d4660..fc2ddd77 100644
> --- a/tests/xfs/234.out
> +++ b/tests/xfs/234.out
> @@ -1,6 +1,4 @@
>  QA output created by 234
>  Create the original file blocks
>  Punch every other block
> -Create metadump file
> -Restore metadump
> -Check restored fs
> +Create metadump file, restore it and check restored fs
> diff --git a/tests/xfs/253 b/tests/xfs/253
> index ce902477..b69a1faf 100755
> --- a/tests/xfs/253
> +++ b/tests/xfs/253
> @@ -52,114 +52,139 @@ function create_file() {
>  echo "Disciplyne of silence is goed."
>  
>  _scratch_mkfs >/dev/null 2>&1
> -_scratch_mount
>  
> -# Initialize and mount the scratch filesystem, then create a bunch
> -# of files that exercise the original problem.
> -#
> -# The problem arose when a file name produced a hash that contained
> -# either 0x00 (string terminator) or 0x27 ('/' character) in a
> -# spot used to determine a character in an obfuscated name.  This
> -# occurred in one of 5 spots at the end of the name, at position
> -# (last-4), (last-3), (last-2), (last-1), or (last).

I wonder, could you create the scratch fs and only then move into
testing v1 and v2 metadump?  Rather than doing the create_file stuff
twice?  Or do we actually end up with a different fs between the two
iterations?

The other two tests here look good enough for now...

--D

> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
>  
> -rm -f "${METADUMP_FILE}"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
>  
> -mkdir -p "${OUTPUT_DIR}"
> +	cd $here
>  
> -cd "${OUTPUT_DIR}"
> -# Start out with some basic test files
> -create_file 'abcde'		# hash 0x1c58f263 ("normal" name)
> +	# Initialize and mount the scratch filesystem, then create a bunch of
> +	# files that exercise the original problem.
> +	#
> +	# The problem arose when a file name produced a hash that contained
> +	# either 0x00 (string terminator) or 0x27 ('/' character) in a spot used
> +	# to determine a character in an obfuscated name.  This occurred in one
> +	# of 5 spots at the end of the name, at position (last-4), (last-3),
> +	# (last-2), (last-1), or (last).
>  
> -create_file 'f'			# hash 0x00000066 (1-byte name)
> -create_file 'gh'		# hash 0x000033e8 (2-byte name)
> -create_file 'ijk'		# hash 0x001a756b (3-byte name)
> -create_file 'lmno'		# hash 0x0d9b776f (4-byte name)
> -create_file 'pqrstu'		# hash 0x1e5cf9f2 (6-byte name)
> -create_file 'abcdefghijklmnopqrstuvwxyz' # a most remarkable word (0x55004ae3)
> +	_scratch_unmount >> $seqres.full 2>&1
>  
> -# Create a short directory name; it won't be obfuscated.  Populate
> -# it with some longer named-files.  The first part of the obfuscated
> -# filenames should use printable characters.
> -mkdir foo
> -create_file 'foo/longer_file_name_1'	# hash 0xe83634ec
> -create_file 'foo/longer_file_name_2'	# hash 0xe83634ef
> -create_file 'foo/longer_file_name_3'	# hash 0xe83634ee
> +	_scratch_mkfs >> $seqres.full 2>&1
>  
> -# Now create a longer directory name
> -mkdir longer_directory_name
> -create_file 'longer_directory_name/f1'	# directory hash 0x9c7accdd
> -create_file 'longer_directory_name/f2'	# filenames are short, no hash
> -create_file 'longer_directory_name/f3'
> +	_scratch_mount
>  
> -# The problematic name originally reported by Arkadiusz Miśkiewicz
> +	rm -f "${METADUMP_FILE}"
>  
> -create_file 'R\323\257NE'	# hash 0x3a4be740, forces  (last-3) = 0x2f
> +	mkdir -p "${OUTPUT_DIR}"
>  
> -# Other names that force a 0x00 byte
> -create_file 'Pbcde'		# hash 0x0c58f260, forces  (last-4) = 0x00
> -create_file 'a\001\203de'	# hash 0x1000f263, forces  (last-3) = 0x00
> -create_file 'ab\001\344e'	# hash 0x1c403263, forces  (last-2) = 0x00
> -create_file 'abc\200e'		# hash 0x1c588063, forces  (last-1) = 0x00
> -create_file 'abcd\006'		# hash 0x1c58f200, forces    (last) = 0x00
> +	cd "${OUTPUT_DIR}"
> +	# Start out with some basic test files
> +	create_file 'abcde'		# hash 0x1c58f263 ("normal" name)
>  
> -# Names that force a 0x2f byte; note no name will ever force (last-4) = 0x2f
> -create_file 'a.\343de'		# hash 0x15f8f263 forces   (last-3) = 0x00
> -create_file 'ac\257de'		# hash 0x1c4bf263, forces  (last-2) = 0x2f
> -create_file 'abe\257e'		# hash 0x1c5917e3, forces  (last-1) = 0x2f
> -create_file 'abcd)'		# hash 0x1c58f22f, forces    (last) = 0x2f
> +	create_file 'f'			# hash 0x00000066 (1-byte name)
> +	create_file 'gh'		# hash 0x000033e8 (2-byte name)
> +	create_file 'ijk'		# hash 0x001a756b (3-byte name)
> +	create_file 'lmno'		# hash 0x0d9b776f (4-byte name)
> +	create_file 'pqrstu'		# hash 0x1e5cf9f2 (6-byte name)
> +	create_file 'abcdefghijklmnopqrstuvwxyz' # a most remarkable word (0x55004ae3)
>  
> -# The following names are possible results of obfuscating the name
> -# "abcde".  Previously, xfs_metadump could get hung up trying to
> -# obfuscate names when too many of the same length had the same hash
> -# value.
> -create_file '!bcda'		# essentially a dup of 'abcde'
> -create_file 'Abcdg'		# essentially a dup of 'abcde'
> -create_file 'qbcdd'		# essentially a dup of 'abcde'
> -create_file '1bcd`'		# essentially a dup of 'abcde'
> -create_file 'Qbcdf'		# essentially a dup of 'abcde'
> -create_file '\001bcdc'		# essentially a dup of 'abcde'
> -create_file 'Qbce\346'		# essentially a dup of 'abcde'
> -create_file 'abb\344e'		# essentially a dup of 'abcde'
> +	# Create a short directory name; it won't be obfuscated.  Populate it
> +	# with some longer named-files.  The first part of the obfuscated
> +	# filenames should use printable characters.
> +	mkdir foo
> +	create_file 'foo/longer_file_name_1'	# hash 0xe83634ec
> +	create_file 'foo/longer_file_name_2'	# hash 0xe83634ef
> +	create_file 'foo/longer_file_name_3'	# hash 0xe83634ee
>  
> -# The orphanage directory (lost+found) should not be obfuscated.
> -# Files thereunder can be, but not if their name is the same as
> -# their inode number.  Test this.
> +	# Now create a longer directory name
> +	mkdir longer_directory_name
> +	create_file 'longer_directory_name/f1'	# directory hash 0x9c7accdd
> +	create_file 'longer_directory_name/f2'	# filenames are short, no hash
> +	create_file 'longer_directory_name/f3'
>  
> -cd "${SCRATCH_MNT}"
> -mkdir -p "${ORPHANAGE}"
> +	# The problematic name originally reported by Arkadiusz Miśkiewicz
>  
> -TEMP_ORPHAN="${ORPHANAGE}/__orphan__"
> -NON_ORPHAN="${ORPHANAGE}/__should_be_obfuscated__"
> +	create_file 'R\323\257NE'	# hash 0x3a4be740, forces  (last-3) = 0x2f
>  
> -# Create an orphan, whose name is the same as its inode number
> -touch "${TEMP_ORPHAN}"
> -INUM=$(ls -i "${TEMP_ORPHAN}" | awk '{ print $1; }')
> -ORPHAN="${SCRATCH_MNT}/lost+found/${INUM}"
> -mv "${TEMP_ORPHAN}" "${ORPHAN}"
> +	# Other names that force a 0x00 byte
> +	create_file 'Pbcde'		# hash 0x0c58f260, forces  (last-4) = 0x00
> +	create_file 'a\001\203de'	# hash 0x1000f263, forces  (last-3) = 0x00
> +	create_file 'ab\001\344e'	# hash 0x1c403263, forces  (last-2) = 0x00
> +	create_file 'abc\200e'		# hash 0x1c588063, forces  (last-1) = 0x00
> +	create_file 'abcd\006'		# hash 0x1c58f200, forces    (last) = 0x00
>  
> -# Create non-orphan, which *should* be obfuscated
> -touch "${NON_ORPHAN}"
> +	# Names that force a 0x2f byte; note no name will ever force (last-4) = 0x2f
> +	create_file 'a.\343de'		# hash 0x15f8f263 forces   (last-3) = 0x00
> +	create_file 'ac\257de'		# hash 0x1c4bf263, forces  (last-2) = 0x2f
> +	create_file 'abe\257e'		# hash 0x1c5917e3, forces  (last-1) = 0x2f
> +	create_file 'abcd)'		# hash 0x1c58f22f, forces    (last) = 0x2f
>  
> -# Get a listing of all the files before obfuscation
> -ls -R >> $seqres.full
> -ls -R | od -c >> $seqres.full
> +	# The following names are possible results of obfuscating the name
> +	# "abcde".  Previously, xfs_metadump could get hung up trying to
> +	# obfuscate names when too many of the same length had the same hash
> +	# value.
> +	create_file '!bcda'		# essentially a dup of 'abcde'
> +	create_file 'Abcdg'		# essentially a dup of 'abcde'
> +	create_file 'qbcdd'		# essentially a dup of 'abcde'
> +	create_file '1bcd`'		# essentially a dup of 'abcde'
> +	create_file 'Qbcdf'		# essentially a dup of 'abcde'
> +	create_file '\001bcdc'		# essentially a dup of 'abcde'
> +	create_file 'Qbce\346'		# essentially a dup of 'abcde'
> +	create_file 'abb\344e'		# essentially a dup of 'abcde'
>  
> -# Now unmount the filesystem and create a metadump file
> -cd $here
> +	# The orphanage directory (lost+found) should not be obfuscated.
> +	# Files thereunder can be, but not if their name is the same as
> +	# their inode number.  Test this.
>  
> -_scratch_unmount
> -_scratch_xfs_metadump $METADUMP_FILE
> +	cd "${SCRATCH_MNT}"
> +	mkdir -p "${ORPHANAGE}"
>  
> -# Now restore the obfuscated one back and take a look around
> -_scratch_xfs_mdrestore "$METADUMP_FILE"
> +	TEMP_ORPHAN="${ORPHANAGE}/__orphan__"
> +	NON_ORPHAN="${ORPHANAGE}/__should_be_obfuscated__"
>  
> -_scratch_mount
> +	# Create an orphan, whose name is the same as its inode number
> +	touch "${TEMP_ORPHAN}"
> +	INUM=$(ls -i "${TEMP_ORPHAN}" | awk '{ print $1; }')
> +	ORPHAN="${SCRATCH_MNT}/lost+found/${INUM}"
> +	mv "${TEMP_ORPHAN}" "${ORPHAN}"
>  
> -# Get a listing of all the files after obfuscation
> -cd ${SCRATCH_MNT}
> -ls -R >> $seqres.full
> -ls -R | od -c >> $seqres.full
> +	# Create non-orphan, which *should* be obfuscated
> +	touch "${NON_ORPHAN}"
> +
> +	# Get a listing of all the files before obfuscation
> +	ls -R >> $seqres.full
> +	ls -R | od -c >> $seqres.full
> +
> +	# Now unmount the filesystem and create a metadump file
> +	cd $here
> +
> +	_scratch_unmount
> +	
> +	_scratch_xfs_metadump $METADUMP_FILE $version
> +
> +	# Now restore the obfuscated one back and take a look around
> +
> +	slogdev=$SCRATCH_LOGDEV
> +	if [[ -z $version || $version == "-v 1" ]]; then
> +		slogdev=""
> +	fi
> +
> +	SCRATCH_LOGDEV=${slogdev} _scratch_xfs_mdrestore "$METADUMP_FILE"
> +
> +	_scratch_mount
> +
> +	cd "${SCRATCH_MNT}"
> +
> +	# Get a listing of all the files after obfuscation
> +	ls -R >> $seqres.full
> +	ls -R | od -c >> $seqres.full
> +done
>  
>  # Finally, re-make the filesystem since to ensure we don't
>  # leave a directory with duplicate entries lying around.
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index 54448497..33193eb7 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -92,10 +92,27 @@ _scratch_xfs_check >> $seqres.full 2>&1 || _fail "xfs_check failed"
>  
>  # Yes they can!  Now...
>  # Can xfs_metadump cope with this monster?
> -_scratch_xfs_metadump $tmp.metadump -a -o || _fail "xfs_metadump failed"
> -SCRATCH_DEV=$tmp.img _scratch_xfs_mdrestore $tmp.metadump || _fail "xfs_mdrestore failed"
> -SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
> -	_fail "xfs_repair of metadump failed"
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
> +
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $tmp.metadump -a -o $version || \
> +		_fail "xfs_metadump failed"
> +
> +	slogdev=$SCRATCH_LOGDEV
> +	if [[ -z $version || $version == "-v 1" ]]; then
> +		slogdev=""
> +	fi
> +	SCRATCH_DEV=$tmp.img SCRATCH_LOGDEV=$slogdev _scratch_xfs_mdrestore \
> +		   $tmp.metadump || _fail "xfs_mdrestore failed"
> +	SCRATCH_DEV=$tmp.img _scratch_xfs_repair -f &>> $seqres.full || \
> +		_fail "xfs_repair of metadump failed"
> +done
>  
>  # Yes it can; success, all done
>  status=0
> diff --git a/tests/xfs/432 b/tests/xfs/432
> index dae68fb2..a215d3ce 100755
> --- a/tests/xfs/432
> +++ b/tests/xfs/432
> @@ -50,6 +50,7 @@ echo "Format and mount"
>  # block.  8187 hashes/dablk / 248 dirents/dirblock = ~33 dirblocks per
>  # dablock.  33 dirblocks * 64k mean that we can expand a directory by
>  # 2112k before we have to allocate another da btree block.
> +
>  _scratch_mkfs -b size=1k -n size=64k > "$seqres.full" 2>&1
>  _scratch_mount >> "$seqres.full" 2>&1
>  
> @@ -85,13 +86,29 @@ extlen="$(check_for_long_extent $dir_inum)"
>  echo "qualifying extent: $extlen blocks" >> $seqres.full
>  test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
>  
> -echo "Try to metadump"
> -_scratch_xfs_metadump $metadump_file -a -o -w
> -SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
> +echo "Try to metadump, restore and check restored metadump image"
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
>  
> -echo "Check restored metadump image"
> -SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
> -	echo "xfs_repair on restored fs returned $?"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -a -o -w $version
> +
> +	slogdev=$SCRATCH_LOGDEV
> +	if [[ -z $version || $version == "-v 1" ]]; then
> +		slogdev=""
> +	fi
> +
> +	SCRATCH_DEV=$metadump_img SCRATCH_LOGDEV=$slogdev \
> +		   _scratch_xfs_mdrestore $metadump_file
> +
> +	SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
> +		echo "xfs_repair on restored fs returned $?"
> +done
>  
>  # success, all done
>  status=0
> diff --git a/tests/xfs/432.out b/tests/xfs/432.out
> index 1f135d16..37bac902 100644
> --- a/tests/xfs/432.out
> +++ b/tests/xfs/432.out
> @@ -2,5 +2,4 @@ QA output created by 432
>  Format and mount
>  Create huge dir
>  Check for > 1000 block extent?
> -Try to metadump
> -Check restored metadump image
> +Try to metadump, restore and check restored metadump image
> diff --git a/tests/xfs/503 b/tests/xfs/503
> index 8805632d..a1479eb6 100755
> --- a/tests/xfs/503
> +++ b/tests/xfs/503
> @@ -29,6 +29,7 @@ testdir=$TEST_DIR/test-$seq
>  _supported_fs xfs
>  
>  _require_command "$XFS_MDRESTORE_PROG" "xfs_mdrestore"
> +_require_loop
>  _require_xfs_copy
>  _require_scratch_nocheck
>  _require_populate_commands
> @@ -40,22 +41,69 @@ _scratch_populate_cached nofill > $seqres.full 2>&1
>  
>  mkdir -p $testdir
>  metadump_file=$testdir/scratch.md
> -metadump_file_a=${metadump_file}.a
> -metadump_file_g=${metadump_file}.g
> -metadump_file_ag=${metadump_file}.ag
>  copy_file=$testdir/copy.img
>  
> -echo metadump
> -_scratch_xfs_metadump $metadump_file -a -o >> $seqres.full
> +check_restored_metadump_image()
> +{
> +	local image=$1
>  
> -echo metadump a
> -_scratch_xfs_metadump $metadump_file_a -a >> $seqres.full
> +	loop_dev=$(_create_loop_device $image)
> +	SCRATCH_DEV=$loop_dev _scratch_mount
> +	SCRATCH_DEV=$loop_dev _check_scratch_fs
> +	SCRATCH_DEV=$loop_dev _scratch_unmount
> +	_destroy_loop_device $loop_dev
> +}
>  
> -echo metadump g
> -_scratch_xfs_metadump $metadump_file_g -g >> $seqres.full
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
>  
> -echo metadump ag
> -_scratch_xfs_metadump $metadump_file_ag -a -g >> $seqres.full
> +echo "metadump and mdrestore"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -a -o $version >> $seqres.full
> +	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> +	check_restored_metadump_image $TEST_DIR/image
> +done
> +
> +echo "metadump a and mdrestore"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -a $version >> $seqres.full
> +	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> +	check_restored_metadump_image $TEST_DIR/image
> +done
> +
> +echo "metadump g and mdrestore"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -g $version >> $seqres.full
> +	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> +	check_restored_metadump_image $TEST_DIR/image
> +done
> +
> +echo "metadump ag and mdrestore"
> +for md_version in $(seq 1 $max_md_version); do
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -a -g $version >> $seqres.full
> +	SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
> +	check_restored_metadump_image $TEST_DIR/image
> +done
>  
>  echo copy
>  $XFS_COPY_PROG $SCRATCH_DEV $copy_file >> $seqres.full
> @@ -67,30 +115,6 @@ _scratch_mount
>  _check_scratch_fs
>  _scratch_unmount
>  
> -echo mdrestore
> -_scratch_xfs_mdrestore $metadump_file
> -_scratch_mount
> -_check_scratch_fs
> -_scratch_unmount
> -
> -echo mdrestore a
> -_scratch_xfs_mdrestore $metadump_file_a
> -_scratch_mount
> -_check_scratch_fs
> -_scratch_unmount
> -
> -echo mdrestore g
> -_scratch_xfs_mdrestore $metadump_file_g
> -_scratch_mount
> -_check_scratch_fs
> -_scratch_unmount
> -
> -echo mdrestore ag
> -_scratch_xfs_mdrestore $metadump_file_ag
> -_scratch_mount
> -_check_scratch_fs
> -_scratch_unmount
> -
>  # success, all done
>  status=0
>  exit
> diff --git a/tests/xfs/503.out b/tests/xfs/503.out
> index 8ef31dbe..496f2516 100644
> --- a/tests/xfs/503.out
> +++ b/tests/xfs/503.out
> @@ -1,12 +1,8 @@
>  QA output created by 503
>  Format and populate
> -metadump
> -metadump a
> -metadump g
> -metadump ag
> +metadump and mdrestore
> +metadump a and mdrestore
> +metadump g and mdrestore
> +metadump ag and mdrestore
>  copy
>  recopy
> -mdrestore
> -mdrestore a
> -mdrestore g
> -mdrestore ag
> -- 
> 2.43.0
> 
> 

Re: [PATCH 4/5] xfs: Add support for testing metadump v2

[Chandan Babu R]

On Tue, Jan 02, 2024 at 09:57:28 PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 02, 2024 at 02:13:51PM +0530, Chandan Babu R wrote:
>> This commit adds the ability to test metadump v2 to existing metadump tests.
>> 
>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>  tests/xfs/129     |  63 ++++++++++++---
>>  tests/xfs/129.out |   4 +-
>>  tests/xfs/234     |  63 ++++++++++++---
>>  tests/xfs/234.out |   4 +-
>>  tests/xfs/253     | 195 ++++++++++++++++++++++++++--------------------
>>  tests/xfs/291     |  25 +++++-
>>  tests/xfs/432     |  29 +++++--
>>  tests/xfs/432.out |   3 +-
>>  tests/xfs/503     |  94 +++++++++++++---------
>>  tests/xfs/503.out |  12 +--
>>  10 files changed, 326 insertions(+), 166 deletions(-)
>> 
>> diff --git a/tests/xfs/129 b/tests/xfs/129
>> index 6f2ef564..7226d57d 100755
>> --- a/tests/xfs/129
>> +++ b/tests/xfs/129
>> @@ -16,7 +16,11 @@ _cleanup()
>>  {
>>      cd /
>>      _scratch_unmount > /dev/null 2>&1
>> -    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image
>> +    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
>> +	    _destroy_loop_device $logdev
>> +    [[ -n $datadev ]] && _destroy_loop_device $datadev
>> +    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/data-image \
>> +       $TEST_DIR/log-image
>>  }
>>  
>>  # Import common functions.
>> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>>  			$testdir/file2 $((nr * blksz)) $blksz >> $seqres.full
>>  done
>>  
>> -echo "Create metadump file"
>>  _scratch_unmount
>> -_scratch_xfs_metadump $metadump_file
>>  
>> -# Now restore the obfuscated one back and take a look around
>> -echo "Restore metadump"
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>> +max_md_version=1
>> +_scratch_metadump_v2_supported && max_md_version=2
>>  
>> -echo "Check restored fs"
>> -_check_generic_filesystem $metadump_file
>> +echo "Create metadump file, restore it and check restored fs"
>> +for md_version in $(seq 1 $max_md_version); do
>> +	# Determine the version to be passed to metadump/mdrestore
>> +	version=""
>> +	if [[ $max_md_version == 2 ]]; then
>> +		version="-v $md_version"
>> +	fi
>> +
>> +	_scratch_xfs_metadump $metadump_file $version
>> +
>> +	# Now restore the obfuscated one back and take a look around
>> +
>> +	# Metadump v2 files can contain contents dumped from an external log
>> +	# device. Use a temporary file to hold the log device contents restored
>> +	# from such a metadump file.
>> +	slogdev=$TEST_DIR/log-image
>> +	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
>> +		slogdev=""
>> +	fi
>> +
>> + 	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
>
>    ^ space before tab
>

Sorry, I will fix it.

>> +		   _scratch_xfs_mdrestore $metadump_file
>> +
>> +	datadev=$(_create_loop_device $TEST_DIR/data-image)
>> +
>> +	logdev=${SCRATCH_LOGDEV}
>> +	if [[ -s $TEST_DIR/log-image ]]; then
>> +		logdev=$(_create_loop_device $TEST_DIR/log-image)
>> +	fi
>> +
>> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
>> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
>> +
>> +	[[ -z $logdev ]] && logdev=none
>> +	_check_xfs_filesystem $datadev $logdev none
>> +
>> +	if [[ -s $TEST_DIR/log-image ]]; then
>> +		_destroy_loop_device $logdev
>> +		logdev=""
>> +		rm -f $TEST_DIR/log-image
>> +	fi
>> +
>> +	_destroy_loop_device $datadev
>> +	datadev=""
>> +	rm -f $TEST_DIR/data-image
>> +done
>
> Given the major differences between v1 and v2, would the two metadump
> tests be easier to understand if this loop were unrolled?
>

Yes, you are right. Unrolling the loop will improve the readability of the
code. I will make the required changes.

> I like that it mdrestores to a separate device to preserve the original
> $SCRATCH_DEV metadata between metadump v1 and v2 testing.
>
>>  # success, all done
>>  status=0
>> diff --git a/tests/xfs/129.out b/tests/xfs/129.out
>> index da6f43fd..0f24c431 100644
>> --- a/tests/xfs/129.out
>> +++ b/tests/xfs/129.out
>> @@ -1,6 +1,4 @@
>>  QA output created by 129
>>  Create the original file blocks
>>  Reflink every other block
>> -Create metadump file
>> -Restore metadump
>> -Check restored fs
>> +Create metadump file, restore it and check restored fs
>> diff --git a/tests/xfs/234 b/tests/xfs/234
>> index 57d447c0..2f6b1f65 100755
>> --- a/tests/xfs/234
>> +++ b/tests/xfs/234
>> @@ -16,7 +16,11 @@ _cleanup()
>>  {
>>      cd /
>>      _scratch_unmount > /dev/null 2>&1
>> -    rm -rf $tmp.* $metadump_file $TEST_DIR/image
>> +    [[ -n $logdev && $logdev != "none" && $logdev != $SCRATCH_LOGDEV ]] && \
>> +	    _destroy_loop_device $logdev
>> +    [[ -n $datadev ]] && _destroy_loop_device $datadev
>> +    rm -rf $tmp.* $testdir $metadump_file $TEST_DIR/image \
>> +       $TEST_DIR/log-image
>>  }
>>  
>>  # Import common functions.
>> @@ -47,18 +51,57 @@ seq 1 2 $((nr_blks - 1)) | while read nr; do
>>  	$XFS_IO_PROG -c "fpunch $((nr * blksz)) $blksz" $testdir/file1 >> $seqres.full
>>  done
>>  
>> -echo "Create metadump file"
>>  _scratch_unmount
>> -_scratch_xfs_metadump $metadump_file
>>  
>> -# Now restore the obfuscated one back and take a look around
>> -echo "Restore metadump"
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>> -SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>> +max_md_version=1
>> +_scratch_metadump_v2_supported && max_md_version=2
>>  
>> -echo "Check restored fs"
>> -_check_generic_filesystem $metadump_file
>> +echo "Create metadump file, restore it and check restored fs"
>> +for md_version in $(seq 1 $max_md_version); do
>> +	# Determine the version to be passed to metadump/mdrestore
>> +	version=""
>> +	if [[ $max_md_version == 2 ]]; then
>> +		version="-v $md_version"
>> +	fi
>> +
>> +	_scratch_xfs_metadump $metadump_file $version
>> +
>> +	# Now restore the obfuscated one back and take a look around
>> +
>> +	# Metadump v2 files can contain contents dumped from an external log
>> +	# device. Use a temporary file to hold the log device contents restored
>> +	# from such a metadump file.
>> +	slogdev=$TEST_DIR/log-image
>> +	if [[ -z $version || $version == "-v 1" || -z $SCRATCH_LOGDEV ]]; then
>> +		slogdev=""
>> +	fi
>> +
>> +	SCRATCH_DEV=$TEST_DIR/data-image SCRATCH_LOGDEV=$slogdev \
>> +		   _scratch_xfs_mdrestore $metadump_file
>> +
>> +	datadev=$(_create_loop_device $TEST_DIR/data-image)
>> +
>> +	logdev=${SCRATCH_LOGDEV}
>> +	if [[ -s $TEST_DIR/log-image ]]; then
>> +		logdev=$(_create_loop_device $TEST_DIR/log-image)
>> +	fi
>> +
>> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_mount
>> +	SCRATCH_DEV=$datadev SCRATCH_LOGDEV=$logdev _scratch_unmount
>> +
>> +	[[ -z $logdev ]] && logdev=none
>> +	_check_xfs_filesystem $datadev $logdev none
>> +
>> +	if [[ -s $TEST_DIR/log-image ]]; then
>> +		_destroy_loop_device $logdev
>> +		logdev=""
>> +		rm -f $TEST_DIR/log-image
>> +	fi
>> +
>> +	_destroy_loop_device $datadev
>> +	datadev=""
>> +	rm -f $TEST_DIR/data-image
>> +done
>>  
>>  # success, all done
>>  status=0
>> diff --git a/tests/xfs/234.out b/tests/xfs/234.out
>> index 463d4660..fc2ddd77 100644
>> --- a/tests/xfs/234.out
>> +++ b/tests/xfs/234.out
>> @@ -1,6 +1,4 @@
>>  QA output created by 234
>>  Create the original file blocks
>>  Punch every other block
>> -Create metadump file
>> -Restore metadump
>> -Check restored fs
>> +Create metadump file, restore it and check restored fs
>> diff --git a/tests/xfs/253 b/tests/xfs/253
>> index ce902477..b69a1faf 100755
>> --- a/tests/xfs/253
>> +++ b/tests/xfs/253
>> @@ -52,114 +52,139 @@ function create_file() {
>>  echo "Disciplyne of silence is goed."
>>  
>>  _scratch_mkfs >/dev/null 2>&1
>> -_scratch_mount
>>  
>> -# Initialize and mount the scratch filesystem, then create a bunch
>> -# of files that exercise the original problem.
>> -#
>> -# The problem arose when a file name produced a hash that contained
>> -# either 0x00 (string terminator) or 0x27 ('/' character) in a
>> -# spot used to determine a character in an obfuscated name.  This
>> -# occurred in one of 5 spots at the end of the name, at position
>> -# (last-4), (last-3), (last-2), (last-1), or (last).
>
> I wonder, could you create the scratch fs and only then move into
> testing v1 and v2 metadump?  Rather than doing the create_file stuff
> twice?  Or do we actually end up with a different fs between the two
> iterations?
>

Yes, Creating the fs once should be sufficient. I will implement the changes
that have been suggested.

> The other two tests here look good enough for now...
>
> --D
>

-- 
Chandan

[PATCH 5/5] xfs: Check correctness of metadump/mdrestore's ability to work with dirty log

[Chandan Babu R]

Add a new test to verify if metadump/mdrestore are able to dump and restore
the contents of a dirty log.

Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
---
 tests/xfs/801     | 115 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/xfs/801.out |  14 ++++++
 2 files changed, 129 insertions(+)
 create mode 100755 tests/xfs/801
 create mode 100644 tests/xfs/801.out

diff --git a/tests/xfs/801 b/tests/xfs/801
new file mode 100755
index 00000000..3ed559df
--- /dev/null
+++ b/tests/xfs/801
@@ -0,0 +1,115 @@
+#! /bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2024 Oracle, Inc.  All Rights Reserved.
+#
+# FS QA Test 801
+#
+# Test metadump/mdrestore's ability to dump a dirty log and restore it
+# correctly.
+#
+. ./common/preamble
+_begin_fstest auto quick metadump log logprint punch
+
+# Override the default cleanup function.
+_cleanup()
+{
+	cd /
+	rm -r -f $tmp.*
+	_scratch_unmount > /dev/null 2>&1
+}
+
+# Import common functions.
+. ./common/dmflakey
+. ./common/inject
+
+# real QA test starts here
+_supported_fs xfs
+_require_scratch
+_require_test
+_require_xfs_debug
+_require_xfs_io_error_injection log_item_pin
+_require_dm_target flakey
+_require_xfs_io_command "pwrite"
+_require_test_program "punch-alternating"
+
+metadump_file=${TEST_DIR}/${seq}.md
+testfile=${SCRATCH_MNT}/testfile
+
+echo "Format filesystem on scratch device"
+_scratch_mkfs >> $seqres.full 2>&1
+
+max_md_version=1
+_scratch_metadump_v2_supported && max_md_version=2
+
+external_log=0
+if [[ $USE_EXTERNAL = yes && -n "$SCRATCH_LOGDEV" ]]; then
+	external_log=1
+fi
+
+if [[ $max_md_version == 1 && $external_log == 1 ]]; then
+	_notrun "metadump v1 does not support external log device"
+fi
+
+echo "Initialize and mount filesystem on flakey device"
+_init_flakey
+_load_flakey_table $FLAKEY_ALLOW_WRITES
+_mount_flakey
+
+echo "Create test file"
+$XFS_IO_PROG -s -f -c "pwrite 0 5M" $testfile >> $seqres.full
+
+echo "Punch alternative blocks of test file"
+$here/src/punch-alternating $testfile
+
+echo "Mount cycle the filesystem on flakey device"
+_unmount_flakey
+_mount_flakey
+
+device=$(readlink -f $FLAKEY_DEV)
+device=$(_short_dev $device)
+
+echo "Pin log items in the AIL"
+echo 1 > /sys/fs/xfs/${device}/errortag/log_item_pin
+
+echo "Create two checkpoint transactions on ondisk log"
+for ct in $(seq 1 2); do
+	offset=$(xfs_io -c 'fiemap' $testfile | tac |  grep -v hole | \
+			 head -n 1 | awk -F '[\\[.]' '{ print $2 * 512; }')
+	$XFS_IO_PROG -c "truncate $offset" -c fsync $testfile
+done
+
+echo "Drop writes to filesystem from here onwards"
+_load_flakey_table $FLAKEY_DROP_WRITES
+
+echo "Unpin log items in AIL"
+echo 0 > /sys/fs/xfs/${device}/errortag/log_item_pin
+
+echo "Unmount filesystem on flakey device"
+_unmount_flakey
+
+echo "Clean up flakey device"
+_cleanup_flakey
+
+echo -n "Filesystem has a "
+_print_logstate
+
+echo "Create metadump file, restore it and check restored fs"
+for md_version in $(seq 1 $max_md_version); do
+	[[ $md_version == 1 && $external_log == 1 ]] && continue
+
+	version=""
+	if [[ $max_md_version == 2 ]]; then
+		version="-v $md_version"
+	fi
+
+	_scratch_xfs_metadump $metadump_file -a -o $version
+	_scratch_xfs_mdrestore $metadump_file
+
+	_scratch_mount
+	_check_scratch_fs
+	_scratch_unmount
+done
+
+# success, all done
+status=0
+exit
diff --git a/tests/xfs/801.out b/tests/xfs/801.out
new file mode 100644
index 00000000..a2f2abca
--- /dev/null
+++ b/tests/xfs/801.out
@@ -0,0 +1,14 @@
+QA output created by 801
+Format filesystem on scratch device
+Initialize and mount filesystem on flakey device
+Create test file
+Punch alternative blocks of test file
+Mount cycle the filesystem on flakey device
+Pin log items in the AIL
+Create two checkpoint transactions on ondisk log
+Drop writes to filesystem from here onwards
+Unpin log items in AIL
+Unmount filesystem on flakey device
+Clean up flakey device
+Filesystem has a dirty log
+Create metadump file, restore it and check restored fs
-- 
2.43.0

Re: [PATCH 5/5] xfs: Check correctness of metadump/mdrestore's ability to work with dirty log

[Darrick J. Wong]

On Tue, Jan 02, 2024 at 02:13:52PM +0530, Chandan Babu R wrote:
> Add a new test to verify if metadump/mdrestore are able to dump and restore
> the contents of a dirty log.
> 
> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
> ---
>  tests/xfs/801     | 115 ++++++++++++++++++++++++++++++++++++++++++++++
>  tests/xfs/801.out |  14 ++++++
>  2 files changed, 129 insertions(+)
>  create mode 100755 tests/xfs/801
>  create mode 100644 tests/xfs/801.out
> 
> diff --git a/tests/xfs/801 b/tests/xfs/801
> new file mode 100755
> index 00000000..3ed559df
> --- /dev/null
> +++ b/tests/xfs/801
> @@ -0,0 +1,115 @@
> +#! /bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# Copyright (c) 2024 Oracle, Inc.  All Rights Reserved.
> +#
> +# FS QA Test 801
> +#
> +# Test metadump/mdrestore's ability to dump a dirty log and restore it
> +# correctly.
> +#
> +. ./common/preamble
> +_begin_fstest auto quick metadump log logprint punch
> +
> +# Override the default cleanup function.
> +_cleanup()
> +{
> +	cd /
> +	rm -r -f $tmp.*
> +	_scratch_unmount > /dev/null 2>&1
> +}
> +
> +# Import common functions.
> +. ./common/dmflakey
> +. ./common/inject
> +
> +# real QA test starts here
> +_supported_fs xfs
> +_require_scratch
> +_require_test
> +_require_xfs_debug
> +_require_xfs_io_error_injection log_item_pin
> +_require_dm_target flakey
> +_require_xfs_io_command "pwrite"
> +_require_test_program "punch-alternating"
> +
> +metadump_file=${TEST_DIR}/${seq}.md
> +testfile=${SCRATCH_MNT}/testfile
> +
> +echo "Format filesystem on scratch device"
> +_scratch_mkfs >> $seqres.full 2>&1
> +
> +max_md_version=1
> +_scratch_metadump_v2_supported && max_md_version=2
> +
> +external_log=0
> +if [[ $USE_EXTERNAL = yes && -n "$SCRATCH_LOGDEV" ]]; then
> +	external_log=1
> +fi
> +
> +if [[ $max_md_version == 1 && $external_log == 1 ]]; then
> +	_notrun "metadump v1 does not support external log device"
> +fi
> +
> +echo "Initialize and mount filesystem on flakey device"
> +_init_flakey
> +_load_flakey_table $FLAKEY_ALLOW_WRITES
> +_mount_flakey
> +
> +echo "Create test file"
> +$XFS_IO_PROG -s -f -c "pwrite 0 5M" $testfile >> $seqres.full
> +
> +echo "Punch alternative blocks of test file"
> +$here/src/punch-alternating $testfile
> +
> +echo "Mount cycle the filesystem on flakey device"
> +_unmount_flakey
> +_mount_flakey
> +
> +device=$(readlink -f $FLAKEY_DEV)
> +device=$(_short_dev $device)
> +
> +echo "Pin log items in the AIL"
> +echo 1 > /sys/fs/xfs/${device}/errortag/log_item_pin

_scratch_inject_error log_item_pin 1

> +
> +echo "Create two checkpoint transactions on ondisk log"
> +for ct in $(seq 1 2); do
> +	offset=$(xfs_io -c 'fiemap' $testfile | tac |  grep -v hole | \

$XFS_IO_PROG

> +			 head -n 1 | awk -F '[\\[.]' '{ print $2 * 512; }')
> +	$XFS_IO_PROG -c "truncate $offset" -c fsync $testfile
> +done
> +
> +echo "Drop writes to filesystem from here onwards"
> +_load_flakey_table $FLAKEY_DROP_WRITES
> +
> +echo "Unpin log items in AIL"
> +echo 0 > /sys/fs/xfs/${device}/errortag/log_item_pin

_scratch_inject_error log_item_pin 0

> +
> +echo "Unmount filesystem on flakey device"
> +_unmount_flakey
> +
> +echo "Clean up flakey device"
> +_cleanup_flakey
> +
> +echo -n "Filesystem has a "
> +_print_logstate
> +
> +echo "Create metadump file, restore it and check restored fs"
> +for md_version in $(seq 1 $max_md_version); do
> +	[[ $md_version == 1 && $external_log == 1 ]] && continue
> +
> +	version=""
> +	if [[ $max_md_version == 2 ]]; then
> +		version="-v $md_version"
> +	fi
> +
> +	_scratch_xfs_metadump $metadump_file -a -o $version
> +	_scratch_xfs_mdrestore $metadump_file

Don't you want to mdrestore to a file or something so that the next
iteration of the loop will still be dumping from the exact same fs?

--D

> +
> +	_scratch_mount
> +	_check_scratch_fs
> +	_scratch_unmount
> +done
> +
> +# success, all done
> +status=0
> +exit
> diff --git a/tests/xfs/801.out b/tests/xfs/801.out
> new file mode 100644
> index 00000000..a2f2abca
> --- /dev/null
> +++ b/tests/xfs/801.out
> @@ -0,0 +1,14 @@
> +QA output created by 801
> +Format filesystem on scratch device
> +Initialize and mount filesystem on flakey device
> +Create test file
> +Punch alternative blocks of test file
> +Mount cycle the filesystem on flakey device
> +Pin log items in the AIL
> +Create two checkpoint transactions on ondisk log
> +Drop writes to filesystem from here onwards
> +Unpin log items in AIL
> +Unmount filesystem on flakey device
> +Clean up flakey device
> +Filesystem has a dirty log
> +Create metadump file, restore it and check restored fs
> -- 
> 2.43.0
> 
> 

Re: [PATCH 5/5] xfs: Check correctness of metadump/mdrestore's ability to work with dirty log

[Chandan Babu R]

On Tue, Jan 02, 2024 at 09:44:00 PM -0800, Darrick J. Wong wrote:
> On Tue, Jan 02, 2024 at 02:13:52PM +0530, Chandan Babu R wrote:
>> Add a new test to verify if metadump/mdrestore are able to dump and restore
>> the contents of a dirty log.
>> 
>> Signed-off-by: Chandan Babu R <chandanbabu@kernel.org>
>> ---
>>  tests/xfs/801     | 115 ++++++++++++++++++++++++++++++++++++++++++++++
>>  tests/xfs/801.out |  14 ++++++
>>  2 files changed, 129 insertions(+)
>>  create mode 100755 tests/xfs/801
>>  create mode 100644 tests/xfs/801.out
>> 
>> diff --git a/tests/xfs/801 b/tests/xfs/801
>> new file mode 100755
>> index 00000000..3ed559df
>> --- /dev/null
>> +++ b/tests/xfs/801
>> @@ -0,0 +1,115 @@
>> +#! /bin/bash
>> +# SPDX-License-Identifier: GPL-2.0
>> +# Copyright (c) 2024 Oracle, Inc.  All Rights Reserved.
>> +#
>> +# FS QA Test 801
>> +#
>> +# Test metadump/mdrestore's ability to dump a dirty log and restore it
>> +# correctly.
>> +#
>> +. ./common/preamble
>> +_begin_fstest auto quick metadump log logprint punch
>> +
>> +# Override the default cleanup function.
>> +_cleanup()
>> +{
>> +	cd /
>> +	rm -r -f $tmp.*
>> +	_scratch_unmount > /dev/null 2>&1
>> +}
>> +
>> +# Import common functions.
>> +. ./common/dmflakey
>> +. ./common/inject
>> +
>> +# real QA test starts here
>> +_supported_fs xfs
>> +_require_scratch
>> +_require_test
>> +_require_xfs_debug
>> +_require_xfs_io_error_injection log_item_pin
>> +_require_dm_target flakey
>> +_require_xfs_io_command "pwrite"
>> +_require_test_program "punch-alternating"
>> +
>> +metadump_file=${TEST_DIR}/${seq}.md
>> +testfile=${SCRATCH_MNT}/testfile
>> +
>> +echo "Format filesystem on scratch device"
>> +_scratch_mkfs >> $seqres.full 2>&1
>> +
>> +max_md_version=1
>> +_scratch_metadump_v2_supported && max_md_version=2
>> +
>> +external_log=0
>> +if [[ $USE_EXTERNAL = yes && -n "$SCRATCH_LOGDEV" ]]; then
>> +	external_log=1
>> +fi
>> +
>> +if [[ $max_md_version == 1 && $external_log == 1 ]]; then
>> +	_notrun "metadump v1 does not support external log device"
>> +fi
>> +
>> +echo "Initialize and mount filesystem on flakey device"
>> +_init_flakey
>> +_load_flakey_table $FLAKEY_ALLOW_WRITES
>> +_mount_flakey
>> +
>> +echo "Create test file"
>> +$XFS_IO_PROG -s -f -c "pwrite 0 5M" $testfile >> $seqres.full
>> +
>> +echo "Punch alternative blocks of test file"
>> +$here/src/punch-alternating $testfile
>> +
>> +echo "Mount cycle the filesystem on flakey device"
>> +_unmount_flakey
>> +_mount_flakey
>> +
>> +device=$(readlink -f $FLAKEY_DEV)
>> +device=$(_short_dev $device)
>> +
>> +echo "Pin log items in the AIL"
>> +echo 1 > /sys/fs/xfs/${device}/errortag/log_item_pin
>
> _scratch_inject_error log_item_pin 1
>

_scratch_inject_error() internally uses $SCRATCH_DEV. This is not suitable for
the test since we are working with $FLAKEY_DEV.

>> +
>> +echo "Create two checkpoint transactions on ondisk log"
>> +for ct in $(seq 1 2); do
>> +	offset=$(xfs_io -c 'fiemap' $testfile | tac |  grep -v hole | \
>
> $XFS_IO_PROG
>

I will fix it up.

>> +			 head -n 1 | awk -F '[\\[.]' '{ print $2 * 512; }')
>> +	$XFS_IO_PROG -c "truncate $offset" -c fsync $testfile
>> +done
>> +
>> +echo "Drop writes to filesystem from here onwards"
>> +_load_flakey_table $FLAKEY_DROP_WRITES
>> +
>> +echo "Unpin log items in AIL"
>> +echo 0 > /sys/fs/xfs/${device}/errortag/log_item_pin
>
> _scratch_inject_error log_item_pin 0
>
>> +
>> +echo "Unmount filesystem on flakey device"
>> +_unmount_flakey
>> +
>> +echo "Clean up flakey device"
>> +_cleanup_flakey
>> +
>> +echo -n "Filesystem has a "
>> +_print_logstate
>> +
>> +echo "Create metadump file, restore it and check restored fs"
>> +for md_version in $(seq 1 $max_md_version); do
>> +	[[ $md_version == 1 && $external_log == 1 ]] && continue
>> +
>> +	version=""
>> +	if [[ $max_md_version == 2 ]]; then
>> +		version="-v $md_version"
>> +	fi
>> +
>> +	_scratch_xfs_metadump $metadump_file -a -o $version
>> +	_scratch_xfs_mdrestore $metadump_file
>
> Don't you want to mdrestore to a file or something so that the next
> iteration of the loop will still be dumping from the exact same fs?

Yes, you are right. I will make the required changes.

-- 
Chandan