[Ocfs2-tools-devel] [patch 4/8] Adds slot remove mechanism in tunefs.ocfs2, take 2

[tao.ma]

Joel Becker wrote:
> On Fri, Jun 15, 2007 at 04:16:01PM +0800, tao.ma at oracle.com wrote:
>   
>> +static errcode_t move_group(ocfs2_filesys *fs,
>> +			    struct relink_ctxt *ctxt,
>> +			    struct moved_group *group)
>> +{
>>     
> <snip>
>   
>> +	ret = ocfs2_write_group_desc(fs, group->blkno, group->gd_buf);
>>     
> <snip>
>   
>> +	ret = ocfs2_write_inode(fs, ctxt->new_inode, ctxt->dst_inode);
>>     
>
> 	I just realized, we're leaving the old chain dangling here.
> That's not good.
>   
Yes, I leave it dangling here since fsck.ocfs2 can handle this problem. 
See the check I add in fsck.ocfs2 for "GROUP_DUPLICATED". I have also 
verified it by crashing the tunefs.ocfs2 after ocfs2_write_inode.
So do you think we really need to update the old group descriptor above 
it and the old chain record accordingly?
That may be two more write function(another write_group_desc and 
write_inode).
>   
>> +static errcode_t relink_system_alloc(ocfs2_filesys *fs,
>> +				     uint16_t removed_slot,
>> +				     enum relink_alloc_action action)
>> +{
>>     
> <snip>
>   
>> +	/*iterate all the chain record and move them to the new slots. */
>> +	for (i = cl->cl_next_free_rec - 1; i >= 0; i--) {
>> +		ctxt.new_slot = i % opts.num_slots;
>> +		if (ctxt.action == RELINK_EXTENT_ALLOC)
>> +			ret = ocfs2_lookup_system_inode(fs,
>> +						EXTENT_ALLOC_SYSTEM_INODE,
>> +						ctxt.new_slot,
>> +						&ctxt.new_inode);
>>     
>
> 	We are assuming that the extent allocs for all nodes >0 will be
> empty, and this is a noop for now, correct?
>   
Yes, currently the kernel allocate all the extent alloc from 
extent_alloc:0000. But I think it is simular like inode_alloc. So I add 
it and it may need some test in future if we use other extent alloc 
files. (Actually I have written a program which exchange 
extent_alloc:0000 and extent_alloc:slot arbitrary and test, it seems 
work well. ;) )