[Ocfs2-tools-devel] [PATCH 06/10] Dir operation modification for inline data.

[Tao Ma]

Joel Becker wrote:
> On Fri, Jul 11, 2008 at 02:20:59PM +0800, Tao Ma wrote:
>> 2. Move expand_dir to ocfs2_link itself. So now the caller of ocfs2_link
>>    doesn't need to expand the dir by itself.
> 
> 	Hooray!  I copied the independent expand_dir thing from libext2,
> and boy was I wrong.
> 
>> +static errcode_t ocfs2_expand_inline_dir(ocfs2_filesys *fs,
>> +					 uint64_t dir)
>> +{
>> +	errcode_t ret = 0;
>> +	uint32_t n_clusters;
>> +	uint64_t blkno;
>> +	char *buf = NULL, *di_buf = NULL;
>> +	struct ocfs2_dinode *di;
>> +
>> +	ret = ocfs2_malloc_block(fs->fs_io, &buf);
>> +	if (ret)
>> +		goto bail;
>> +
>> +	ret = ocfs2_malloc_block(fs->fs_io, &di_buf);
>> +	if (ret)
>> +		goto bail;
>> +
>> +	ret = ocfs2_read_inode(fs, dir, di_buf);
>> +	if (ret)
>> +		goto bail;
>> +	di = (struct ocfs2_dinode *)di_buf;
>> +
>> +	ret = ocfs2_new_clusters(fs, 1, 1, &blkno, &n_clusters);
>> +	if (ret)
>> +		goto bail;
>> +
>> +	memcpy(buf, di->id2.i_data.id_data, di->i_size);
>> +	memset(buf + di->i_size, 0, fs->fs_blocksize - di->i_size);
>> +	ocfs2_expand_last_dirent(buf, di->i_size, fs->fs_blocksize);
>> +
>> +	ret = ocfs2_write_dir_block(fs, blkno, buf);
>> +	if (ret)
>> +		goto bail;
>> +
>> +	di->i_dyn_features &= ~OCFS2_INLINE_DATA_FL;
>> +
>> +	ocfs2_dinode_new_extent_list(fs, di);
>> +
>> +	di->i_size = 0;
>> +	di->i_ctime = di->i_mtime = time(NULL);
>> +
>> +	ret = ocfs2_write_inode(fs, dir, di_buf);
> 
> 	This is a little weird, writing out a zero size, even
> temporarily.  If we crash, we lose the dir entries, right?
yes, you see ocfs2_insert_extent below needs us to send the blkno and 
read the inode info by itself, so I have to write it out here. by now, I 
haven't find a perfect way to handle this(no journal in user space is 
really painful).
> 
>> +	if (ret)
>> +		goto bail;
>> +
>> +	ret = ocfs2_insert_extent(fs, di->i_blkno, 0, blkno, n_clusters, 0);
>> +	if (ret)
>> +		goto bail;
>> +
>> +	/* re-read the buffer since ocfs2_insert_extent has modified it. */
>> +	ret = ocfs2_read_inode(fs, dir, di_buf);
>> +	if (ret)
>> +		goto bail;
>> +	di->i_size = fs->fs_blocksize;
>> +	/* update the size of the inode */
>> +	ret = ocfs2_write_inode(fs, dir, di_buf);
> 
> <snip>
> 
>> diff --git a/libocfs2/inode.c b/libocfs2/inode.c
>> index 33d9c9d..31dc2b3 100644
>> --- a/libocfs2/inode.c
>> +++ b/libocfs2/inode.c
>> @@ -89,6 +89,21 @@ static void ocfs2_swap_inode_third(struct ocfs2_dinode *di)
>>  			rec->t_start    = bswap_32(rec->t_start);
>>  			rec->t_clusters = bswap_32(rec->t_clusters);
>>  		}
>> +	} else if (di->i_dyn_features & OCFS2_INLINE_DATA_FL &&
>> +		   S_ISDIR(di->i_mode)) {
>> +		struct ocfs2_dir_entry *de;
>> +		uint16_t rec_len;
>> +		char *de_buf = (char *)di->id2.i_data.id_data;
>> +		char *limit = de_buf + di->i_size;
>> +
>> +		while (de_buf < limit) {
>> +			de = (struct ocfs2_dir_entry *)de_buf;
>> +			de->inode = bswap_64(de->inode);
>> +			rec_len = de->rec_len;
>> +			de->rec_len = bswap_16(de->rec_len);
>> +
>> +			de_buf += rec_len;
>> +		}
> 
> 	Should this loop be shared with swap_dir_block() somehow?
yes, this really is somehow like ocfs2_swap_dir_entries_direction, I 
will try to share them later.
> 
>> @@ -128,15 +129,24 @@ errcode_t ocfs2_link(ocfs2_filesys *fs, uint64_t dir, const char *name,
>>  	ls.flags = flags;
>>  	ls.done = 0;
>>  	ls.sb = fs->fs_super;
>> -
>> +again:
>>  	retval = ocfs2_dir_iterate(fs, dir,
>>                                     OCFS2_DIRENT_FLAG_INCLUDE_EMPTY,
>>                                     NULL, link_proc, &ls);
>>  	if (retval)
>>  		return retval;
>>  
>> -	if (!ls.done)
>> -		return OCFS2_ET_DIR_NO_SPACE;
>> +	if (!ls.done) {
>> +		if (allocation)
>> +			return OCFS2_ET_INTERNAL_FAILURE;
>> +
>> +		retval = ocfs2_expand_dir(fs, dir);
>> +		if (retval)
>> +			return retval;
>> +
>> +		allocation = 1;
>> +		goto again;
>> +	}
> 
> 	Can we make this a loop instead of a goto?  It's not complex.
en, not agree, if we use loop, use "for" or "while"?
if with "for", then use "for(i=0;i<2;i++)" isn't looks as clear as this 
one. the same goes with "whle(i<2)".

Regards,
Tao