Re: [PATCH 1/6] xattr consolidation v2 - generic xattr API

[Will Dyson <will.dyson@gmail.com>]

On Sat, 18 Sep 2004 03:20:37 -0400 (EDT), James Morris
<jmorris@redhat.com> wrote:
> This patch consolidates common xattr handling logic into the core fs code,
> with modifications suggested by Christoph Hellwig (hang off superblock,
> remove locking, use generic code as methods), for use by ext2, ext3 and
> devpts, as well as upcoming tmpfs xattr code.

Hi James,

I'd like to raise an issue with the documentation your patch
introduces. The lack of it, actually.

While it might be obvious to someone who is familiar with the ext2 and
ext3 xattr code, I had a rather hard time understanding how a
filesystem would make use of the generic functions that your patch
introduces. While I think I have it now, that is 30 minutes of my life
that I will never get back :) . 30 minutes is not a long time (I've
spent as much thinking about this message), but this sort of thing is
an issue all throughout the generic vfs code.

When I was working on the readonly befs filesystem, I may have spent
as much as 20 percent of my time actually thinking about on-disk data
and how to use it. Most of the rest of that time was spent reading
code from the vfs layer (or other existing filesystems), trying to
figure out how I should call helper functions or how they would call
my code. Some of that code is very clever and solves complex problems,
which makes it hard to grok on the first (or third) read-through.

I don't plan on holding my breath while waiting for "The linux vfs
layer for dummies" to come out. But a quick comment to explain the
purpose and use of a block of code can make a world of difference to
someone trying to come up to speed.

For example:

/*
In order to implement different sets of xattr operations for each
xattr prefix, a filesystem should create a null-terminated array of
struct xattr_handler (one for each prefix) and hang a pointer to it
off of the s_xattr field of the superblock. The generic_fooxattr
functions will search this list for a xattr_handler with a prefix
field that matches the prefix of the xattr we are dealing with and
call the apropriate function pointer from that xattr_handler.
*/

-- 
Will Dyson - Consultant
http://www.lucidts.com/