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

[Sunil Mushran]

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 */
>