[Ocfs2-tools-devel] [PATCH 5/6] Resend: Add unwritten extent support in ocfs2-tools, take 2

[tao.ma]

Joel Becker wrote:
> On Wed, Sep 26, 2007 at 04:12:44PM -0700, Mark Fasheh wrote:
>   
>>> @@ -2251,6 +3698,7 @@ errcode_t ocfs2_extend_allocation(ocfs2_filesys *fs, uint64_t ino,
>>>  	uint64_t blkno, file_size;
>>>  	char *buf = NULL;
>>>  	struct ocfs2_dinode* di = NULL;
>>> +	uint16_t flag = 0;
>>>  
>>>  	if (!(fs->fs_flags & OCFS2_FLAG_RW))
>>>  		return OCFS2_ET_RO_FILESYS;
>>> @@ -2265,6 +3713,9 @@ errcode_t ocfs2_extend_allocation(ocfs2_filesys *fs, uint64_t ino,
>>>  
>>>  	di = (struct ocfs2_dinode *)buf;
>>>  
>>> +	if (ocfs2_writes_unwritten_extents(OCFS2_RAW_SB(fs->fs_super)))
>>> +		flag = OCFS2_EXT_UNWRITTEN;
>>>       
>> Uh-oh... Doesn't this function get called for expanding dirs and creating
>> journals?
>>
>> It's illegal (and pointless anyway) for a directory or a journal to have an
>> unwritten extent.
>>     
>
> 	This brings up another issue.  In your sixth patch, you modify
> file_read() and file_write() to handle unwritten extents.  However,
> anyone using ocfs2_block_iterate() or ocfs2_extent_iterate() needs to
> understand unwritten extents.  They can, of course, see them on the
> rec->flags, but we need to make sure all users handle this.  It would be
> slicker if we can handle it for them, but that might take more work :-)
>   
Yeah, you are somewhat right. ;)

For ocfs2_extent_iterate, we just gave the extent record to the user. so 
the user's callback should handle it. So there is no need for modify the 
iteration process. Maybe I need to check all the caller to check whether 
their callbacks handle this.

In ocfs2_block_iterate, we just iterate all the extent records and give 
the block num to the user's callback.  As for unwritten extent record, 
do you think we should add a new parameter in the callback's definition 
that this block exist in a unwritten extents? I am not quite sure of it.

Tao