diff mbox series

[v2] ext4/053: update the test_dummy_encryption tests

Message ID 20220530173044.156375-1-ebiggers@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v2] ext4/053: update the test_dummy_encryption tests | expand

Commit Message

Eric Biggers May 30, 2022, 5:30 p.m. UTC 
From: Eric Biggers <ebiggers@google.com>

Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
supported") tightened the requirements on when the test_dummy_encryption
mount option is accepted.  Update ext4/053 accordingly.

Move the test cases to later in the file to group them with the other
test cases that use do_mkfs to add custom mkfs options instead of using
the "default" filesystem that the test creates at the beginning.

Reviewed-by: Lukas Czerner <lczerner@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@google.com>
---

v2: mention the commit ID now that it is merged, and add a Reviewed-by

 tests/ext4/053 | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)


base-commit: d3cc66012a287b6db81aad408b6970a4a96a67da

Comments

Theodore Ts'o May 30, 2022, 7:07 p.m. UTC | #1 
On Mon, May 30, 2022 at 10:30:44AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
> supported") tightened the requirements on when the test_dummy_encryption
> mount option is accepted.  Update ext4/053 accordingly.
> 
> Move the test cases to later in the file to group them with the other
> test cases that use do_mkfs to add custom mkfs options instead of using
> the "default" filesystem that the test creates at the beginning.
> 
> Reviewed-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>

I thought we were going to put in a kernel version check in so that
this won't break on pre-5.19 kernels?  The thinking was that we
wouldn't be backporting commit 5f41fdaea63d to LTS or distro kernels,
so the version number check would be reliable in this instance.

Otherwise people who are testing enterprise kernels, LTS kernels,
etc., will see this test fail.

					- Ted
Eric Biggers May 30, 2022, 7:47 p.m. UTC | #2 
On Mon, May 30, 2022 at 03:07:46PM -0400, Theodore Ts'o wrote:
> On Mon, May 30, 2022 at 10:30:44AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
> > supported") tightened the requirements on when the test_dummy_encryption
> > mount option is accepted.  Update ext4/053 accordingly.
> > 
> > Move the test cases to later in the file to group them with the other
> > test cases that use do_mkfs to add custom mkfs options instead of using
> > the "default" filesystem that the test creates at the beginning.
> > 
> > Reviewed-by: Lukas Czerner <lczerner@redhat.com>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> I thought we were going to put in a kernel version check in so that
> this won't break on pre-5.19 kernels?  The thinking was that we
> wouldn't be backporting commit 5f41fdaea63d to LTS or distro kernels,
> so the version number check would be reliable in this instance.
> 
> Otherwise people who are testing enterprise kernels, LTS kernels,
> etc., will see this test fail.
> 
> 					- Ted

This was already discussed in the original thread.  IIUC, both Zorro and Lukas
prefer *not* having the kernel version check.

The whole test script is still gated on 5.12, so I'll only have to backport the
commit to 5.15 (unless the 5.12 version check gets removed).

- Eric
Zorro Lang June 3, 2022, 5:31 a.m. UTC | #3 
On Mon, May 30, 2022 at 10:30:44AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
> supported") tightened the requirements on when the test_dummy_encryption
> mount option is accepted.  Update ext4/053 accordingly.
> 
> Move the test cases to later in the file to group them with the other
> test cases that use do_mkfs to add custom mkfs options instead of using
> the "default" filesystem that the test creates at the beginning.
> 
> Reviewed-by: Lukas Czerner <lczerner@redhat.com>
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> 
> v2: mention the commit ID now that it is merged, and add a Reviewed-by

Hi Eric,

If I don't remember wrong, it was a patchset with 2 patches. Now you only
send this patch out, do you hope to merge this one only, or merge both?

Thanks,
Zorro

