Re: [xfstests PATCH 1/2] ext4/053: update the test_dummy_encryption tests

[Eric Biggers <ebiggers@kernel.org>]

On Mon, May 02, 2022 at 05:46:11AM -0700, tytso wrote:
> On Sat, Apr 30, 2022 at 10:19:27PM -0700, Eric Biggers wrote:
> > 
> > The kernel patch "ext4: only allow test_dummy_encryption when supported"
> > will tighten the requirements on when the test_dummy_encryption mount
> > option will be accepted.  Update ext4/053 accordingly.
> 
> One of the problems with ext4/053 is that it is very implementation
> dependent.  It was useful when we were making the change to the new
> mount API, but the problem is any future changes to the mount option
> handling is going to break the patch.
> 
> So for example, the kernel patch which Eric has proposed, "ext4: only
> allow test_dummy_encryption when supported", breaks ext4/053, which I
> noted in the review the patch.  But then this patch will break kernels
> as they currently stand without this patch, and for kernels that
> haven't moved to the new mount API, fixing it is going to be a mess.
> 
> Perhaps ext4/053 is still useful in that it will flag changes that
> might unintentionally make user-visible changes mount options handling
> in ext4, but for cases like this one, where we are changing a mount
> option which is really intended for kernel developers, perhaps the
> right approach here is to just remove the parts of ext4/053 that are
> testing the behaviour of test_dummy_encryption in such a
> super-nit-picky way?
> 
> What do folks think?

I'd like to keep the test_dummy_encryption test cases.  Trying to add a couple
new test cases (patch 2) actually found a regression.

We could gate them on the kernel version, similar to the whole ext4/053 which
already only runs on kernel version 5.12.  (Kernel versions checks suck, but
maybe it's the right choice for this very-nit-picky test.)  Alternatively, I
could just backport "ext4: only allow test_dummy_encryption when supported" to
5.15, which would be the only relevant LTS kernel version.

> 
> > 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.
> 
> Note: this patch doesn't apply because ext4/053 currently has this
> line:
> 
> 		not_mnt test_dummy_encryption=v3
> 
> and the patch is trying to remove this line in the first patch chunk:
> 
> 		mnt test_dummy_encryption=v3 ^test_dummy_encryption=v3
> 
> I checked the upstream version of ext4/053 just in case I had some
> local modification of ext4/053 in my tree via "git diff -r
> origin/master tests/ext4/053" but that returned no deltas.
> 
> Eric, do you have a local modification to this test in your tree
> already, perhaps?

Sorry about that; as I mentioned in the cover letter, this is based on my other
patch "ext4/053: fix the rejected mount option testing".  As-is, 'not_mnt'
doesn't really work at all, so I wanted to fix that first.

- Eric