[PATCH -v2 0/6] Miscellaneous ext4 test fix ups

[PATCH -v2 0/6] Miscellaneous ext4 test fix ups

[Theodore Ts'o]

This addresses the review comments of the patch series at:

https://lore.kernel.org/r/20220625030718.1215980-1-tytso@mit.edu

There is also an additional patch to fix test failures when the config
is testing ext4's encrypt feature to address tests which overrides
MKFS_OPTIONS.

Theodore Ts'o (6):
  ext4/050: support indirect as well as extent mapped journals
  ext4/044: skip test if the file system does not have a journal
  ext4/045: _require_dumpe2fs doesn't take any arguments so remove them
  ext4/054: skip test if the dax mount option is enabled
  ext4/055: skip test if dax mount option is used
  ext4: create file systems with the enrypt feature if necessary

 common/rc         |  3 +++
 tests/ext4/003    |  6 ++++-
 tests/ext4/035    |  5 +++-
 tests/ext4/044    |  1 +
 tests/ext4/045    |  2 +-
 tests/ext4/050    | 58 ++++-------------------------------------------
 tests/ext4/054    |  5 ++--
 tests/ext4/055    |  1 +
 tests/ext4/306    |  3 +++
 tests/generic/260 |  5 +++-
 10 files changed, 29 insertions(+), 60 deletions(-)

-- 
2.31.0

[PATCH 1/6] ext4/050: support indirect as well as extent mapped journals

[Theodore Ts'o]

Simplify the test and fix ext4/050 failures when running ext4 without
extents enabled (e.g., in ext3 emulation mode).

Instead of relying on parsing debugfs output's (which varies depending
on whether the journal inode is extent mapped or indirect block
mapped), use debugfs's "cat" command to get the contents of the
journal.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
 tests/ext4/050 | 58 +++++---------------------------------------------
 1 file changed, 5 insertions(+), 53 deletions(-)

diff --git a/tests/ext4/050 b/tests/ext4/050
index 79961957..6f93b86d 100755
--- a/tests/ext4/050
+++ b/tests/ext4/050
@@ -22,55 +22,6 @@ _require_command "$DEBUGFS_PROG" debugfs
 checkpoint_journal=$here/src/checkpoint_journal
 _require_test_program "checkpoint_journal"
 
-# convert output from stat<journal_inode> to list of block numbers
-get_journal_extents() {
-	inode_info=$($DEBUGFS_PROG $SCRATCH_DEV -R "stat <8>" 2>> $seqres.full)
-	echo -e "\nJournal info:" >> $seqres.full
-	echo "$inode_info" >> $seqres.full
-
-	extents_line=$(echo "$inode_info" | awk '/EXTENTS:/{ print NR; exit }')
-	get_extents=$(echo "$inode_info" | sed -n "$(($extents_line + 1))"p)
-
-	# get just the physical block numbers
-	get_extents=$(echo "$get_extents" |  perl -pe 's|\(.*?\):||g' | sed -e 's/, /\n/g' | perl -pe 's|(\d+)-(\d+)|\1 \2|g')
-
-	echo "$get_extents"
-}
-
-# checks all extents are zero'd out except for the superblock
-# arg 1: extents (output of get_journal_extents())
-check_extents() {
-	echo -e "\nChecking extents:" >> $seqres.full
-	echo "$1" >> $seqres.full
-
-	super_block="true"
-	echo "$1" | while IFS= read line; do
-		start_block=$(echo $line | cut -f1 -d' ')
-		end_block=$(echo $line | cut -f2 -d' ' -s)
-
-		# if first block of journal, shouldn't be wiped
-		if [ "$super_block" == "true" ]; then
-			super_block="false"
-
-			#if super block only block in this extent, skip extent
-			if [ -z "$end_block" ]; then
-				continue;
-			fi
-			start_block=$(($start_block + 1))
-		fi
-
-		if [ ! -z "$end_block" ]; then
-			blocks=$(($end_block - $start_block + 1))
-		else
-			blocks=1
-		fi
-
-		check=$(od $SCRATCH_DEV --skip-bytes=$(($start_block * $blocksize)) --read-bytes=$(($blocks * $blocksize)) -An -v | sed -e 's/[0 \t\n\r]//g')
-
-		[ ! -z "$check" ] && echo "error" && break
-	done
-}
-
 testdir="${SCRATCH_MNT}/testdir"
 
 _scratch_mkfs_sized $((64 * 1024 * 1024)) >> $seqres.full 2>&1
@@ -93,11 +44,12 @@ sync --file-system $testdir/1
 # call ioctl to checkpoint and zero-fill journal blocks
 $checkpoint_journal $SCRATCH_MNT --erase=zeroout || _fail "ioctl returned error"
 
-extents=$(get_journal_extents)
-
 # check journal blocks zeroed out
-ret=$(check_extents "$extents")
-[ "$ret" = "error" ] && _fail "Journal was not zero-filled"
+$DEBUGFS_PROG $SCRATCH_DEV -R "cat <8>" 2> /dev/null | od >> $seqres.full
+check=$($DEBUGFS_PROG $SCRATCH_DEV -R "cat <8>" 2> /dev/null | \
+	    od --skip-bytes="$blocksize" -An -v | sed -e '/^[0 \t]*$/d')
+
+[ ! -z "$check" ] && _fail "Journal was not zeroed"
 
 _scratch_unmount >> $seqres.full 2>&1
 
-- 
2.31.0

[PATCH 2/6] ext4/044: skip test if the file system does not have a journal

[Theodore Ts'o]

This test mounts the file system using "mount -t ext3"; if the file
system config creates the file system without the jbd2 journal, the
"mount -t ext3" will fail.  So skip this test in that case.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/ext4/044 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ext4/044 b/tests/ext4/044
index 46e44053..158e2ac5 100755
--- a/tests/ext4/044
+++ b/tests/ext4/044
@@ -16,6 +16,7 @@ _begin_fstest auto quick
 _supported_fs ext4
 _require_scratch
 _require_test_program "t_get_file_time"
+_require_metadata_journaling
 
 echo "Silence is golden"
 
-- 
2.31.0

[PATCH 3/6] ext4/045: _require_dumpe2fs doesn't take any arguments so remove them

[Theodore Ts'o]

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/ext4/045 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/ext4/045 b/tests/ext4/045
index ee7c0de3..4f0ad4aa 100755
--- a/tests/ext4/045
+++ b/tests/ext4/045
@@ -22,7 +22,7 @@ _supported_fs ext4
 _require_scratch
 _require_test_program "t_create_short_dirs"
 _require_test_program "t_create_long_dirs"
-_require_dumpe2fs "$DUMPE2FS_PROG" dumpe2fs
+_require_dumpe2fs
 
 echo "Silence is golden"
 
-- 
2.31.0

[PATCH 4/6] ext4/054: skip test if the dax mount option is enabled

[Theodore Ts'o]

The ext4/054 test explicitly creates a file system with a 1k
blocksize.  This can't possibly work on if the dax mount option is
enabled, so change ext4/054 to use _scratch_mkfs_blocksized, and and a
check to _scratch_mkfs_blocksized to _notrun the test if the block
size is less than the page size.

Also remove an unnecessary _require_test declaration since this test
does not use the test device.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
 common/rc      | 3 +++
 tests/ext4/054 | 5 ++---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/common/rc b/common/rc
index 9378ff26..3014f5fc 100644
--- a/common/rc
+++ b/common/rc
@@ -1220,6 +1220,9 @@ _scratch_mkfs_blocksized()
 	if ! [[ $blocksize =~ $re ]] ; then
 		_notrun "error: _scratch_mkfs_sized: block size \"$blocksize\" not an integer."
 	fi
+	if [ $blocksize -lt $(get_page_size) ]; then
+		_exclude_scratch_mount_option dax
+	fi
 
 	case $FSTYP in
 	btrfs)
diff --git a/tests/ext4/054 b/tests/ext4/054
index 9a11719f..e23acbb1 100755
--- a/tests/ext4/054
+++ b/tests/ext4/054
@@ -19,7 +19,6 @@ _begin_fstest auto quick dangerous_fuzzers
 
 # real QA test starts here
 _supported_fs ext4
-_require_test
 _require_scratch_nocheck
 _require_xfs_io_command "falloc"
 _require_xfs_io_command "pwrite"
@@ -28,8 +27,8 @@ _require_xfs_io_command "fpunch"
 _require_command "$DEBUGFS_PROG" debugfs
 
 # In order to accurately construct the damaged extent in the following
-# test steps, the blocksize is set to 1024 here
-_scratch_mkfs "-b 1024" > $seqres.full 2>&1
+# test steps, the block size is set to 1024 here
+_scratch_mkfs_blocksized 1024 >> $seqres.full 2>&1
 _scratch_mount
 
 TEST_FILE="${SCRATCH_MNT}/testfile"
-- 
2.31.0

[PATCH 5/6] ext4/055: skip test if dax mount option is used

[Theodore Ts'o]

This test creates a 1k block file system with the quota feature
enabled.   As such, it is incompatible with DAX.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/ext4/055 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/ext4/055 b/tests/ext4/055
index 8f466f1b..aa15cfe9 100755
--- a/tests/ext4/055
+++ b/tests/ext4/055
@@ -26,6 +26,7 @@ _require_command "$DEBUGFS_PROG" debugfs
 echo "Silence is golden"
 
 # The 1K blocksize is designed for debugfs.
+_exclude_scratch_mount_option dax
 _scratch_mkfs "-F -O quota -b 1024" > $seqres.full 2>&1
 
 # Start from 0, fill block 1 with 6，replace the original 2.
-- 
2.31.0

[PATCH 6/6] ext4: create file systems with the enrypt feature if necessary

[Theodore Ts'o]

The linux kernel commit 5f41fdaea63d ("ext4: only allow
test_dummy_encryption when supported") the kernel will reject mounts
with the test_dummy_encryption option if the ext4 file system does not
have the encrypt feature enabled.

There are a handful of tests (ext4/003, ext4/035, ext4/306, and
generic/260) which will format the scratch file system using a
hard-coded set of mkfs.ext4 parameters ignoring the MKFS_OPTION that
is set by the file system test config.

For file system config that includes test_dummy_encryption in
MOUNT_OPTIONS and "-O encrypt" in MKFS_OPTIONS, we need to test for
test_dummy_encryption and force the hard-coded mkfs options to enable
the encrypt feature.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---
 tests/ext4/003    | 6 +++++-
 tests/ext4/035    | 5 ++++-
 tests/ext4/306    | 3 +++
 tests/generic/260 | 5 ++++-
 4 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/tests/ext4/003 b/tests/ext4/003
index 773bcb03..6a4518e3 100755
--- a/tests/ext4/003
+++ b/tests/ext4/003
@@ -27,7 +27,11 @@ _require_scratch
 _require_scratch_ext4_feature "bigalloc"
 
 BLOCK_SIZE=$(get_page_size)
-$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
+FEATURES=bigalloc
+if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
+    FEATURES=${FEATURES},encrypt
+fi
+$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O ${FEATURES} -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
 	>> $seqres.full 2>&1
 _scratch_mount
 
diff --git a/tests/ext4/035 b/tests/ext4/035
index 1f26e766..a1ccce45 100755
--- a/tests/ext4/035
+++ b/tests/ext4/035
@@ -25,7 +25,10 @@ _require_scratch
 _exclude_scratch_mount_option dax
 _require_command "$RESIZE2FS_PROG" resize2fs
 
-$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $SCRATCH_DEV 32768 >> $seqres.full 2>&1
+if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
+    ENCRYPT="-O encrypt"
+fi
+$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $ENCRYPT $SCRATCH_DEV 32768 >> $seqres.full 2>&1
 if [ $? -ne 0 ]; then
     _notrun "Can't make file system with a block size of 1024"
 fi
diff --git a/tests/ext4/306 b/tests/ext4/306
index 2ff88537..a6b7eab5 100755
--- a/tests/ext4/306
+++ b/tests/ext4/306
@@ -33,6 +33,9 @@ features="^extents"
 if grep -q 64bit /etc/mke2fs.conf ; then
     features="^extents,^64bit"
 fi
+if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
+    features=${features},encrypt
+fi
 
 blksz=$(get_page_size)
 
diff --git a/tests/generic/260 b/tests/generic/260
index b4d72e0f..e86afe9c 100755
--- a/tests/generic/260
+++ b/tests/generic/260
@@ -120,7 +120,10 @@ case $FSTYP in
 		bsize=4096
 		start=$(_math "$base*$agsize*$bsize")
 		len=$start
-		export MKFS_OPTIONS="-F -b $bsize -g $agsize"
+		if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
+		    ENCRYPT="-O encrypt"
+		fi
+		export MKFS_OPTIONS="-F -b $bsize -g $agsize $ENCRYPT"
 		;;
 	xfs)
 		agsize=65538
-- 
2.31.0

Re: [PATCH 6/6] ext4: create file systems with the enrypt feature if necessary

[Eric Biggers]

On Tue, Jun 28, 2022 at 02:25:45PM -0400, Theodore Ts'o wrote:
> The linux kernel commit 5f41fdaea63d ("ext4: only allow
> test_dummy_encryption when supported") the kernel will reject mounts
> with the test_dummy_encryption option if the ext4 file system does not
> have the encrypt feature enabled.
> 
> There are a handful of tests (ext4/003, ext4/035, ext4/306, and
> generic/260) which will format the scratch file system using a
> hard-coded set of mkfs.ext4 parameters ignoring the MKFS_OPTION that
> is set by the file system test config.
> 
> For file system config that includes test_dummy_encryption in
> MOUNT_OPTIONS and "-O encrypt" in MKFS_OPTIONS, we need to test for
> test_dummy_encryption and force the hard-coded mkfs options to enable
> the encrypt feature.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  tests/ext4/003    | 6 +++++-
>  tests/ext4/035    | 5 ++++-
>  tests/ext4/306    | 3 +++
>  tests/generic/260 | 5 ++++-
>  4 files changed, 16 insertions(+), 3 deletions(-)

Thanks, I didn't notice these.  The general approach looks good.  A few nits:

- The hunk in tests/ext4/035 doesn't apply; it seems you have a local change?

- "enrypt" in the subject should be "encrypt"

> diff --git a/tests/ext4/003 b/tests/ext4/003
> index 773bcb03..6a4518e3 100755
> --- a/tests/ext4/003
> +++ b/tests/ext4/003
> @@ -27,7 +27,11 @@ _require_scratch
>  _require_scratch_ext4_feature "bigalloc"
>  
>  BLOCK_SIZE=$(get_page_size)
> -$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
> +FEATURES=bigalloc
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    FEATURES=${FEATURES},encrypt
> +fi

Bash supports appending to variables: FEATURES+=",encrypt"

Also, lower case "features" would be better.

> diff --git a/tests/ext4/035 b/tests/ext4/035
> index 1f26e766..a1ccce45 100755
> --- a/tests/ext4/035
> +++ b/tests/ext4/035
> @@ -25,7 +25,10 @@ _require_scratch
>  _exclude_scratch_mount_option dax
>  _require_command "$RESIZE2FS_PROG" resize2fs
>  
> -$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $SCRATCH_DEV 32768 >> $seqres.full 2>&1
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    ENCRYPT="-O encrypt"
> +fi

Similarly, lower case "encrypt".  Also, starting out with "encrypt=" would be a
good idea, to avoid relying on an uninitialized variable.

> +$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $ENCRYPT $SCRATCH_DEV 32768 >> $seqres.full 2>&1
>  if [ $? -ne 0 ]; then
>      _notrun "Can't make file system with a block size of 1024"
>  fi
> diff --git a/tests/ext4/306 b/tests/ext4/306
> index 2ff88537..a6b7eab5 100755
> --- a/tests/ext4/306
> +++ b/tests/ext4/306
> @@ -33,6 +33,9 @@ features="^extents"
>  if grep -q 64bit /etc/mke2fs.conf ; then
>      features="^extents,^64bit"
>  fi
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    features=${features},encrypt
> +fi

Similarly, features+=",encrypt"

Also adding the ^64bit option can look like: features+=",^64bit"

> diff --git a/tests/generic/260 b/tests/generic/260
> index b4d72e0f..e86afe9c 100755
> --- a/tests/generic/260
> +++ b/tests/generic/260
> @@ -120,7 +120,10 @@ case $FSTYP in
>  		bsize=4096
>  		start=$(_math "$base*$agsize*$bsize")
>  		len=$start
> -		export MKFS_OPTIONS="-F -b $bsize -g $agsize"
> +		if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +		    ENCRYPT="-O encrypt"
> +		fi
> +		export MKFS_OPTIONS="-F -b $bsize -g $agsize $ENCRYPT"
>  		;;

This one could be:

	export MKFS_OPTIONS="-F -b $bsize -g $agsize"
	if echo "$MOUNT_OPTIONS" | grep -q 'test_dummy_encryption'; then
		MKFS_OPTIONS+=" -O encrypt"
	fi

Re: [PATCH 6/6] ext4: create file systems with the enrypt feature if necessary

[Theodore Ts'o]

On Tue, Jun 28, 2022 at 10:16:34PM -0700, Eric Biggers wrote:
> 
> Thanks, I didn't notice these.  The general approach looks good.  A few nits:
> 
> - The hunk in tests/ext4/035 doesn't apply; it seems you have a local change?

Yeah, sorry, I had forgotten about that out of tree patch.  I'll pull
that into this patch series, and I should really try to pay down some
of that tech debt.

> - "enrypt" in the subject should be "encrypt"

Thanks, fixed.

						- Ted

Re: [PATCH 6/6] ext4: create file systems with the enrypt feature if necessary

[Zorro Lang]

On Tue, Jun 28, 2022 at 02:25:45PM -0400, Theodore Ts'o wrote:
> The linux kernel commit 5f41fdaea63d ("ext4: only allow
> test_dummy_encryption when supported") the kernel will reject mounts
> with the test_dummy_encryption option if the ext4 file system does not
> have the encrypt feature enabled.
> 
> There are a handful of tests (ext4/003, ext4/035, ext4/306, and
> generic/260) which will format the scratch file system using a
> hard-coded set of mkfs.ext4 parameters ignoring the MKFS_OPTION that
> is set by the file system test config.
> 
> For file system config that includes test_dummy_encryption in
> MOUNT_OPTIONS and "-O encrypt" in MKFS_OPTIONS, we need to test for
> test_dummy_encryption and force the hard-coded mkfs options to enable
> the encrypt feature.
> 
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> ---
>  tests/ext4/003    | 6 +++++-
>  tests/ext4/035    | 5 ++++-
>  tests/ext4/306    | 3 +++
>  tests/generic/260 | 5 ++++-
>  4 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/ext4/003 b/tests/ext4/003
> index 773bcb03..6a4518e3 100755
> --- a/tests/ext4/003
> +++ b/tests/ext4/003
> @@ -27,7 +27,11 @@ _require_scratch
>  _require_scratch_ext4_feature "bigalloc"
>  
>  BLOCK_SIZE=$(get_page_size)
> -$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O bigalloc -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
> +FEATURES=bigalloc
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    FEATURES=${FEATURES},encrypt
> +fi
> +$MKFS_EXT4_PROG -F -b $BLOCK_SIZE -O ${FEATURES} -C $(($BLOCK_SIZE * 16)) -g 256 $SCRATCH_DEV 512m \
>  	>> $seqres.full 2>&1
>  _scratch_mount
>  
> diff --git a/tests/ext4/035 b/tests/ext4/035
> index 1f26e766..a1ccce45 100755
> --- a/tests/ext4/035
> +++ b/tests/ext4/035
> @@ -25,7 +25,10 @@ _require_scratch
>  _exclude_scratch_mount_option dax
>  _require_command "$RESIZE2FS_PROG" resize2fs
>  
> -$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $SCRATCH_DEV 32768 >> $seqres.full 2>&1
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    ENCRYPT="-O encrypt"
> +fi
> +$MKFS_EXT4_PROG -F -b 1024 -E "resize=262144" $ENCRYPT $SCRATCH_DEV 32768 >> $seqres.full 2>&1
>  if [ $? -ne 0 ]; then
>      _notrun "Can't make file system with a block size of 1024"
>  fi

This _notrun part isn't in mainline fstests. That cause merging aborted
at here :)

Thanks,
Zorro

> diff --git a/tests/ext4/306 b/tests/ext4/306
> index 2ff88537..a6b7eab5 100755
> --- a/tests/ext4/306
> +++ b/tests/ext4/306
> @@ -33,6 +33,9 @@ features="^extents"
>  if grep -q 64bit /etc/mke2fs.conf ; then
>      features="^extents,^64bit"
>  fi
> +if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +    features=${features},encrypt
> +fi
>  
>  blksz=$(get_page_size)
>  
> diff --git a/tests/generic/260 b/tests/generic/260
> index b4d72e0f..e86afe9c 100755
> --- a/tests/generic/260
> +++ b/tests/generic/260
> @@ -120,7 +120,10 @@ case $FSTYP in
>  		bsize=4096
>  		start=$(_math "$base*$agsize*$bsize")
>  		len=$start
> -		export MKFS_OPTIONS="-F -b $bsize -g $agsize"
> +		if echo "${MOUNT_OPTIONS}" | grep -q 'test_dummy_encryption' ; then
> +		    ENCRYPT="-O encrypt"
> +		fi
> +		export MKFS_OPTIONS="-F -b $bsize -g $agsize $ENCRYPT"
>  		;;
>  	xfs)
>  		agsize=65538
> -- 
> 2.31.0
>