Re: [Nbd] [PATCH] doc: Fix shortcoming of NBD_OPT_GO before it gets implemented

[Wouter Verhelst <w@...112...>]

Thanks, applied.

On Thu, Mar 31, 2016 at 09:06:06PM -0600, Eric Blake wrote:
> Without the SELECT extension, a client is guaranteed that the
> transmission flags sent in response to NBD_OPT_EXPORT_NAME
> represent the final state of all other option requests.  But
> it is possible that option requests handled between
> NBD_OPT_SELECT and NBD_OPT_GO could change the set of
> transmission flags.  Therefore, NBD_OPT_GO must give the client
> at least as much information as the reply to NBD_OPT_EXPORT_NAME;
> although we want it to be structured rather than the old-style
> reply, so that the client can distinguish and recover from errors.
> Since NBD_REP_ACK carries no data, the easiest solution is to
> instead reuse the NBD_REP_SERVER layout present in NBD_OPT_SELECT.
> 
> While at it, make it obvious that the option request that ends
> handshake phase MUST be the last reply sent by the server (moving
> to transmission phase while stranding out-of-order option replies
> does no one any good).  Also, the trailing zeroes only occur
> after NBD_OPT_EXPORT_NAME (or oldstyle handshake), and should never
> appear after NBD_OPT_GO.
> 
> Signed-off-by: Eric Blake <eblake@...696...>
> ---
>  doc/proto.md | 32 +++++++++++++++++++++++---------
>  1 file changed, 23 insertions(+), 9 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index c1e05c5..2600098 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -155,11 +155,13 @@ would have to abort negotiation as well.
>  To fix these two issues, the following changes were implemented:
> 
>  - The server will set the handshake flag `NBD_FLAG_FIXED_NEWSTYLE`, to
> -  signal that it supports fixed newstyle negotiation
> +  signal that it supports fixed newstyle negotiation.
>  - The client should reply with `NBD_FLAG_C_FIXED_NEWSTYLE` set in its flags
>    field too, though its side of the protocol does not change incompatibly.
>  - The client may now send other options to the server as appropriate, in
>    the generic format for sending an option as described above.
> +- The server MUST NOT send a response to `NBD_OPT_EXPORT_NAME` until all
> +  other pending option requests have had their final reply.
>  - The server will reply to any option apart from `NBD_OPT_EXPORT_NAME`
>    with reply packets in the following format:
> 
> @@ -615,8 +617,13 @@ option reply type.
>      - `NBD_REP_ERR_TLS_REQD`: The server does not wish to export this
>        block device unless the client negotiates TLS first.
>      - `NBD_REP_SERVER`: The server accepts the chosen export. In this
> -      case, the `NBD_REP_SERVER` message's data contains the following
> -      fields:
> +      case, the `NBD_REP_SERVER` message's data takes on a different
> +      interpretation than the default (so as to provide additional
> +      binary information normally sent in reply to
> +      `NBD_OPT_EXPORT_NAME`, in place of the default UTF-8 free-form
> +      string); this layout is shared with the successful response to
> +      `NBD_OPT_GO`.  The option reply length MUST be *length of
> +      name* + 14, and the option data has the following layout:
> 
>        - 32 bits, length of name (unsigned)
>        - Name of the export. This name MAY be different from the one
> @@ -625,9 +632,6 @@ option reply type.
>        - 64 bits, size of the export in bytes (unsigned)
>        - 16 bits, transmission flags
> 
> -      That is, the `NBD_REP_SERVER` message is extended to also include
> -      the data sent in reply to the `NBD_OPT_EXPORT_NAME` option, and
> -      the option reply length MUST be the length of name + 14.
>      - For backwards compatibility, clients should be prepared to also
>        handle `NBD_REP_ERR_UNSUP`. In this case, they should fall back to
>        using `NBD_OPT_EXPORT_NAME`.
> @@ -637,9 +641,19 @@ option reply type.
>      The client wishes to terminate the negotiation phase and progress to
>      the transmission phase. Possible replies from the server include:
> 
> -    - `NBD_REP_ACK`: The server acknowledges. After receiving this
> -      reply, the client and the server have both moved to the
> -      transmission phase.
> +    - `NBD_REP_SERVER`: The server acknowledges, using the same format
> +      for the reply as in `NBD_OPT_SELECT` (thus allowing the client
> +      to receive the same transmission flags as would have been sent
> +      by `NBD_OPT_EXPORT_NAME`).  The values of the transmission flags
> +      MAY differ from what was sent earlier in response to
> +      `NBD_OPT_SELECT`, based on other options that were negotiated in
> +      the meantime.  The server MUST NOT send any zero padding bytes
> +      after the `NBD_REP_SERVER` data, whether or not the client
> +      negotiated the `NBD_FLAG_C_NO_ZEROES` flag.  After receiving
> +      this reply, the client and the server have both moved to the
> +      transmission phase; therefore, the server MUST NOT send this
> +      particular reply until all other pending option requests have
> +      had their final reply.
>      - `NBD_REP_ERR_INVALID`: No `NBD_OPT_SELECT` command was
>        previously issued during this negotiation (there is no default);
>        or, the most recent `NBD_OPT_SELECT` command resulted in an error
> -- 
> 2.5.5
> 
> 
> ------------------------------------------------------------------------------
> Transform Data into Opportunity.
> Accelerate data analysis in your applications with
> Intel Data Analytics Acceleration Library.
> Click to learn more.
> http://pubads.g.doubleclick.net/gampad/clk?id=278785471&iu=/4140
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12