[PATCHSET 0/3] fstests: refactor ext4-specific code

[PATCHSET 0/3] fstests: refactor ext4-specific code

[Darrick J. Wong]

Hi all,

This series aims to make it so that fstests can install device mapper
filters for external log devices.  Before we can do that, however, we
need to change fstests to pass the device path of the jbd2 device to
mount and mkfs.  Before we can do /that/, refactor all the ext4-specific
code out of common/rc into a separate common/ext4 file.

If you're going to start using this mess, you probably ought to just
pull from my git trees, which are linked below.

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

--D

fstests git tree:
https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=refactor-ext4-helpers
---
 common/config |    4 +
 common/ext4   |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/rc     |  177 ++-------------------------------------------------------
 common/xfs    |   23 +++++++
 4 files changed, 208 insertions(+), 172 deletions(-)
 create mode 100644 common/ext4

[PATCH 1/3] common/rc: move ext4-specific helpers into a separate common/ext4 file

[Darrick J. Wong]

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

Move the ext4-specific parts of common/rc into a separate file and
source it when we test that.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/config |    4 +
 common/ext4   |  156 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 common/rc     |  153 --------------------------------------------------------
 3 files changed, 160 insertions(+), 153 deletions(-)
 create mode 100644 common/ext4


diff --git a/common/config b/common/config
index c30eec6d..5eaae447 100644
--- a/common/config
+++ b/common/config
@@ -512,6 +512,10 @@ _source_specific_fs()
 		;;
 	ext4)
 		[ "$MKFS_EXT4_PROG" = "" ] && _fatal "mkfs.ext4 not found"
+		. ./common/ext4
+		;;
+	ext2|ext3|ext4dev)
+		. ./common/ext4
 		;;
 	f2fs)
 		[ "$MKFS_F2FS_PROG" = "" ] && _fatal "mkfs.f2fs not found"
diff --git a/common/ext4 b/common/ext4
new file mode 100644
index 00000000..287705af
--- /dev/null
+++ b/common/ext4
@@ -0,0 +1,156 @@
+#
+# ext4 specific common functions
+#
+
+_setup_large_ext4_fs()
+{
+	local fs_size=$1
+	local tmp_dir=/tmp/
+
+	[ "$LARGE_SCRATCH_DEV" != yes ] && return 0
+	[ -z "$SCRATCH_DEV_EMPTY_SPACE" ] && SCRATCH_DEV_EMPTY_SPACE=0
+	[ $SCRATCH_DEV_EMPTY_SPACE -ge $fs_size ] && return 0
+
+	# Default free space in the FS is 50GB, but you can specify more via
+	# SCRATCH_DEV_EMPTY_SPACE
+	local space_to_consume=$(($fs_size - 50*1024*1024*1024 - $SCRATCH_DEV_EMPTY_SPACE))
+
+	# mount the filesystem and create 16TB - 4KB files until we consume
+	# all the necessary space.
+	_try_scratch_mount 2>&1 >$tmp_dir/mnt.err
+	local status=$?
+	if [ $status -ne 0 ]; then
+		echo "mount failed"
+		cat $tmp_dir/mnt.err >&2
+		rm -f $tmp_dir/mnt.err
+		return $status
+	fi
+	rm -f $tmp_dir/mnt.err
+
+	local file_size=$((16*1024*1024*1024*1024 - 4096))
+	local nfiles=0
+	while [ $space_to_consume -gt $file_size ]; do
+
+		xfs_io -F -f \
+			-c "truncate $file_size" \
+			-c "falloc -k 0 $file_size" \
+			$SCRATCH_MNT/.use_space.$nfiles 2>&1
+		status=$?
+		if [ $status -ne 0 ]; then
+			break;
+		fi
+
+		space_to_consume=$(( $space_to_consume - $file_size ))
+		nfiles=$(($nfiles + 1))
+	done
+
+	# consume the remaining space.
+	if [ $space_to_consume -gt 0 ]; then
+		xfs_io -F -f \
+			-c "truncate $space_to_consume" \
+			-c "falloc -k 0 $space_to_consume" \
+			$SCRATCH_MNT/.use_space.$nfiles 2>&1
+		status=$?
+	fi
+	export NUM_SPACE_FILES=$nfiles
+
+	_scratch_unmount
+	if [ $status -ne 0 ]; then
+		echo "large file prealloc failed"
+		cat $tmp_dir/mnt.err >&2
+		return $status
+	fi
+	return 0
+}
+
+_scratch_mkfs_ext4()
+{
+	local mkfs_cmd="$MKFS_EXT4_PROG -F"
+	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
+	local tmp=`mktemp -u`
+	local mkfs_status
+
+	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
+	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
+
+	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
+	mkfs_status=$?
+
+	if [ $mkfs_status -eq 0 -a "$LARGE_SCRATCH_DEV" = yes ]; then
+		# manually parse the mkfs output to get the fs size in bytes
+		local fs_size=`cat $tmp.mkfsstd | awk ' \
+			/^Block size/ { split($2, a, "="); bs = a[2] ; } \
+			/ inodes, / { blks = $3 } \
+			/reserved for the super user/ { resv = $1 } \
+			END { fssize = bs * blks - resv; print fssize }'`
+
+		_setup_large_ext4_fs $fs_size
+		mkfs_status=$?
+	fi
+
+	# output mkfs stdout and stderr
+	cat $tmp.mkfsstd
+	cat $tmp.mkfserr >&2
+	rm -f $tmp.mkfserr $tmp.mkfsstd
+
+	return $mkfs_status
+}
+
+_ext4_metadump()
+{
+	local device="$1"
+	local dumpfile="$2"
+	local compressopt="$3"
+
+	test -n "$E2IMAGE_PROG" || _fail "e2image not installed"
+	$E2IMAGE_PROG -Q "$device" "$dumpfile"
+	[ "$compressopt" = "compress" ] && [ -n "$DUMP_COMPRESSOR" ] &&
+		$DUMP_COMPRESSOR -f "$dumpfile" &>> "$seqres.full"
+}
+
+# this test requires the ext4 kernel support crc feature on scratch device
+#
+_require_scratch_ext4_crc()
+{
+	_scratch_mkfs_ext4 >/dev/null 2>&1
+	dumpe2fs -h $SCRATCH_DEV 2> /dev/null | grep -q metadata_csum || _notrun "metadata_csum not supported by this filesystem"
+	_try_scratch_mount >/dev/null 2>&1 \
+	   || _notrun "Kernel doesn't support metadata_csum feature"
+	_scratch_unmount
+}
+
+# Check whether the specified feature whether it is supported by
+# mkfs.ext4 and the kernel.
+_require_scratch_ext4_feature()
+{
+    if [ -z "$1" ]; then
+        echo "Usage: _require_scratch_ext4_feature feature"
+        exit 1
+    fi
+    $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
+		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
+	|| _notrun "mkfs.ext4 doesn't support $1 feature"
+    _try_scratch_mount >/dev/null 2>&1 \
+	|| _notrun "Kernel doesn't support the ext4 feature(s): $1"
+    _scratch_unmount
+}
+
+# Disable extent zeroing for ext4 on the given device
+_ext4_disable_extent_zeroout()
+{
+	local dev=${1:-$TEST_DEV}
+	local sdev=`_short_dev $dev`
+
+	[ -f /sys/fs/ext4/$sdev/extent_max_zeroout_kb ] && \
+		echo 0 >/sys/fs/ext4/$sdev/extent_max_zeroout_kb
+}
+
+_require_scratch_richacl_ext4()
+{
+	_scratch_mkfs -O richacl >/dev/null 2>&1 \
+		|| _notrun "can't mkfs $FSTYP with option -O richacl"
+	_try_scratch_mount >/dev/null 2>&1 \
+		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
+	_scratch_unmount
+}
diff --git a/common/rc b/common/rc
index 197c9415..52dd3b41 100644
--- a/common/rc
+++ b/common/rc
@@ -559,113 +559,6 @@ _scratch_do_mkfs()
 	return $mkfs_status
 }
 
