Subject: [nfsv4] [PATCH 1/4] COPY: fix up language assuming COPY could create a file

From: Christoph Hellwig <[email-masked]>
Date: Thu, 19 Mar 2015 17:57:36 +0100

Signed-off-by: Christoph Hellwig <[email-masked]>
---
 nfsv42_middle_op_copy.xml | 47 +++++------------------------------------------
 1 file changed, 5 insertions(+), 42 deletions(-)

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index d623b98..47f592a 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -32,13 +32,14 @@
       The COPY operation is used for both intra-server and inter-server
       copies. In both cases, the COPY is always sent from the client
       to the destination server of the file copy. The COPY operation
-      requests that a file be copied from the location specified by the
-      SAVED_FH value to the location specified by the CURRENT_FH.
+      requests that a range in the file specified by SAVED_FH
+      is copied to a range in the file specified by CURRENT_FH.
     </t>
 
     <t>
-      The SAVED_FH must be a regular file. If SAVED_FH is not a
-      regular file, the operation MUST fail and return NFS4ERR_WRONG_TYPE.
+      Both SAVED_FH and CURRENT_FH must be regular files. If SAVED_FH or
+      CURRENT_FH is not a regular file, the operation MUST fail and
+      return NFS4ERR_WRONG_TYPE.
     </t>
 
     <t>
@@ -88,17 +89,6 @@
     </t>
 
     <t>
-      The CURRENT_FH specifies the destination of the copy operation.
-      The CURRENT_FH MUST be a regular file and not a directory.
-      Note, the file MUST exist before the COPY operation begins.
-      It is the responsibility of the client to create the file if
-      necessary, regardless of the actual copy protocol used.  If
-      the file cannot be created in the destination file system
-      (due to file name restrictions, such as case or length), the
-      COPY operation MUST NOT be called.
-    </t>
-
-    <t>
       The ca_src_offset is the offset within the source file from
       which the data will be read, the ca_dst_offset is the offset
       within the destination file to which the data will be written,
@@ -198,33 +188,6 @@
     </t>
 
     <t>
-      The copying of any and all attributes on the source file is
-      the responsibility of both the client and the copy protocol.
-      Any attribute which is both exposed via the NFS protocol on
-      the source file and set SHOULD be copied to the destination file.
-      Any attribute supported by the destination server that is not
-      set on the source file SHOULD be left unset. If the client
-      cannot copy an attribute from the source to destination, it
-      MAY fail the copy transaction.
-    </t>
-
-    <t>
-      Metadata attributes not exposed via the NFS protocol SHOULD
-      be copied to the destination file where appropriate via the
-      copy protocol. Note that if the copy protocol is NFSv4.x, then
-      these attributes will be lost.
-    </t>
-
-    <t>
-      The destination file's named attributes are not duplicated
-      from the source file. After the copy process completes, the
-      client MAY attempt to duplicate named attributes using standard
-      NFSv4 operations. However, the destination file's named
-      attribute capabilities MAY be different from the source
-      file's named attribute capabilities.
-    </t>
-
-    <t>
       If the operation does not result in an immediate failure, the
       server will return NFS4_OK, and the CURRENT_FH will remain
       the destination's filehandle.
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Thu, 19 Mar 2015 17:57:51 +0100

There is no language indicating it could change, so mentioning
it can't change for a specific case seems pointless.

Signed-off-by: Christoph Hellwig <[email-masked]>
---
 nfsv42_middle_op_copy.xml | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index 47f592a..f8c2f80 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -189,8 +189,7 @@
 
     <t>
       If the operation does not result in an immediate failure, the
-      server will return NFS4_OK, and the CURRENT_FH will remain
-      the destination's filehandle.
+      server will return NFS4_OK.
     </t>
 
     <t>
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Thu, 19 Mar 2015 17:58:13 +0100

I can't see anything else but copy using this wording in NFSv4.1 or 4.2,
and my last changes got rid of all other occurances.

Signed-off-by: Christoph Hellwig <[email-masked]>
---
 nfsv42_middle_op_copy.xml | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index f8c2f80..c55eaa4 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -69,7 +69,7 @@
       If a server supports the inter-server-to-server COPY feature, a
       PUTFH followed by a SAVEFH MUST NOT return NFS4ERR_STALE for
       either operation. These restrictions do not pose substantial
-      difficulties for servers. The CURRENT_FH and SAVED_FH may be
+      difficulties for servers. CURRENT_FH and SAVED_FH may be
       validated in the context of the operation referencing them
       and an NFS4ERR_STALE error returned for an invalid file handle
       at that point.
-- 
1.9.1

---

From: Christoph Hellwig <[email-masked]>
Date: Thu, 19 Mar 2015 17:58:32 +0100

