Re: [Ocfs2-devel] [PATCH 13/39] ocfs2: Add extended attribute support

From: Tao Ma
Date: Wed Oct 08 2008 - 10:05:26 EST




Christoph Hellwig wrote:
On Wed, Oct 08, 2008 at 09:56:41AM +0800, Tiger Yang wrote:
I have looked the patch for btrfs about this. We are different.
Btrfs store the whole xattr name including the prefix "user." "trusted.", we store index number instead of it.

I looked at the git tree and there are two users of
ocfs2_xattr_handler().

(1) for using the ->list handler in listattr. That's something I fixed
in btrfs that I wanted to point you to. The whole concept of a
->list handler is stupid, and it was only added as a hack for
the tmpfs "generic" xattr support which is a mess. Instead of
looking up a handler that would only do the same thing anyway
for all on-disk attributes just call the code directly and
have a map from index to prefix (look at
fs/xfs/linux-2.6/xfs_xattr.c for an example). You
also have a check for OCFS2_MOUNT_NOUSERXATTR for the user
attributes, but that's much easier done by just checking the
index in an if (and I'd personally just kill it completely, the
options doesn't seem useful - but that's an unrelated bit)
yes, you are right. The handler for list is borrowed from ext3 and somewhat ugly. We just need the prefix name but use such a complicated method. Just a map from index to prefix should work fine.
(2) For generating the hash. I don't quite understand why you want to
also hash the prefix if it's not store on disk anyway but sorted
into the numeric buckets.
This is done intentionally. See the design doc http://oss.oracle.com/osswiki/OCFS2/DesignDocs/ExtendedAttributes.
"Each entry has a 32-bit hash value associated with it. The hash value is calculated using the full (prefix.suffix) name of the xattr to avoid hash collisions when the same suffix is used in multiple attribute namespaces. "
So Mark, do you think we need this prefix hash?
Anyway, if we make consensus that the hash calculation doesn't need prefix any more, we can remove the ocfs2_xattr_handler safely.

Regards,
Tao
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/