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

[Date Prev] [Date Next] [Thread Prev] [Thread Next] [Date Index] [Thread Index]



"Darrick J. Wong" <djwong@xxxxxxxxxx> writes:

> On Fri, Mar 29, 2024 at 06:50:20PM -0400, Theodore Ts'o wrote:
>> 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@xxxxxxxxx>
>> 
>> 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?
>
> "Qwklgjwlqaetwqjetlweqqlgqgqtrrt", most likely.
>
>> 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.)
>
> I'm ok with either, though _scratch_mkfs -O has_journal is more
> consistent with the way XFS fuzz tests do things.
>
> Sorry about that. :/

Thank you both for the feedback.  I'll send out v2 shortly.

Cheers,
-- 
Luis




[Index of Archives]     [Linux Filesystems Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux