* [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'
@ 2024-03-28 17:06 Luis Henriques (SUSE)
2024-03-29 22:50 ` Theodore Ts'o
0 siblings, 1 reply; 4+ messages in thread
From: Luis Henriques (SUSE) @ 2024-03-28 17:06 UTC (permalink / raw)
To: fstests; +Cc: Luis Henriques (SUSE)
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>
---
tests/ext4/012 | 2 +-
tests/ext4/019 | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tests/ext4/012 b/tests/ext4/012
index c49e8ef4483b..4cef19bf6be5 100755
--- a/tests/ext4/012
+++ b/tests/ext4/012
@@ -32,7 +32,7 @@ TESTDIR="${SCRATCH_MNT}/scratchdir"
TESTFILE="${TESTDIR}/testfile"
echo "+ create scratch fs"
-_scratch_mkfs_ext4 -O journal > /dev/null 2>&1
+_scratch_mkfs_ext4 > /dev/null 2>&1
echo "+ mount fs image"
_scratch_mount
diff --git a/tests/ext4/019 b/tests/ext4/019
index ab5f50c66b6f..9db7f23bb608 100755
--- a/tests/ext4/019
+++ b/tests/ext4/019
@@ -32,7 +32,7 @@ TESTDIR="${SCRATCH_MNT}/scratchdir"
TESTFILE="${TESTDIR}/testfile"
echo "+ create scratch fs"
-_scratch_mkfs_ext4 -O journal > /dev/null 2>&1
+_scratch_mkfs_ext4 > /dev/null 2>&1
echo "+ mount fs image"
_scratch_mount
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'
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
0 siblings, 1 reply; 4+ messages in thread
From: Theodore Ts'o @ 2024-03-29 22:50 UTC (permalink / raw)
To: Luis Henriques (SUSE), djwong; +Cc: fstests
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'
2024-03-29 22:50 ` Theodore Ts'o
@ 2024-03-29 23:08 ` Darrick J. Wong
2024-04-01 9:45 ` Luis Henriques
0 siblings, 1 reply; 4+ messages in thread
From: Darrick J. Wong @ 2024-03-29 23:08 UTC (permalink / raw)
To: Theodore Ts'o; +Cc: Luis Henriques (SUSE), fstests
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. :/
--D
> - Ted
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ext4/01{2,9}: remove invalid filesystem option 'journal'
2024-03-29 23:08 ` Darrick J. Wong
@ 2024-04-01 9:45 ` Luis Henriques
0 siblings, 0 replies; 4+ messages in thread
From: Luis Henriques @ 2024-04-01 9:45 UTC (permalink / raw)
To: Darrick J. Wong; +Cc: Theodore Ts'o, fstests
"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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-01 9:46 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
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 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).