-_setup_large_ext4_fs()
-{
-	local fs_size=$1
-	local tmp_dir=/tmp/
-
-	[ "$LARGE_SCRATCH_DEV" != yes ] && return 0
-	[ -z "$SCRATCH_DEV_EMPTY_SPACE" ] && SCRATCH_DEV_EMPTY_SPACE=0
-	[ $SCRATCH_DEV_EMPTY_SPACE -ge $fs_size ] && return 0
-
-	# Default free space in the FS is 50GB, but you can specify more via
-	# SCRATCH_DEV_EMPTY_SPACE
-	local space_to_consume=$(($fs_size - 50*1024*1024*1024 - $SCRATCH_DEV_EMPTY_SPACE))
-
-	# mount the filesystem and create 16TB - 4KB files until we consume
-	# all the necessary space.
-	_try_scratch_mount 2>&1 >$tmp_dir/mnt.err
-	local status=$?
-	if [ $status -ne 0 ]; then
-		echo "mount failed"
-		cat $tmp_dir/mnt.err >&2
-		rm -f $tmp_dir/mnt.err
-		return $status
-	fi
-	rm -f $tmp_dir/mnt.err
-
-	local file_size=$((16*1024*1024*1024*1024 - 4096))
-	local nfiles=0
-	while [ $space_to_consume -gt $file_size ]; do
-
-		xfs_io -F -f \
-			-c "truncate $file_size" \
-			-c "falloc -k 0 $file_size" \
-			$SCRATCH_MNT/.use_space.$nfiles 2>&1
-		status=$?
-		if [ $status -ne 0 ]; then
-			break;
-		fi
-
-		space_to_consume=$(( $space_to_consume - $file_size ))
-		nfiles=$(($nfiles + 1))
-	done
-
-	# consume the remaining space.
-	if [ $space_to_consume -gt 0 ]; then
-		xfs_io -F -f \
-			-c "truncate $space_to_consume" \
-			-c "falloc -k 0 $space_to_consume" \
-			$SCRATCH_MNT/.use_space.$nfiles 2>&1
-		status=$?
-	fi
-	export NUM_SPACE_FILES=$nfiles
-
-	_scratch_unmount
-	if [ $status -ne 0 ]; then
-		echo "large file prealloc failed"
-		cat $tmp_dir/mnt.err >&2
-		return $status
-	fi
-	return 0
-}
-
-_scratch_mkfs_ext4()
-{
-	local mkfs_cmd="$MKFS_EXT4_PROG -F"
-	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
-	local tmp=`mktemp -u`
-	local mkfs_status
-
-	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
-	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
-	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
-
-	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
-	mkfs_status=$?
-
-	if [ $mkfs_status -eq 0 -a "$LARGE_SCRATCH_DEV" = yes ]; then
-		# manually parse the mkfs output to get the fs size in bytes
-		local fs_size=`cat $tmp.mkfsstd | awk ' \
-			/^Block size/ { split($2, a, "="); bs = a[2] ; } \
-			/ inodes, / { blks = $3 } \
-			/reserved for the super user/ { resv = $1 } \
-			END { fssize = bs * blks - resv; print fssize }'`
-
-		_setup_large_ext4_fs $fs_size
-		mkfs_status=$?
-	fi
-
-	# output mkfs stdout and stderr
-	cat $tmp.mkfsstd
-	cat $tmp.mkfserr >&2
-	rm -f $tmp.mkfserr $tmp.mkfsstd
-
-	return $mkfs_status
-}
-
-_ext4_metadump()
-{
-	local device="$1"
-	local dumpfile="$2"
-	local compressopt="$3"
-
-	test -n "$E2IMAGE_PROG" || _fail "e2image not installed"
-	$E2IMAGE_PROG -Q "$device" "$dumpfile"
-	[ "$compressopt" = "compress" ] && [ -n "$DUMP_COMPRESSOR" ] &&
-		$DUMP_COMPRESSOR -f "$dumpfile" &>> "$seqres.full"
-}
-
 # Capture the metadata of a filesystem in a dump file for offline analysis.
 # This is not supported by all filesystem types, so this function should only
 # be used after a test has already failed.
@@ -2245,33 +2138,6 @@ _require_non_zoned_device()
 	fi
 }
 
-# this test requires the ext4 kernel support crc feature on scratch device
-#
-_require_scratch_ext4_crc()
-{
-	_scratch_mkfs_ext4 >/dev/null 2>&1
-	dumpe2fs -h $SCRATCH_DEV 2> /dev/null | grep -q metadata_csum || _notrun "metadata_csum not supported by this filesystem"
-	_try_scratch_mount >/dev/null 2>&1 \
-	   || _notrun "Kernel doesn't support metadata_csum feature"
-	_scratch_unmount
-}
-
-# Check whether the specified feature whether it is supported by
-# mkfs.ext4 and the kernel.
-_require_scratch_ext4_feature()
-{
-    if [ -z "$1" ]; then
-        echo "Usage: _require_scratch_ext4_feature feature"
-        exit 1
-    fi
-    $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
-		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
-	|| _notrun "mkfs.ext4 doesn't support $1 feature"
-    _try_scratch_mount >/dev/null 2>&1 \
-	|| _notrun "Kernel doesn't support the ext4 feature(s): $1"
-    _scratch_unmount
-}
-
 # this test requires that external log/realtime devices are not in use
 #
 _require_nonexternal()
