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.
|