[Ocfs2-tools-devel] [PATCH 4/5] Add "Set sparse flag" to ocfs2-tools, take 2

[Mark Fasheh]

On Wed, Oct 31, 2007 at 02:22:21PM +0800, tao.ma wrote:
>
>>> +
>>> +static errcode_t set_func(ocfs2_filesys *fs, struct ocfs2_dinode *di)
>>> +{
>>> +	errcode_t ret;
>>> +	ocfs2_cached_inode *ci = NULL;
>>> +
>>> +	ret = ocfs2_read_cached_inode(fs, di->i_blkno, &ci);
>>> +	if (ret)
>>> +		goto out;
>>> +
>>> +	ret = ocfs2_zero_tail_and_truncate(fs, ci, di->i_size, NULL);
>>>     
>>
>> Are we missing writeout of the inode block here, for the case where
>> i_clusters changed?
>>   
> Do we really need to handle this issue here?

Yes - ocfs2_zero_tail_and_truncate() might remove clusters from the file --
that's why we're calling it. If you truncate part of an inode, you need to
update i_clusters.

ocfs2_truncate() does an i_clusters update after calling
ocfs2_zero_tail_and_truncate():

		ret = ocfs2_zero_tail_and_truncate(fs, ci, new_i_size,
						   &new_clusters);
 		if (ret)
 			goto out;
 
		ci->ci_inode->i_clusters = new_clusters;
...
 		ci->ci_inode->i_size = new_i_size;
 		ret = ocfs2_write_cached_inode(fs, ci);


By the way, 'set_func' can ignore the write 99% of the time just by
comparing i_clusters before the zero and afterwards.


> This is the work of fsck.ocfs2, not tunefs.ocfs2. If we have modified
> i_clusters, it means that the file is already corrupted.

I wouldn't call it a corruption - the inode tree is completely valid, for a
nonsparse file system.

We're removing uninitialized data from clusters past i_size which might have
been left around from a failed extend in the non-sparse code. Since ocfs2
without sparse files always zero's, we'd never see that data. Ocfs2 _with_
sparse file support could expose that data via an ftruncate. tunefs.ocfs2 is
turned on sparse file support, and should make sure that inodes are in a
state which the sparse file code expects.
	--Mark

--
Mark Fasheh
Senior Software Developer, Oracle
mark.fasheh at oracle.com