[Ocfs2-tools-devel] [PATCH] mount.ocfs2: return -EROFS if file system is readonly

[piaojun]

Hi Eric & Joseph,

I will try to define a new error code in patch v2.

thanks,
Jun

On 2017/12/28 14:41, Eric Ren wrote:
> Hi Jun,
> 
>>>>>> On 17/12/27 17:12, piaojun wrote:
>>>>>>> If metadata is corrupted such as 'invalid inode block', we will get
>>>>>>> -EROFS by calling 'mount()'. In this situation we need fix it by fsck,
>>>>>>> and mount again rather than doing meaningless retry.
>>>>>>>
>>>>>>> Signed-off-by: Jun Piao <piaojun at huawei.com>
>>>>>>> ---
>>>>>>>    mount.ocfs2/mount.ocfs2.c | 13 ++++++++++---
>>>>>>>    1 file changed, 10 insertions(+), 3 deletions(-)
>>>>>>>
>>>>>>> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
>>>>>>> index e3d8871..1ab07ac 100644
>>>>>>> --- a/mount.ocfs2/mount.ocfs2.c
>>>>>>> +++ b/mount.ocfs2/mount.ocfs2.c
>>>>>>> @@ -434,8 +434,15 @@ int main(int argc, char **argv)
>>>>>>>                o2cb_complete_group_join(&cluster, &desc, errno);
>>>>>>>            }
>>>>>>>            block_signals (SIG_UNBLOCK);
>>>>>>> -        com_err(progname, ret, "while mounting %s on %s. Check 'dmesg' "
>>>>>>> -            "for more information on this error.", mo.dev, mo.dir);
>>>>>>> +        if (ret == -EROFS)
>>>>>>> +            com_err(progname, 0, "file system is now readonly due to "
>>>>>>> +                "on-disk corruption while mounting %s on %s, please "
>>>>>>> +                "fix this by fsck.ocfs2 and then retry mounting",
>>>>>>> +                mo.dev, mo.dir);
>>>>>> Could we use a meaningful error code instead of 0?
>>>>>>
>>>>>> Thanks,
>>>>>> Joseph
>>>>> I looked over 'ocfs2_err.et', and did not find a suitable error code.
>>>>> Should we define a new error code like this?
>>>>>
>>>>> -- 
>>>>> ec OCFS2_ET_CORRUPT_INODE_BLOCK,
>>>>> "Inode block is corrupt"
>>>>> -- 
>>>>>
>>>> Yes, I think it should be acceptable.
>>> Does it mean we will always interpret EROFS code ( returned from kernel?) as
>>> "inode block corruption"? Is there some other internal error returning EROFS?
>>>
>>> Thanks,
>>> Eric
>>> .
>>>
>> Yes, when readonly device detected but readonly mount was not
>> specified, we will also get -EROFS from kernel as below:
> 
> Hmm, I just feel it's strange to return OCFS2_ET_CORRUPT_INODE_BLOCK to
> user while kernel actually kernel returns EROFS. Well, I could be wrong.
> 
>>
>> ocfs2_fill_super
>>    ...
>>    if (bdev_read_only(sb->s_bdev)) {
>>        ...
>>        ocfs2_set_ro_flag(osb, 1);
>>        ...
>>    }
>>
>> That is hard-ro error. Sadly we can hardly distinguish it from soft-ro,
> 
> Hmm, I think it's OK to see hard-ro and soft-ro as readonly. So, why not just
> return "readonly" error code to user? define it if it doesn't exist?
> 
>> and fsck seems useless. I just come up with a solution that we could
>> use -EACCES instead of -EROFS in this case. I'm glad to hear your
>> suggestion.
> Hmm, EACCES seems not fit here:
> 
> #define EACCES          13      /* Permission denied */
> 
> Thanks,
> Eric
> 
> .
>