Subject: [nfsv4] 3530bis: remove paragraph incorrectly copied from 4.1 spec

From: "J. Bruce Fields" <[email-masked]>
Date: Tue, 23 Aug 2011 09:35:43 -0400

Below I'm suggesting a paragraph stating the opposite--but I haven't
checked if it's redundant given the rest of the text.  Best might be
just to delete this paragraph.

--b.

diff --git a/nfsv4_middle_file_locking.xml b/nfsv4_middle_file_locking.xml
index 280276e..4dc1171 100644
--- a/nfsv4_middle_file_locking.xml
+++ b/nfsv4_middle_file_locking.xml
@@ -365,11 +365,9 @@
       </t>
 
       <t>
-        The server may assign stateids independently for different clients.
-        A stateid with the same bit pattern for one client may designate an
-        entirely different set of locks for a different client.  The stateid
-        is always interpreted with respect to the client ID associated with
-        the current session.
+        Each stateid must be unique to the server.  Many operations take
+	a stateid as an argument but not a clientid, so the server must
+	be able to infer the client from the stateid.
       </t>
 
       <section anchor="stateid_types" title="Stateid Types">

---

From: <[email-masked]>
Date: Tue, 23 Aug 2011 11:47:17 -0400

I don't see any redundancy.

The definition of stateid says:

   Stateid:  A stateid is a 128-bit quantity returned by a server that
      uniquely identifies the open and locking states provided by the
      server for a specific open-owner or lock-owner/open-owner pair for
      a specific file and type of lock.

And you could interpret "uniquely identifies" as implying what your paragraph
does and making it thus redundant.

The only problem with that argument is that RFC5661 says exactly the same
thing, so if it means what your paragraph says, then it is totally wrong in
RFC5661.  As it is, it is ambiguous in both specs.
 
I think putting in your replacement paragraph is a step in the right direction,
by which I mean "toward clarity." 

-----Original Message-----
From: [email-masked] [mailto:[email-masked]] On Behalf Of J. Bruce Fields
Sent: Tuesday, August 23, 2011 9:36 AM
To: Tom Haynes
Cc: [email-masked]
Subject: [nfsv4] 3530bis: remove paragraph incorrectly copied from 4.1 spec

--b.

       <t>
 
       <section anchor="stateid_types" title="Stateid Types">
_______________________________________________
nfsv4 mailing list
[email-masked]
https://www.ietf.org/mailman/listinfo/nfsv4

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 23 Aug 2011 13:05:43 -0400

> by which I mean "toward clarity." 

Right, but there is a significant difference between stateids in the two
minor versions:

In minor version 0, the stateid MUST somehow allow the server to figure
out to which clientid it is attached.

In minor version 1, the server can rely on the fact that any operation
containing a stateid, will be preceded by a SEQUENCE op, which will tell
the server which session and clientid to use.

Cheers
  Trond

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
[email-masked]
www.netapp.com

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 16:48:57 -0400

> just to delete this paragraph.

Actually I think the problem is more extensive: much of 9.1.3 is true
only for 4.1, not 4.0.  Other examples:

	- it allows stateid's with seqid field 0 (the "give me the
	  most-recent stateid" case)
	- "layouts" are included as a type of lock in one place
	- special stateid types newly defined in 5661 are included.

This section at least needs a thorough review to make sure we haven't
missed anything else.  I haven't had the chance to do that yet.

The discussion of seqid wraparound is new to 4.0 as well, I think.  I
assume it's OK to add, as it would be good to have an agreed-upon
convention here and I don't think anything was specified in 3530.

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 16:56:45 -0400

> 	- "layouts" are included as a type of lock in one place

BTW, just grepping for "layout" I notice one other instance: in "State
Management Errors": "Depending on the operation, the stateid when valid
may designate opens, byte-range locks, file delegations, layouts, or
device maps."

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 18:00:12 -0400

> missed anything else.  I haven't had the chance to do that yet.

OK, I attempted that and the following patches fix most of the issues I
found.

--b.

---

From: [email-masked]
Date: Sun, 28 Aug 2011 18:04:40 -0400

From: "J. Bruce Fields" <[email-masked]>

In 4.1 the in-stateid seqid field allows the client to tell the server
what order it expects certain operations to be performed in.  In 4.0 the
out-of-stateid seqid field serves this purpose.  Also the special
interpretation of 0 in the seqid field exists only in 4.1.
---
 nfsv4_middle_file_locking.xml |   35 +----------------------------------
 1 files changed, 1 insertions(+), 34 deletions(-)

