On Tue, Aug 08, 2023 at 12:34:49AM +0200, David Disseldorp wrote:
> Thanks for the feedback, Zorro...
>
> On Tue, 8 Aug 2023 01:48:35 +0800, Zorro Lang wrote:
>
> > On Mon, Aug 07, 2023 at 01:28:50PM +0200, David Disseldorp wrote:
> > > fsck.exfat doesn't support the '-f' flag, so add a special case to
> > > _repair_test_fs().
> >
> > I'm wondering why _repair_scratch_fs() doesn't have the '-f', but the
> > _repair_test_fs() has it. Looks like the '-f' option was for extN fs
> > originally, it's not a fsck common option, but in fsck.ext4.
> >
> > So I think the '-f' might not be a necessary option. As _repair_scratch_fs
> > works without it, can we just remove the '-f' from _repair_test_fs()?
>
> The fsck.ext4 usage states:
> -f Force checking even if filesystem is marked clean
>
> As _repair_test_fs() is only called on _check_test_fs() failure, I
> suppose '-f' removal should be fine. Still, to avoid any behavioural
> changes I could also just add an explicit ext case with '-f'.
>
> There's also a question of what btrfs should do here, as calling the
> "fsck.btrfs" no-op script following btrfs check failure likely
> doesn't make much sense.
Hmm, I think you're right. I just checked the commit log, the code (check
calls _repair_test_fs if TEST_DEV is corrupted) is added by this commit:
commit c7d81cdecbefd5768163a195e8d5257279216a34
Author: Theodore Ts'o <tytso@xxxxxxx>
Date: Fri Apr 21 10:51:31 2023 -0700
check: try to fix the test device if it gets corrupted
That commit cares about xfs and extN more, didn't think about other fs
likes btrfs. If btrfs would like to call btrfs-check, not fsck.btrfs,
we should do that.
And as the "-f" isn't a common option of fsck, so we'd better to not use
it in " *)" branch. If extN hope to use it, we'd better to have a "ext4)"
branch.
If btrfs would like to call btrfs-check in _repair_test_fs(), we need
to do the same thing in _repair_scratch_fs(). CC btrfs list to get more
review/suggestion about that.
Thanks,
Zorro
>
> Cheers, David
>
