[Ocfs2-tools-devel] [PATCH] Add extent flag as the parameter to block iteration function.

[tao.ma]

Mark Fasheh wrote:
> On Tue, Oct 09, 2007 at 02:01:38PM +0800, tao.ma wrote:
>   
>> As we have added unwritten extent in ocfs2, block iteration function
>> needs to know whether the content in the block is good. So add a new
>> parameter in block iteration function.
>>
>> As for the block writting, please don't use these block iteration
>> functions. Just use ocfs2_file_write since it knows everything about
>> unwritten extents. ;)
>>     
>
> Ok, most of that looks good - we're deprecating this api so adding the flags
> is questionable, but I think in the end it's probably useful to complete it
> in case the existing callers need a quick fix (for example, fsck should
> probably check for and correct the unwritten flag in directory extents).
>
> Can you please move the OCFS2_EXTENT_*, OCFS2_BLOCK_* flag / return code definitions
> and ocfs2_extent_iterate{_inode}/ocfs2_block_iterate{_inode} prototypes to
> the bottom of libocfs2/include/ocfs2.h?
>
> Above them, we should add a comment indicating that the api is now
> deprecated.
>
> Maybe something like this:
>
> /*
>  * Deprecated extent/block iterate functions.
>  */
>
>
>   
OK, no problem. I will move it.
>> diff --git a/libocfs2/fileio.c b/libocfs2/fileio.c
>> index 85730b2..d10d141 100644
>> --- a/libocfs2/fileio.c
>> +++ b/libocfs2/fileio.c
>> @@ -45,12 +45,17 @@ struct read_whole_context {
>>  static int read_whole_func(ocfs2_filesys *fs,
>>  			   uint64_t blkno,
>>  			   uint64_t bcount,
>> +			   uint16_t ext_flags,
>>  			   void *priv_data)
>>  {
>>  	struct read_whole_context *ctx = priv_data;
>>  
>> -	ctx->errcode = io_read_block(fs->fs_io, blkno,
>> -				     1, ctx->ptr);
>> +	if (ext_flags & OCFS2_EXT_UNWRITTEN) {
>> +		memset(ctx->ptr, 0, fs->fs_blocksize);
>> +		ctx->errcode = 0;
>> +	} else
>> +		ctx->errcode = io_read_block(fs->fs_io, blkno,
>> +					     1, ctx->ptr);
>>  	if (ctx->errcode)
>>  		return OCFS2_BLOCK_ABORT;
>>     
>
> Can you just fix up ocfs2_read_whole_file() to use ocfs2_file_read()
> instead? This way it won't need any modification for inline-data.
>   
After I re-read the function of ocfs2_read_whole_file, I have some new 
thought about it. If you go through the code, you will see that it only 
allocate the di->i_clusters and iterate all the clusters and read them. 
So considering that now we have sparse file support, what should we do 
if we meet with a large hole? It is useless for us to read many number 
of zeros from ocfs2_file_read. Thanks to god that after I go through all 
the caller of this function in ocfs2-tools, it is only used for slot_map 
and heartbeat files.
I have to say this function should also be deprecated. Maybe I have to 
also move its definition to the bottom of libocfs2/include/ocfs2.h since 
it isn't suitable for sparse file now.
So my suggestion is that just leave it as what I have modified. You 
don't have to modify it for inline-data and I will move the definition 
to the bottom of libocfs2/include/ocfs2.h. Is it OK?