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

[Mark Fasheh]

On Wed, Oct 17, 2007 at 05:26:14PM +0800, tao.ma wrote:
> "Set sparse" is used when we want to add "sparse" features for an
> old-formatted OCFS2 volume. Now the user can use
> "--fs-features=sparse" to add this feature to those volumes and
> enable the new "sparse" feature.
> 
> Signed-off-by: Tao Ma <tao.ma at oracle.com>
> ---
>  libocfs2/include/feature_string.h |    8 --
>  libocfs2/include/ocfs2.h          |    8 ++
>  tunefs.ocfs2/Makefile             |    2 +-
>  tunefs.ocfs2/features.c           |   59 ++++++++++++++++
>  tunefs.ocfs2/sparse_file.c        |  137 +++++++++++++++++++++++++++++++++++++
>  tunefs.ocfs2/tunefs.c             |   61 +++++++++++++++--
>  tunefs.ocfs2/tunefs.h             |    8 ++
>  7 files changed, 269 insertions(+), 14 deletions(-)
>  create mode 100644 tunefs.ocfs2/features.c
> 

Again, I really like how you're keeping the feature check / parsing code
seperate. This should make it easier for us to add support for other
features in the future.


> +/*
> + * Check whether we can add or remove a feature.
> + *
> + * Currently, we only handle "sparse files".
> + * More feature check may be added if we want to
> + * support more options in tunefs.ocfs2.
> + */
> +errcode_t feature_check(ocfs2_filesys *fs)
> +{
> +	errcode_t ret = 0;
> +
> +	if ((opts.set_feature.incompat &
> +	     OCFS2_FEATURE_INCOMPAT_SPARSE_ALLOC) &&
> +	    ocfs2_sparse_alloc(OCFS2_RAW_SB(fs->fs_super)))
> +		ret = 1;
> +
> +	return ret;
> +}

Shouldn't feature_check() fail if the user asks for a feature which tunefs
can't handle yet?


