Subject: [nfsv4] [PATCH] Clarify use of pNFS and stateids

From: Thomas Haynes <[email-masked]>
Date: Fri, 25 Apr 2014 15:49:24 -0700

From: loghyr <[email-masked]>

This is item #3 in the issues file. I scoured the mailing list
to pick up the thread in November.

As suggested on the WG ML, I've moved ALLOCATE and DEALLOCATE
out of the allowed DS operations. I've also explained that
these need to be atomic from the MDS.

Finally, I've clarified that the exeception to rule #16 is
a NFSv4.2 MDS working with a downrev DS.

Signed-off-by: Thomas Haynes <[email-masked]>
Suggested-by: Christoph Hellwig <[email-masked]>
---
 Makefile                 |  1 +
 nfsv42_middle_minorv.xml |  9 ++++++++-
 nfsv42_middle_pnfs.xml   | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 nfsv42_middle_space.xml  | 35 +++++++++++++++++++++--------------
 4 files changed, 76 insertions(+), 15 deletions(-)
 create mode 100644 nfsv42_middle_pnfs.xml

diff --git a/Makefile b/Makefile
index 9ba2101..1139b93 100755
--- a/Makefile
+++ b/Makefile
@@ -428,6 +428,7 @@ IDXMLSRC_BASE = \
 	${DOC_PREFIX}_middle_start.xml \
 	${DOC_PREFIX}_middle_introduction.xml \
 	${DOC_PREFIX}_middle_minorv.xml \
+	${DOC_PREFIX}_middle_pnfs.xml \
 	${DOC_PREFIX}_middle_copy.xml \
 	${DOC_PREFIX}_middle_advise.xml \
 	${DOC_PREFIX}_middle_sparse.xml \
diff --git a/nfsv42_middle_minorv.xml b/nfsv42_middle_minorv.xml
index 064ec47..a25813d 100644
--- a/nfsv42_middle_minorv.xml
+++ b/nfsv42_middle_minorv.xml
@@ -209,10 +209,17 @@
       </t>
 
       <t>
-        A client MUST NOT attempt to use a stateid, filehandle, or
+        A client SHOULD NOT attempt to use a stateid, filehandle, or
         similar returned object from the COMPOUND procedure with minor
         version X for another COMPOUND procedure with minor version Y,
         where X != Y.
+          <vspace blankLines="1"/>
+        Note that this is a "SHOULD" and not a "MUST" because of the
+        possibility that a NFSv4.2 metadata server could hand out a
+        layout to a NFSv4.1 storage device. Section 13.9.1 of
+        <xref target='RFC5661' /> discusses how the client gets
+        a stateid from the metadata server to present to a storage
+        device.
       </t>
     </list>
   </t>
