Subject: [nfsv4] Current stateid and CREATE

From: Tigran Mkrtchyan <[email-masked]>
Date: Sun, 18 Dec 2011 18:34:54 +0100

Dear NFS gurus,

The rfc5661 in 16.2.3.1.2 says:

   If an operation sets the current filehandle but does not return a stateid,
   the current stateid MUST be set to the all-zeros special stateid,
i.e., (seqid, other) = (0, 0).

Nevertheless  operations like PUTFH, PUTPUBFH and some others
explicitly mention mentioned to clear current stateid.
Is it just redundant or others like LOOKUP and CREATE left current
stateid unchanged.

Thanks,
   Tigran.

---

From: Benny Halevy <[email-masked]>
Date: Mon, 19 Dec 2011 14:19:36 +0200

> Is it just redundant or others like LOOKUP and CREATE left current

Since the current stateid is defined as follows (in the same section):
   A "current stateid" is the stateid that is associated with the
   current filehandle.

When the current filehandle changes the previous value of the current
stateid does not refer to the new filehandle and therefore it MUST either
be set to a new value by the operation if it returns a stateid or
be reset to the all-zeroes value if the operation does not return a stateid.

Regarding LOOKUP, further in the section it is said:
   Operations like
   LOOKUP that transform a current filehandle and component name into a
   new current filehandle will also change the current state to {0, 0}.

Benny

> 
> Thanks,
>    Tigran.
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Tigran Mkrtchyan <[email-masked]>
Date: Mon, 19 Dec 2011 13:37:56 +0100

>   new current filehandle will also change the current state to {0, 0}.

Yes, that's cleat. The main question is:

it the description of other operations redundant or 16.2.3.1.2 is not enough to
make a decision on stateid invalidation.

In both cases I will suggest to make it more clean in following etteras.

Thanks,
   Tigran.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 19 Dec 2011 14:23:41 -0500

> In both cases I will suggest to make it more clean in following etteras.

We could remove the redundant text from the operation descriptions.

Or we could put it in a standard place in every operation description,
and hope that will make it harder to forget when adding new operations.

E.g., perhaps it could go into comments in the xdr, with the current
filehandle:

	18.13.1. ARGUMENT

	struct LOOKUP4args {
		/* CURRENT_FH: directory */
		component4      objname;
	};

	18.13.2.  RESULTS

	struct LOOKUP4res {
		/* New CURRENT_FH: object */
		/* New CURRENT_STATEID: all zeroes */
		nfsstat4 status;
	};

--b.

---

From: Benny Halevy <[email-masked]>
Date: Tue, 20 Dec 2011 10:13:55 +0200

>                /* New CURRENT_STATEID: all zeroes */

I really like this approach. I think it's clear to any reader of the spec
and in particular, to the implementer.

Benny

               nfsstat4 status;

---

From: <[email-masked]>
Date: Wed, 21 Dec 2011 08:43:15 -0500

18.13.2.  RESULTS

       struct LOOKUP4res {

> and in particular, to the implementer.

Agree that this is what readers would want to see.  Now we have to talk about getting this done, i.e.
get someone to actually make the changes.  I don't see that errata is the right mechanism.  Since the
spec is not really unclear, errata are not intended for a big change like this, focused on making the spec,
"the best it can be".

should we do this first in the XDR for 4.2?  Do we have a volunteer to make that change?

If there is a 5661bis (and so a 5662bis), we could make this sort of change for v4.1 then.

________________________________
From: [email-masked] [mailto:[email-masked]] On Behalf Of Benny Halevy
Sent: Tuesday, December 20, 2011 3:14 AM
To: J. Bruce Fields
Cc: [email-masked]
Subject: Re: [nfsv4] Current stateid and CREATE

On Mon, Dec 19, 2011 at 9:23 PM, J. Bruce Fields <[email-masked]<mailto:[email-masked]>> wrote:
On Mon, Dec 19, 2011 at 01:37:56PM +0100, Tigran Mkrtchyan wrote:
> On Mon, Dec 19, 2011 at 1:19 PM, Benny Halevy <[email-masked]<mailto:[email-masked]>> wrote:

       18.13.1. ARGUMENT

       struct LOOKUP4args {

       18.13.2.  RESULTS

       struct LOOKUP4res {

Benny

               nfsstat4 status;
       };

--b.

---

From: Trond Myklebust <[email-masked]>
Date: Wed, 21 Dec 2011 14:47:10 +0100

On Tue, 2011-12-20 at 10:13 +0200, Benny Halevy wrote: 
> On Mon, Dec 19, 2011 at 9:23 PM, J. Bruce Fields
> <[email-masked]> wrote:

> and in particular, to the implementer.

I agree that it is nice to have this explicitly noted in all functions
which are affected, but I'm mystified as to why people are getting up in
arms about something that is, for once, actually 100% clear in the spec.
Redundancy is fine, as long as the protocol remains consistent.

There are plenty of other little nits and inconsistencies in that same
document which are more worthy of bandwidth on these lists.

Trond
-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
[email-masked]
www.netapp.com