Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 2 May 2022 05:46:11 -0700
From:   tytso <tytso@mit.edu>
To:     Eric Biggers <ebiggers@kernel.org>
Cc:     fstests@vger.kernel.org, linux-fscrypt@vger.kernel.org,
        linux-ext4@vger.kernel.org, Lukas Czerner <lczerner@redhat.com>
Subject: Re: [xfstests PATCH 1/2] ext4/053: update the test_dummy_encryption
 tests
Message-ID: <Ym/Sk7D17iCeQIJa@mit.edu>
References: <20220501051928.540278-1-ebiggers@kernel.org>
 <20220501051928.540278-2-ebiggers@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220501051928.540278-2-ebiggers@kernel.org>
Precedence: bulk

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?

> 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?

						- Ted