Subject: [nfsv4] [PATCH 1/4] misc CLONE updates

From: Christoph Hellwig <[email-masked]>
Date: Sat, 25 Apr 2015 15:33:00 +0200

Based on the revіew from David Noveck
---
 nfsv42_middle_op_clone.xml | 73 +++++++++++++++++++++++-----------------------
 1 file changed, 37 insertions(+), 36 deletions(-)

diff --git a/nfsv42_middle_op_clone.xml b/nfsv42_middle_op_clone.xml
index f93e78d..72ce4ab 100644
--- a/nfsv42_middle_op_clone.xml
+++ b/nfsv42_middle_op_clone.xml
@@ -1,7 +1,8 @@
 <!-- Copyright (C) The IETF Trust (2011-2014) -->
 <!-- Copyright (C) The Internet Society (2011-2014) -->
 
-<section anchor="op:clone" title="Operation 71: CLONE - Clone a file">
+<section anchor="op:clone"
+	 title="Operation 71: CLONE - Clone a range of file into another file">
   <section toc='exclude' title="ARGUMENT">
     <t>
       &lt;CODE BEGINS&gt;
@@ -30,36 +31,23 @@
     <t>
       The CLONE operation is used to clone file content from a source
       file specified by the SAVED_FH value into a destination file
-      specified by CURRENT_FH without actually copying the data, e.g.,
-      by using a copy on write mechanism.
+      specified by CURRENT_FH without actually copying the data, e.g.
+      by using a copy-on-write mechanism.
     </t>
 
     <t>
       Both SAVED_FH and CURRENT_FH must be regular files. If either
-      SAVED_FH or CURRENT_FH are not regular files, the operation
+      SAVED_FH or CURRENT_FH is not a regular files, the operation
       MUST fail and return NFS4ERR_WRONG_TYPE.
     </t>
 
     <t>
       SAVED_FH and CURRENT_FH must be different files.  If SAVED_FH and
-      CURRENT_FH refer to the same file, the operation will fail with
+      CURRENT_FH refer to the same file, the operation MUST fail with
       NFS4ERR_INVAL.
     </t>
 
     <t>
-      In order to set SAVED_FH to the source file handle, the
-      compound procedure requesting the CLONE will include a
-      sub-sequence of operations such as
-    </t>
-
-    <figure>
-      <artwork>
-   PUTFH source-fh
-   SAVEFH
-      </artwork>
-    </figure>
-
-    <t>
       The ca_dst_stateid MUST refer to a stateid that is valid for
       a WRITE operation and follows the rules for stateids in
       Sections 8.2.5 and 18.32.3 of <xref target='RFC5661' />.
@@ -71,36 +59,49 @@
     </t>
 
     <t>
-      The cl_src_offset is the offset within the source file from
-      which the data will be read, the cl_dst_offset is the offset
-      within the destination file to which the data will be written,
-      and the cl_count is the number of bytes that will be cloned.
-      An offset of 0 (zero) specifies the start of the file. A count
-      of 0 (zero) requests that all bytes from ca_src_offset through
-      EOF be cloned to the destination. Both cl_src_offset and
-      cl_dst_offset must be aligned to the clone block size
-      <xref target='ss:fattr:clone' />.  cl_count must be aligned to
-      the clone block size, unless cl_src_offset + cl_count are
-      equal to the source file size.
+      The cl_src_offset is the starting offset within the source file from which
+      the data to be cloned will be obtained and the cl_dst_offset is the
+      starting offset of the target region into which the cloned data will be
+      placed. An offset of 0 (zero) indicates the start of the respective file.
+      The number of bytes to be cloned is obtained from cl_count, except that a
+      cl_count of 0 (zero) indicates that the number of bytes to be cloned is the
+      count of bytes between cl_src_offset and the EOF of the source file. Both
+      cl_src_offset and cl_dst_offset must be aligned to the clone block size
+      <xref target='ss:fattr:clone' />. The number of bytes to be cloned must be a
+      multiple of the clone block size, except in the case in which cl_src_offset
+      plus the number of bytes to be cloned is equal to the source file size.
     </t>
 
     <t>
       If the source offset or the source offset plus count is greater