@@ -2894,16 +2760,6 @@ _require_fail_make_request()
  not found. Seems that CONFIG_FAULT_INJECTION_DEBUG_FS kernel config option not enabled"
 }
 
-# Disable extent zeroing for ext4 on the given device
-_ext4_disable_extent_zeroout()
-{
-	local dev=${1:-$TEST_DEV}
-	local sdev=`_short_dev $dev`
-
-	[ -f /sys/fs/ext4/$sdev/extent_max_zeroout_kb ] && \
-		echo 0 >/sys/fs/ext4/$sdev/extent_max_zeroout_kb
-}
-
 # The default behavior of SEEK_HOLE is to always return EOF.
 # Filesystems that implement non-default behavior return the offset
 # of holes with SEEK_HOLE. There is no way to query the filesystem
@@ -3001,15 +2857,6 @@ _require_scratch_richacl_xfs()
 	_scratch_unmount
 }
 
-_require_scratch_richacl_ext4()
-{
-	_scratch_mkfs -O richacl >/dev/null 2>&1 \
-		|| _notrun "can't mkfs $FSTYP with option -O richacl"
-	_try_scratch_mount >/dev/null 2>&1 \
-		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
-	_scratch_unmount
-}
-
 _require_scratch_richacl_support()
 {
 	_scratch_mount

[PATCH 2/3] common/rc: move XFS-specific parts of _scratch_options into common/xfs

[Darrick J. Wong]

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

Move all the non-XFS code in _scratch_options into a
_scratch_xfs_options helper in common/xfs, in preparation to add ext4
bits.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/rc  |   23 +++--------------------
 common/xfs |   23 +++++++++++++++++++++++
 2 files changed, 26 insertions(+), 20 deletions(-)


diff --git a/common/rc b/common/rc
index 52dd3b41..dc1d65c3 100644
--- a/common/rc
+++ b/common/rc
@@ -172,30 +172,13 @@ _clear_mount_stack()
 
 _scratch_options()
 {
-    local type=$1
-    local rt_opt=""
-    local log_opt=""
     SCRATCH_OPTIONS=""
 
-    if [ "$FSTYP" != "xfs" ]; then
-        return
-    fi
-
-    case $type in
-    mkfs)
-	SCRATCH_OPTIONS="$SCRATCH_OPTIONS -f"
-	rt_opt="-r"
-        log_opt="-l"
-	;;
-    mount)
-	rt_opt="-o"
-        log_opt="-o"
+    case "$FSTYP" in
+    "xfs")
+	_scratch_xfs_options "$@"
 	;;
     esac
-    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
-	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${rt_opt}rtdev=$SCRATCH_RTDEV"
-    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
-	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}logdev=$SCRATCH_LOGDEV"
 }
 
 _test_options()
diff --git a/common/xfs b/common/xfs
index 9f84dffb..f6f4cdd2 100644
--- a/common/xfs
+++ b/common/xfs
@@ -265,6 +265,29 @@ _xfs_check()
 	return $status
 }
 
+_scratch_xfs_options()
+{
+    local type=$1
+    local rt_opt=""
+    local log_opt=""
+
+    case $type in
+    mkfs)
+	SCRATCH_OPTIONS="$SCRATCH_OPTIONS -f"
+	rt_opt="-r"
+        log_opt="-l"
+	;;
+    mount)
+	rt_opt="-o"
+        log_opt="-o"
+	;;
+    esac
+    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
+	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${rt_opt}rtdev=$SCRATCH_RTDEV"
+    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}logdev=$SCRATCH_LOGDEV"
+}
+
 _scratch_xfs_db_options()
 {
 	SCRATCH_OPTIONS=""

[PATCH 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

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

Create a _scratch_options backend for ext* so that we can inject
pathnames to external log devices into the scratch fs mount options.
This enables common/dm* to install block device filters, e.g. dm-error
for stress testing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/ext4 |   20 ++++++++++++++++++++
 common/rc   |    3 +++
 2 files changed, 23 insertions(+)


diff --git a/common/ext4 b/common/ext4
index 287705af..819f9786 100644
--- a/common/ext4
+++ b/common/ext4
@@ -154,3 +154,23 @@ _require_scratch_richacl_ext4()
 		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
 	_scratch_unmount
 }
+
+_scratch_ext4_options()
+{
+    local type=$1
+    local log_opt=""
+
+    case $type in
+    mkfs)
+        log_opt="-J device=$SCRATCH_LOGDEV"
+	;;
+    mount)
+	# As of kernel 5.19, the kernel mount option path parser only accepts
+	# direct paths to block devices--the final path component cannot be a
+	# symlink.
+        log_opt="-o journal_path=$(realpath $SCRATCH_LOGDEV)"
+	;;
+    esac
+    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
+}
diff --git a/common/rc b/common/rc
index dc1d65c3..b82bb36b 100644
--- a/common/rc
+++ b/common/rc
@@ -178,6 +178,9 @@ _scratch_options()
     "xfs")
 	_scratch_xfs_options "$@"
 	;;
+    ext2|ext3|ext4|ext4dev)
+	_scratch_ext4_options "$@"
+	;;
     esac
 }
 

Re: [PATCH 3/3] common/ext4: provide custom ext4 scratch fs options

[Zorro Lang]

On Tue, Aug 02, 2022 at 09:21:57PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a _scratch_options backend for ext* so that we can inject
> pathnames to external log devices into the scratch fs mount options.
> This enables common/dm* to install block device filters, e.g. dm-error
> for stress testing.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> ---
>  common/ext4 |   20 ++++++++++++++++++++
>  common/rc   |    3 +++
>  2 files changed, 23 insertions(+)
> 
> 
> diff --git a/common/ext4 b/common/ext4
> index 287705af..819f9786 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -154,3 +154,23 @@ _require_scratch_richacl_ext4()
>  		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
>  	_scratch_unmount
>  }
> +
> +_scratch_ext4_options()
> +{
> +    local type=$1
> +    local log_opt=""
> +
> +    case $type in
> +    mkfs)
> +        log_opt="-J device=$SCRATCH_LOGDEV"

