Re: [PATCH] use generic_*xattr routines

[Christoph Hellwig <hch@lst.de>]

On Tue, May 27, 2008 at 08:50:48PM +1000, Timothy Shimmin wrote:
> >  - the callback is supplied by the xfs_attr_list caller, not set based
> >    on options
> 
> Oh, okay. For example, instead of setting the flags to ATTR_KERNOVAL
> such as in xfs_vn_listxattr when size is 0, one could just set the callback
> to xfs_attr_kern_list_sizes and pass it in etc...

Yes.  I have an initial patch that goes directly to xfs_attr_list_int
from xfs_xattr.c and kills most of the ATTR_KERN flags.  It's a quite
nice cleanup already.  Next step will be to convert dmapi to use it's
own callback aswell.  This will be an even bigger cleanup as
put_listent gets the xattr value aswell and we can kill the additional
xfs_attr_get calls, making this code simpler and more efficient.

> >  - there will be an opaque object supplied to xfs_attr_list that is to
> >    be used by put_listent so that we don't have to pass down
> >    implementation-specific arguments directly.
> >
> 
> Ok.
> So instead of overloading fields in xfs_attr_list_context_t,
> you'll pass down a void* argument or some such for callback specific data.

I've started looking at this and after some investigattion I think
we should just pass the xfs_inode directly to all the functions and then
a void parameter, yes.  We'll need to find a solution for the
seen_enough paramter, but I think this could be handled similar to
filldir.  There's also some functions directly touching the attr cursor
which seems solveable, too.

> > I'd also like to move the attrlist_cursor_kern_t into this callback
> > opaque context because it doesn't make sense for the normal xattr API,
> > but I'll have to see if that's actually feasible.
> 
> BTW, the cursor stuff is a bit flawed. Like the dir1 code (I believe),
> if from userspace you use the cursor and modifications happen to the EAs
> (add or removal) between calls, 
> we can end up repeating elements in the list or miss some.
> We don't preserve the position and we can compact the data etc.

Yes, I think this whole cursor is a rather bad idea.  But given that
it's used by xfsdump we can't easily get rid of it.