Re: [nfsv4] xattr model in draft-naik-nfsv4-xattrs-01?

[Manoj Naik <[e-mail]>]

Christoph Hellwig <[e-mail]> wrote on 10/05/2014 12:22:28 AM:

> From: Christoph Hellwig <[e-mail]>
> To: Manoj Naik/Almaden/IBM@IBMUS
> Cc: [e-mail]
> Date: 10/05/2014 12:22 AM
> Subject: Re: xattr model in draft-naik-nfsv4-xattrs-01?
>
> On Sat, Oct 04, 2014 at 01:34:04PM -0700, Manoj Naik wrote:
> > The operations capture those requirements.
>
> While they happen to also implement those the focus seems to be on a lot
> of more complicated cruft, thus making the straight forward
> implementation of those requirements unessecarily complex.
>
> > > On the GETXATTR side you conflate the LISTXATTR operation with
GETXATTR,
> > > as well as adding a new bulk get one that isn't present in any OS API
> > > I know.  Both the argument and return structures are basically
entirely
> > > different for the different cases.
> >
> > Agree that GETXATTR includes what is traditionally provided by
listxattr()
> > call. The idea was to limit the number of new operations provided to
two -
> > GETXATTR and SETXATTR. But if there is consensus, we don't have an
issue
> > with adding a LISTXATTR instead.
>
>
> Why would you want to limit the number of operations?  Adding a new
> compound operation just is a new entry in a new dispatch table, which
> adding tons of unions complicates parsing at runtime, bloats data
> structures and makes the standard unreadable, thus placing a burden on
> human and machine.

I think you're referring to this:

        union getxattr_res4 switch (getxattr_type4 gr_type) {
         case GETXATTR4_LIST:
                xattrname4    gr_names<>;
         case GETXATTR4_ONE:
                xattrvalue4   gr_value;
         case GETXATTR4_ALL:
                xattr4        gr_xattrs<>;
        };

This isn't any more bloated/unreadable than a lot of existing protocol
definitions and operations. I am not arguing against LISTXATTR/REMOVEXATTR
being separate operations, but there is precedence in the existing spec for
operations that are far more complicated that this :)

>
> > I think the ability to get/set all xattrs of a given file
> > (getallxattrs/setallxattrs?) is useful, especially for backup/restore
> > programs. Granted that there isn't an OS API that exposes this, but
perhaps
> > there should be one? I know some backup utilities that work directly
with a
> > filesystem API to do this. Especially over NFS, I believe a single RPC
that
> > accomplishes this may be desirable.
>
> Setting multiple attributes can easily be archives by multiple SETXATTR
> operations in a compound.  Getting multiple attributes in one operation
> might sound useful, but there is a reason why it hasn't be implemented
> so far, and the main reason is that it's not sanely implementable at
> the OS lewvel.  More on that below.

An operation that replaces ALL the xattr keys of a file with another set of
keys could be done in a compound if we added a separate REMOVEXATTR that
allowed deleting all the xattrs of a file. Otherwise this would need a
LISTXATTR, delete individual xattrs and SETXATTR each one, and this cannot
be achieved in a single COMPOUND.

>
> > > Also how is the client supposed to cope with sizes of both each
> > > xattr and the list?  Don't we need a maxcount/mincount scheme similar
> > > to READDIR or GETDEVICEINFO?
> >
> > Explain? There is no limit imposed on the size of each xattr (as long
as it
> > is <= total size of all xattrs, i.e. maxxattrsize).
>
> Both setxattr and listxattr return data that basically is unbound for
> a simple request.  While this isn't a problem at the XDR level it is a
> problem at the syscall level.  If you look how the get and list
operations
> are specified you'll see that they take special care of this at the API
> level.

This is not unbounded for the protocol either. Section 4 states:

   Similar to ACLs, the amount of xattr data exchanged between the
   client and server for get/set operations can be considered to fit in
   a single COMPOUND request, bounded by the channel's negotiated
   maximum size for requests.

More on relaxing this requirement (for list/getall/setall) separately.

>
> All of them get passed a fixed sized pointer for the return buffer.  On
> IRIX the size paramter for the attr_get operation is an IN/OUT argument
> using a pointer, so when the size is not large enough it returns the
> expected size. This baiscally is a 1:1 mapping to the maxcount/mincount
> scheme in XDR.  Linux and FreeBSD aren't quite as smart and instead
> just return an error, but then expect the user to do a get or list
> call with a 0 size and NULL buffer to query the size of the attribute
> or attribute list.
>
> This also is a reason why the get all argument operation was never added
> to any OS - expressing this concept as a C API is incredibly complex
> with all those buffer sizing issue - you basically have to pass in the
> biggest possible buffers for the largest possible number of attributes,
> unless you want to do callee allocated buffers which isn't possible
> for syscall-level APIs.
>
> > Don't have a problem with this - we can add separate
LISTXATTR/REMOVEXATTR
> > if there is consensus. I'd still like to retain the ALL option though.
>
> Again they aren't in OS APIs for a good reason.  Besides an utterly
> horrible and crufty inferface they also are against the concept of the
> attr model where each attribute stands on it's own.  The concept of
> replacing or deleting all xattrs does not fit into any of the OS ABIs.
>
> I strongly disagree with NFS standards creating new unimplementable
> operations that further increase the deadwood in the specification.
>
> > > Instead of mentioning NTFS you should mention the API, preferably
with
> > > an informative reference.
> >
> > Probably - I can't find any informative references to this though.
> > Pointers?
>
> Ask the Microsoft representatives on the WG?  If it's not important
enough
> we might as well strike the reference, as an ondisk format isn't very
> relevant for NFS standardization.
>
> > I agree. Tom pointed out in his review of the first draft that it might
> > make sense to encapsulate some security attrs as xattrs if they could
be
> > generalized to be platform-independent. Hence I added this to Section
3:
> >
> >                                                Attributes from other
> >    namespaces are typically vendor-specific, but some of them may be
> >    generalized into well-defined set of names that promote
interoperable
> >    implementations.
> >
> > What wording can we add to enforce this? We've said the xattrs are
> > completely opaque (both in terms of key names and values), so adding a
> > namespace prefix in the key should be avoided. I like the idea of the
> > (Linux) client/server stripping off the "user." prefix, but can we do
> > better than adding strong verbiage that only attributes from the user
> > namespace can be specified?
>
> I don't think standardizing anything else is even remotely possible,
> all other attrs are higly OS-specific non-opaque data structures, so
> even leaving the options for them open is contrary to wording in much
> of the draft.
>
> > > Section 5.1:  a very useful attribute missing here is the maximum
size
> > > of each individual attribute.
> >
> > Why do we need this? I see file systems limit the total space taken by
> > xattrs, but not the maximum size of individual attributes. Of course,
we
> > can add it if needed.
>
> I don't known of a any (Linux) filesystem that has limits on the total
> size of all extended attributes.  There limits on the size of each
> attribute, further complicated by the fact that the extN limitations
> if for key+value combined, while the others I know only have individual
> limits for key and value.
>

OK, so we need an option to specify limit on individual xattr size. But
there are file systems that place limits on the total size as well.

Manoj.