In _scratch_mkfs_ext4, it deals with mkfs with SCRATCH_LOGDEV:

  [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
     $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
     mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"

So is there a conflict or duplication?

> +	;;
> +    mount)
> +	# As of kernel 5.19, the kernel mount option path parser only accepts
> +	# direct paths to block devices--the final path component cannot be a
> +	# symlink.
> +        log_opt="-o journal_path=$(realpath $SCRATCH_LOGDEV)"
> +	;;
> +    esac
> +    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> +	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
> +}
> diff --git a/common/rc b/common/rc
> index dc1d65c3..b82bb36b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -178,6 +178,9 @@ _scratch_options()
>      "xfs")
>  	_scratch_xfs_options "$@"
>  	;;
> +    ext2|ext3|ext4|ext4dev)
> +	_scratch_ext4_options "$@"
> +	;;
>      esac
>  }
>  
> 

Re: [PATCH 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

On Thu, Aug 04, 2022 at 02:28:43AM +0800, Zorro Lang wrote:
> On Tue, Aug 02, 2022 at 09:21:57PM -0700, Darrick J. Wong wrote:
> > From: Darrick J. Wong <djwong@kernel.org>
> > 
> > Create a _scratch_options backend for ext* so that we can inject
> > pathnames to external log devices into the scratch fs mount options.
> > This enables common/dm* to install block device filters, e.g. dm-error
> > for stress testing.
> > 
> > Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> > ---
> >  common/ext4 |   20 ++++++++++++++++++++
> >  common/rc   |    3 +++
> >  2 files changed, 23 insertions(+)
> > 
> > 
> > diff --git a/common/ext4 b/common/ext4
> > index 287705af..819f9786 100644
> > --- a/common/ext4
> > +++ b/common/ext4
> > @@ -154,3 +154,23 @@ _require_scratch_richacl_ext4()
> >  		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
> >  	_scratch_unmount
> >  }
> > +
> > +_scratch_ext4_options()
> > +{
> > +    local type=$1
> > +    local log_opt=""
> > +
> > +    case $type in
> > +    mkfs)
> > +        log_opt="-J device=$SCRATCH_LOGDEV"
> 
> In _scratch_mkfs_ext4, it deals with mkfs with SCRATCH_LOGDEV:
> 
>   [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
>      $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
>      mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
> 
> So is there a conflict or duplication?

Nope, I simply missed that.  I'll respin the patch.

--D

> > +	;;
> > +    mount)
> > +	# As of kernel 5.19, the kernel mount option path parser only accepts
> > +	# direct paths to block devices--the final path component cannot be a
> > +	# symlink.
> > +        log_opt="-o journal_path=$(realpath $SCRATCH_LOGDEV)"
> > +	;;
> > +    esac
> > +    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> > +	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
> > +}
> > diff --git a/common/rc b/common/rc
> > index dc1d65c3..b82bb36b 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -178,6 +178,9 @@ _scratch_options()
> >      "xfs")
> >  	_scratch_xfs_options "$@"
> >  	;;
> > +    ext2|ext3|ext4|ext4dev)
> > +	_scratch_ext4_options "$@"
> > +	;;
> >      esac
> >  }
> >  
> > 
> 

[PATCH v1.2 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

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

Create a _scratch_options backend for ext* so that we can inject
pathnames to external log devices into the scratch fs mount options.
This enables common/dm* to install block device filters, e.g. dm-error
for stress testing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
v1.2: refactor _scratch_mkfs_ext4 to use _scratch_options too
---
 common/ext4 |   34 +++++++++++++++++++++++++++++++---
 common/rc   |    3 +++
 2 files changed, 34 insertions(+), 3 deletions(-)

diff --git a/common/ext4 b/common/ext4
index 287705af..8a3385af 100644
--- a/common/ext4
+++ b/common/ext4
@@ -63,16 +63,24 @@ _setup_large_ext4_fs()
 	return 0
 }
 
+_scratch_mkfs_ext4_opts()
+{
+	mkfs_opts=$*
+
+	_scratch_options mkfs
+
+	echo "$MKFS_EXT4_PROG -F $SCRATCH_OPTIONS $mkfs_opts"
+}
+
 _scratch_mkfs_ext4()
 {
-	local mkfs_cmd="$MKFS_EXT4_PROG -F"
+	local mkfs_cmd="`_scratch_mkfs_ext4_opts`"
 	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
 	local tmp=`mktemp -u`
 	local mkfs_status
 
 	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
-	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
-	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
+	    $MKFS_EXT4_PROG -F -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV
 
 	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
 	mkfs_status=$?
@@ -154,3 +162,23 @@ _require_scratch_richacl_ext4()
 		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
 	_scratch_unmount
 }
+
+_scratch_ext4_options()
+{
+    local type=$1
+    local log_opt=""
+
+    case $type in
+    mkfs)
+        log_opt="-J device=$SCRATCH_LOGDEV"
+	;;
+    mount)
+	# As of kernel 5.19, the kernel mount option path parser only accepts
+	# direct paths to block devices--the final path component cannot be a
+	# symlink.
+        log_opt="-o journal_path=$(realpath $SCRATCH_LOGDEV)"
+	;;
+    esac
+    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
+}
diff --git a/common/rc b/common/rc
index dc1d65c3..b82bb36b 100644
--- a/common/rc
+++ b/common/rc
@@ -178,6 +178,9 @@ _scratch_options()
     "xfs")
 	_scratch_xfs_options "$@"
 	;;
+    ext2|ext3|ext4|ext4dev)
+	_scratch_ext4_options "$@"
+	;;
     esac
 }
 

Re: [PATCH v1.2 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

On Wed, Aug 03, 2022 at 05:25:11PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a _scratch_options backend for ext* so that we can inject
> pathnames to external log devices into the scratch fs mount options.
> This enables common/dm* to install block device filters, e.g. dm-error
> for stress testing.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> v1.2: refactor _scratch_mkfs_ext4 to use _scratch_options too

Self NAK, this still has broken bits...

> ---
>  common/ext4 |   34 +++++++++++++++++++++++++++++++---
>  common/rc   |    3 +++
>  2 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/common/ext4 b/common/ext4
> index 287705af..8a3385af 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -63,16 +63,24 @@ _setup_large_ext4_fs()
>  	return 0
>  }
>  
> +_scratch_mkfs_ext4_opts()
> +{
> +	mkfs_opts=$*
> +
> +	_scratch_options mkfs
> +
> +	echo "$MKFS_EXT4_PROG -F $SCRATCH_OPTIONS $mkfs_opts"

...the -F should go in _scratch_ext4_options...

> +}
> +
>  _scratch_mkfs_ext4()
>  {
> -	local mkfs_cmd="$MKFS_EXT4_PROG -F"
> +	local mkfs_cmd="`_scratch_mkfs_ext4_opts`"
>  	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
>  	local tmp=`mktemp -u`
>  	local mkfs_status
>  
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> -	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
> -	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
> +	    $MKFS_EXT4_PROG -F -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV
>  
>  	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	mkfs_status=$?
> @@ -154,3 +162,23 @@ _require_scratch_richacl_ext4()
>  		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
>  	_scratch_unmount
>  }
> +
> +_scratch_ext4_options()
> +{
> +    local type=$1
> +    local log_opt=""
> +
> +    case $type in
> +    mkfs)
> +        log_opt="-J device=$SCRATCH_LOGDEV"
> +	;;
> +    mount)
> +	# As of kernel 5.19, the kernel mount option path parser only accepts
> +	# direct paths to block devices--the final path component cannot be a
> +	# symlink.
> +        log_opt="-o journal_path=$(realpath $SCRATCH_LOGDEV)"

