[Ocfs2-tools-devel] Patch for journal truncate of ocfs2-tools.

[tao.ma]

Sunil,
    Since the test script works good now, Can I also check in this patch 
using your sob?
Sunil Mushran wrote:
> Tao,
>
> Looks good. However, I would like few tests added in conjuction with
> this.
>
> 1. Test for ocfs2_truncate(). Test should create a file with lots of
> extents (using the same scheme of allocating multiple clusters but
> appending to the file in reverse order) and then truncate it. Run fsck
> to check correctness. We should be able to run the same basic test in
> multiple combinations of bs/cs. For each combination, we should make the
> tree level 0 and level 1 trees.
>
> 2. Specific test for resizing journals in tunefs. Test should not try to
> stress test truncate but concentrate on ensuring that the user not
> resize the journal in conjunction with num slot increase.
>
> Thanks
> Sunil 
>
>
> On Sat, 2007-04-28 at 14:11 +0800, tao.ma wrote:
>   
>> Sunil Mushran wrote:
>>     
>>>> From the code it seems that we used to make the journal size smaller 
>>>> only under the condition that we change the numbers of slots, so here we 
>>>> have the check of "if (!opts.num_slots)". But now we can decrease the 
>>>> journal size without touching the number of slots, so we shouldn't care 
>>>> whether we change num_slots or not. Am I wrong? Please point it out.
>>>>     
>>>>         
>>> The code used to handle the situation in which the user extends the
>>> journal size and adds slots at the same time. The current code cannot
>>> handle that.
>>>
>>> Again, I am ok with making the two ops exclusive. As in, disallow
>>> changing slots and changing journal files at the same time. It may make
>>> life easier both coding/testing wise and not take much from the user.
>>>   
>>>       
>> Sorry for not reading the code clearly about the process of changing 
>> num_slots.
>> First paste the original code here:
>>
>> 		if (opts.jrnl_size > def_jrnl_size)
>>  			printf("Changing journal size %"PRIu64" to %"PRIu64"\n",
>>  			       def_jrnl_size, opts.jrnl_size);
>>  		else {
>> 			if (!opts.num_slots) {
>> 				com_err(opts.progname, 0, "Journal size %"PRIu64" "
>> 					"has to be larger " "than %"PRIu64"",
>> 					opts.jrnl_size, def_jrnl_size);
>> 				goto unlock;
>> 			}
>>  		}
>>  	
>>
>> The code is used for an additional check of journal_size. When the user 
>> want to change num_slots, tunefs will set jrnl_size equals to the 
>> default size, so that here may meet with the situation "opts.jrnl_size 
>> == def_jrnl_size" and it should't be an error. During the following 
>> steps, update_slots will add the nodes and update_journal_size will do 
>> the work of filling the new journal files with the journal header.
>> Now as we can decrease the journal size, all the possible values of 
>> opts.jrnl_size is right. So I removed the com_err completely.
>> As for extending the journal size and adding slots at the same time, I 
>> think they can coexist with each other, since we will add the slots and 
>> fill the journal file later.
>> Attached is the new patch. Please review.
>>
>>
>> plain text document attachment (journal_truncate.patch)
>> Index: libocfs2/mkjournal.c
>> ===================================================================
>> --- libocfs2/mkjournal.c	(revision 1346)
>> +++ libocfs2/mkjournal.c	(working copy)
>> @@ -301,6 +301,19 @@ errcode_t ocfs2_make_journal(ocfs2_files
>>  		ret = ocfs2_write_inode(fs, blkno, (char *)di);
>>  		if (ret)
>>  			goto out;
>> +	} else if (clusters < di->i_clusters) {
>> +		uint64_t new_size = clusters <<
>> +			   OCFS2_RAW_SB(fs->fs_super)->s_clustersize_bits;
>> +		ret = ocfs2_truncate(fs, blkno, new_size);
>> +		if (ret)
>> +			goto out;
>> +
>> +		ocfs2_free_cached_inode(fs, ci);
>> +		ret = ocfs2_read_cached_inode(fs, blkno, &ci);
>> +		if (ret) {
>> +			ci = NULL;
>> +			goto out;
>> +		}
>>  	}
>>  
>>  	ret = ocfs2_format_journal(fs, ci);
>> Index: tunefs.ocfs2/tunefs.c
>> ===================================================================
>> --- tunefs.ocfs2/tunefs.c	(revision 1346)
>> +++ tunefs.ocfs2/tunefs.c	(working copy)
>> @@ -976,7 +976,7 @@ static errcode_t update_journal_size(ocf
>>  		}
>>  
>>  		di = (struct ocfs2_dinode *)buf;
>> -		if (num_clusters <= di->i_clusters)
>> +		if (num_clusters == di->i_clusters)
>>  			continue;
>>  
>>  		printf("Updating %s...  ", jrnl_file);
>> @@ -1445,17 +1445,9 @@ int main(int argc, char **argv)
>>  		opts.jrnl_size = num_clusters <<
>>  				OCFS2_RAW_SB(fs->fs_super)->s_clustersize_bits;
>>  
>> -		if (opts.jrnl_size > def_jrnl_size)
>> +		if (opts.jrnl_size != def_jrnl_size)
>>  			printf("Changing journal size %"PRIu64" to %"PRIu64"\n",
>>  			       def_jrnl_size, opts.jrnl_size);
>> -		else {
>> -			if (!opts.num_slots) {
>> -				com_err(opts.progname, 0, "Journal size %"PRIu64" "
>> -					"has to be larger " "than %"PRIu64"",
>> -					opts.jrnl_size, def_jrnl_size);
>> -				goto unlock;
>> -			}
>> -		}
>>  	}
>>  
>>  	/* validate volume size */
>>
>>     
>
>   


-- 
* **     Tao Ma
*     Member of Techincal Staff *

Oracle Asia Research & Development Center
Open Source Technologies Development
*
Tel:        +86 10 8278 6026
URL:       OARDC Intranet <http://cdc.oraclecorp.com/>, Oracle.com/cdc 
<http://www.oracle.com/cdc/>