-      than the size of the source file, the operation will fail with
+      than the size of the source file, the operation MUST fail with
       NFS4ERR_INVAL. The destination offset or destination offset plus
       count may be greater than the size of the destination file.
     </t>
 
     <t>
-      If the CLONE wrote past the end of the file on the destination, then the
-      last byte written to will determine the new file size. The contents of
-      any block not written to and past the original size of the file will be
-      as if a normal WRITE extended the file.
+      If the target area of the clone operation ends beyond the end of the
+      destination file, the offset at the end of the target area will determine
+      the new size of the destination file.  The contents of any block not part
+      of the target area will be the same as if the file size were extended by a
+      WRITE.
+    </t>
+
+    <t>
+      If the area to be cloned is not a multiple of the clone block size and the
+      size of the destination file is past the end of the target area, the area
+      between the end of the target area and the next multiple o the clone block
+      size wlll be zeroed.
     </t>
 
     <t>
-      The CLONE operation is atomic, that is either all changes or
-      no changes are seen by the client or other clients.
+      The CLONE operation is atomic in that other operations may not see see any
+      intermediate states between the state of the two files before the operation
+      and that after the operation.    READs of the destination file will never
+      see some blocks of the target area cloned without all of them being
+      cloned.  WRITEs of the source area will either have no effect on the data
+      of the target file or be fully reflected in the target area of the
+      destination file.
     </t>
 
     <t>
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Sat, 25 Apr 2015 15:33:14 +0200

Based on the CLONE revіew from David Noveck
---
 nfsv42_middle_op_copy.xml | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index cfab1c0..2ca640f 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -38,30 +38,17 @@
 
     <t>
       Both SAVED_FH and CURRENT_FH must be regular files. If either
-      SAVED_FH or CURRENT_FH are not regular files, the operation
+      SAVED_FH or CURRENT_FH is not a regular file, the operation
       MUST fail and return NFS4ERR_WRONG_TYPE.
     </t>
 
     <t>
       SAVED_FH and CURRENT_FH must be different files.  If SAVED_FH and
-      CURRENT_FH refer to the same file, the operation will fail
+      CURRENT_FH refer to the same file, the operation MUST fail
       with NFS4ERR_INVAL.
     </t>
 
     <t>
-      In order to set SAVED_FH to the source file handle, the
-      compound procedure requesting the COPY will include a
-      sub-sequence of operations such as
-    </t>
-
-    <figure>
-      <artwork>
-   PUTFH source-fh
-   SAVEFH
-      </artwork>
-    </figure>
-
-    <t>
       If the request is for an inter-server-to-server copy, the source-fh
       is a filehandle from the source server and the compound
       procedure is being executed on the destination server. In
@@ -118,7 +105,7 @@
     <t>
       If the source offset or the source offset plus count is greater
       than the size of the source file, the operation
-      will fail with NFS4ERR_INVAL. The destination offset or
+      MUST fail with NFS4ERR_INVAL. The destination offset or
       destination offset plus count may be greater than the size
       of the destination file. This allows for the client to issue
       parallel copies to implement operations such as
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Sat, 25 Apr 2015 15:33:28 +0200

As pointed out by Trond.
---
 nfsv42_middle_errors.xml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/nfsv42_middle_errors.xml b/nfsv42_middle_errors.xml
index cd2cd68..090c197 100644
--- a/nfsv42_middle_errors.xml
+++ b/nfsv42_middle_errors.xml
@@ -1422,6 +1422,7 @@
         NFS4ERR_DEADSESSION,
         NFS4ERR_DELAY,
         NFS4ERR_DELEG_REVOKED,
