Subject: Re: [nfsv4] [PATCH] Adding consecutive byte indicator to source server list

From: Thomas Haynes <[email-masked]>
Date: Sun, 20 Apr 2014 23:11:22 -0700

On Apr 20, 2014, at 11:09 PM, Thomas Haynes <[email-masked]> wrote:

> --compose failed.
> 
> This new flag will allow servers to inform clients if a consecutive byte oder copied
> will be done. In turn this allows clients to restart after interruptions.
> 
> Comments?
> 
> On Apr 20, 2014, at 11:07 PM, Thomas Haynes <[email-masked]> wrote:
> 
>> From: loghyr <[email-masked]>
>> 
>> ---
>> Makefile                         |  6 ++++--
>> dotx.d/Makefile                  |  3 ++-
>> dotx.d/copy_args.x               |  2 +-
>> dotx.d/copy_notify_res.x         |  2 +-
>> dotx.d/head.x                    |  2 ++
>> dotx.d/spit_types.sh             | 14 +++++++++++++-
>> nfsv42_middle_copy.xml           |  8 ++++----
>> nfsv42_middle_op_copy.xml        | 22 ++++++++++++++--------
>> nfsv42_middle_op_copy_notify.xml | 19 ++++++++++++-------
>> 9 files changed, 53 insertions(+), 25 deletions(-)
>> 
>> diff --git a/Makefile b/Makefile
>> index 1c04c33..9a73acf 100755
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -174,7 +174,8 @@ SPITGEN =	dotx.d/type_nfstime4.x \
>> 		dotx.d/data4.x \
>> 		dotx.d/data_content4.x \
>> 		dotx.d/stable_how4.x \
>> -		dotx.d/write_response4.x
>> +		dotx.d/write_response4.x \
>> +		dotx.d/type_source_server4.x
>> 
>> SPITGENXML =	autogen/type_nfstime4.xml \
>> 		autogen/type_time_how4.xml \
>> @@ -252,7 +253,8 @@ SPITGENXML =	autogen/type_nfstime4.xml \
>> 		autogen/data4.xml \
>> 		autogen/data_content4.xml \
>> 		autogen/stable_how4.xml \
>> -		autogen/write_response4.xml
>> +		autogen/write_response4.xml \
>> +		autogen/type_source_server4.xml
>> 
>> $(SPITGEN): dotx.d/spit_types.sh
>> 	cd dotx.d ; sh spit_types.sh `basename $@`
>> diff --git a/dotx.d/Makefile b/dotx.d/Makefile
>> index dd1a2b5..524dc6f 100755
>> --- a/dotx.d/Makefile
>> +++ b/dotx.d/Makefile
>> @@ -238,7 +238,8 @@ SPITGEN =	basic_types.x \
>> 		type_chattr_type.x \
>> 		type_label_format.x \
>> 		stable_how4.x \
>> -		write_response4.x
>> +		write_response4.x \
>> +		type_source_server4.x
>> 
>> AUTOGEN =	hand_edit_warning.x \
>> 		fore_ops.x \
>> diff --git a/dotx.d/copy_args.x b/dotx.d/copy_args.x
>> index d2929dd..7c1000b 100644
>> --- a/dotx.d/copy_args.x
>> +++ b/dotx.d/copy_args.x
>> @@ -6,6 +6,6 @@ struct COPY4args {
>> 	offset4		ca_src_offset;
>> 	offset4		ca_dst_offset;
>> 	length4		ca_count;
>> -	netloc4		ca_source_server<>;
>> +	source_server4	ca_copy_source<>;
>> };
>> 
>> diff --git a/dotx.d/copy_notify_res.x b/dotx.d/copy_notify_res.x
>> index e28cbd6..3652510 100644
>> --- a/dotx.d/copy_notify_res.x
>> +++ b/dotx.d/copy_notify_res.x
>> @@ -1,6 +1,6 @@
>> struct COPY_NOTIFY4resok {
>> 	nfstime4	cnr_lease_time;
>> -	netloc4		cnr_source_server<>;
>> +	source_server4	cnr_copy_source<>;
>> };
>> 
>> union COPY_NOTIFY4res switch (nfsstat4 cnr_status) {
>> diff --git a/dotx.d/head.x b/dotx.d/head.x
>> index 2cbe72d..6d16026 100644
>> --- a/dotx.d/head.x
>> +++ b/dotx.d/head.x
>> @@ -413,6 +413,8 @@ include(stable_how4.x)
>> 
>> include(write_response4.x)
>> 
>> +include(type_source_server4.x)
>> +
>> /*
>> * NFSv4.1 attributes
>> */
>> diff --git a/dotx.d/spit_types.sh b/dotx.d/spit_types.sh
>> index 8e3b227..d264d68 100755
>> --- a/dotx.d/spit_types.sh
>> +++ b/dotx.d/spit_types.sh
>> @@ -1342,7 +1342,7 @@ cat << EOF > $i
>> struct copy_to_auth_priv {
>> 	/* equal to cfap_shared_secret */
>> 	secret4              ctap_shared_secret;
>> -	netloc4              ctap_source;
>> +	netloc4              ctap_copy_source;
>> 	/* the NFSv4 user name that the user principal maps to */
>> 	utf8str_mixed        ctap_username;
>> 	/*
>> @@ -1469,6 +1469,18 @@ EOF
>> 
>> 	;;
>> 
>> +	type_source_server4.x )
>> +
>> +cat << EOF > $i
>> +
>> +struct source_server4 {
>> +	netloc4		ss_copy_as;
>> +	bool		ss_bytes_consecutive;
>> +};
>> +EOF
>> +
>> +	;;
>> +
>> 	* )
>> 		echo $0: Error: $i not recognized target.
>> 
>> diff --git a/nfsv42_middle_copy.xml b/nfsv42_middle_copy.xml
>> index d09c99e..10e5e16 100644
>> --- a/nfsv42_middle_copy.xml
>> +++ b/nfsv42_middle_copy.xml
>> @@ -1,5 +1,5 @@
>> -<!-- Copyright (C) The IETF Trust (2011-2013) -->
>> -<!-- Copyright (C) The Internet Society (2011-2013) -->
>> +<!-- Copyright (C) The IETF Trust (2011-2014) -->
>> +<!-- Copyright (C) The Internet Society (2011-2014) -->
>> 
>> <section anchor="sec:copy" title="Server-side Copy">
>>  <section title="Introduction">
>> @@ -828,7 +828,7 @@
>> 
>>            <t>
>>             An instance of copy_to_auth_priv is filled in with the shared
>> -             secret, the cnr_source_server list returned by COPY_NOTIFY, and
>> +             secret, the cnr_copy_source list returned by COPY_NOTIFY, and
>>             the NFSv4 user id of the user principal.
>>             The next four fields are passed in the copy_to_auth privilege
>>             to be used by the copy_confirm_auth rpc_gss3_gss_binding fields
>> @@ -885,7 +885,7 @@
>>        <t>
>>         When the client sends a COPY request to the destination server, it
>>         uses the privileged "copy_to_auth" RPCSEC_GSSv3 handle.
>> -         ca_source_server list in COPY MUST be the same as ctap_source list
>> +         ca_copy_source list in COPY MUST be the same as ctap_copy_source list
>>         specified in copy_to_auth_priv.  Otherwise, COPY will fail
>>         with NFS4ERR_ACCESS.  The destination server verifies that the
>>         privilege &lt;"copy_to_auth", user id, source list, nounce, nounce MIC,
>> diff --git a/nfsv42_middle_op_copy.xml b/nfsv42_middle_op_copy.xml
>> index 81f4736..e7e1690 100644
>> --- a/nfsv42_middle_op_copy.xml
>> +++ b/nfsv42_middle_op_copy.xml
>> @@ -1,8 +1,9 @@
>> -<!-- Copyright (C) The IETF Trust (2011-2013) -->
>> -<!-- Copyright (C) The Internet Society (2011-2013) -->
>> +<!-- Copyright (C) The IETF Trust (2011-2014) -->
>> +<!-- Copyright (C) The Internet Society (2011-2014) -->
>> 
>> <section anchor="op:copy" title="Operation 59: COPY - Initiate a server-side copy">
>>  <section toc='exclude' title="ARGUMENT">
>> +    <?rfc include='autogen/type_source_server4.xml'?>
>>    <?rfc include='autogen/copy_args.xml'?>
>>  </section>
>> 
>> @@ -109,13 +110,13 @@
>>    </t>
>> 
>>    <t>
>> -      If the ca_source_server list is specified, then this is an inter-server
>> +      If the ca_copy_source list is specified, then this is an inter-server
>>      copy operation and the source file is on a remote server.  The client
>>      is expected to have previously issued a successful COPY_NOTIFY request
>> -      to the remote source server. The ca_source_server list MUST be the
>> -      same as the COPY_NOTIFY response's cnr_source_server list. If the client
>> -      includes the entries from the COPY_NOTIFY response's cnr_source_server
>> -      list in the ca_source_server list, the source server can indicate a
>> +      to the remote source server. The ca_copy_source list MUST be the
>> +      same as the COPY_NOTIFY response's cnr_copy_source list. If the client
>> +      includes the entries from the COPY_NOTIFY response's cnr_copy_source
>> +      list in the ca_copy_source list, the source server can indicate a
>>      specific copy protocol for the destination server to use by returning
>>      a URL, which specifies both a protocol service and server name.
>>      Server-to-server copy protocol considerations are described in
>> @@ -160,7 +161,12 @@
>>      If an immediate failure does occur, cr_bytes_copied will be
>>      set to the number of bytes copied to the destination file before
>>      the error occurred. The cr_bytes_copied value indicates the number
>> -      of bytes copied but not which specific bytes have been copied.
>> +      of bytes copied but not which specific bytes have been copied. Note
>> +      that if the ss_bytes_consecutive flag had been sent, then the
>> +      source and destination MUST have copied the bytes consecutively
>> +      from ca_src_offset to cr_bytes_copied. Assuming that the file
>> +      still exists and the permissions still allow the copy, the client
>> +      MUST be able to restart the copy from the new offset of cr_bytes_copied.
>>    </t>
>> 
>>    <t>
>> diff --git a/nfsv42_middle_op_copy_notify.xml b/nfsv42_middle_op_copy_notify.xml
>> index dcca637..df57ab8 100644
>> --- a/nfsv42_middle_op_copy_notify.xml
>> +++ b/nfsv42_middle_op_copy_notify.xml
>> @@ -1,5 +1,5 @@
>> -<!-- Copyright (C) The IETF Trust (2011-2012) -->
>> -<!-- Copyright (C) The Internet Society (2011-2012) -->
>> +<!-- Copyright (C) The IETF Trust (2011-2014) -->
>> +<!-- Copyright (C) The Internet Society (2011-2014) -->
>> 
>> <section anchor="op:copy_notify" title="Operation 61: COPY_NOTIFY - Notify a source server of a future copy">
>>  <section toc='exclude' title="ARGUMENT">
>> @@ -7,6 +7,7 @@
>>  </section>
>> 
>>  <section toc='exclude' title="RESULT">
>> +    <?rfc include='autogen/type_source_server4.xml'?>
>>    <?rfc include='autogen/copy_notify_res.xml'?>
>>  </section>
>> 
>> @@ -68,11 +69,15 @@
>> 
>>    <t>
>>      A successful response will also contain a list of netloc4
>> -      network location formats
>> -      called cnr_source_server, on which the source is willing to
>> -      accept connections from the destination. These might not be reachable
>> -      from the client and might be located on networks to which the client
>> -      has no connection.
>> +      network location formats inside cnr_copy_source, on which
>> +      the source is willing to accept connections from the
>> +      destination. These might not be reachable from the client
>> +      and might be located on networks to which the client
>> +      has no connection. The source can inform the client 
>> +      as to whether the bytes will be copied consecutively
>> +      via the cnr_copy_source.ss_bytes_consecutive flag. This
>> +      flag can be used by the client to restart a copy at a
>> +      different location in case the transaction is interrupted.
>>    </t>
>> 
>>    <t>
>> -- 
>> 1.8.2
>> 
>

---

From: Thomas Haynes <[email-masked]>
Date: Sun, 20 Apr 2014 23:09:27 -0700

--compose failed.

Comments?

---

From: Tom Haynes <[email-masked]>
Date: Mon, 21 Apr 2014 14:21:04 -0700

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

> On Sun, Apr 20, 2014 at 11:09:27PM -0700, Thomas Haynes wrote:
>> --compose failed.
>> 
> This looks useful for me, but one thing this doesn't address is failure
> behavior if the flag is not set.  I think the only sensible failure
> behavior for a non-consecutive copy is that no bytes must appear as
> copied in the filesystem either during a copy or after a failed copy,
> while for a consecutive we could allow it to a) be visible partially
> during the copy, and b) remain in place in the truncated form.
> 

I started to argue that the failure behavior if the flag is not set is what
we currently have in place.

The problem with what I originally proposed is that the client:

1) has no way of influencing the destination to use a contiguous copy or not.

2) determining which copy engine that the destination chose

The client would need to add to the COPY operation that it would
prefer that the destination use the contiguous copy. But there might
be no contiguous copy methods available between the destination
and source.

And while we could return the copy method selected to the client in
the COPY result, perhaps a simpler approach would be to return in
the write_response4 whether the corresponding asynchronous operation
was done consecutively or not.