[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220510094308.mhzvcgq5wrat5qao@fedora>
Date: Tue, 10 May 2022 11:43:08 +0200
From: Lukas Czerner <lczerner@...hat.com>
To: Eric Biggers <ebiggers@...nel.org>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org
Subject: Re: [xfstests PATCH] ext4/053: fix the rejected mount option testing
On Mon, May 09, 2022 at 04:42:03PM -0700, Eric Biggers wrote:
> On Sat, Apr 30, 2022 at 12:21:30PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@...gle.com>
> >
> > 'not_mnt OPTIONS' seems to have been intended to test that the
> > filesystem cannot be mounted at all with the given OPTIONS, meaning that
> > the mount fails as opposed to the options being ignored. However, this
> > doesn't actually work, as shown by the fact that the test case 'not_mnt
> > test_dummy_encryption=v3' is passing in the !CONFIG_FS_ENCRYPTION case.
> > Actually ext4 ignores this mount option when !CONFIG_FS_ENCRYPTION.
> > (The ext4 behavior might be changed, but that is besides the point.)
> >
> > The problem is that the do_mnt() helper function is being misused in a
> > context where a mount failure is expected, and it does some additional
> > remount tests that don't make sense in that context. So if the mount
> > unexpectedly succeeds, then one of these later tests can still "fail",
> > causing the unexpected success to be shadowed by a later failure, which
> > causes the overall test case to pass since it expects a failure.
> >
> > Fix this by reworking not_mnt() and not_remount_noumount() to use
> > simple_mount() in cases where they are expecting a failure. Also fix
> > up some of the naming and calling conventions to be less confusing.
> > Finally, make sure to test that remounting fails too, not just mounting.
> >
> > Signed-off-by: Eric Biggers <ebiggers@...gle.com>
> > ---
> > tests/ext4/053 | 148 ++++++++++++++++++++++++++-----------------------
> > 1 file changed, 78 insertions(+), 70 deletions(-)
>
> Lukas, any thoughts on this patch? You're the author of this test.
>
> - Eric
Haven't tested it myself but the change looks fine, thanks.
You can add
Reviewed-by: Lukas Czerner <lczerner@...hat.com>
Powered by blists - more mailing lists