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

[Coly Li]

On 2010年01月29日 05:12, Mark Fasheh Wrote:
> On Fri, Jan 29, 2010 at 04:42:29AM +0800, Coly Li wrote:
>> +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()).
> 

Copied, fixed in my local tree.

> 
>> +static int disable_indexed_dirs(ocfs2_filesys *fs, int flags)
>> +{
>> +	errcode_t ret = 0;
[snip]
>> +	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...

If clean_indexed_dirs()  does not truncate all the indexed trees and returns an error...
In this condition, IMHO, the indexed dir flag should be removed from the super block, but there are still some indexed
trees not truncated. This comment means, in fsck.ocfs2, if (!ocfs2_supports_indexed_dirs(supper)), it's also necessary
to check the direcotry inode's dynamic feature flag, if there is indexed tree, truncate it.

Maybe I used an incorrect term 'orphan', does it have specific meaning in ocfs2 ?

Thanks.
-- 
Coly Li
SuSE Labs