diff --git a/nfsv4_middle_file_locking.xml b/nfsv4_middle_file_locking.xml
index 4dc1171..c36d1d1 100644
--- a/nfsv4_middle_file_locking.xml
+++ b/nfsv4_middle_file_locking.xml
@@ -439,40 +439,7 @@
           NFS4_UINT32_MAX, and one (not zero) is the next seqid value.
           The purpose of the incrementing of the seqid is to allow the server
           to communicate to the client the order in which operations that
-          modified locking state associated with a stateid have been processed
-          and to make it possible for the client to send requests that are
-          conditional on the set of locks not having changed since the stateid
-          in question was returned.
-        </t>
-
-        <t>
-          When a client sends a
-          stateid to the server, it has two choices with regard to the seqid
-          sent.  It may set the seqid to zero to indicate to the server that it
-          wishes the most up-to-date seqid for that stateid's "other" field to
-          be used.  This would be the common choice in the case of a stateid
-          sent with a READ or WRITE operation.  It also may set a non-zero
-          value, in which case the server checks if that seqid is the correct
-          one.  In that case, the server is required to return
-          NFS4ERR_OLD_STATEID if the seqid is lower than the most current value
-          and NFS4ERR_BAD_STATEID if the seqid is greater than the most current
-          value.  This would be the common choice in the case of stateids sent
-          with a CLOSE or OPEN_DOWNGRADE.  Because OPENs may be sent in
-          parallel for the same owner, a client might close a file without
-          knowing that an OPEN upgrade had been done by the server, changing
-          the lock in question.  If CLOSE were sent with a zero seqid, the OPEN
-          upgrade would be canceled before the client even received an
-          indication that an upgrade had happened.
-        </t>
-
-        <t>
-          When a stateid is sent by the server to the client as part of a
-          callback operation, it is not subject to checking for a current seqid
-          and returning NFS4ERR_OLD_STATEID.  This is because the client is not
-          in a position to know the most up-to-date seqid and thus cannot
-          verify it.  Unless specially noted, the seqid value for a stateid
-          sent by the server to the client as part of a callback is required to
-          be zero with NFS4ERR_BAD_STATEID returned if it is not.
+          modified locking state associated with a stateid have been processed.t
         </t>
 
         <t>
-- 
1.7.6

---

From: [email-masked]
Date: Sun, 28 Aug 2011 18:04:43 -0400

---
 nfsv4_middle_file_locking.xml |    5 -----
 1 files changed, 0 insertions(+), 5 deletions(-)

diff --git a/nfsv4_middle_file_locking.xml b/nfsv4_middle_file_locking.xml
index 857eb59..fef7598 100644
--- a/nfsv4_middle_file_locking.xml
+++ b/nfsv4_middle_file_locking.xml
@@ -627,11 +627,6 @@
               filehandle, return NFS4ERR_BAD_STATEID.
             </t>
             <t>
-              If the client ID in the table entry does not match the
-              client ID associated with the current session,
-              return NFS4ERR_BAD_STATEID.
-            </t>
-            <t>
               If the stateid represents revoked state, then return
               NFS4ERR_EXPIRED, NFS4ERR_ADMIN_REVOKED, or
               NFS4ERR_DELEG_REVOKED, as appropriate.
-- 
1.7.6

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 18:11:56 -0400

I also grepped for "session" and noticed one more small problem.

I'm not completely sure my fix is right--perhaps there should be mention
of how a client tests whether state was transferred in the 4.0 case, if
they do something other than just continuing to use the same stateid's
and seeing if they get an error.

--b.

commit 8d692edbd6f6cafe59c207ec709544e9f60c295d
Author: J. Bruce Fields <[email-masked]>
Date:   Sun Aug 28 18:01:53 2011 -0400

    Omit mention of sessions from migration discussion

diff --git a/nfsv4_middle_mars.xml b/nfsv4_middle_mars.xml
index 7761a2a..b6269ac 100644
--- a/nfsv4_middle_mars.xml
+++ b/nfsv4_middle_mars.xml
@@ -739,10 +739,7 @@
       <t>
         This state transfer will reduce disruption to the client
         when a file system transition occurs.  If the servers are successful in