> 
>  tests/ext4/053 | 35 +++++++++++++++++++++--------------
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/ext4/053 b/tests/ext4/053
> index 187a2515..23e553c5 100755
> --- a/tests/ext4/053
> +++ b/tests/ext4/053
> @@ -511,20 +511,6 @@ for fstype in ext2 ext3 ext4; do
>  	mnt noinit_itable
>  	mnt max_dir_size_kb=4096
>  
> -	if _has_kernel_config CONFIG_FS_ENCRYPTION; then
> -		mnt test_dummy_encryption
> -		mnt test_dummy_encryption=v1
> -		mnt test_dummy_encryption=v2
> -		not_mnt test_dummy_encryption=v3
> -		not_mnt test_dummy_encryption=
> -	else
> -		mnt test_dummy_encryption ^test_dummy_encryption
> -		mnt test_dummy_encryption=v1 ^test_dummy_encryption=v1
> -		mnt test_dummy_encryption=v2 ^test_dummy_encryption=v2
> -		mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3
> -		not_mnt test_dummy_encryption=
> -	fi
> -
>  	if _has_kernel_config CONFIG_FS_ENCRYPTION_INLINE_CRYPT; then
>  		mnt inlinecrypt
>  	else
> @@ -686,6 +672,27 @@ for fstype in ext2 ext3 ext4; do
>  	mnt_then_not_remount defaults jqfmt=vfsv1
>  	remount defaults grpjquota=,usrjquota= ignored
>  
> +	echo "== Testing the test_dummy_encryption option" >> $seqres.full
> +	# Since kernel commit 5f41fdaea63d ("ext4: only allow
> +	# test_dummy_encryption when supported"), the test_dummy_encryption
> +	# option is only allowed when the filesystem has the encrypt feature and
> +	# the kernel has CONFIG_FS_ENCRYPTION.  The encrypt feature requirement
> +	# implies that this option is never allowed on ext2 or ext3 mounts.
> +	if [[ $fstype == ext4 ]] && _has_kernel_config CONFIG_FS_ENCRYPTION; then
> +		do_mkfs -O encrypt $SCRATCH_DEV ${SIZE}k
> +		mnt test_dummy_encryption
> +		mnt test_dummy_encryption=v1
> +		mnt test_dummy_encryption=v2
> +		not_mnt test_dummy_encryption=bad
> +		not_mnt test_dummy_encryption=
> +		do_mkfs -O ^encrypt $SCRATCH_DEV ${SIZE}k
> +	fi
> +	not_mnt test_dummy_encryption
> +	not_mnt test_dummy_encryption=v1
> +	not_mnt test_dummy_encryption=v2
> +	not_mnt test_dummy_encryption=bad
> +	not_mnt test_dummy_encryption=
> +
>  done #for fstype in ext2 ext3 ext4; do
>  
>  $UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1
> 
> base-commit: d3cc66012a287b6db81aad408b6970a4a96a67da
> -- 
> 2.36.1
>
Eric Biggers June 3, 2022, 5:48 a.m. UTC | #4 
On Fri, Jun 03, 2022 at 01:31:43PM +0800, Zorro Lang wrote:
> On Mon, May 30, 2022 at 10:30:44AM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
> > supported") tightened the requirements on when the test_dummy_encryption
> > mount option is accepted.  Update ext4/053 accordingly.
> > 
> > Move the test cases to later in the file to group them with the other
> > test cases that use do_mkfs to add custom mkfs options instead of using
> > the "default" filesystem that the test creates at the beginning.
> > 
> > Reviewed-by: Lukas Czerner <lczerner@redhat.com>
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> > 
> > v2: mention the commit ID now that it is merged, and add a Reviewed-by
> 
> Hi Eric,
> 
> If I don't remember wrong, it was a patchset with 2 patches. Now you only
> send this patch out, do you hope to merge this one only, or merge both?
> 
> Thanks,
> Zorro
> 

Just this one for now.  The second patch would add a test for the bug fix
https://lore.kernel.org/linux-ext4/20220526040412.173025-1-ebiggers@kernel.org,
but that wasn't applied for 5.19 due to a cross-tree dependency.  I'll be
resending that test patch later.  One step at a time...

