[PATCHSET 0/4] fstests: fix broken fuzzing xfs_mdrestore calls

[PATCHSET 0/4] fstests: fix broken fuzzing xfs_mdrestore calls

[Darrick J. Wong]

Hi all,

Back in 2021, I amended the xfs create-a-filesystem helper code to
support the generation of compressed metadumps.  Unfortunately, I
neglected to submit the other half of that code which detects a
*compressed* metadump file and uses it, which means that the metadump
caching used by all the xfs fuzz tests have been broken ever since, but
only if DUMP_COMPRESSOR is set by the user.

Fast-forward to 2022, and I noticed that fuzz tests runs would
occasionally complain about POPULATE_METADUMP not pointing to a valid
file if DUMP_COMPRESSOR is set, and no dump file exists, compressed or
uncompressed.  After some curious looks from Zorro, I finally realized
that POPULATE_METADUMP points to the uncompressed file, so checking for
its existence is not correct.

Hence port all the code that fixed mdrestore of compressed metadumps
further down the patch stack, and fix the broken conditional to get
things running again.

This is an extraordinary way to destroy everything.  Enjoy!
Comments and questions are, as always, welcome.

--D
---
 common/ext4     |   21 +++++++++++++++++++++
 common/fuzzy    |    5 ++---
 common/populate |   15 ++-------------
 common/xfs      |   29 +++++++++++++++++++++++++++++
 tests/xfs/129   |    2 +-
 tests/xfs/234   |    2 +-
 tests/xfs/253   |    2 +-
 tests/xfs/284   |    2 +-
 tests/xfs/291   |    2 +-
 tests/xfs/336   |    2 +-
 tests/xfs/432   |    2 +-
 tests/xfs/503   |    8 ++++----
 12 files changed, 65 insertions(+), 27 deletions(-)

[PATCH 1/4] common/populate: create helpers to handle restoring metadumps

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Refactor _scratch_populate_restore_cached so that the actual commands
for restoring metadumps are filesystem-specific helpers.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/ext4     |   10 ++++++++++
 common/fuzzy    |    2 +-
 common/populate |    4 ++--
 common/xfs      |    9 +++++++++
 4 files changed, 22 insertions(+), 3 deletions(-)


diff --git a/common/ext4 b/common/ext4
index 4a2eaa157f..dc2e4e59cc 100644
--- a/common/ext4
+++ b/common/ext4
@@ -125,6 +125,16 @@ _ext4_metadump()
 		$DUMP_COMPRESSOR -f "$dumpfile" &>> "$seqres.full"
 }
 
+_ext4_mdrestore()
+{
+	local metadump="$1"
+	local device="$2"
+	shift; shift
+	local options="$@"
+
+	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
+}
+
 # this test requires the ext4 kernel support crc feature on scratch device
 #
 _require_scratch_ext4_crc()
diff --git a/common/fuzzy b/common/fuzzy
index 2d688fd27b..fad79124e5 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -159,7 +159,7 @@ __scratch_xfs_fuzz_mdrestore()
 	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
 
 	__scratch_xfs_fuzz_unmount
-	$XFS_MDRESTORE_PROG "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
+	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
 }
 
 __fuzz_notify() {
diff --git a/common/populate b/common/populate
index 6e00499734..f382c40aca 100644
--- a/common/populate
+++ b/common/populate
@@ -861,7 +861,7 @@ _scratch_populate_restore_cached() {
 
 	case "${FSTYP}" in
 	"xfs")
-		$XFS_MDRESTORE_PROG "${metadump}" "${SCRATCH_DEV}"
+		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
 		res=$?
 		test $res -ne 0 && return $res
 
@@ -876,7 +876,7 @@ _scratch_populate_restore_cached() {
 		return $res
 		;;
 	"ext2"|"ext3"|"ext4")
-		$E2IMAGE_PROG -r "${metadump}" "${SCRATCH_DEV}"
+		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}"
 		ret=$?
 		test $ret -ne 0 && return $ret
 
diff --git a/common/xfs b/common/xfs
index f466d2c42f..27d6ac84e3 100644
--- a/common/xfs
+++ b/common/xfs
@@ -638,6 +638,15 @@ _xfs_metadump() {
 	return $res
 }
 
+_xfs_mdrestore() {
+	local metadump="$1"
+	local device="$2"
+	shift; shift
+	local options="$@"
+
+	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
+}
+
 # Snapshot the metadata on the scratch device
 _scratch_xfs_metadump()
 {

[PATCH 2/4] common/xfs: create a helper for restoring metadumps to the scratch devs

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Refactor the open-coded $XFS_MDRESTORE_PROG calls into a proper
_scratch_xfs_mdrestore helper.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/xfs    |    9 +++++++++
 tests/xfs/129 |    2 +-
 tests/xfs/234 |    2 +-
 tests/xfs/253 |    2 +-
 tests/xfs/284 |    2 +-
 tests/xfs/291 |    2 +-
 tests/xfs/336 |    2 +-
 tests/xfs/432 |    2 +-
 tests/xfs/503 |    8 ++++----
 9 files changed, 20 insertions(+), 11 deletions(-)


diff --git a/common/xfs b/common/xfs
index 27d6ac84e3..216dab3bcd 100644
--- a/common/xfs
+++ b/common/xfs
@@ -660,6 +660,15 @@ _scratch_xfs_metadump()
 	_xfs_metadump "$metadump" "$SCRATCH_DEV" "$logdev" nocompress "$@"
 }
 
