[Ocfs2-tools-devel] [PATCH] fsck.ocfs2: close ost_fs in case of check superblock fails

[Joseph Qi]

Yes, you are right.
maybe_replay_journals may close and open fs again. And dlm and cluster
info will be lost in this case.
So we have to think a solution for this issue as well.

Thanks,
Joseph

On 2016/9/25 15:33, piaojun wrote:
> Hi Joseph,
> Please see my comments below:
> 
> On 2016-9-24 11:55, Joseph Qi wrote:
>> open_and_check may happen that check_superblock fails after ocfs2_open.
>> Currently it goes to out without close ost->ost_fs. So close it in this
>> case.
>> And check ost->ost_fs to make maybe_replay_journals works properly in
>> all cases.
>>
>> Signed-off-by: Joseph Qi <joseph.qi at huawei.com>
>> ---
>>  fsck.ocfs2/fsck.c | 15 +++++++++------
>>  1 file changed, 9 insertions(+), 6 deletions(-)
>>
>> diff --git a/fsck.ocfs2/fsck.c b/fsck.ocfs2/fsck.c
>> index dc5209d..f71fb21 100644
>> --- a/fsck.ocfs2/fsck.c
>> +++ b/fsck.ocfs2/fsck.c
>> @@ -471,6 +471,7 @@ static errcode_t open_and_check(o2fsck_state *ost, char *filename,
>>  	if (ret) {
>>  		printf("fsck saw unrecoverable errors in the super block and "
>>  		       "will not continue.\n");
>> +		ocfs2_close(ost->ost_fs);
>>  		goto out;
>>  	}
>>
>> @@ -1097,12 +1098,14 @@ close:
>>  		ocfs2_shutdown_dlm(ost->ost_fs, whoami);
>>  	block_signals(SIG_UNBLOCK);
>>
>> -	ret = ocfs2_close(ost->ost_fs);
>> -	if (ret) {
>> -		com_err(whoami, ret, "while closing file \"%s\"", filename);
>> -		/* XXX I wonder about this error.. */
>> -		fsck_mask |= FSCK_ERROR;
>> -	}
>> +	if (ost->ost_fs) {
>> +		ret = ocfs2_close(ost->ost_fs);
>> +		if (ret) {
>> +			com_err(whoami, ret, "while closing file \"%s\"", filename);
>> +			/* XXX I wonder about this error.. */
>> +			fsck_mask |= FSCK_ERROR;
>> +		}
>> +	}
>>
>>  out:
>>  	return fsck_mask;
>>
> If 'ost->ost_fs' is NULL, the access of 'ost->ost_fs->fs_dlm_ctxt' will
> lead NULL pointer error, so more exceptional situations need to be
> considered. I think more NULL-checkers need be added.
> Thanks,
> Jun
> 
> 
> .
>