From: Luis Henriques <luis.henriques@linux.dev>
To: "Darrick J. Wong" <djwong@kernel.org>
Cc: Theodore Ts'o <tytso@mit.edu>, fstests@vger.kernel.org
Subject: Re: [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'
Date: Mon, 01 Apr 2024 10:45:56 +0100 [thread overview]
Message-ID: <871q7p8lgr.fsf@brahms.olymp> (raw)
In-Reply-To: <20240329230859.GF6379@frogsfrogsfrogs> (Darrick J. Wong's message of "Fri, 29 Mar 2024 16:08:59 -0700")
"Darrick J. Wong" <djwong@kernel.org> 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@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?
>
> "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
prev parent reply other threads:[~2024-04-01 9:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-28 17:06 [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal' Luis Henriques (SUSE)
2024-03-29 22:50 ` Theodore Ts'o
2024-03-29 23:08 ` Darrick J. Wong
2024-04-01 9:45 ` Luis Henriques [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=871q7p8lgr.fsf@brahms.olymp \
--to=luis.henriques@linux.dev \
--cc=djwong@kernel.org \
--cc=fstests@vger.kernel.org \
--cc=tytso@mit.edu \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).