> +errcode_t update_feature(ocfs2_filesys *fs)
> +{
> +	errcode_t ret = 0;
> +
> +	if (opts.set_feature.incompat & OCFS2_FEATURE_INCOMPAT_SPARSE_ALLOC)
> +		ret = set_sparse_file_flag(fs, opts.progname);
> +
> +	return ret;
> +}
> diff --git a/tunefs.ocfs2/sparse_file.c b/tunefs.ocfs2/sparse_file.c
> index 4877408..3499ae4 100644
> --- a/tunefs.ocfs2/sparse_file.c
> +++ b/tunefs.ocfs2/sparse_file.c
> @@ -463,3 +463,140 @@ bail:
>  		ocfs2_free(&buf);
>  	return ret;
>  }
> +
> +static errcode_t fill_file_end(ocfs2_filesys *fs, struct ocfs2_dinode *di)
> +{
> +	errcode_t ret = 0;
> +	char *buf = NULL, *data_buf = NULL;;
> +	struct ocfs2_extent_rec *rec = NULL;
> +	struct ocfs2_extent_list *el = NULL;
> +	struct ocfs2_extent_block *eb = NULL;
> +	uint64_t blkno, len_in_blk, last_cluster_start;
> +	uint16_t offset, blk_off, bpc = fs->fs_clustersize / fs->fs_blocksize;
> +
> +	ret = ocfs2_malloc_block(fs->fs_io, &data_buf);
> +	if (ret)
> +		goto bail;
> +
> +	el = &di->id2.i_list;
> +	if (el->l_tree_depth > 0) {
> +		ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +		if (ret)
> +			goto bail;
> +
> +		ret = ocfs2_read_extent_block(fs, di->i_last_eb_blk, buf);
> +		if (ret)
> +			goto bail;
> +
> +		eb = (struct ocfs2_extent_block *)buf;
> +		el = &eb->h_list;
> +	}

Use something like ocfs2_get_clusters for this instead...

Actually, doesn't ocfs2_zero_tail_for_truncate() do most of this for you?


> +	if (!el->l_next_free_rec)
> +		goto bail;
> +
> +	rec = &el->l_recs[el->l_next_free_rec - 1];
> +
> +	offset = di->i_size % fs->fs_blocksize;
> +	len_in_blk = (di->i_size + fs->fs_blocksize - 1) / fs->fs_blocksize;
> +	blk_off = len_in_blk % bpc;
> +
> +	/*
> +	 * Although in a non-sparse file system a extent record doesn't have
> +	 * the extent flag, 2-bytes are still enough to store the length of
> +	 * the extent record. And the disk storage of OCFS2 is little endian,
> +	 * so we can use e_leaf_cluster here safely.
> +	 */
> +	last_cluster_start = rec->e_blkno + (rec->e_leaf_clusters - 1) * bpc;
> +	blkno = last_cluster_start + blk_off - 1;
> +
> +	/* first emtpy the extra byte in the last used block. */
> +	ret = io_read_block(fs->fs_io, blkno, 1,data_buf);
> +	if (ret)
> +		goto bail;
> +
> +	memset(data_buf + offset, 0, fs->fs_blocksize - offset);
> +	ret = io_write_block(fs->fs_io, blkno, 1, data_buf);
> +	if (ret)
> +		goto bail;
> +
> +	/* Then empty the extra blocks in the last cluster. */
> +	memset(data_buf, 0, fs->fs_blocksize);
> +	while (++blkno % bpc != 0) {
> +		ret = io_write_block(fs->fs_io, blkno, 1, data_buf);
> +		if (ret)
> +			goto bail;
> +	}
> +
> +bail:
> +	if (data_buf)
> +		ocfs2_free(&data_buf);
> +	if (buf)
> +		ocfs2_free(&buf);
> +
> +	return ret;
> +}
> +
> +errcode_t set_sparse_file_flag(ocfs2_filesys *fs, char *progname)
> +{
> +	errcode_t ret;
> +	uint64_t blkno;
> +	char *buf;
> +	struct ocfs2_dinode *di;
> +	ocfs2_inode_scan *scan;
> +	struct ocfs2_super_block *super = OCFS2_RAW_SB(fs->fs_super);
> +
> +	ret = ocfs2_malloc_block(fs->fs_io, &buf);
> +	if (ret)
> +		goto out;
> +
> +	di = (struct ocfs2_dinode *)buf;
> +
> +	ret = ocfs2_open_inode_scan(fs, &scan);
> +	if (ret) {
> +		com_err(progname, ret, "while opening inode scan");
> +		goto out_free;
> +	}
> +
> +	for(;;) {
> +		ret = ocfs2_get_next_inode(scan, &blkno, buf);
> +		if (ret) {
> +			com_err(progname, ret,
> +				"while getting next inode");
> +			goto out_close_scan;
> +		}
> +		if (blkno == 0)
> +			break;
> +
> +		if (memcmp(di->i_signature, OCFS2_INODE_SIGNATURE,
> +			    strlen(OCFS2_INODE_SIGNATURE)))
> +			continue;
> +
> +		ocfs2_swap_inode_to_cpu(di);
> +
> +		if (di->i_fs_generation != fs->fs_super->i_fs_generation)
> +			continue;
> +
> +		if (di->i_flags & OCFS2_SYSTEM_FL)
> +			continue;
> +
> +		if (S_ISREG(di->i_mode)) {
> +			if (!(di->i_size % fs->fs_clustersize))
> +				continue;
> +
> +			ret = fill_file_end(fs, di);
> +			if (ret)
> +				goto out_close_scan;

We should also truncate all clusters past i_size. The file system might have
left them allocated without the proper i_size change if there was a failed
extend.

The only complication is this check in ocfs2_truncate():

	if (ci->ci_inode->i_size == new_i_size)
		goto out;

We don't actually want to change i_size from what it is, but we want to do
the iteration over 

Maybe we should abstract out the truncate part of ocfs2_truncate() into a
seperate function and just have this code call that? It looks like it'd
call ocfs2_zero_tail_for_truncate() for you.
	--Mark

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