Without a valid src stateid the server must perform an implicit
open, which would not be able to respect locking or delegation.

Additionally at least the Linux server requires the stateid to
easily lookup the file object, as demonstrated by the COPY
prototype implementation which would fail to copy with an invalid
src stateid.

Signed-off-by: Christoph Hellwig <[email-masked]>
---
 nfsv42_middle_op_copy.xml | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index c55eaa4..7dec12d 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -76,15 +76,15 @@
     </t>
 
     <t>
-      For an inter-server copy, the ca_dst_stateid MUST refer to
-      either delegation, locking, or open states provided earlier
-      by the server (see <xref target="ss:copy:lock" />). The order
-      of selection is explained in Section 8.2.5 of
-      <xref target='RFC5661' />.  And the ca_src_stateid MUST be
-      the cnr_stateid returned from the earlier COPY_NOTIFY.  If
-      either stateid is invalid, then the operation MUST fail.  If
-      the request is for an intra-server copy, then the ca_src_stateid
-      can be ignored.  If ca_dst_stateid is invalid, then the
+      The ca_dst_stateid MUST refer to either delegation, locking,
+      or open states provided earlier by the server (see
+      <xref target="ss:copy:lock" />). The order of selection is
+      explained in Section 8.2.5 of <xref target='RFC5661' />.
+      For an inter-server copy, the ca_src_stateid MUST be the
+      cnr_stateid returned from the earlier COPY_NOTIFY operation,
+      while for an intra-server copy ca_src_stateid MUST also
+      refer to either delegation, locking, or open states provided
+      earlier by the server. If either stateid is invalid, then the
       operation MUST fail.
     </t>
 
-- 
1.9.1

---

From: Tom Haynes <[email-masked]>
Date: Thu, 19 Mar 2015 10:27:49 -0700

> On Mar 19, 2015, at 9:57 AM, Christoph Hellwig <[email-masked]> wrote:
> 

Hi Christoph,

I’ve applied the proposed changes.

Note: I did tweak the following:

Both SAVED_FH and CURRENT_FH must be regular files. If SAVED_FH or
CURRENT_FH is not a regular file, the operation MUST fail and
return NFS4ERR_WRONG_TYPE.

to:

      Both SAVED_FH and CURRENT_FH must be regular files. If neither SAVED_FH nor
      CURRENT_FH are regular files, the operation MUST fail and 
      return NFS4ERR_WRONG_TYPE.

Thanks for the updates.
Tom

---

From: David Noveck <[email-masked]>
Date: Thu, 19 Mar 2015 14:20:23 -0400

>      If neither SAVED_FH nor

As written, this seems to say that if one of {SAVED,CURRENT}_FH is
not a regular file, the operation doesn't need to fail. I don't think
that's right.

I could bring up the issue of the possible use of NFS4ERR_{ISDIR,SYMLINK}
if one or more of source/destination are syminks or directories, but ..

a) that doesn't make sense
b) the time for  that sort of issue has passed, as it's no longer Friday,
the 13th :-).

On Thu, Mar 19, 2015 at 1:27 PM, Tom Haynes <[email-masked]>
wrote:

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

---

From: Christoph Hellwig <[email-masked]>
Date: Fri, 20 Mar 2015 07:19:35 +0100

> not a regular file, the operation doesn't need to fail. I don't think

It's not.  See a patch to fix it up below.

> b) the time for  that sort of issue has passed, as it's no longer Friday,

I think we could easily add it.  Note that DEALLOCATE, READ_PLUS and
WRITE_SAME are also specified to only work on regular files and would benefit
from coherent and precise wording common to all operations.

ALLOCATE probably should also have this language, but currently doesn't.

---

From: Christoph Hellwig <[email-masked]>
Date: Fri, 20 Mar 2015 07:20:00 +0100

diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
index 057201a..bcaf752 100644
--- a/nfsv42_middle_op_copy.xml
+++ b/nfsv42_middle_op_copy.xml
@@ -37,9 +37,9 @@
     </t>
 
     <t>
-      Both SAVED_FH and CURRENT_FH must be regular files. If neither SAVED_FH nor
-      CURRENT_FH are regular files, the operation MUST fail and
-      return NFS4ERR_WRONG_TYPE.
+      Both SAVED_FH and CURRENT_FH must be regular files. If either
+      SAVED_FH or CURRENT_FH are not regular files, the operation
+      MUST fail and return NFS4ERR_WRONG_TYPE.
     </t>
 
     <t>

---

From: David Noveck <[email-masked]>
Date: Fri, 20 Mar 2015 04:40:18 -0400

> I think we could easily add it.

I don't agree with the "easily" part.  None of the operations you mention
operate on two files and I just don't see the benefit of making the spec
explcitly address the issue of the error to return when copying a directory
into a symlink, or the reverse.