Re: [PATCH v2] common/rc: drop 'fsck -f' parameter from _repair_test_fs

[Anand Jain <anand.jain@oracle.com>]

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
>>
>