Re: [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'

["Theodore Ts'o" <tytso@mit.edu>]

On Thu, Mar 28, 2024 at 05:06:20PM +0000, Luis Henriques (SUSE) wrote:
> Creating an ext4 filesystem using '-O journal' will fail with:
> 
>     Invalid filesystem option set: journal
> 
> I didn't do any archaeological investigation to check if this option ever
> existed, but the two tests using it will fail to create the scratch
> filesystems.
> 
> Signed-off-by: Luis Henriques (SUSE) <luis.henriques@linux.dev>

The feature name has never been journal, but rather has_journal.  The
reason why no once noticed is because the file system was created by
the _require_attrs before the attempted _scratch_mkfs_ext4.  So when
_scratch_mkfs_ext4 failed, it was a no-op that didn't actually do
anything, and there was still a file system the default configuration
for the test scenario.

What puzzles me is why there was an attempt to enable the journal
feature in the first place.  As near as I can tell, the tests don't
change what gets tested whether or not the journal is enabled.
Darrick; you had added these tests were you were working on ext4's
metadata checksum feature; do you remember your thinking at the time?

In any case, either better fix is to replace:

_scratch_mkfs_ext4 -O journal > /dev/null 2>&1

with:

_scratch_mkfs_ext4 -O has_journal >> $seqres.full 2>&1

Or:

_scratch_mkfs -O has_journal >> $seqres.full 2>&1

My preference would be latter, since I'm regularly testing with and
without the journal, and I'd much rather run the test with whatever
configuration I'm currently testing (e.g., ext4/4k, ext4/1k,
ext4/nojournal, ext4/ext3conv, ext4/bigalloc, etc.)

						- Ted