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

[Manoj Naik <[e-mail]>]

Christoph Hellwig <[e-mail]> wrote on 10/04/2014 09:17:50 AM:

> From: Christoph Hellwig <[e-mail]>
> To: Manoj Naik/Almaden/IBM@IBMUS, [e-mail]
> Date: 10/04/2014 09:17 AM
> Subject: xattr model in draft-naik-nfsv4-xattrs-01?
>
> Hi Manoj,
>
> can you explain what xattr model your draft is based on?  It seems to
> have extremly complicated semantics that include a lot of semantics
> not present in Linux/IRIX/FreeBSD xattrs.

Why do you say they are complicated? Section 5.3 lists what we need:

   Applications need to perform the following operations on a given
   file's extended attributes [5]:

   o  Given a file, return a list of all of the file's assigned extended
      attribute keys.

   o  Given a file and a key, return the corresponding value.

   o  Given a file, a key, and a value, assign that value to the key.

   o  Given a file and a key, remove that extended attribute from the
      file.

The operations capture those requirements.

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

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. I know somebody on the list or at the
IETF meeting asked for this specifically too.

>
> I think we would be much better of starting with:
>
> GETXATTR:
>
> struct GETXATTR4args {
>    /* CURRENT_FH: file */
>    xattrname4   ga_name;
> }
>
> struct GETXATTR4res {
>    xattrvalue4   gr_value;
> }
>
> LISTXATTR:
>
> struct LISTXATTR4args {
>    /* CURRENT_FH: file */
> }
>
> struct LISTXATTR4res {
>    xattrname4   gr_names<>
> };
>
> 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).

>
>
> In the SETXATTR I would suggest to remove the _ALL types, and the
> multiplexing not present in any OS API.  Additionally we need an
> anything goes type that can create or replace to properly support
> the existing Linux/IRIX/FreeBSD semantics.  The remove case could
> be roled into this - while none of the OS APIs do this, the Linux
> kernel internal API does and it works fairly well, although I still
> think a separate operation would be cleaner protocol wise:
>
> enum setxattr_type4 {
>    SETXATTR4_ANY      = 0,
>    SETXATTR4_CREATE   = 1,
>    SETXATTR4_REPLACE   = 2,
> }
>
> struct SETXATTR4args {
>    /* CURRENT_FH: file */
>    setxattr_type4   sa_type;
>    xattrname4   sa_xattrname;
> }
>
> struct SETXATTR4res {
>    nfsstat4 sr_res;
> }
>
> struct REMOVEXATTR4args {
>    /* CURRENT_FH: file */
>    xattrname4   sa_xattrname;
> }
>
> struct REMOVEXATTR4res {
>    nfsstat4 sr_res;
> }
>

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.

>
> A few more comments on the standard outside of this high level model:
>
> In Section 2. mentioning Solaris is wrong - Solaris never exposed
xattr-like
> APIs, just name attributes which are different and supported in NFSv4.

OK. See the referenced
http://docs.oracle.com/cd/E19253-01/816-5175/6mbba7f02/. Solaris still
calls them "extended file attributes", but you're right and I'll remove it.

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

>
> Asking about not interpreting the name is a desaster waiting to happen
> given that Linux differs from the other implementations in how it handles
> the name.  I sent a separate mail about this earlier today, but I think
> trying to handle anything but the "user" xattrs in this protocol would
> be a grave mistake, so a Linux client would have to strip the "user."
> and a Linux server would have to add this "user."

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?

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

Thanks for the comments.

Manoj.

>
>