[Ocfs2-tools-devel] [PATCH 07/15] dx_dirs v10: more library support for directory indexing

[Coly Li]

On 04/01/2010 06:45 AM, Mark Fasheh Wrote:
> On Thu, Mar 25, 2010 at 03:50:57AM +0800, Coly Li wrote:
>> This patch adds more library support for indexed dirs,
>> - dx_root alloc/delete
>> - dx_leaf alloc/delete
>> - dx_root read/write
>> - dx_leaf read/write
>> - indexed tree insert/truncate
>> - dx_root extent tree operations
>> - indexed dir support in ocfs2_lookup_system_inode(),
>>   ocfs2_init_dir(), ocfs2_link(), ocfs2_unlink(),
>>   ocfs2_lookup().
>> With this patch, indexed dirs support in fsck.ocfs2 is possible.
> 
> This is looking great. I have only one comment below,
> 
> 
>> +/*
>> + * This function overwite the indexed dir attribute of
>> + * the given inode. The caller should make sure the dir's
>> + * indexed tree is truncated.
>> + */
>> +errcode_t ocfs2_dx_dir_build(ocfs2_filesys *fs,
>> +			uint64_t dir)
>> +{
> 
> 
> I noticed that dx_dir_build() will not take care to ensure that there is
> space for the trailers, before installing them. That's ok though -
> tunefs.ocfs2 is the only caller (correct?) and it installs the trailers in
> one of your next patches by re-using the metaecc code.
> 
> What bothers me is that I had to read pretty deep into the function before
> realizing that. IMHO, two things need to be fixed:
> 
> - This limitation should be documented in the function description above.
> 
> - The function should still check that it won't be over writing an existing
>   dirent when it re-initializes a trailer.
> 

The comments are added in v11 patches.
Add an function ocfs2_check_dir_trailer_space() to do the check, it can be found in v11 patches.

Thanks for the review.
-- 
Coly Li
SuSE Labs