diff --git a/nfsv42_middle_pnfs.xml b/nfsv42_middle_pnfs.xml
new file mode 100644
index 0000000..7fde106
--- /dev/null
+++ b/nfsv42_middle_pnfs.xml
@@ -0,0 +1,46 @@
+<!-- Copyright (C) The IETF Trust (2014) -->
+<!-- Copyright (C) The Internet Society (2014) -->
+
+<section anchor="ss:pnfs" title="pNFS considerations for New Operations">
+  <section anchor="ss:pnfs:atom" title="Atomicty for ALLOCATE and DEALLOCATE">
+    <t>
+      Both ALLOCATE (see <xref target='op:allocate' />) and
+      DEALLOCATE (see <xref target='op:deallocate' />) are sent to the
+      metadata server, which is responsible for coordinating the
+      changes onto the storage devices. In particular, both operations
+      must either fully succeed or fail, it cannot be the case that
+      one storage device succeeds whilst another fails.
+    </t>
+  </section>
+
+  <section anchor="ss:pnfs:flt" title="NFSv4.2 as a Storage Protocol in pNFS: the File Layout Type">
+    <t>
+      A file layout provided by a NFSv4.2 server may refer either to a DS
+      that only implements NFSv4.1 as specified in <xref target='RFC5661' />,
+      or to a DS that implements additions from NFSv4.2, in which
+      case the rules in <xref target='ss:pnfs:ops' /> apply. As the File
+      Layout Type does not provide a means for informing the client as
+      to which minor version a particular DS is providing, it will have
+      to negotiate this via the normal RPC semantics of major and minor
+      version discovery.
+    </t>
+
+    <section anchor="ss:pnfs:ops" title="Operations Sent to NFSv4.2 Data Servers">
+      <t>
+        In addition to the commands listed in <xref target='RFC5661' />,
+        NFSv4.2 data servers MAY accept a COMPOUND containing the following
+        additional operations: READ_PLUS (see
+        <xref target='op:read_plus' />), WRITE_SAME (see
+        <xref target='op:write_same' />), and SEEK (see
+        <xref target='op:seek' />), which will be treated like the
+        subset specified as "Operations Sent to NFSv4.1 Data Servers"
+        in Section 13.6 of <xref target='RFC5661' />.
+      </t>
+
+      <t>
+        Additional details on the implementation of these operations in a pNFS
+        context are documented in the operation specific sections.
+      </t>
+    </section>
+  </section>
+</section>
diff --git a/nfsv42_middle_space.xml b/nfsv42_middle_space.xml
index 22df40d..63be968 100644
--- a/nfsv42_middle_space.xml
+++ b/nfsv42_middle_space.xml
@@ -83,7 +83,7 @@
 
     <t>
       The following operations and attributes can be used to resolve
-      this issues:
+      these issues:
     </t>
 
     <t>
@@ -93,14 +93,20 @@
           deleted, taking block sharing into consideration.
         </t>
 
-        <t hangText="WRITE_HOLE">
-          This operation zeros and/or deallocates the blocks
-          backing a region of the file.
+        <t hangText="DEALLOCATE">
+          This operation delallocates the blocks backing a
+          region of the file.
+        </t>
+
+        <t hangText="WRITE_SAME">
+          This operation zeros the blocks backing a region of the
+          file. It does not deallocate the blocks, so it does not
+          reduce the space reservation.
         </t>
 
-	<t hangText="READ_PLUS or SEEK">
+        <t hangText="READ_PLUS or SEEK">
           These operations can return the reservation status of blocks
-	  backing a region of the file.
+          backing a region of the file.
         </t>
       </list>
     </t>
@@ -146,18 +152,19 @@
       over-reported.  However, over-reporting is better than under-reporting.
     </t>
   </section>
+
   <section anchor='ss:space:info' title='Space Reservation Information'>
   <?rfc include='autogen/type_space_info4.xml'?>
       <t>
         The space_info4 type is used to report the reservation space