-        transferring all state, the client can attempt to establish
-        sessions associated with the client ID used for the source
-        file system instance.  If the server accepts that as a valid
-        client ID, then the client may use the existing stateids
+        transferring all state, then the client may use the existing stateids
         associated with that client ID for the old file system instance
         in connection with that same client ID in connection with
         the transitioned file system instance.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 18:09:36 -0400

> OK, I attempted that and the following patches fix most of the issues I

One remaining issue I was confused about:

     The server however should not try to enforce these ordering rules and
     should use whatever information is available to properly process I/O
     requests.  In particular, when a client has a delegation for a given
     file, it SHOULD take note of this fact in processing a request, even
     if it is sent with a special stateid.
  
  9.1.3.6. Stateid Use for SETATTR Operations

     In the case of SETATTR operations, a stateid is present.  In cases
     other than those that set the file size, the client may send either a
     special stateid or, when a delegation is held for the file in
     question, a delegation stateid.  While the server SHOULD validate the
     stateid and may use the stateid to optimize the determination as to
     whether a delegation is held, it SHOULD note the presence of a
     delegation even when a special stateid is sent, and MUST accept a
     valid delegation stateid when sent.

The text assumes you can map operations containing special stateids to clients.
That's because it's assuming the operation came in a session that maps to a
client.  So this is clearly wrong for 4.0.

I'm not sure how to fix it.  Why are special stateid's and delegation stateid's
the only cases considered in the second paragraph?  Are lock or open stateid's
not allowed in the non-size-setting setattr case?

--b.

---

From: [email-masked]
Date: Sun, 28 Aug 2011 18:04:39 -0400

I replaced this by a paragraph stating the opposite.
---
 nfsv4_middle_file_locking.xml |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)

       <t>

---

From: [email-masked]
Date: Sun, 28 Aug 2011 18:04:41 -0400

---
 nfsv4_middle_file_locking.xml |   49 ++--------------------------------------
 1 files changed, 3 insertions(+), 46 deletions(-)

diff --git a/nfsv4_middle_file_locking.xml b/nfsv4_middle_file_locking.xml
index c36d1d1..e528ed1 100644
--- a/nfsv4_middle_file_locking.xml
+++ b/nfsv4_middle_file_locking.xml
@@ -486,37 +486,6 @@
               access, even if access would normally be
               denied to READ requests.
             </t>
-
-            <t>
-              When "other" is zero and "seqid" is one,
-              the stateid represents the current stateid,
-              which is whatever value is the last stateid
-              returned by an operation within the COMPOUND.
-              In the case of an OPEN, the stateid returned
-              for the open file, and not the delegation is
-              used.  The stateid passed to the operation in
-              place of the special value has its "seqid"
-              value set to zero, except when the current
-              stateid is used by the operation CLOSE or
-              OPEN_DOWNGRADE.  If there is no operation
-              in the COMPOUND which has returned a stateid
-              value, the server MUST return the error
-              NFS4ERR_BAD_STATEID. As illustrated  in <xref
-              target="csid_example4"/>, if the value of a
-              current stateid is a special stateid, and the
-              stateid of an operation's arguments has
-              "other" set to zero, and "seqid" set to one,
-              then the server MUST return the error
-              NFS4ERR_BAD_STATEID.
-            </t>
-
-            <t>
-              When "other" is zero and "seqid" is NFS4_UINT32_MAX,
-              the stateid represents a reserved stateid
-              value defined to be invalid.  When this
-              stateid is used, the server MUST return the error
-              NFS4ERR_BAD_STATEID.
-            </t>
           </list>
         </t>
 
@@ -529,13 +498,7 @@
         <t>
           Special stateids, unlike other stateids, are not associated with
           individual client IDs or filehandles and can be used with all valid
-          client IDs and filehandles.  In the case of a special
-          stateid designating the current stateid, the current stateid
-          value substituted for the special stateid is associated with a
-          particular client ID and filehandle, and so, if it is used where
-          current filehandle does not match that associated with the current
-          stateid, the operation to which the stateid is passed will return
-          NFS4ERR_BAD_STATEID.
+          client IDs and filehandles.
         </t>
       </section>
 
@@ -630,12 +593,6 @@
               NFS4ERR_BAD_STATEID is returned.
             </t>
             <t>
