[Ocfs2-tools-devel] [PATCH 9/9] ocfs2-tools: add xattr support in tunefs.ocfs2

[Joel Becker]

On Fri, Dec 19, 2008 at 05:42:19PM +0800, Tiger Yang wrote:
> This patch add feature xattr in tunefs.ocfs2.
> So you can enable or disable feature xattr on a ocfs2 volume.
> Disable feature xattr will remove all xattrs which have already been set,
> include acl, security xattrs.

<snip>

> +errcode_t ocfs2_xattr_remove_value_outside(ocfs2_filesys *fs,
> +					   struct ocfs2_extent_list *el,
> +					   int flags,
> +					   int (*func)(ocfs2_filesys *fs,
> +						struct ocfs2_extent_rec *rec,
> +						int tree_depth,
> +						uint32_t ccount,
> +						uint64_t ref_blkno,
> +						int ref_recno,
> +						void *priv_data),
> +					   void *priv_data)
> +{

	This really could be called ocfs2_extent_iterate_xattr_value(),
you know?  Just like extent_iterate_inode below it.

> +	int i;
> +	int iret = 0;
> +	errcode_t ret;
> +	struct extent_context ctxt;
> +
> +	if (el->l_tree_depth) {
> +		ret = ocfs2_malloc0(sizeof(char *) * el->l_tree_depth,
> +				    &ctxt.eb_bufs);
> +		if (ret)
> +			goto out;
> +
> +		ret = ocfs2_malloc0(fs->fs_blocksize *
> +				    el->l_tree_depth,
> +				    &ctxt.eb_bufs[0]);
> +		if (ret)
> +			goto out_eb_bufs;
> +
> +		for (i = 1; i < el->l_tree_depth; i++) {
> +			ctxt.eb_bufs[i] = ctxt.eb_bufs[0] +
> +				i * fs->fs_blocksize;
> +		}
> +	} else
> +		ctxt.eb_bufs = NULL;
> +
> +	ctxt.fs = fs;
> +	ctxt.func = func;
> +	ctxt.priv_data = priv_data;
> +	ctxt.flags = flags;
> +	ctxt.ccount = 0;
> +	ctxt.last_eb_blkno = 0;
> +	ctxt.last_eb_cpos = 0;
> +
> +	ret = 0;
> +	iret |= extent_iterate_el(el, 0, &ctxt);
> +	if (iret & OCFS2_EXTENT_ERROR)
> +		ret = ctxt.errcode;
> +

	This never writes out the buffer containing the
ocfs2_extent_list.  Now, in this patch we are just deleting the entire
xattr set, so that's not necessary.  But this function should be the
generic ocfs2_extent_iterate_xattr_value(), and it should write out the
block if changed.  Or perhaps let the caller know to do so.

> +static errcode_t remove_xattr_index_block(ocfs2_filesys *fs,
> +					  struct ocfs2_dinode *di,
> +					  struct ocfs2_xattr_block *xb)
> +{
> +	struct ocfs2_extent_list *el = &xb->xb_attrs.xb_root.xt_list;
> +	errcode_t ret = 0;
> +	uint32_t name_hash = UINT_MAX, e_cpos = 0, num_clusters = 0;
> +	uint64_t p_blkno = 0;
> +
> +	if (el->l_next_free_rec == 0)
> +		return 0;
> +
> +	while (name_hash > 0) {
> +		ret = ocfs2_xattr_get_rec(fs, xb, name_hash, &p_blkno,
> +					  &e_cpos, &num_clusters);
> +		if (ret) {
> +			tcom_err(ret, "while getting bucket record "
> +				 "of xattr data.");
> +			goto out;
> +		}
> +
> +		ret = remove_xattr_buckets(fs, di, p_blkno, num_clusters);
> +		if (ret) {
> +			tcom_err(ret, "while iterating buckets of xattr data");
> +			goto out;
> +		}

	This removes the buckets, but it doesn't remove the extent
blocks that make up the xattr index tree.  Those need to get removed.

> +static int enable_xattr(ocfs2_filesys *fs, int flag)
> +{
> +	errcode_t ret = 0;
> +	struct ocfs2_super_block *super = OCFS2_RAW_SB(fs->fs_super);
> +
> +	if (ocfs2_support_xattr(super)) {
> +		verbosef(VL_APP,
> +			 "The xattr feature is already enabled; "
> +			 "nothing to enable\n");
> +		goto out;
> +	}
> +
> +	if (!tools_interact("Enable the xattr feature on device "
> +			     "\"%s\"? ",
> +			     fs->fs_devname))
> +		goto out;

	The 'interact' strings are most likely to be seen.  Let's say
"extended attributes" instead of "xattr" in those strings.

Joel

-- 

Life's Little Instruction Book #356

	"Be there when people need you."

Joel Becker
Principal Software Developer
Oracle
E-mail: joel.becker at oracle.com
Phone: (650) 506-8127