diff mbox series

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

Message ID 20220628182545.1834145-7-tytso@mit.edu (mailing list archive)
State New, archived
Headers show
Series Miscellaneous ext4 test fix ups | expand

Commit Message

Theodore Ts'o June 28, 2022, 6:25 p.m. UTC 
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(-)

Comments

Eric Biggers June 29, 2022, 5:16 a.m. UTC | #1 
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
Zorro Lang June 29, 2022, 8:58 a.m. UTC | #2 
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
>
Theodore Ts'o June 29, 2022, 12:28 p.m. UTC | #3 
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
diff mbox series

Patch

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