+        NFS4ERR_DQUOT,
         NFS4ERR_EXPIRED,
         NFS4ERR_FBIG,
         NFS4ERR_FHEXPIRED,
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Sat, 25 Apr 2015 15:33:43 +0200

As pointed out by Trond.
---
 nfsv42_middle_new_errors.xml | 1 -
 1 file changed, 1 deletion(-)

diff --git a/nfsv42_middle_new_errors.xml b/nfsv42_middle_new_errors.xml
index f0203bf..770e455 100644
--- a/nfsv42_middle_new_errors.xml
+++ b/nfsv42_middle_new_errors.xml
@@ -212,7 +212,6 @@
         NFS4ERR_IO,
         NFS4ERR_ISDIR,
         NFS4ERR_LOCKED,
-        NFS4ERR_METADATA_NOTSUPP,
         NFS4ERR_MOVED,
         NFS4ERR_NOFILEHANDLE,
         NFS4ERR_NOSPC,
-- 
1.9.1

---

From: Thomas Haynes <[email-masked]>
Date: Sat, 25 Apr 2015 15:11:34 -0700

applied

---

From: Christoph Hellwig <[email-masked]>
Date: Mon, 27 Apr 2015 15:36:29 +0200

> applied

Here is another fix, NFS4ERR_DQUOT also needs to be added to the second
error table.

diff --git a/nfsv42_middle_new_errors.xml b/nfsv42_middle_new_errors.xml
index 770e455..c79e6df 100644
--- a/nfsv42_middle_new_errors.xml
+++ b/nfsv42_middle_new_errors.xml
@@ -168,6 +168,7 @@
         NFS4ERR_DEADSESSION,
         NFS4ERR_DELAY,
         NFS4ERR_DELEG_REVOKED,
+        NFS4ERR_DQUOT,
         NFS4ERR_EXPIRED,
         NFS4ERR_FBIG,
         NFS4ERR_FHEXPIRED,

---

From: Tom Haynes <[email-masked]>
Date: Mon, 27 Apr 2015 13:52:01 -0700

> +++ b/nfsv42_middle_new_errors.xml

Applied - I thought we also agreed to NFS4ERR_XDEV?

We had, but it is not in this list:

jhereg:NFSv4.2 loghyr$ git diff
diff --git a/nfsv42_middle_new_errors.xml b/nfsv42_middle_new_errors.xml
index 770e455..0088fcc 100644
--- a/nfsv42_middle_new_errors.xml
+++ b/nfsv42_middle_new_errors.xml
@@ -168,6 +168,7 @@
         NFS4ERR_DEADSESSION,
         NFS4ERR_DELAY,
         NFS4ERR_DELEG_REVOKED,
+        NFS4ERR_DQUOT,
         NFS4ERR_EXPIRED,
         NFS4ERR_FBIG,
         NFS4ERR_FHEXPIRED,
@@ -191,7 +192,8 @@
         NFS4ERR_STALE,
         NFS4ERR_SYMLINK,
         NFS4ERR_TOO_MANY_OPS,
-        NFS4ERR_WRONG_TYPE
+        NFS4ERR_WRONG_TYPE,
+        NFS4ERR_XDEV
       </c>
 
       <c>COPY</c>

---

From: Benny Halevy <[email-masked]>
Date: Tue, 28 Apr 2015 08:04:18 +0300

We agreed on XDEV for CLONE, not for COPY.
On Apr 27, 2015 23:52, "Tom Haynes" <[email-masked]> wrote:

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

---

From: Christoph Hellwig <[email-masked]>
Date: Tue, 28 Apr 2015 15:45:56 +0200

> We had, but it is not in this list:

Oh.  I wonder if there is some sort of #include statement that would
allow us to keep the error lists in a single place instead of this
error prone duplication.  In fact it would be really useful if the
list of errors could be kept in the same XML file as the operation
defintion.