-	in the READ_PLUS (see <xref target='op:read_plus' />) and SEEK
-	(see <xref target='op:seek' /> operations to report the
-	reservation state of a data block or hole.  A data block or
-	hole may either be reserved, that means a future write MUST not
-	return NFS4ERR_NOSPC, or it might be unreserved in which case
-	a future write may return NFS4ERR_NOSPC.  Servers may not know
-	the allocation state, in which case it might return unknown
-	in the space_info fields in the READ_PLUS and SEEK return values.
+        in the READ_PLUS (see <xref target='op:read_plus' />) and SEEK
+        (see <xref target='op:seek' /> operations to report the
+        reservation state of a data block or hole.  A data block or
+        hole may either be reserved, that means a future write MUST not
+        return NFS4ERR_NOSPC, or it might be unreserved in which case
+        a future write may return NFS4ERR_NOSPC.  Servers may not know
+        the allocation state, in which case it might return unknown
+        in the space_info fields in the READ_PLUS and SEEK return values.
       </t>
   </section>
 </section>
-- 
1.9.0

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 28 Apr 2014 06:58:22 +0200

> these need to be atomic from the MDS.

This part looks good to me.

> a NFSv4.2 MDS working with a downrev DS.

I understand that point, but suggesting that stateids aren't reusable
over minor versions just to say that in the only case where this applies
for far seems odd.  Maybe the standard should just say that from 4.1 to 4.2
they are, but that no such guarantee exists for future minor versions and
each standards document will have explicitly state the compatibility?

---

From: Thomas Haynes <[email-masked]>
Date: Sun, 27 Apr 2014 22:07:36 -0700

> each standards document will have explicitly state the compatibility?

How about these changes:

        Unless explicitly documented in a minor version standard's document,
        a client MUST NOT attempt to use a stateid, filehandle, or
        similar returned object from the COMPOUND procedure with minor
        version X for another COMPOUND procedure with minor version Y,
        where X != Y.

And then push this:

        A NFSv4.2 metadata server could hand out a
        layout to a NFSv4.1 storage device. Section 13.9.1 of
        <xref target='RFC5661' /> discusses how the client gets
        a stateid from the metadata server to present to a storage
        device.

into the next section, the one on pNFS.

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 28 Apr 2014 07:09:20 +0200

> into the next section, the one on pNFS.

This sounds good to me!

---

From: Thomas Haynes <[email-masked]>
Date: Wed, 30 Apr 2014 11:25:08 -0700

Expanding the audience:

On Apr 30, 2014, at 3:41 AM, Christoph Hellwig <[email-masked]> wrote:

> On Mon, Apr 28, 2014 at 05:14:30PM -0700, Tom Haynes wrote:
>> 
>> On Apr 27, 2014, at 10:10 PM, Christoph Hellwig <[email-masked]> wrote:
>> 
>>> 
>>> All the high-level bits look good to me, but I'd love to see a bit
>>> more attention to the COPY failure behavior.
>> 
>> In what sense?
> 
> I can't find any place in 15.2. that says what happens on an error.
> 
> There's a couple interesting case:
> 
> 1) a COPY that does not extend the file size
> 
>  What parts of the region COPYed to are clobbered after a failed operation?
> 
>  For a consecutive COPY I would expect the behavior to be the same as
>  WRITE: N bytes are written, those have the new data, everything else is
>  unmodified.
> 
>  But what happens for a non-consecutive?  Should all of the region
>  be assumed to be clobbered and may contain random sub-regions of the
>  old and new data?

Unfortunately, yes.

> 
> 2) a COPY that does extend the file size:
> 
>  In addition to the above considerations what will the file size after
>  a failed COPY be?
> 
>  Writes in posix file systems make sure that the visible file size is only
>  extended to the offset that now contains data, usually by truncating
>  the file on a failed extending write.  If we want to implement this
>  for COPY how do we exclude other writes into the same region beyond
>  the file size?

Doesn't the lock protect us here?

> 
>  Should non-consecutive COPYs that span the file size extent the file
>  size at all in the error case, and if yes to the full size of the COPY
>  or to the last offset that contains copied data.  What will subregions
>  that haven't been copied but are lower in the file than a copied subregion
>  contain?

Were the sub-regions to be copied or not?

If not, then the normal semantics apply.

> 
>  For asynchronous COPYs that fails beyond the original file size, do
>  we allow a larger file size to be seen by other clients during the
>  operation even if it has to be truncated in the failure case?
> 

I think we need to craft responses based on consecutive and
non-consecutive copies. I think the client and destination should
both be aware of what type of copy is being done for this.

In the non-consecutive case, all bets are off. We basically have
to assume that a series of random writes occurred to the range
and let the client redrive fixing things if possible.

For extending, since we do not have a way to rollback, other
clients can see the larger file size.

For consecutive copies, I think we can safely fall back on the
semantics of a normal client-based copy. It is basically an
ordered sequence of writes and we get stuck with what happens
on the last successful one.