Subject: Re: [nfsv4] AD Review of draft-ietf-nfsv4-umask-03

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 1 May 2017 15:53:03 -0400

On Fri, Apr 28, 2017 at 05:54:07PM -0400, David Noveck wrote:
> > You should also be able to do this by trying an operation and checking
> > for NFS4ERR_NOTSUPP, shouldn't you?
> 
> It's not quite that simple.  In some cases you, could get an OP_ILLEGAL or
> even an XDR decode error.   This is discussed in the last paragraph of
> Section 8.4
> of xattr-04.

Sure.  That doesn't seem so bad to me, but...

> You can still do the more complicated thing but the document
> recommends,
> quite reasonably, that you check the attribute first.  There is no
> "RECOMMENDATION",
> but most clients would prefer to do the simpler thing regardless.
> 
> I don't see the point of deleting this.  It simplifies life for the client,
> and the burden on the
> server side is trivial.

OK, fair enough.  Reading through that part, it sounds OK.

My only request is that they bump FATTR4_XATTR_SUPPORT to 82 to avoid
conflict with FATTR4_MODE_UMASK.

A couple other nits (sorry, this probably isn't the right thread):

- "For file systems which either do not support the xattr_support
  attribute or which returns FALSE when it is interrogated, all of these
  operations MUST NOT be supported."  Might be clearer to say the
  operations either must not be supported or, if they're supported, must
  return NFS4ERR_NOTSUPP, which I don't think of as exactly the same
  thing.

- NFS4ERR_NOXATTR "The specified xattr does not exist or the server is
  unable to retrieve it."

  Surely that's wrong.  Applications have to be able to tell the
  difference between those two cases.  Probably the latter should be
  NFS4ERR_IO or something.

--b.

---

From: David Noveck <[email-masked]>
Date: Mon, 1 May 2017 16:32:31 -0400

> A couple other nits (sorry, this probably isn't the right thread)

The right thread was WGLC a number of months ago.  In the absence
of a time machine, we might as well deal with these issues here and now.

I've added Marc and Manoj to the To-list.

> conflict with FATTR4_MODE_UMASK.

Clearly, the same value should not be used.  I'm not sure exactly how to
decide which feature has priority.  This may turn on the distribution of
prototype implemenatations.  What is known about that?  We certainly want to
invalidate interoperating implementations that people are using.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 1 May 2017 16:47:24 -0400

> invalidate interoperating implementations that people are using.

Client and server implementations of the umask draft are both in
released versions of the Linux kernel.  (I don't think they're in a
version of the kernel that's been distributed by enterprise
distributions yet.)

There have been Linux client xattr patches floating around, but I don't
think they've been updated recently.  I really think it's time to get
those patches properly reviewed and included--experience has shown that
often finds specification issues.  I'm not sure of the status of server
side xattr implementations--I think there may have been patches for
Ganesha, but I'm not clear what level of review they've had.

--b.

---

From: Nico Williams <[email-masked]>
Date: Mon, 1 May 2017 15:47:50 -0500

> of a time machine, we might as well deal with these issues here and now.

"Stuff happens".  You get to fix these now or in errata.

> invalidate interoperating implementations that people are using.

This is a failure of how allocation is done.  I recommend having a range
of private use code for these purposes.  I also recommend allowing
allocations ahead of I-Ds reaching the RFC-Editor queue IANA state for
at least WG work items.

Nico
--

---

From: "Marc Eshel" <[email-masked]>
Date: Mon, 1 May 2017 14:11:32 -0700

The Ganesha 2.5 code includes xattr support and it is using 
FATTR4_XATTR_SUPPORT 81 but it can be changed.
Marc.

From:   "J. Bruce Fields" <[email-masked]>
To:     David Noveck <[email-masked]>
Cc:     Marc Eshel/Almaden/IBM@IBMUS, Manoj Naik <[email-masked]>, 
Spencer Dawkins at IETF <[email-masked]>, 
[email-masked], "[email-masked]" 
<[email-masked]>, NFSv4 <[email-masked]>
Date:   05/01/2017 01:47 PM
Subject:        Re: [nfsv4] AD Review of draft-ietf-nfsv4-umask-03

--b.