...and this ought to be `realpath -q "$SCRATCH_LOGDEV"' to avoid
breaking the non-external-journal case.

> +	;;
> +    esac
> +    [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> +	SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"

Tab/space insanity here.

--D

> +}
> diff --git a/common/rc b/common/rc
> index dc1d65c3..b82bb36b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -178,6 +178,9 @@ _scratch_options()
>      "xfs")
>  	_scratch_xfs_options "$@"
>  	;;
> +    ext2|ext3|ext4|ext4dev)
> +	_scratch_ext4_options "$@"
> +	;;
>      esac
>  }
>  

[PATCH v1.3 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

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

Create a _scratch_options backend for ext* so that we can inject
pathnames to external log devices into the scratch fs mount options.
This enables common/dm* to install block device filters, e.g. dm-error
for stress testing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
v1.1: bad at counting
v1.2: refactor _scratch_mkfs_ext4 to use _scratch_options too
v1.3: quiet down realpath usage when SCRATCH_LOGDEV is unset
---
 common/ext4 |   35 ++++++++++++++++++++++++++++++++---
 common/rc   |    3 +++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/common/ext4 b/common/ext4
index 287705af..f2df888c 100644
--- a/common/ext4
+++ b/common/ext4
@@ -63,16 +63,24 @@ _setup_large_ext4_fs()
 	return 0
 }
 
+_scratch_mkfs_ext4_opts()
+{
+	mkfs_opts=$*
+
+	_scratch_options mkfs
+
+	echo "$MKFS_EXT4_PROG $SCRATCH_OPTIONS $mkfs_opts"
+}
+
 _scratch_mkfs_ext4()
 {
-	local mkfs_cmd="$MKFS_EXT4_PROG -F"
+	local mkfs_cmd="`_scratch_mkfs_ext4_opts`"
 	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
 	local tmp=`mktemp -u`
 	local mkfs_status
 
 	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
-	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
-	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
+	    $MKFS_EXT4_PROG -F -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV
 
 	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
 	mkfs_status=$?
@@ -154,3 +162,24 @@ _require_scratch_richacl_ext4()
 		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
 	_scratch_unmount
 }
+
+_scratch_ext4_options()
+{
+	local type=$1
+	local log_opt=""
+
+	case $type in
+	mkfs)
+		SCRATCH_OPTIONS="$SCRATCH_OPTIONS -F"
+		log_opt="-J device=$SCRATCH_LOGDEV"
+		;;
+	mount)
+		# As of kernel 5.19, the kernel mount option path parser only
+		# accepts direct paths to block devices--the final path
+		# component cannot be a symlink.
+		log_opt="-o journal_path=$(realpath -q "$SCRATCH_LOGDEV")"
+		;;
+	esac
+	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+		SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
+}
diff --git a/common/rc b/common/rc
index dc1d65c3..b82bb36b 100644
--- a/common/rc
+++ b/common/rc
@@ -178,6 +178,9 @@ _scratch_options()
     "xfs")
 	_scratch_xfs_options "$@"
 	;;
+    ext2|ext3|ext4|ext4dev)
+	_scratch_ext4_options "$@"
+	;;
     esac
 }
 

Re: [PATCH v1.3 3/3] common/ext4: provide custom ext4 scratch fs options

[Zorro Lang]

On Thu, Aug 04, 2022 at 09:29:01AM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <djwong@kernel.org>
> 
> Create a _scratch_options backend for ext* so that we can inject
> pathnames to external log devices into the scratch fs mount options.
> This enables common/dm* to install block device filters, e.g. dm-error
> for stress testing.
> 
> Signed-off-by: Darrick J. Wong <djwong@kernel.org>
> v1.1: bad at counting
> v1.2: refactor _scratch_mkfs_ext4 to use _scratch_options too
> v1.3: quiet down realpath usage when SCRATCH_LOGDEV is unset

I think you'd like move these 3 change log lines after the "---" ?

As usual, I'm going to give this patchset enough testing before merging
it, due to it affect ext4 testing too much. And hope to get review from
ext4 list, or "no objection" means all good by default :)

Thanks,
Zorro

> ---
>  common/ext4 |   35 ++++++++++++++++++++++++++++++++---
>  common/rc   |    3 +++
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/common/ext4 b/common/ext4
> index 287705af..f2df888c 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -63,16 +63,24 @@ _setup_large_ext4_fs()
>  	return 0
>  }
>  
> +_scratch_mkfs_ext4_opts()
> +{
> +	mkfs_opts=$*
> +
> +	_scratch_options mkfs
> +
> +	echo "$MKFS_EXT4_PROG $SCRATCH_OPTIONS $mkfs_opts"
> +}
> +
>  _scratch_mkfs_ext4()
>  {
> -	local mkfs_cmd="$MKFS_EXT4_PROG -F"
> +	local mkfs_cmd="`_scratch_mkfs_ext4_opts`"
>  	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
>  	local tmp=`mktemp -u`
>  	local mkfs_status
>  
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> -	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
> -	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
> +	    $MKFS_EXT4_PROG -F -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV
>  
>  	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
>  	mkfs_status=$?
> @@ -154,3 +162,24 @@ _require_scratch_richacl_ext4()
>  		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
>  	_scratch_unmount
>  }
> +
> +_scratch_ext4_options()
> +{
> +	local type=$1
> +	local log_opt=""
> +
> +	case $type in
> +	mkfs)
> +		SCRATCH_OPTIONS="$SCRATCH_OPTIONS -F"
> +		log_opt="-J device=$SCRATCH_LOGDEV"
> +		;;
> +	mount)
> +		# As of kernel 5.19, the kernel mount option path parser only
> +		# accepts direct paths to block devices--the final path
> +		# component cannot be a symlink.
> +		log_opt="-o journal_path=$(realpath -q "$SCRATCH_LOGDEV")"
> +		;;
> +	esac
> +	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> +		SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
> +}
> diff --git a/common/rc b/common/rc
> index dc1d65c3..b82bb36b 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -178,6 +178,9 @@ _scratch_options()
>      "xfs")
>  	_scratch_xfs_options "$@"
>  	;;
> +    ext2|ext3|ext4|ext4dev)
> +	_scratch_ext4_options "$@"
> +	;;
>      esac
>  }
>  
> 

Re: [PATCHSET 0/3] fstests: refactor ext4-specific code

[Zorro Lang]

On Tue, Aug 02, 2022 at 09:21:40PM -0700, Darrick J. Wong wrote:
> Hi all,
> 
> This series aims to make it so that fstests can install device mapper
> filters for external log devices.  Before we can do that, however, we
> need to change fstests to pass the device path of the jbd2 device to
> mount and mkfs.  Before we can do /that/, refactor all the ext4-specific
> code out of common/rc into a separate common/ext4 file.
> 
> If you're going to start using this mess, you probably ought to just
> pull from my git trees, which are linked below.
> 
> This is an extraordinary way to destroy everything.  Enjoy!
> Comments and questions are, as always, welcome.
> 
> --D
> 
> fstests git tree:
> https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=refactor-ext4-helpers
> ---

Hi Darrick,

There're 3 failures[1] if test ext4 with external logdev, after merging this
patchset.
The g/629 is always failed with or without this patchset, it fails if test
with external logdev.
The g/250 and g/252 fail due to _scratch_mkfs_sized doesn't use common ext4
mkfs helper, so can't deal with SCRATCH_LOGDEV well.

Thanks,
Zorro

[1]
SECTION       -- logdev
FSTYP         -- ext4
PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 5.19.0-0.rc2.21.fc37.x86_64+debug #1 SMP PREEMPT_DYNAMIC Mon Jun 13 14:55:18 UTC 2022
MKFS_OPTIONS  -- -F -J device=/dev/loop0 /dev/sda3
MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:root_t:s0 -o journal_path=/dev/loop0 /dev/sda3 /mnt/scratch

generic/250 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/250.out.bad)
    --- tests/generic/250.out   2022-04-29 23:07:23.262498285 +0800
    +++ /root/git/xfstests/results//logdev/generic/250.out.bad  2022-08-06 22:26:45.179294149 +0800
    @@ -1,9 +1,19 @@
     QA output created by 250
     Format and mount
    +umount: /mnt/scratch: not mounted.
    +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
    +       dmesg(1) may have more information after failed mount system call.
     Create the original files
    +umount: /mnt/scratch: not mounted.
    ...
    (Run 'diff -u /root/git/xfstests/tests/generic/250.out /root/git/xfstests/results//logdev/generic/250.out.bad'  to see the entire diff)
generic/252 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/252.out.bad)
    --- tests/generic/252.out   2022-04-29 23:07:23.264498308 +0800
    +++ /root/git/xfstests/results//logdev/generic/252.out.bad  2022-08-06 22:26:48.495330525 +0800
    @@ -1,10 +1,19 @@
     QA output created by 252
     Format and mount
    +umount: /mnt/scratch: not mounted.
    +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
    +       dmesg(1) may have more information after failed mount system call.
     Create the original files
    +umount: /mnt/scratch: not mounted.
    ...
    (Run 'diff -u /root/git/xfstests/tests/generic/252.out /root/git/xfstests/results//logdev/generic/252.out.bad'  to see the entire diff)
generic/629 3s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/629.out.bad)
    --- tests/generic/629.out   2022-04-29 23:07:23.545501491 +0800
    +++ /root/git/xfstests/results//logdev/generic/629.out.bad  2022-08-06 22:26:50.810355920 +0800
    @@ -1,4 +1,5 @@
     QA output created by 629
    +mke2fs 1.46.5 (30-Dec-2021)
     test o_sync write
     310f146ce52077fcd3308dcbe7632bb2  SCRATCH_MNT/0
     test unaligned copy range o_sync
    ...
    (Run 'diff -u /root/git/xfstests/tests/generic/629.out /root/git/xfstests/results//logdev/generic/629.out.bad'  to see the entire diff)
Ran: generic/250 generic/252 generic/629
Failures: generic/250 generic/252 generic/629
Failed 3 of 3 tests


>  common/config |    4 +
>  common/ext4   |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  common/rc     |  177 ++-------------------------------------------------------
>  common/xfs    |   23 +++++++
>  4 files changed, 208 insertions(+), 172 deletions(-)
>  create mode 100644 common/ext4
> 

Re: [PATCHSET 0/3] fstests: refactor ext4-specific code

[Darrick J. Wong]

On Sat, Aug 06, 2022 at 10:36:06PM +0800, Zorro Lang wrote:
> On Tue, Aug 02, 2022 at 09:21:40PM -0700, Darrick J. Wong wrote:
> > Hi all,
> > 
> > This series aims to make it so that fstests can install device mapper
> > filters for external log devices.  Before we can do that, however, we
> > need to change fstests to pass the device path of the jbd2 device to
> > mount and mkfs.  Before we can do /that/, refactor all the ext4-specific
> > code out of common/rc into a separate common/ext4 file.
> > 
> > If you're going to start using this mess, you probably ought to just
> > pull from my git trees, which are linked below.
> > 
> > This is an extraordinary way to destroy everything.  Enjoy!
> > Comments and questions are, as always, welcome.
> > 
> > --D
> > 
> > fstests git tree:
> > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=refactor-ext4-helpers
> > ---
> 
> Hi Darrick,
> 
> There're 3 failures[1] if test ext4 with external logdev, after merging this
> patchset.
> The g/629 is always failed with or without this patchset, it fails if test
> with external logdev.
> The g/250 and g/252 fail due to _scratch_mkfs_sized doesn't use common ext4
> mkfs helper, so can't deal with SCRATCH_LOGDEV well.

Totally different helper, but yes, I'll add that to my list if nothing
else than to get this patchset moving.

--D

> Thanks,
> Zorro
> 
> [1]
> SECTION       -- logdev
> FSTYP         -- ext4
> PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 5.19.0-0.rc2.21.fc37.x86_64+debug #1 SMP PREEMPT_DYNAMIC Mon Jun 13 14:55:18 UTC 2022
> MKFS_OPTIONS  -- -F -J device=/dev/loop0 /dev/sda3
> MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:root_t:s0 -o journal_path=/dev/loop0 /dev/sda3 /mnt/scratch
> 
> generic/250 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/250.out.bad)
>     --- tests/generic/250.out   2022-04-29 23:07:23.262498285 +0800
>     +++ /root/git/xfstests/results//logdev/generic/250.out.bad  2022-08-06 22:26:45.179294149 +0800
>     @@ -1,9 +1,19 @@
>      QA output created by 250
>      Format and mount
>     +umount: /mnt/scratch: not mounted.
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
>     +       dmesg(1) may have more information after failed mount system call.
>      Create the original files
>     +umount: /mnt/scratch: not mounted.
>     ...
>     (Run 'diff -u /root/git/xfstests/tests/generic/250.out /root/git/xfstests/results//logdev/generic/250.out.bad'  to see the entire diff)
> generic/252 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/252.out.bad)
>     --- tests/generic/252.out   2022-04-29 23:07:23.264498308 +0800
>     +++ /root/git/xfstests/results//logdev/generic/252.out.bad  2022-08-06 22:26:48.495330525 +0800
>     @@ -1,10 +1,19 @@
>      QA output created by 252
>      Format and mount
>     +umount: /mnt/scratch: not mounted.
>     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
>     +       dmesg(1) may have more information after failed mount system call.
>      Create the original files
>     +umount: /mnt/scratch: not mounted.
>     ...
>     (Run 'diff -u /root/git/xfstests/tests/generic/252.out /root/git/xfstests/results//logdev/generic/252.out.bad'  to see the entire diff)
> generic/629 3s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/629.out.bad)
>     --- tests/generic/629.out   2022-04-29 23:07:23.545501491 +0800
>     +++ /root/git/xfstests/results//logdev/generic/629.out.bad  2022-08-06 22:26:50.810355920 +0800
>     @@ -1,4 +1,5 @@
>      QA output created by 629
>     +mke2fs 1.46.5 (30-Dec-2021)
>      test o_sync write
>      310f146ce52077fcd3308dcbe7632bb2  SCRATCH_MNT/0
>      test unaligned copy range o_sync
>     ...
>     (Run 'diff -u /root/git/xfstests/tests/generic/629.out /root/git/xfstests/results//logdev/generic/629.out.bad'  to see the entire diff)
> Ran: generic/250 generic/252 generic/629
> Failures: generic/250 generic/252 generic/629
> Failed 3 of 3 tests
> 
> 
> >  common/config |    4 +
> >  common/ext4   |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  common/rc     |  177 ++-------------------------------------------------------
> >  common/xfs    |   23 +++++++
> >  4 files changed, 208 insertions(+), 172 deletions(-)
> >  create mode 100644 common/ext4
> > 
> 

Re: [PATCHSET 0/3] fstests: refactor ext4-specific code

[Zorro Lang]

On Sun, Aug 07, 2022 at 09:30:28AM -0700, Darrick J. Wong wrote:
> On Sat, Aug 06, 2022 at 10:36:06PM +0800, Zorro Lang wrote:
> > On Tue, Aug 02, 2022 at 09:21:40PM -0700, Darrick J. Wong wrote:
> > > Hi all,
> > > 
> > > This series aims to make it so that fstests can install device mapper
> > > filters for external log devices.  Before we can do that, however, we
> > > need to change fstests to pass the device path of the jbd2 device to
> > > mount and mkfs.  Before we can do /that/, refactor all the ext4-specific
> > > code out of common/rc into a separate common/ext4 file.
> > > 
> > > If you're going to start using this mess, you probably ought to just
> > > pull from my git trees, which are linked below.
> > > 
> > > This is an extraordinary way to destroy everything.  Enjoy!
> > > Comments and questions are, as always, welcome.
> > > 
> > > --D
> > > 
> > > fstests git tree:
> > > https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=refactor-ext4-helpers
> > > ---
> > 
> > Hi Darrick,
> > 
> > There're 3 failures[1] if test ext4 with external logdev, after merging this
> > patchset.
> > The g/629 is always failed with or without this patchset, it fails if test
> > with external logdev.
> > The g/250 and g/252 fail due to _scratch_mkfs_sized doesn't use common ext4
> > mkfs helper, so can't deal with SCRATCH_LOGDEV well.
> 
> Totally different helper, but yes, I'll add that to my list if nothing
> else than to get this patchset moving.

Yes, just due to you try to help common/dmerror to support external logdev,
and these two eio test cases use _scratch_mkfs_sized, it's not compatible
with your new change on dmerror, but it's not regression :)

I think we can fix visible errors at first, then improve ext4 external logdev
supporting bit by bit.

Thanks,
Zorro

> 
> --D
> 
> > Thanks,
> > Zorro
> > 
> > [1]
> > SECTION       -- logdev
> > FSTYP         -- ext4
> > PLATFORM      -- Linux/x86_64 hp-dl380pg8-01 5.19.0-0.rc2.21.fc37.x86_64+debug #1 SMP PREEMPT_DYNAMIC Mon Jun 13 14:55:18 UTC 2022
> > MKFS_OPTIONS  -- -F -J device=/dev/loop0 /dev/sda3
> > MOUNT_OPTIONS -- -o acl,user_xattr -o context=system_u:object_r:root_t:s0 -o journal_path=/dev/loop0 /dev/sda3 /mnt/scratch
> > 
> > generic/250 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/250.out.bad)
> >     --- tests/generic/250.out   2022-04-29 23:07:23.262498285 +0800
> >     +++ /root/git/xfstests/results//logdev/generic/250.out.bad  2022-08-06 22:26:45.179294149 +0800
> >     @@ -1,9 +1,19 @@
> >      QA output created by 250
> >      Format and mount
> >     +umount: /mnt/scratch: not mounted.
> >     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
> >     +       dmesg(1) may have more information after failed mount system call.
> >      Create the original files
> >     +umount: /mnt/scratch: not mounted.
> >     ...
> >     (Run 'diff -u /root/git/xfstests/tests/generic/250.out /root/git/xfstests/results//logdev/generic/250.out.bad'  to see the entire diff)
> > generic/252 2s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/252.out.bad)
> >     --- tests/generic/252.out   2022-04-29 23:07:23.264498308 +0800
> >     +++ /root/git/xfstests/results//logdev/generic/252.out.bad  2022-08-06 22:26:48.495330525 +0800
> >     @@ -1,10 +1,19 @@
> >      QA output created by 252
> >      Format and mount
> >     +umount: /mnt/scratch: not mounted.
> >     +mount: /mnt/scratch: wrong fs type, bad option, bad superblock on /dev/mapper/error-test, missing codepage or helper program, or other error.
> >     +       dmesg(1) may have more information after failed mount system call.
> >      Create the original files
> >     +umount: /mnt/scratch: not mounted.
> >     ...
> >     (Run 'diff -u /root/git/xfstests/tests/generic/252.out /root/git/xfstests/results//logdev/generic/252.out.bad'  to see the entire diff)
> > generic/629 3s ... - output mismatch (see /root/git/xfstests/results//logdev/generic/629.out.bad)
> >     --- tests/generic/629.out   2022-04-29 23:07:23.545501491 +0800
> >     +++ /root/git/xfstests/results//logdev/generic/629.out.bad  2022-08-06 22:26:50.810355920 +0800
> >     @@ -1,4 +1,5 @@
> >      QA output created by 629
> >     +mke2fs 1.46.5 (30-Dec-2021)
> >      test o_sync write
> >      310f146ce52077fcd3308dcbe7632bb2  SCRATCH_MNT/0
> >      test unaligned copy range o_sync
> >     ...
> >     (Run 'diff -u /root/git/xfstests/tests/generic/629.out /root/git/xfstests/results//logdev/generic/629.out.bad'  to see the entire diff)
> > Ran: generic/250 generic/252 generic/629
> > Failures: generic/250 generic/252 generic/629
> > Failed 3 of 3 tests
> > 
> > 
> > >  common/config |    4 +
> > >  common/ext4   |  176 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > >  common/rc     |  177 ++-------------------------------------------------------
> > >  common/xfs    |   23 +++++++
> > >  4 files changed, 208 insertions(+), 172 deletions(-)
> > >  create mode 100644 common/ext4
> > > 
> > 
> 

Re: [PATCH 3/3] common/ext4: provide custom ext4 scratch fs options

[Christoph Hellwig]

Looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[PATCH 3/3] common/ext4: provide custom ext4 scratch fs options

[Darrick J. Wong]

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

Create a _scratch_options backend for ext* so that we can inject
pathnames to external log devices into the scratch fs mount options.
This enables common/dm* to install block device filters, e.g. dm-error
for stress testing.

Signed-off-by: Darrick J. Wong <djwong@kernel.org>
---
 common/ext4 |   45 +++++++++++++++++++++++++++++++++++++++++----
 common/rc   |   12 +++++++++++-
 2 files changed, 52 insertions(+), 5 deletions(-)


diff --git a/common/ext4 b/common/ext4
index 287705af..f4c3c413 100644
--- a/common/ext4
+++ b/common/ext4
@@ -63,16 +63,32 @@ _setup_large_ext4_fs()
 	return 0
 }
 
+_scratch_mkfs_ext4_opts()
+{
+	mkfs_opts=$*
+
+	_scratch_options mkfs
+
+	echo "$MKFS_EXT4_PROG $SCRATCH_OPTIONS $mkfs_opts"
+}
+
 _scratch_mkfs_ext4()
 {
-	local mkfs_cmd="$MKFS_EXT4_PROG -F"
+	local mkfs_cmd="`_scratch_mkfs_ext4_opts`"
 	local mkfs_filter="grep -v -e ^Warning: -e \"^mke2fs \" | grep -v \"^$\""
 	local tmp=`mktemp -u`
 	local mkfs_status
 
-	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
-	    $mkfs_cmd -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV && \
-	    mkfs_cmd="$mkfs_cmd -J device=$SCRATCH_LOGDEV"
+	if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ]; then
+		$MKFS_EXT4_PROG -F -O journal_dev $MKFS_OPTIONS $* $SCRATCH_LOGDEV 2>$tmp.mkfserr 1>$tmp.mkfsstd
+		mkjournal_status=$?
+
+		if [ $mkjournal_status -ne 0 ]; then
+			cat $tmp.mkfsstd
+			cat $tmp.mkfserr >&2
+			return $mkjournal_status
+		fi
+	fi
 
 	_scratch_do_mkfs "$mkfs_cmd" "$mkfs_filter" $* 2>$tmp.mkfserr 1>$tmp.mkfsstd
 	mkfs_status=$?
@@ -154,3 +170,24 @@ _require_scratch_richacl_ext4()
 		|| _notrun "kernel doesn't support richacl feature on $FSTYP"
 	_scratch_unmount
 }
+
+_scratch_ext4_options()
+{
+	local type=$1
+	local log_opt=""
+
+	case $type in
+	mkfs)
+		SCRATCH_OPTIONS="$SCRATCH_OPTIONS -F"
+		log_opt="-J device=$SCRATCH_LOGDEV"
+		;;
+	mount)
+		# As of kernel 5.19, the kernel mount option path parser only
+		# accepts direct paths to block devices--the final path
+		# component cannot be a symlink.
+		log_opt="-o journal_path=$(realpath -q "$SCRATCH_LOGDEV")"
+		;;
+	esac
+	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
+		SCRATCH_OPTIONS="$SCRATCH_OPTIONS ${log_opt}"
+}
diff --git a/common/rc b/common/rc
index dc1d65c3..e20c494c 100644
--- a/common/rc
+++ b/common/rc
@@ -178,6 +178,9 @@ _scratch_options()
     "xfs")
 	_scratch_xfs_options "$@"
 	;;
+    ext2|ext3|ext4|ext4dev)
+	_scratch_ext4_options "$@"
+	;;
     esac
 }
 
@@ -964,6 +967,13 @@ _scratch_mkfs_sized()
 		fi
 		;;
 	ext2|ext3|ext4|ext4dev)
+		# Can't use _scratch_mkfs_ext4 here because the block count has
+		# to come after the device path.
+		if [ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ]; then
+			${MKFS_PROG} -F -O journal_dev $MKFS_OPTIONS $SCRATCH_LOGDEV || \
+				_notrun "Could not make scratch logdev"
+			MKFS_OPTIONS="$MKFS_OPTIONS -J device=$SCRATCH_LOGDEV"
+		fi
 		${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV $blocks
 		;;
 	gfs2)
@@ -1093,7 +1103,7 @@ _scratch_mkfs_blocksized()
 		_scratch_mkfs_xfs $MKFS_OPTIONS -b size=$blocksize
 		;;
 	ext2|ext3|ext4)
-		${MKFS_PROG} -t $FSTYP -F $MKFS_OPTIONS -b $blocksize $SCRATCH_DEV
+		_scratch_mkfs_ext4 $MKFS_OPTIONS -b $blocksize
 		;;
 	gfs2)
 		${MKFS_PROG} -t $FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV