[Ocfs2-tools-devel] [PATCH 10/10] dx_dirs v6: add disable indexed-dirs support in tunefs.ocfs2

[Mark Fasheh]

On Fri, Jan 29, 2010 at 04:42:29AM +0800, Coly Li wrote:
> This patch truncates all directories' indexed tree if '--fs-features noindexed-tree' option is given. The indexed dirs
> related flags on directory inodes and superblock are cleared too.
> 
> Signed-off-by: Coly Li <coly.li at suse.de>
> Cc: Mark Fasheh <mfasheh at suse.com>

I haven't read the library patch yet so my comments might be a little off.

Overall though, great to see that the tunefs patch reequires so little code - that's
exactly how we like it :)

> ---
>  tunefs.ocfs2/feature_indexed_dirs.c |  191 ++++++++++++++++++++++++++++++++++-
>  tunefs.ocfs2/o2ne_err.et            |    9 ++
>  2 files changed, 198 insertions(+), 2 deletions(-)
> 
> diff --git a/tunefs.ocfs2/feature_indexed_dirs.c b/tunefs.ocfs2/feature_indexed_dirs.c
> index 368eb87..f3d097b 100644
> --- a/tunefs.ocfs2/feature_indexed_dirs.c
> +++ b/tunefs.ocfs2/feature_indexed_dirs.c

> @@ -71,6 +82,182 @@ out:
>  	return ret;
>  }
> 
> +static errcode_t dx_dir_iterate(ocfs2_filesys *fs, struct ocfs2_dinode *di,
> +				void *user_data)
> +{
> +	errcode_t ret = 0;
> +	struct dx_dirs_inode *dx_di = NULL;
> +	struct dx_dirs_context *ctxt= (struct dx_dirs_context *)user_data;
> +
> +	if (!S_ISDIR(di->i_mode))
> +		goto bail;
> +
> +	ret = ocfs2_malloc0(sizeof(struct dx_dirs_inode), &dx_di);
> +	if (ret) {
> +		ret = TUNEFS_ET_NO_MEMORY;
> +		goto bail;
> +	}
> +
> +	if (!(di->i_dyn_features & OCFS2_INDEXED_DIR_FL))
> +		goto bail;

You're leaking 'dx_di' here - probably you just need to move the check above
the malloc. Also, it might not hurt to also check that the inode is properly
marked as a directory  (S_ISDIR()).


> +static int disable_indexed_dirs(ocfs2_filesys *fs, int flags)
> +{
> +	errcode_t ret = 0;
> +	struct ocfs2_super_block *super = OCFS2_RAW_SB(fs->fs_super);
> +	struct dx_dirs_context ctxt;
> +	struct tools_progress *prog = NULL;
> +
> +	if (!ocfs2_supports_indexed_dirs(super)) {
> +		verbosef(VL_APP,
> +			"Directory indexing feature is not enabled; "
> +			"nothing to disable\n");
> +		goto out;
> +	}
> +
> +	if (!tools_interact("Disabling the directory indexing feature on "
> +			    "device \"%s\"? ",
> +			    fs->fs_devname))
> +		goto out;
> +
> +	prog = tools_progress_start("Disable directory indexing", "no dir idx", 2);
> +	if (!prog) {
> +		ret = TUNEFS_ET_NO_MEMORY;
> +		tcom_err(ret, "while initializing the progress display");
> +		goto out;
> +	}
> +
> +	memset(&ctxt, 0, sizeof (struct dx_dirs_context));
> +	INIT_LIST_HEAD(&ctxt.inodes);
> +	ret = find_indexed_dirs(fs, &ctxt);
> +	if (ret) {
> +		tcom_err(ret, "while scanning indexed directories");
> +		goto out_cleanup;
> +	}
> +
> +	tools_progress_step(prog, 1);
> +
> +	tunefs_block_signals();
> +	ret = clean_indexed_dirs(fs, &ctxt);
> +	if (ret) {
> +		tcom_err(ret, "while truncate indexed directories");
> +	}
> +	/* We already touched file system, must disable dx dirs flag here.
> +	 * XXX: fsck.ocfs2 will handle the orphan indexed trees. */

Is the "XXX: fsck.ocfs2 will handle the orphan indexed trees." comment still
true? It should only be a matter of a few lines of code to iterate the
system directory...
	--Mark

--
Mark Fasheh