| Message ID | 20230816103330.961-1-ddiss@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] common/rc: drop 'fsck -f' parameter from _repair_test_fs | expand |
On Wed, Aug 16, 2023 at 12:33:30PM +0200, David Disseldorp wrote: > The '-f' parameter is fsck.ext# specific, where it's documented to: > Force checking even if filesystem is marked clean > > _repair_test_fs() is only called on _check_test_fs() failure, so > dropping the parameter should be possible without changing ext# > behaviour. > Doing so fixes _repair_test_fs() on exfat, where fsck.exfat doesn't > support '-f'. > > Signed-off-by: David Disseldorp <ddiss@suse.de> > --- > v2: drop -f from default case instead of splitting out exfat case This version is good to me. Reviewed-by: Zorro Lang <zlang@redhat.com> I remembered you hope to add a btrfs branch to _repair_scratch_fs and _repair_test_fs [1]. Is that still in your plan? Anand, could you provide more suggestions about that? [1] https://lore.kernel.org/fstests/20230808091454.4skdyjnaxjqa7zyi@zlang-mailbox/ > > common/rc | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/common/rc b/common/rc > index 5c4429ed..66d270ac 100644 > --- a/common/rc > +++ b/common/rc > @@ -1231,7 +1231,7 @@ _repair_test_fs() > ;; > *) > # Let's hope fsck -y suffices... > - fsck -t $FSTYP -fy $TEST_DEV >$tmp.repair 2>&1 > + fsck -t $FSTYP -y $TEST_DEV >$tmp.repair 2>&1 > res=$? > if test "$res" -lt 4 ; then > res=0 > -- > 2.35.3 >
On Wed, 16 Aug 2023 22:55:43 +0800, Zorro Lang wrote: > On Wed, Aug 16, 2023 at 12:33:30PM +0200, David Disseldorp wrote: > > The '-f' parameter is fsck.ext# specific, where it's documented to: > > Force checking even if filesystem is marked clean > > > > _repair_test_fs() is only called on _check_test_fs() failure, so > > dropping the parameter should be possible without changing ext# > > behaviour. > > Doing so fixes _repair_test_fs() on exfat, where fsck.exfat doesn't > > support '-f'. > > > > Signed-off-by: David Disseldorp <ddiss@suse.de> > > --- > > v2: drop -f from default case instead of splitting out exfat case > > This version is good to me. > > Reviewed-by: Zorro Lang <zlang@redhat.com> Thanks. > I remembered you hope to add a btrfs branch to _repair_scratch_fs and > _repair_test_fs [1]. Is that still in your plan? Anand, could you provide > more suggestions about that? Yes, that's still my plan, but I was hoping to get some more input on whether "btrfs check --repair ..." is sufficient before proposing the change. Cheers, David
On 16/8/23 22:55, Zorro Lang wrote: > On Wed, Aug 16, 2023 at 12:33:30PM +0200, David Disseldorp wrote: >> The '-f' parameter is fsck.ext# specific, where it's documented to: >> Force checking even if filesystem is marked clean >> >> _repair_test_fs() is only called on _check_test_fs() failure, so >> dropping the parameter should be possible without changing ext# >> behaviour. >> Doing so fixes _repair_test_fs() on exfat, where fsck.exfat doesn't >> support '-f'. >> >> Signed-off-by: David Disseldorp <ddiss@suse.de> >> --- >> v2: drop -f from default case instead of splitting out exfat case > > This version is good to me. > > Reviewed-by: Zorro Lang <zlang@redhat.com> > > I remembered you hope to add a btrfs branch to _repair_scratch_fs and > _repair_test_fs [1]. Is that still in your plan? Anand, could you provide > more suggestions about that? > > [1] > https://lore.kernel.org/fstests/20230808091454.4skdyjnaxjqa7zyi@zlang-mailbox/ Thanks for bringing to my notice. The reason fstyp=btrfs has not detected this missing part so far is that 'fsck -t btrfs' returns 0, along with a message to use 'btrfs check', which means repair is never invoked for the 'fstyp=btrfs'. The appropriate repair command to use here is 'btrfs check --repair'. However, it would be better to address this in a separate patch as I guess some tests may fail. I will send a patch for it. Thanks, Anand > >> >> common/rc | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/common/rc b/common/rc >> index 5c4429ed..66d270ac 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -1231,7 +1231,7 @@ _repair_test_fs() >> ;; >> *) >> # Let's hope fsck -y suffices... >> - fsck -t $FSTYP -fy $TEST_DEV >$tmp.repair 2>&1 >> + fsck -t $FSTYP -y $TEST_DEV >$tmp.repair 2>&1 >> res=$? >> if test "$res" -lt 4 ; then >> res=0 >> -- >> 2.35.3 >> >
diff --git a/common/rc b/common/rc index 5c4429ed..66d270ac 100644 --- a/common/rc +++ b/common/rc @@ -1231,7 +1231,7 @@ _repair_test_fs() ;; *) # Let's hope fsck -y suffices... - fsck -t $FSTYP -fy $TEST_DEV >$tmp.repair 2>&1 + fsck -t $FSTYP -y $TEST_DEV >$tmp.repair 2>&1 res=$? if test "$res" -lt 4 ; then res=0
The '-f' parameter is fsck.ext# specific, where it's documented to: Force checking even if filesystem is marked clean _repair_test_fs() is only called on _check_test_fs() failure, so dropping the parameter should be possible without changing ext# behaviour. Doing so fixes _repair_test_fs() on exfat, where fsck.exfat doesn't support '-f'. Signed-off-by: David Disseldorp <ddiss@suse.de> --- v2: drop -f from default case instead of splitting out exfat case common/rc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)