[Ocfs2-tools-devel] [RFC 1/4] dx_dirs v2: indexed dir for fsck.ocfs2

[Joel Becker]

On Sat, Dec 26, 2009 at 03:22:42AM +0800, Coly Li wrote:
> +errcode_t o2fsck_try_add_reidx_dir(struct rb_root *root, uint64_t dino)
> +{
> +	errcode_t ret = 1;

	ret is an errcode_t.  It needs to be a proper error, not '1'.

> +	uint64_t ino;
> +	ino = o2fsck_search_reidx_dir(root, dino);
> +	if (ino) {
> +		ret = 0;
> +		goto out;
> +	}
> +	ret = o2fsck_add_reidx_dir_ino(root, dino);
> +	if (ret)
> +		printf("some error happens.\n");

	This needs to be using com_err().  There are multiple places
where you should be using com_err() to report errcode_t errors.

> +static int ocfs2_rebuild_indexed_dir(ocfs2_filesys *fs, uint64_t ino)
> +{
> +	int ret = 0;

	ret needs to be an errcode_t, and this function needs to return
errcode_t.  All the functions it calls use errcode_t.

> +	char *di_buf = NULL, *dx_root_buf = NULL;
> +	struct ocfs2_dinode *di;
> +
> +	ret = ocfs2_malloc_block(fs->fs_io, &di_buf);
> +	if (ret)
> +		goto out;
> +	
> +	ret = ocfs2_read_inode(fs, ino, di_buf);
> +	if (ret)
> +		goto out;
> +	di = (struct ocfs2_dinode *)di_buf;
> +
> +	ret = ocfs2_dx_dir_truncate(fs, di);
> +	if (ret)
> +		goto out;
> +
> +
> +	/* do not rebuild indexed tree for inline directory */
> +	if (di->i_dyn_features & OCFS2_INLINE_DATA_FL) {
> +		printf("inode with OCFS2_INLINE_DATA_FL, do nothing.\n");
> +		goto out;
> +	}

	Don't print anything when we do nothing.  The user doesn't care.
You can, of course, have a printf for debugging; it gets removed before
we commit the changes.

> +//#		build new indexed-tree

	Please use C comments, not C++/C99.  And put "XXX:" in front of
TODO items:

	/* XXX: Build new indexed tree */

> +	ret = ocfs2_write_inode(fs, ino, di_buf);
> +out:
> +	if (di_buf)
> +		ocfs2_free(&di_buf);
> +	if (dx_root_buf)
> +		ocfs2_free(&dx_root_buf);
> +	return ret;
> +}


> +
> +
> +void o2fsck_rebuild_indexed_dirs(ocfs2_filesys *fs, struct rb_root *root)
> +{
> +	struct rb_node *node;
> +	o2fsck_dirblock_entry *dbe;
> +	uint64_t ino;
> +	int ret;
> +
> +	for (node = rb_first(root); node; node = rb_next(node)) {
> +		dbe = rb_entry(node, o2fsck_dirblock_entry, e_node);
> +		ino = dbe->e_ino;
> +		ret = ocfs2_rebuild_indexed_dir(fs, ino);
> +		if (ret)
> +			printf("o2fsck_rebuilt_indexed_dir failed on %lu\n", ino);

	Again ret needs to be errcode_t, and the error should be printed
with com_err().

> +		if (ocfs2_supports_indexed_dirs(OCFS2_RAW_SB(dd->fs->fs_super)) &&
> +		    !(di->i_dyn_features & OCFS2_INLINE_DATA_FL)) {
> +			ret = o2fsck_try_add_reidx_dir(&dd->re_idx_dirs, dbe->e_ino);
> +			if (ret)
> +				printf("there is something wrong\n");

	com_err() again.

Joel

-- 

"Here's a nickle -- get yourself a better X server."
	- Keith Packard

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127