- Eric
Zorro Lang June 3, 2022, 7:40 a.m. UTC | #5 
On Thu, Jun 02, 2022 at 10:48:28PM -0700, Eric Biggers wrote:
> On Fri, Jun 03, 2022 at 01:31:43PM +0800, Zorro Lang wrote:
> > On Mon, May 30, 2022 at 10:30:44AM -0700, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > Kernel commit 5f41fdaea63d ("ext4: only allow test_dummy_encryption when
> > > supported") tightened the requirements on when the test_dummy_encryption
> > > mount option is accepted.  Update ext4/053 accordingly.
> > > 
> > > Move the test cases to later in the file to group them with the other
> > > test cases that use do_mkfs to add custom mkfs options instead of using
> > > the "default" filesystem that the test creates at the beginning.
> > > 
> > > Reviewed-by: Lukas Czerner <lczerner@redhat.com>
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > ---
> > > 
> > > v2: mention the commit ID now that it is merged, and add a Reviewed-by
> > 
> > Hi Eric,
> > 
> > If I don't remember wrong, it was a patchset with 2 patches. Now you only
> > send this patch out, do you hope to merge this one only, or merge both?
> > 
> > Thanks,
> > Zorro
> > 
> 
> Just this one for now.  The second patch would add a test for the bug fix
> https://lore.kernel.org/linux-ext4/20220526040412.173025-1-ebiggers@kernel.org,
> but that wasn't applied for 5.19 due to a cross-tree dependency.  I'll be
> resending that test patch later.  One step at a time...

Sure, thanks for clarify that :)

Thanks,
Zorro

> 
> - Eric
>
diff mbox series

Patch

diff --git a/tests/ext4/053 b/tests/ext4/053
index 187a2515..23e553c5 100755
--- a/tests/ext4/053
+++ b/tests/ext4/053
@@ -511,20 +511,6 @@  for fstype in ext2 ext3 ext4; do
 	mnt noinit_itable
 	mnt max_dir_size_kb=4096
 
-	if _has_kernel_config CONFIG_FS_ENCRYPTION; then
-		mnt test_dummy_encryption
-		mnt test_dummy_encryption=v1
-		mnt test_dummy_encryption=v2
-		not_mnt test_dummy_encryption=v3
-		not_mnt test_dummy_encryption=
-	else
-		mnt test_dummy_encryption ^test_dummy_encryption
-		mnt test_dummy_encryption=v1 ^test_dummy_encryption=v1
-		mnt test_dummy_encryption=v2 ^test_dummy_encryption=v2
-		mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3
-		not_mnt test_dummy_encryption=
-	fi
-
 	if _has_kernel_config CONFIG_FS_ENCRYPTION_INLINE_CRYPT; then
 		mnt inlinecrypt
 	else
@@ -686,6 +672,27 @@  for fstype in ext2 ext3 ext4; do
 	mnt_then_not_remount defaults jqfmt=vfsv1
 	remount defaults grpjquota=,usrjquota= ignored
 
+	echo "== Testing the test_dummy_encryption option" >> $seqres.full
+	# Since kernel commit 5f41fdaea63d ("ext4: only allow
+	# test_dummy_encryption when supported"), the test_dummy_encryption
+	# option is only allowed when the filesystem has the encrypt feature and
+	# the kernel has CONFIG_FS_ENCRYPTION.  The encrypt feature requirement
+	# implies that this option is never allowed on ext2 or ext3 mounts.
+	if [[ $fstype == ext4 ]] && _has_kernel_config CONFIG_FS_ENCRYPTION; then
+		do_mkfs -O encrypt $SCRATCH_DEV ${SIZE}k
+		mnt test_dummy_encryption
+		mnt test_dummy_encryption=v1
+		mnt test_dummy_encryption=v2
+		not_mnt test_dummy_encryption=bad
+		not_mnt test_dummy_encryption=
+		do_mkfs -O ^encrypt $SCRATCH_DEV ${SIZE}k
+	fi
+	not_mnt test_dummy_encryption
+	not_mnt test_dummy_encryption=v1
+	not_mnt test_dummy_encryption=v2
+	not_mnt test_dummy_encryption=bad
+	not_mnt test_dummy_encryption=
+
 done #for fstype in ext2 ext3 ext4; do
 
 $UMOUNT_PROG $SCRATCH_MNT > /dev/null 2>&1