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
