[Top] [All Lists]

Re: [PATCH] use generic_*xattr routines

To: Timothy Shimmin <tes@xxxxxxx>
Subject: Re: [PATCH] use generic_*xattr routines
From: Christoph Hellwig <hch@xxxxxx>
Date: Thu, 29 May 2008 14:39:10 +0200
Cc: Christoph Hellwig <hch@xxxxxx>, xfs-oss <xfs@xxxxxxxxxxx>
In-reply-to: <483BE788.4050504@xxxxxxx>
References: <20080430112217.GB16966@xxxxxx> <20080521081656.GA2638@xxxxxx> <48365486.3060503@xxxxxxx> <20080523054848.GA29507@xxxxxx> <483A15CE.9060409@xxxxxxx> <20080526053759.GA17825@xxxxxx> <483BE788.4050504@xxxxxxx>
Sender: xfs-bounce@xxxxxxxxxxx
User-agent: Mutt/1.3.28i
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.

<Prev in Thread] Current Thread [Next in Thread>