-              If the special stateid is one designating the current
-              stateid, and there is a current stateid, then the current
-              stateid is substituted for the special stateid and the
-              checks appropriate to non-special stateids in performed.
-            </t>
-            <t>
               If the combination is valid in general but is not
               appropriate to the context in which the stateid is used
               (e.g. an all-zero stateid is used when an open stateid
@@ -696,8 +653,8 @@
               current "other" field, return NFS4ERR_BAD_STATEID.
             </t>
             <t>
-              If the "seqid" field is not zero, and it is less
-              than the current sequence value corresponding the
+              If the "seqid" field is less than
+              the current sequence value corresponding the
               current "other" field, return NFS4ERR_OLD_STATEID.
             </t>
             <t>
-- 
1.7.6

---

From: [email-masked]
Date: Sun, 28 Aug 2011 18:04:42 -0400

---
 nfsv4_middle_file_locking.xml |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/nfsv4_middle_file_locking.xml b/nfsv4_middle_file_locking.xml
index e528ed1..857eb59 100644
--- a/nfsv4_middle_file_locking.xml
+++ b/nfsv4_middle_file_locking.xml
@@ -417,8 +417,8 @@
           identifying the specific set of locks and a 32-bit "seqid" sequence
           value.  Except in the case of special stateids (see <xref target="ss:fl:special" />), a
           particular value of the "other" field denotes a set of locks of the
-          same type (for example, byte-range locks, opens, delegations, or
-          layouts), for a specific file or directory, and sharing the same
+          same type (for example, byte-range locks, opens, or delegations),
+          for a specific file or directory, and sharing the same
           ownership characteristics.  The seqid designates a specific instance
           of such a set of locks, and is incremented to indicate changes in
           such a set of locks, either by the addition or deletion of locks from
-- 
1.7.6

---

From: "J. Bruce Fields" <[email-masked]>
Date: Sun, 28 Aug 2011 18:23:30 -0400

> 	- special stateid types newly defined in 5661 are included.

The introduction suggests this is intentional:

	-  The definition of stateid was strengthened, which had the
	   side effect of introducing a semantic change in a COMPOUND
	   structure having a current stateid and a saved stateid.

But that can't be correct; a bis shouldn't require a server to implement
an entirely new feature just to stay compliant.

--b.

---

From: Thomas Haynes <[email-masked]>
Date: Mon, 29 Aug 2011 09:56:53 -0700

> convention here and I don't think anything was specified in 3530.

This latter point was why we brought that discussion into 3530bis.

---

From: Thomas Haynes <[email-masked]>
Date: Mon, 29 Aug 2011 10:11:48 -0700

> +++ b/nfsv4_middle_file_locking.xml

Why add a 't' at the end?

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 29 Aug 2011 13:25:59 -0400

> Why add a 't' at the end?

It's just such a good letter, you know?  You can never have too many
t's.

--b.

---

From: Thomas Haynes <[email-masked]>
Date: Mon, 29 Aug 2011 10:13:09 -0700

> +++ b/nfsv4_middle_file_locking.xml

e.g.,

---

From: Thomas Haynes <[email-masked]>
Date: Mon, 29 Aug 2011 10:26:02 -0700

> an entirely new feature just to stay compliant.

No over the wire changes, so this would be okay. :->

Why was this change pulled back?

From the history and task list, I see this:

20) Explanatory Text on Stateid Contradicts Self and Rest of Protocol

See the thread beginning
[http://www1.ietf.org/mail-archive/web/nfsv4/current/msg03159.html]
and particularly the message:
[http://www1.ietf.org/mail-archive/web/nfsv4/current/msg03193.html]
for details.

Action: Pulled back from 5661
Owner: Tom
State: Finished
Target: IETF 80

Okay, that does not require that we introduce the changes you are proposing we
pull out.

Which suggests we would also have to pull out everything after the comma in the
Introduction point you quoted above?

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 29 Aug 2011 13:59:44 -0400

> This latter point was why we brought that discussion into 3530bis.

OK.  There's still a little overkill there:

	"... When such a set of locks is first created, the server
	returns a stateid with seqid value of one."

That's in 5661, I think, partly to allow for a larger space of special
stateid's by ensuring seqid 0 is reserved.  I'm not sure we really care
in 4.0.

In any case, if we do want that, then we need to remove "The starting
value of the seqid field is undefined." from 2.2.16 (the description of
stateid4).

--b.

---

From: "J. Bruce Fields" <[email-masked]>
Date: Mon, 29 Aug 2011 14:04:38 -0400

> Introduction point you quoted above?

Sounds good.

--b.