Re: Possible issues with fsck of f2fs root

[Chao Yu <yuchao0@huawei.com>]

On 2019/4/22 18:05, Hagbard Celine wrote:
> 2019-04-22 11:26 GMT+02:00, Chao Yu <yuchao0@huawei.com>:
>> On 2019/4/22 17:05, Hagbard Celine wrote:
>>> 2019-04-22 9:37 GMT+02:00, Chao Yu <yuchao0@huawei.com>:
>>>> On 2019/4/22 15:11, Hagbard Celine wrote:
>>>>> With this patch the one problem with opening the device in RO mode is
>>>>> fixed.
>>>>
>>>> Oops, with default preen mode fsck should not open ro mounted image,
>>>> that's
>>>> the
>>>> rule we keep line with ext4...
>>>>
>>>> How about changing to use -f in your scenario ( on RO mounted root image
>>>> )?
>>>
>>> This was with -f. Without -f it still refuses to open the device.
>>
>> What I mean is we'd better to keep line with ext4, just refusing to open ro
>> mounted device without -f, since triggering fsck and repair on a mounted
>> device
>> is dangerous, it can easily make inconsistency in between in-memory data
>> and
>> on-disk data of filesystem. Refusing fsck without -f is to make user being
>> aware
>> of such danger.
> 
> I am sorry, I've apparently added the -f after my first report. After
> re-testing it seems that fsck.f2fs is opening the RO partition even
> without this patch if I use -f. So the part about fsck.f2fs not being
> able to open RO mounted partition during boot was a user error.

I've sent a patch for your second issue, could you please have a try with it?

[PATCH] fsck.f2fs: fix to repair ro mounted device w/ -f

But one concern is that, with this patch, not like the fsck.ext4, fsck.f2fs
won't show any interaction with below reminding word to remind user to decide
repair or not, it may increase the risk of damaging the device.

Do you want to restore lost files into ./lost_found/?
Do you want to fix this partition? [Y/N]

Jaegeuk, Hagbard,

Any suggestion on this, in current scenario, how about implement:
1. fsck.f2fs -f ro_mounted_device: check; show interaction words if there is
corruption;
2. fsck.f2fs -f -a ro_moutned_device: check and repair automatically;

Thanks,

> 
>>
>> Thanks,
>>
>>>
>>>
>>>> Thanks,
>>>>
>>>>> But as far as I can understand it will still only check the fs, not fix
>>>>> it.
>>>>>
>>>>>
>>>>> 2019-04-21 12:27 GMT+02:00, Jaegeuk Kim <jaegeuk@kernel.org>:
>>>>>
>>>>>>
>>>>>> New version of the patch is:
>>>>>>
>>>>>> From 3221692b060649378f1f69b898ed85a814af3dbf Mon Sep 17 00:00:00 2001
>>>>>> From: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>>> Date: Tue, 16 Apr 2019 11:46:31 -0700
>>>>>> Subject: [PATCH] fsck.f2fs: open ro disk if we want to check fs only
>>>>>>
>>>>>> This patch fixes the "open failure" issue on ro disk, reported by
>>>>>> Hagbard.
>>>>>>
>>>>>> "
>>>>>>  If I boot with kernel option "ro rootfstype=f2fs
>>>>>>  I get the following halfway trough boot:
>>>>>>
>>>>>>   * Checking local filesystems  ...
>>>>>>  Info: Use default preen mode
>>>>>>  Info: Mounted device!
>>>>>>  Info: Check FS only due to RO
>>>>>>          Error: Failed to open the device!
>>>>>>   * Filesystems couldn't be fixed
>>>>>> "
>>>>>>
>>>>>> Reported-by: Hagbard Celine <hagbardcelin@gmail.com>
>>>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
>>>>>> ---
>>>>>>  lib/libf2fs.c | 25 +++++++++++++++++++++----
>>>>>>  1 file changed, 21 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/libf2fs.c b/lib/libf2fs.c
>>>>>> index d30047f..853e713 100644
>>>>>> --- a/lib/libf2fs.c
>>>>>> +++ b/lib/libf2fs.c
>>>>>> @@ -789,6 +789,15 @@ void get_kernel_uname_version(__u8 *version)
>>>>>>  #endif /* APPLE_DARWIN */
>>>>>>
>>>>>>  #ifndef ANDROID_WINDOWS_HOST
>>>>>> +static int open_check_fs(char *path, int flag)
>>>>>> +{
>>>>>> +	if (c.func != FSCK || c.fix_on || c.auto_fix)
>>>>>> +		return -1;
>>>>>> +
>>>>>> +	/* allow to open ro */
>>>>>> +	return open(path, O_RDONLY | flag);
>>>>>> +}
>>>>>> +
>>>>>>  int get_device_info(int i)
>>>>>>  {
>>>>>>  	int32_t fd = 0;
>>>>>> @@ -810,8 +819,11 @@ int get_device_info(int i)
>>>>>>  	if (c.sparse_mode) {
>>>>>>  		fd = open(dev->path, O_RDWR | O_CREAT | O_BINARY, 0644);
>>>>>>  		if (fd < 0) {
>>>>>> -			MSG(0, "\tError: Failed to open a sparse file!\n");
>>>>>> -			return -1;
>>>>>> +			fd = open_check_fs(dev->path, O_BINARY);
>>>>>> +			if (fd < 0) {
>>>>>> +				MSG(0, "\tError: Failed to open a sparse file!\n");
>>>>>> +				return -1;
>>>>>> +			}
>>>>>>  		}
>>>>>>  	}
>>>>>>
>>>>>> @@ -825,10 +837,15 @@ int get_device_info(int i)
>>>>>>  			return -1;
>>>>>>  		}
>>>>>>
>>>>>> -		if (S_ISBLK(stat_buf->st_mode) && !c.force)
>>>>>> +		if (S_ISBLK(stat_buf->st_mode) && !c.force) {
>>>>>>  			fd = open(dev->path, O_RDWR | O_EXCL);
>>>>>> -		else
>>>>>> +			if (fd < 0)
>>>>>> +				fd = open_check_fs(dev->path, O_EXCL);
>>>>>> +		} else {
>>>>>>  			fd = open(dev->path, O_RDWR);
>>>>>> +			if (fd < 0)
>>>>>> +				fd = open_check_fs(dev->path, 0);
>>>>>> +		}
>>>>>>  	}
>>>>>>  	if (fd < 0) {
>>>>>>  		MSG(0, "\tError: Failed to open the device!\n");
>>>>>> --
>>>>>> 2.19.0.605.g01d371f741-goog
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> Linux-f2fs-devel mailing list
>>>>> Linux-f2fs-devel@lists.sourceforge.net
>>>>> https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
>>>>> .
>>>>>
>>>>
>>> .
>>>
>>
> .
>