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

[Joseph Qi]

On 18/1/4 08:42, piaojun wrote:
> Hi Joseph,
> 
> On 2018/1/3 20:06, Joseph Qi wrote:
>>
>>
>> On 18/1/3 11:58, piaojun wrote:
>>> Hi Joseph,
>>>
>>> Could you help reviewing this new patch again?
>>>
>>> thanks,
>>> Jun
>>>
>>> On 2017/12/29 14:13, 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. In addition, an
>>>> indentation problem in 'ocfs2_err.et' is fixed.
>>>>
>>>> Signed-off-by: Jun Piao <piaojun at huawei.com>
>>>> ---
>>>>  libocfs2/ocfs2_err.et     |  4 ++--
>>>>  mount.ocfs2/mount.ocfs2.c | 13 ++++++++++---
>>>>  2 files changed, 12 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/libocfs2/ocfs2_err.et b/libocfs2/ocfs2_err.et
>>>> index 34cefed..3dc0ce2 100644
>>>> --- a/libocfs2/ocfs2_err.et
>>>> +++ b/libocfs2/ocfs2_err.et
>>>> @@ -168,8 +168,8 @@ ec	OCFS2_ET_NO_IONICE,
>>>>  ec	OCFS2_ET_NO_BACKUP_SUPER,
>>>>  	"Backup superblock not found"
>>>>
>>>> -ec      OCFS2_ET_TOO_MANY_SLOTS,
>>>> -        "Too many slots for slot map"
>>>> +ec	OCFS2_ET_TOO_MANY_SLOTS,
>>>> +	"Too many slots for slot map"
>>>>
>>>>  ec	OCFS2_ET_CANNOT_INLINE_DATA,
>>>>  	"Unable to write the data inline"
>>>> diff --git a/mount.ocfs2/mount.ocfs2.c b/mount.ocfs2/mount.ocfs2.c
>>>> index e3d8871..65937e7 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, OCFS2_ET_RO_FILESYS, "while mounting %s "
>>>> +				"on %s, please try fixing this by fsck.ocfs2 and then "
>>>> +				"retry mounting", mo.dev, mo.dir);
>>>> +		else
>>>> +			com_err(progname, OCFS2_ET_INTERNAL_FAILURE,
>>>> +				"while mounting %s on %s. Check 'dmesg' for more "
>>>> +				"information on this error %d.", mo.dev, mo.dir,
>>>> +				(int)ret);
>>>>  		goto bail;
>>>>  	}
>>>>  	if (group_join) {
>>>> @@ -478,5 +485,5 @@ bail:
>>>>  	if (opts_string)
>>>>  		free(opts_string);
>>>>
>>>> -	return ret ? 1 : 0;
>>>> +	return ret;
>> Umm...I think we couldn't return ret directly as the exit code of a
>> program can only be 0 ~ 255. For example, -1 will be converted to 255.
>> And the baddest case is -256 will be converted to 0.
>> Sorry for not pointing this out in your last version.
>>
>> Thanks,
>> Joseph
>>
> 
> Error code like 0~255 seems weird indeed, could we just return 1 and
> let user be aware of read-only case by error message printed?
> 
Yes, that's my point.
You can refer TLDP doc for details:
https://urldefense.proofpoint.com/v2/url?u=http-3A__tldp.org_LDP_abs_html_exitcodes.html&d=DwICaQ&c=RoP1YumCXCgaWHvlZYR8PZh8Bv7qIrMUB65eapI_JnE&r=O5cCAbu465xBYvKnRkqjGxKXxI8YKZ1I5XfW6gFnHbE&m=Td_2_ce8gyADsKP2heoryPLwizjTFAYlf_GqpkPWyYY&s=6py8Z5EsWr7VviB5GNqSs_5ap7r_4szTLBwHd4iOeaE&e=

> thanks,
> Jun
> 
>>>>  }
>>>>
>> .
>>