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

[Mark Fasheh]

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.

Thanks,
	--Mark

--
Mark Fasheh