[Ocfs2-tools-devel] [PATCH RESEND 3/3] fsck.ocfs2: support append direct io incompat feature

[Eric Ren]

Hi Joseph,
...
>> Support to truncate direct io orphan entry in fsck.ocfs2.
>>
>> Signed-off-by: Joseph Qi<joseph.qi at huawei.com>
Acked-by: Eric Ren <zren at suse.com>
>> ---
>>   fsck.ocfs2/pass4.c  |  9 +++++++++
>>   libocfs2/truncate.c | 12 ++++++++++++
>>   2 files changed, 21 insertions(+)
....
>> It make sense to skip _delte_inode() stuff. But I cannot understand very well what the code
>> after out_check: is supposed to do? Looks it collects info about link count and inode refs,
>> and then do check_link_counts() where inode would be moved to lost+found dir if no dirent
>> refer to it. So is it possible to lose the inode unexpectedly?
>>
> Normally it won't. Otherwise I think it is a bug for inode link count.
>> Could you explain a bit more the reason why we should add this lines of code here?
>> Because fix_dups_func()
>>            ->clone_one_inode() or delete_one_inode()
>> 	    ->1355         ret = ocfs2_truncate_full(ost->ost_fs, ino, 0,
>> 	      1356                                   pass1d_free_clusters, fd);
>>
>> As you can see the 3th parameter(new_i_size) is 0, so it won't hit go-out. Why do we
>> need this line:
>> 	new_i_size = ci->ci_inode->i_size;
>> ?
>>
> Though replay_orphan_iterate->ocfs2_truncate passes new_i_size with 0,
> but for dio entry, the new_i_size will be re-assigned to the current
> inode i_size. As I explained before, this is to reclaim allocated
> clusters.
> In other words, clusters are allocated, but inode i_size has
> not been updated and written to disk. So we have to make the inode go
> back to the former status after fsck.
Thanks for your explainations. I can understand more. But considering my 
very limited knowledge about this,
I think I can just give a acked-by, hope other experts can also review 
this;-)

Btw, reflink feature also uses orphan directory to do the similar thing. 
I'm just wondering did reflink take this into consideration,
and can both of them has a common handling in fsck?

Eric