+# Restore snapshotted metadata on the scratch device
+_scratch_xfs_mdrestore()
+{
+	local metadump=$1
+	shift
+
+	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
+}
+
 # run xfs_check and friends on a FS.
 _check_xfs_filesystem()
 {
diff --git a/tests/xfs/129 b/tests/xfs/129
index 09d40630d0..6f2ef5640d 100755
--- a/tests/xfs/129
+++ b/tests/xfs/129
@@ -53,7 +53,7 @@ _scratch_xfs_metadump $metadump_file
 
 # Now restore the obfuscated one back and take a look around
 echo "Restore metadump"
-$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
+SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
 SCRATCH_DEV=$TEST_DIR/image _scratch_mount
 SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
 
diff --git a/tests/xfs/234 b/tests/xfs/234
index cc1ee9a8ca..57d447c056 100755
--- a/tests/xfs/234
+++ b/tests/xfs/234
@@ -53,7 +53,7 @@ _scratch_xfs_metadump $metadump_file
 
 # Now restore the obfuscated one back and take a look around
 echo "Restore metadump"
-$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
+SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
 SCRATCH_DEV=$TEST_DIR/image _scratch_mount
 SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
 
diff --git a/tests/xfs/253 b/tests/xfs/253
index 1cfc218088..ce90247777 100755
--- a/tests/xfs/253
+++ b/tests/xfs/253
@@ -152,7 +152,7 @@ _scratch_unmount
 _scratch_xfs_metadump $METADUMP_FILE
 
 # Now restore the obfuscated one back and take a look around
-$XFS_MDRESTORE_PROG "${METADUMP_FILE}" "${SCRATCH_DEV}"
+_scratch_xfs_mdrestore "$METADUMP_FILE"
 
 _scratch_mount
 
diff --git a/tests/xfs/284 b/tests/xfs/284
index e2bd05d4c7..58f330035e 100755
--- a/tests/xfs/284
+++ b/tests/xfs/284
@@ -49,7 +49,7 @@ _scratch_unmount
 # xfs_mdrestore should refuse to restore to a mounted device
 _scratch_xfs_metadump $METADUMP_FILE
 _scratch_mount
-$XFS_MDRESTORE_PROG $METADUMP_FILE $SCRATCH_DEV 2>&1 | filter_mounted
+_scratch_xfs_mdrestore $METADUMP_FILE 2>&1 | filter_mounted
 _scratch_unmount
 
 # Test xfs_copy to a mounted device
diff --git a/tests/xfs/291 b/tests/xfs/291
index f5fea7f9a5..600dcb2eba 100755
--- a/tests/xfs/291
+++ b/tests/xfs/291
@@ -93,7 +93,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"
-$XFS_MDRESTORE_PROG $tmp.metadump $tmp.img || _fail "xfs_mdrestore 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 ee8ec649cb..5bcac976e4 100755
--- a/tests/xfs/336
+++ b/tests/xfs/336
@@ -65,7 +65,7 @@ _scratch_xfs_metadump $metadump_file
 
 # Now restore the obfuscated one back and take a look around
 echo "Restore metadump"
-$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
+SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
 SCRATCH_DEV=$TEST_DIR/image _scratch_mount
 SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
 
diff --git a/tests/xfs/432 b/tests/xfs/432
index 676be9bd8a..66315b0398 100755
--- a/tests/xfs/432
+++ b/tests/xfs/432
@@ -87,7 +87,7 @@ test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
 
 echo "Try to metadump"
 _scratch_xfs_metadump $metadump_file -w
-$XFS_MDRESTORE_PROG $metadump_file $metadump_img
+SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
 
 echo "Check restored metadump image"
 SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
diff --git a/tests/xfs/503 b/tests/xfs/503
index 18bd8694c8..c786b04ccd 100755
--- a/tests/xfs/503
+++ b/tests/xfs/503
@@ -66,25 +66,25 @@ _check_scratch_fs
 _scratch_unmount
 
 echo mdrestore
-$XFS_MDRESTORE_PROG $metadump_file $SCRATCH_DEV
+_scratch_xfs_mdrestore $metadump_file
 _scratch_mount
 _check_scratch_fs
 _scratch_unmount
 
 echo mdrestore a
-$XFS_MDRESTORE_PROG $metadump_file_a $SCRATCH_DEV
+_scratch_xfs_mdrestore $metadump_file_a
 _scratch_mount
 _check_scratch_fs
 _scratch_unmount
 
 echo mdrestore g
-$XFS_MDRESTORE_PROG $metadump_file_g $SCRATCH_DEV
+_scratch_xfs_mdrestore $metadump_file_g
 _scratch_mount
 _check_scratch_fs
 _scratch_unmount
 
 echo mdrestore ag
-$XFS_MDRESTORE_PROG $metadump_file_ag $SCRATCH_DEV
+_scratch_xfs_mdrestore $metadump_file_ag
 _scratch_mount
 _check_scratch_fs
 _scratch_unmount

[PATCH 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Move the metadump decompression code to the per-filesystem mdrestore
commands so that everyone can take advantage of them.  This enables the
XFS and ext4 _mdrestore helpers to handle metadata dumps compressed with
their respective _metadump helpers.

In turn, this means that the xfs fuzz tests can now handle the
compressed metadumps created by the _scratch_populate_cached helper.
This is key to unbreaking fuzz testing for xfs.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/ext4     |   13 ++++++++++++-
 common/fuzzy    |    2 +-
 common/populate |   15 ++-------------
 common/xfs      |   15 +++++++++++++--
 4 files changed, 28 insertions(+), 17 deletions(-)


diff --git a/common/ext4 b/common/ext4
index dc2e4e59cc..cadf1a7974 100644
--- a/common/ext4
+++ b/common/ext4
@@ -129,9 +129,20 @@ _ext4_mdrestore()
 {
 	local metadump="$1"
 	local device="$2"
-	shift; shift
+	local compressopt="$3"
+	shift; shift; shift
 	local options="$@"
 
+	# If we're configured for compressed dumps and there isn't already an
+	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
+	# something.
+	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
+		for compr in "$metadump".*; do
+			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
+		done
+	fi
+	test -r "$metadump" || return 1
+
 	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
 }
 
diff --git a/common/fuzzy b/common/fuzzy
index fad79124e5..e634815eec 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -159,7 +159,7 @@ __scratch_xfs_fuzz_mdrestore()
 	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
 
 	__scratch_xfs_fuzz_unmount
-	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
+	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
 }
 
 __fuzz_notify() {
diff --git a/common/populate b/common/populate
index f382c40aca..96866ee4cf 100644
--- a/common/populate
+++ b/common/populate
@@ -848,20 +848,9 @@ _scratch_populate_cache_tag() {
 _scratch_populate_restore_cached() {
 	local metadump="$1"
 
-	# If we're configured for compressed dumps and there isn't already an
-	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
-	# something.
-	if [ -n "$DUMP_COMPRESSOR" ]; then
-		for compr in "$metadump".*; do
-			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
-		done
-	fi
-
-	test -r "$metadump" || return 1
-
 	case "${FSTYP}" in
 	"xfs")
-		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
+		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
 		res=$?
 		test $res -ne 0 && return $res
 
@@ -876,7 +865,7 @@ _scratch_populate_restore_cached() {
 		return $res
 		;;
 	"ext2"|"ext3"|"ext4")
-		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}"
+		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
 		ret=$?
 		test $ret -ne 0 && return $ret
 
diff --git a/common/xfs b/common/xfs
index 216dab3bcd..833c2f4368 100644
--- a/common/xfs
+++ b/common/xfs
@@ -641,9 +641,20 @@ _xfs_metadump() {
 _xfs_mdrestore() {
 	local metadump="$1"
 	local device="$2"
-	shift; shift
+	local compressopt="$3"
+	shift; shift; shift
 	local options="$@"
 
+	# If we're configured for compressed dumps and there isn't already an
+	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
+	# something.
+	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
+		for compr in "$metadump".*; do
+			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
+		done
+	fi
+	test -r "$metadump" || return 1
+
 	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
 }
 
@@ -666,7 +677,7 @@ _scratch_xfs_mdrestore()
 	local metadump=$1
 	shift
 
-	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
+	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" nocompress "$@"
 }
 
 # run xfs_check and friends on a FS.

[PATCH 4/4] fuzzy: don't fail on compressed metadumps

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

This line in __scratch_xfs_fuzz_mdrestore:

	test -e "${POPULATE_METADUMP}"

Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
the metadump files, because the metadump files get the compression
program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
is wrong, and since the naming policy is an implementation detail of
_xfs_mdrestore, let's get rid of the -e test.

However, we still need a way to fail the test if the metadump cannot be
restored.  _xfs_mdrestore returns nonzero on failure, so use that
instead.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/fuzzy |    5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


diff --git a/common/fuzzy b/common/fuzzy
index e634815eec..49c850f2d5 100644
--- a/common/fuzzy
+++ b/common/fuzzy
@@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
 # Restore metadata to scratch device prior to field-fuzzing.
 __scratch_xfs_fuzz_mdrestore()
 {
-	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
-
 	__scratch_xfs_fuzz_unmount
-	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
+	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
+		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
 }
 
 __fuzz_notify() {

Re: [PATCH 1/4] common/populate: create helpers to handle restoring metadumps

[Zorro Lang]

On Tue, Dec 13, 2022 at 11:45:15AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Refactor _scratch_populate_restore_cached so that the actual commands
> for restoring metadumps are filesystem-specific helpers.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/ext4     |   10 ++++++++++
>  common/fuzzy    |    2 +-
>  common/populate |    4 ++--
>  common/xfs      |    9 +++++++++
>  4 files changed, 22 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/common/ext4 b/common/ext4
> index 4a2eaa157f..dc2e4e59cc 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -125,6 +125,16 @@ _ext4_metadump()
>  		$DUMP_COMPRESSOR -f "$dumpfile" &>> "$seqres.full"
>  }
>  
> +_ext4_mdrestore()
> +{
> +	local metadump="$1"
> +	local device="$2"
> +	shift; shift
> +	local options="$@"
> +
> +	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
> +}
> +
>  # this test requires the ext4 kernel support crc feature on scratch device
>  #
>  _require_scratch_ext4_crc()
> diff --git a/common/fuzzy b/common/fuzzy
> index 2d688fd27b..fad79124e5 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -159,7 +159,7 @@ __scratch_xfs_fuzz_mdrestore()
>  	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
>  
>  	__scratch_xfs_fuzz_unmount
> -	$XFS_MDRESTORE_PROG "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
> +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
>  }
>  
>  __fuzz_notify() {
> diff --git a/common/populate b/common/populate
> index 6e00499734..f382c40aca 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -861,7 +861,7 @@ _scratch_populate_restore_cached() {
>  
>  	case "${FSTYP}" in
>  	"xfs")
> -		$XFS_MDRESTORE_PROG "${metadump}" "${SCRATCH_DEV}"
> +		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
>  		res=$?
>  		test $res -ne 0 && return $res
>  
> @@ -876,7 +876,7 @@ _scratch_populate_restore_cached() {
>  		return $res
>  		;;
>  	"ext2"|"ext3"|"ext4")
> -		$E2IMAGE_PROG -r "${metadump}" "${SCRATCH_DEV}"
> +		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}"
>  		ret=$?
>  		test $ret -ne 0 && return $ret
>  
> diff --git a/common/xfs b/common/xfs
> index f466d2c42f..27d6ac84e3 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -638,6 +638,15 @@ _xfs_metadump() {
>  	return $res
>  }
>  
> +_xfs_mdrestore() {
> +	local metadump="$1"
> +	local device="$2"
> +	shift; shift
> +	local options="$@"
> +
> +	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
> +}
> +
>  # Snapshot the metadata on the scratch device
>  _scratch_xfs_metadump()
>  {
> 

Re: [PATCH 2/4] common/xfs: create a helper for restoring metadumps to the scratch devs

[Zorro Lang]

On Tue, Dec 13, 2022 at 11:45:20AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Refactor the open-coded $XFS_MDRESTORE_PROG calls into a proper
> _scratch_xfs_mdrestore helper.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/xfs    |    9 +++++++++
>  tests/xfs/129 |    2 +-
>  tests/xfs/234 |    2 +-
>  tests/xfs/253 |    2 +-
>  tests/xfs/284 |    2 +-
>  tests/xfs/291 |    2 +-
>  tests/xfs/336 |    2 +-
>  tests/xfs/432 |    2 +-
>  tests/xfs/503 |    8 ++++----
>  9 files changed, 20 insertions(+), 11 deletions(-)
> 
> 
> diff --git a/common/xfs b/common/xfs
> index 27d6ac84e3..216dab3bcd 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -660,6 +660,15 @@ _scratch_xfs_metadump()
>  	_xfs_metadump "$metadump" "$SCRATCH_DEV" "$logdev" nocompress "$@"
>  }
>  
> +# Restore snapshotted metadata on the scratch device
> +_scratch_xfs_mdrestore()
> +{
> +	local metadump=$1
> +	shift
> +
> +	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
> +}
> +
>  # run xfs_check and friends on a FS.
>  _check_xfs_filesystem()
>  {
> diff --git a/tests/xfs/129 b/tests/xfs/129
> index 09d40630d0..6f2ef5640d 100755
> --- a/tests/xfs/129
> +++ b/tests/xfs/129
> @@ -53,7 +53,7 @@ _scratch_xfs_metadump $metadump_file
>  
>  # Now restore the obfuscated one back and take a look around
>  echo "Restore metadump"
> -$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
> +SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>  SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>  SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>  
> diff --git a/tests/xfs/234 b/tests/xfs/234
> index cc1ee9a8ca..57d447c056 100755
> --- a/tests/xfs/234
> +++ b/tests/xfs/234
> @@ -53,7 +53,7 @@ _scratch_xfs_metadump $metadump_file
>  
>  # Now restore the obfuscated one back and take a look around
>  echo "Restore metadump"
> -$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
> +SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>  SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>  SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>  
> diff --git a/tests/xfs/253 b/tests/xfs/253
> index 1cfc218088..ce90247777 100755
> --- a/tests/xfs/253
> +++ b/tests/xfs/253
> @@ -152,7 +152,7 @@ _scratch_unmount
>  _scratch_xfs_metadump $METADUMP_FILE
>  
>  # Now restore the obfuscated one back and take a look around
> -$XFS_MDRESTORE_PROG "${METADUMP_FILE}" "${SCRATCH_DEV}"
> +_scratch_xfs_mdrestore "$METADUMP_FILE"
>  
>  _scratch_mount
>  
> diff --git a/tests/xfs/284 b/tests/xfs/284
> index e2bd05d4c7..58f330035e 100755
> --- a/tests/xfs/284
> +++ b/tests/xfs/284
> @@ -49,7 +49,7 @@ _scratch_unmount
>  # xfs_mdrestore should refuse to restore to a mounted device
>  _scratch_xfs_metadump $METADUMP_FILE
>  _scratch_mount
> -$XFS_MDRESTORE_PROG $METADUMP_FILE $SCRATCH_DEV 2>&1 | filter_mounted
> +_scratch_xfs_mdrestore $METADUMP_FILE 2>&1 | filter_mounted
>  _scratch_unmount
>  
>  # Test xfs_copy to a mounted device
> diff --git a/tests/xfs/291 b/tests/xfs/291
> index f5fea7f9a5..600dcb2eba 100755
> --- a/tests/xfs/291
> +++ b/tests/xfs/291
> @@ -93,7 +93,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"
> -$XFS_MDRESTORE_PROG $tmp.metadump $tmp.img || _fail "xfs_mdrestore 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 ee8ec649cb..5bcac976e4 100755
> --- a/tests/xfs/336
> +++ b/tests/xfs/336
> @@ -65,7 +65,7 @@ _scratch_xfs_metadump $metadump_file
>  
>  # Now restore the obfuscated one back and take a look around
>  echo "Restore metadump"
> -$XFS_MDRESTORE_PROG $metadump_file $TEST_DIR/image
> +SCRATCH_DEV=$TEST_DIR/image _scratch_xfs_mdrestore $metadump_file
>  SCRATCH_DEV=$TEST_DIR/image _scratch_mount
>  SCRATCH_DEV=$TEST_DIR/image _scratch_unmount
>  
> diff --git a/tests/xfs/432 b/tests/xfs/432
> index 676be9bd8a..66315b0398 100755
> --- a/tests/xfs/432
> +++ b/tests/xfs/432
> @@ -87,7 +87,7 @@ test -n "$extlen" || _notrun "could not create dir extent > 1000 blocks"
>  
>  echo "Try to metadump"
>  _scratch_xfs_metadump $metadump_file -w
> -$XFS_MDRESTORE_PROG $metadump_file $metadump_img
> +SCRATCH_DEV=$metadump_img _scratch_xfs_mdrestore $metadump_file
>  
>  echo "Check restored metadump image"
>  SCRATCH_DEV=$metadump_img _scratch_xfs_repair -n &>> $seqres.full || \
> diff --git a/tests/xfs/503 b/tests/xfs/503
> index 18bd8694c8..c786b04ccd 100755
> --- a/tests/xfs/503
> +++ b/tests/xfs/503
> @@ -66,25 +66,25 @@ _check_scratch_fs
>  _scratch_unmount
>  
>  echo mdrestore
> -$XFS_MDRESTORE_PROG $metadump_file $SCRATCH_DEV
> +_scratch_xfs_mdrestore $metadump_file
>  _scratch_mount
>  _check_scratch_fs
>  _scratch_unmount
>  
>  echo mdrestore a
> -$XFS_MDRESTORE_PROG $metadump_file_a $SCRATCH_DEV
> +_scratch_xfs_mdrestore $metadump_file_a
>  _scratch_mount
>  _check_scratch_fs
>  _scratch_unmount
>  
>  echo mdrestore g
> -$XFS_MDRESTORE_PROG $metadump_file_g $SCRATCH_DEV
> +_scratch_xfs_mdrestore $metadump_file_g
>  _scratch_mount
>  _check_scratch_fs
>  _scratch_unmount
>  
>  echo mdrestore ag
> -$XFS_MDRESTORE_PROG $metadump_file_ag $SCRATCH_DEV
> +_scratch_xfs_mdrestore $metadump_file_ag
>  _scratch_mount
>  _check_scratch_fs
>  _scratch_unmount
> 

Re: [PATCH 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore

[Zorro Lang]

On Tue, Dec 13, 2022 at 11:45:28AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Move the metadump decompression code to the per-filesystem mdrestore
> commands so that everyone can take advantage of them.  This enables the
> XFS and ext4 _mdrestore helpers to handle metadata dumps compressed with
> their respective _metadump helpers.
> 
> In turn, this means that the xfs fuzz tests can now handle the
> compressed metadumps created by the _scratch_populate_cached helper.
> This is key to unbreaking fuzz testing for xfs.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/ext4     |   13 ++++++++++++-
>  common/fuzzy    |    2 +-
>  common/populate |   15 ++-------------
>  common/xfs      |   15 +++++++++++++--
>  4 files changed, 28 insertions(+), 17 deletions(-)
> 
> 
> diff --git a/common/ext4 b/common/ext4
> index dc2e4e59cc..cadf1a7974 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -129,9 +129,20 @@ _ext4_mdrestore()
>  {
>  	local metadump="$1"
>  	local device="$2"
> -	shift; shift
> +	local compressopt="$3"

I didn't see this variable is used in this function, do I missed something?

> +	shift; shift; shift
>  	local options="$@"
>  
> +	# If we're configured for compressed dumps and there isn't already an
> +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> +	# something.
> +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> +		for compr in "$metadump".*; do
> +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> +		done
> +	fi
> +	test -r "$metadump" || return 1
> +
>  	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
>  }
>  
> diff --git a/common/fuzzy b/common/fuzzy
> index fad79124e5..e634815eec 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -159,7 +159,7 @@ __scratch_xfs_fuzz_mdrestore()
>  	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
>  
>  	__scratch_xfs_fuzz_unmount
> -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
> +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
>  }
>  
>  __fuzz_notify() {
> diff --git a/common/populate b/common/populate
> index f382c40aca..96866ee4cf 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -848,20 +848,9 @@ _scratch_populate_cache_tag() {
>  _scratch_populate_restore_cached() {
>  	local metadump="$1"
>  
> -	# If we're configured for compressed dumps and there isn't already an
> -	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> -	# something.
> -	if [ -n "$DUMP_COMPRESSOR" ]; then
> -		for compr in "$metadump".*; do
> -			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> -		done
> -	fi
> -
> -	test -r "$metadump" || return 1
> -
>  	case "${FSTYP}" in
>  	"xfs")
> -		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
> +		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
>  		res=$?
>  		test $res -ne 0 && return $res
>  
> @@ -876,7 +865,7 @@ _scratch_populate_restore_cached() {
>  		return $res
>  		;;
>  	"ext2"|"ext3"|"ext4")
> -		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}"
> +		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
>  		ret=$?
>  		test $ret -ne 0 && return $ret
>  
> diff --git a/common/xfs b/common/xfs
> index 216dab3bcd..833c2f4368 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -641,9 +641,20 @@ _xfs_metadump() {
>  _xfs_mdrestore() {
>  	local metadump="$1"
>  	local device="$2"
> -	shift; shift
> +	local compressopt="$3"

I didn't see this variable is used in this function either.

> +	shift; shift; shift
>  	local options="$@"
>  
> +	# If we're configured for compressed dumps and there isn't already an
> +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> +	# something.
> +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> +		for compr in "$metadump".*; do
> +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> +		done
> +	fi
> +	test -r "$metadump" || return 1
> +
>  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
>  }
>  
> @@ -666,7 +677,7 @@ _scratch_xfs_mdrestore()
>  	local metadump=$1
>  	shift
>  
> -	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
> +	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" nocompress "$@"
>  }
>  
>  # run xfs_check and friends on a FS.
> 

Re: [PATCH 4/4] fuzzy: don't fail on compressed metadumps

[Zorro Lang]

On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> This line in __scratch_xfs_fuzz_mdrestore:
> 
> 	test -e "${POPULATE_METADUMP}"
> 
> Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> the metadump files, because the metadump files get the compression
> program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> is wrong, and since the naming policy is an implementation detail of
> _xfs_mdrestore, let's get rid of the -e test.
> 
> However, we still need a way to fail the test if the metadump cannot be
> restored.  _xfs_mdrestore returns nonzero on failure, so use that
> instead.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---

Looks good to me,
Reviewed-by: Zorro Lang <zlang@redhat.com>

>  common/fuzzy |    5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> 
> diff --git a/common/fuzzy b/common/fuzzy
> index e634815eec..49c850f2d5 100644
> --- a/common/fuzzy
> +++ b/common/fuzzy
> @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
>  # Restore metadata to scratch device prior to field-fuzzing.
>  __scratch_xfs_fuzz_mdrestore()
>  {
> -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> -
>  	__scratch_xfs_fuzz_unmount
> -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
> +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
>  }
>  
>  __fuzz_notify() {
> 

Re: [PATCH 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore

[Darrick J. Wong]

On Sat, Dec 17, 2022 at 02:50:15PM +0800, Zorro Lang wrote:
> On Tue, Dec 13, 2022 at 11:45:28AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Move the metadump decompression code to the per-filesystem mdrestore
> > commands so that everyone can take advantage of them.  This enables the
> > XFS and ext4 _mdrestore helpers to handle metadata dumps compressed with
> > their respective _metadump helpers.
> > 
> > In turn, this means that the xfs fuzz tests can now handle the
> > compressed metadumps created by the _scratch_populate_cached helper.
> > This is key to unbreaking fuzz testing for xfs.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  common/ext4     |   13 ++++++++++++-
> >  common/fuzzy    |    2 +-
> >  common/populate |   15 ++-------------
> >  common/xfs      |   15 +++++++++++++--
> >  4 files changed, 28 insertions(+), 17 deletions(-)
> > 
> > 
> > diff --git a/common/ext4 b/common/ext4
> > index dc2e4e59cc..cadf1a7974 100644
> > --- a/common/ext4
> > +++ b/common/ext4
> > @@ -129,9 +129,20 @@ _ext4_mdrestore()
> >  {
> >  	local metadump="$1"
> >  	local device="$2"
> > -	shift; shift
> > +	local compressopt="$3"
> 
> I didn't see this variable is used in this function, do I missed something?

Oops.  I missed that this variable isn't necessary at all. :(

--D

> > +	shift; shift; shift
> >  	local options="$@"
> >  
> > +	# If we're configured for compressed dumps and there isn't already an
> > +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> > +	# something.
> > +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> > +		for compr in "$metadump".*; do
> > +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> > +		done
> > +	fi
> > +	test -r "$metadump" || return 1
> > +
> >  	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
> >  }
> >  
> > diff --git a/common/fuzzy b/common/fuzzy
> > index fad79124e5..e634815eec 100644
> > --- a/common/fuzzy
> > +++ b/common/fuzzy
> > @@ -159,7 +159,7 @@ __scratch_xfs_fuzz_mdrestore()
> >  	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> >  
> >  	__scratch_xfs_fuzz_unmount
> > -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}"
> > +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> >  }
> >  
> >  __fuzz_notify() {
> > diff --git a/common/populate b/common/populate
> > index f382c40aca..96866ee4cf 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -848,20 +848,9 @@ _scratch_populate_cache_tag() {
> >  _scratch_populate_restore_cached() {
> >  	local metadump="$1"
> >  
> > -	# If we're configured for compressed dumps and there isn't already an
> > -	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> > -	# something.
> > -	if [ -n "$DUMP_COMPRESSOR" ]; then
> > -		for compr in "$metadump".*; do
> > -			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> > -		done
> > -	fi
> > -
> > -	test -r "$metadump" || return 1
> > -
> >  	case "${FSTYP}" in
> >  	"xfs")
> > -		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
> > +		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
> >  		res=$?
> >  		test $res -ne 0 && return $res
> >  
> > @@ -876,7 +865,7 @@ _scratch_populate_restore_cached() {
> >  		return $res
> >  		;;
> >  	"ext2"|"ext3"|"ext4")
> > -		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}"
> > +		_ext4_mdrestore "${metadump}" "${SCRATCH_DEV}" compress
> >  		ret=$?
> >  		test $ret -ne 0 && return $ret
> >  
> > diff --git a/common/xfs b/common/xfs
> > index 216dab3bcd..833c2f4368 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -641,9 +641,20 @@ _xfs_metadump() {
> >  _xfs_mdrestore() {
> >  	local metadump="$1"
> >  	local device="$2"
> > -	shift; shift
> > +	local compressopt="$3"
> 
> I didn't see this variable is used in this function either.
> 
> > +	shift; shift; shift
> >  	local options="$@"
> >  
> > +	# If we're configured for compressed dumps and there isn't already an
> > +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> > +	# something.
> > +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> > +		for compr in "$metadump".*; do
> > +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> > +		done
> > +	fi
> > +	test -r "$metadump" || return 1
> > +
> >  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
> >  }
> >  
> > @@ -666,7 +677,7 @@ _scratch_xfs_mdrestore()
> >  	local metadump=$1
> >  	shift
> >  
> > -	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" "$@"
> > +	_xfs_mdrestore "$metadump" "$SCRATCH_DEV" nocompress "$@"
> >  }
> >  
> >  # run xfs_check and friends on a FS.
> > 
> 

[PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore

[Darrick J. Wong]

From: Darrick J. Wong <djwong@kernel.org>

Move the metadump decompression code to the per-filesystem mdrestore
commands so that everyone can take advantage of them.  This enables the
XFS and ext4 _mdrestore helpers to handle metadata dumps compressed with
their respective _metadump helpers.

In turn, this means that the xfs fuzz tests can now handle the
compressed metadumps created by the _scratch_populate_cached helper.
This is key to unbreaking fuzz testing for xfs.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
v1.1: remove unnecessary parameter
---
 common/ext4     |   10 ++++++++++
 common/populate |   11 -----------
 common/xfs      |   10 ++++++++++
 3 files changed, 20 insertions(+), 11 deletions(-)

diff --git a/common/ext4 b/common/ext4
index dc2e4e59cc..8fd6dbc682 100644
--- a/common/ext4
+++ b/common/ext4
@@ -132,6 +132,16 @@ _ext4_mdrestore()
 	shift; shift
 	local options="$@"
 
+	# If we're configured for compressed dumps and there isn't already an
+	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
+	# something.
+	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
+		for compr in "$metadump".*; do
+			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
+		done
+	fi
+	test -r "$metadump" || return 1
+
 	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
 }
 
diff --git a/common/populate b/common/populate
index f382c40aca..0a8a6390d4 100644
--- a/common/populate
+++ b/common/populate
@@ -848,17 +848,6 @@ _scratch_populate_cache_tag() {
 _scratch_populate_restore_cached() {
 	local metadump="$1"
 
-	# If we're configured for compressed dumps and there isn't already an
-	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
-	# something.
-	if [ -n "$DUMP_COMPRESSOR" ]; then
-		for compr in "$metadump".*; do
-			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
-		done
-	fi
-
-	test -r "$metadump" || return 1
-
 	case "${FSTYP}" in
 	"xfs")
 		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
diff --git a/common/xfs b/common/xfs
index 216dab3bcd..60848a5b8a 100644
--- a/common/xfs
+++ b/common/xfs
@@ -644,6 +644,16 @@ _xfs_mdrestore() {
 	shift; shift
 	local options="$@"
 
+	# If we're configured for compressed dumps and there isn't already an
+	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
+	# something.
+	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
+		for compr in "$metadump".*; do
+			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
+		done
+	fi
+	test -r "$metadump" || return 1
+
 	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
 }
 

Re: [PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore

[Zorro Lang]

On Sat, Dec 17, 2022 at 12:18:34AM -0800, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Move the metadump decompression code to the per-filesystem mdrestore
> commands so that everyone can take advantage of them.  This enables the
> XFS and ext4 _mdrestore helpers to handle metadata dumps compressed with
> their respective _metadump helpers.
> 
> In turn, this means that the xfs fuzz tests can now handle the
> compressed metadumps created by the _scratch_populate_cached helper.
> This is key to unbreaking fuzz testing for xfs.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
> v1.1: remove unnecessary parameter

This version looks good to me,

Reviewed-by: Zorro Lang <zlang@redhat.com>

> ---
>  common/ext4     |   10 ++++++++++
>  common/populate |   11 -----------
>  common/xfs      |   10 ++++++++++
>  3 files changed, 20 insertions(+), 11 deletions(-)
> 
> diff --git a/common/ext4 b/common/ext4
> index dc2e4e59cc..8fd6dbc682 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -132,6 +132,16 @@ _ext4_mdrestore()
>  	shift; shift
>  	local options="$@"
>  
> +	# If we're configured for compressed dumps and there isn't already an
> +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> +	# something.
> +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> +		for compr in "$metadump".*; do
> +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> +		done
> +	fi
> +	test -r "$metadump" || return 1
> +
>  	$E2IMAGE_PROG $options -r "${metadump}" "${SCRATCH_DEV}"
>  }
>  
> diff --git a/common/populate b/common/populate
> index f382c40aca..0a8a6390d4 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -848,17 +848,6 @@ _scratch_populate_cache_tag() {
>  _scratch_populate_restore_cached() {
>  	local metadump="$1"
>  
> -	# If we're configured for compressed dumps and there isn't already an
> -	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> -	# something.
> -	if [ -n "$DUMP_COMPRESSOR" ]; then
> -		for compr in "$metadump".*; do
> -			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> -		done
> -	fi
> -
> -	test -r "$metadump" || return 1
> -
>  	case "${FSTYP}" in
>  	"xfs")
>  		_xfs_mdrestore "${metadump}" "${SCRATCH_DEV}"
> diff --git a/common/xfs b/common/xfs
> index 216dab3bcd..60848a5b8a 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -644,6 +644,16 @@ _xfs_mdrestore() {
>  	shift; shift
>  	local options="$@"
>  
> +	# If we're configured for compressed dumps and there isn't already an
> +	# uncompressed dump, see if we can use DUMP_COMPRESSOR to decompress
> +	# something.
> +	if [ ! -e "$metadump" ] && [ -n "$DUMP_COMPRESSOR" ]; then
> +		for compr in "$metadump".*; do
> +			[ -e "$compr" ] && $DUMP_COMPRESSOR -d -f -k "$compr" && break
> +		done
> +	fi
> +	test -r "$metadump" || return 1
> +
>  	$XFS_MDRESTORE_PROG $options "${metadump}" "${device}"
>  }
>  
> 

Re: [PATCH 4/4] fuzzy: don't fail on compressed metadumps

[Zorro Lang]

On Sat, Dec 17, 2022 at 03:03:29PM +0800, Zorro Lang wrote:
> On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > This line in __scratch_xfs_fuzz_mdrestore:
> > 
> > 	test -e "${POPULATE_METADUMP}"
> > 
> > Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> > the metadump files, because the metadump files get the compression
> > program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> > is wrong, and since the naming policy is an implementation detail of
> > _xfs_mdrestore, let's get rid of the -e test.
> > 
> > However, we still need a way to fail the test if the metadump cannot be
> > restored.  _xfs_mdrestore returns nonzero on failure, so use that
> > instead.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> 
> Looks good to me,
> Reviewed-by: Zorro Lang <zlang@redhat.com>
> 
> >  common/fuzzy |    5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > 
> > diff --git a/common/fuzzy b/common/fuzzy
> > index e634815eec..49c850f2d5 100644
> > --- a/common/fuzzy
> > +++ b/common/fuzzy
> > @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
> >  # Restore metadata to scratch device prior to field-fuzzing.
> >  __scratch_xfs_fuzz_mdrestore()
> >  {
> > -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> > -
> >  	__scratch_xfs_fuzz_unmount
> > -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> > +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \

FYI, I've also removed the "compress" parameter according to:
  [PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore
When I merged this patch.

> > +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
> >  }
> >  
> >  __fuzz_notify() {
> > 

Re: [PATCH 4/4] fuzzy: don't fail on compressed metadumps

[Darrick J. Wong]

On Sat, Dec 17, 2022 at 06:33:33PM +0800, Zorro Lang wrote:
> On Sat, Dec 17, 2022 at 03:03:29PM +0800, Zorro Lang wrote:
> > On Tue, Dec 13, 2022 at 11:45:33AM -0800, Darrick J. Wong wrote:
> > > From: Darrick J. Wong <djwong@kernel.org>
> > > 
> > > This line in __scratch_xfs_fuzz_mdrestore:
> > > 
> > > 	test -e "${POPULATE_METADUMP}"
> > > 
> > > Breaks spectacularly on a setup that uses DUMP_COMPRESSOR to compress
> > > the metadump files, because the metadump files get the compression
> > > program added to the name (e.g. "${POPULATE_METADUMP}.xz").  The check
> > > is wrong, and since the naming policy is an implementation detail of
> > > _xfs_mdrestore, let's get rid of the -e test.
> > > 
> > > However, we still need a way to fail the test if the metadump cannot be
> > > restored.  _xfs_mdrestore returns nonzero on failure, so use that
> > > instead.
> > > 
> > > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > > ---
> > 
> > Looks good to me,
> > Reviewed-by: Zorro Lang <zlang@redhat.com>
> > 
> > >  common/fuzzy |    5 ++---
> > >  1 file changed, 2 insertions(+), 3 deletions(-)
> > > 
> > > 
> > > diff --git a/common/fuzzy b/common/fuzzy
> > > index e634815eec..49c850f2d5 100644
> > > --- a/common/fuzzy
> > > +++ b/common/fuzzy
> > > @@ -156,10 +156,9 @@ __scratch_xfs_fuzz_unmount()
> > >  # Restore metadata to scratch device prior to field-fuzzing.
> > >  __scratch_xfs_fuzz_mdrestore()
> > >  {
> > > -	test -e "${POPULATE_METADUMP}" || _fail "Need to set POPULATE_METADUMP"
> > > -
> > >  	__scratch_xfs_fuzz_unmount
> > > -	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress
> > > +	_xfs_mdrestore "${POPULATE_METADUMP}" "${SCRATCH_DEV}" compress || \
> 
> FYI, I've also removed the "compress" parameter according to:
>   [PATCH v1.1 3/4] common/populate: move decompression code to _{xfs,ext4}_mdrestore
> When I merged this patch.

Thank you!  Sorry for the mess. :/

--D

> > > +		_fail "${POPULATE_METADUMP}: Could not find metadump to restore?"
> > >  }
> > >  
> > >  __fuzz_notify() {
> > > 
>