Re: [Nbd] [PATCH v3 4/5] doc: Propose STRUCTURED_REPLY extension

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

We're getting close.

On Thu, Mar 31, 2016 at 12:06:23AM -0600, Eric Blake wrote:
> -#### Reply message
> +#### Simple reply message
> 
> -The server replies with:
> +[option #A1 - only replies with payload are affected]
> +The simple reply message MUST be sent by the server in response to a
> +request that requires no data payload.  It MUST also be used for the
> +`NBD_CMD_READ` command if the experimental `STRUCTURED_REPLY`
> +extension was not negotiated.  The message looks as follows:
> 
> -S: 32 bits, 0x67446698, magic (`NBD_REPLY_MAGIC`)  
> -S: 32 bits, error  
> +[option #A2 - enabling structured replies MAY affect all other commands]
> +The simple reply message MUST be sent by the server in response to all
> +requests if the experimental `STRUCTURED_REPLY` extension was not
> +negotiated.  It MAY also be used for requests that require no data
> +payload, even when structured replies are in use.  The message looks
> +as follows:
> +
> +[option #A3 - enabling structured replies MUST affect all commands]
> +The simple reply message MUST be sent by the server in response to all
> +requests if the experimental `STRUCTURED_REPLY` extension was not
> +negotiated, and MUST NOT be sent otherwise.  The message looks as
> +follows:
> +
> +[all options]
> +S: 32 bits, 0x67446698, magic (`NBD_SIMPLE_REPLY_MAGIC`)  
> +S: 32 bits, error (MAY be zero)  
>  S: 64 bits, handle  
>  S: (*length* bytes of data if the request is of type `NBD_CMD_READ`)  

I still think #1 is best, but I think Markus' input is valuable too, so
we should probably hold off on deciding this until we've seen his
opinion.

I'm going to veto #3, though.

I'm not entirely convinced about the merit of going into too much detail
about structured replies here yet, but that's a detail.

> @@ -261,6 +288,8 @@ immediately after the handshake flags field in oldstyle negotiation:
>    schedule I/O accesses as for a rotational medium
>  - bit 5, `NBD_FLAG_SEND_TRIM`; should be set to 1 if the server supports
>    `NBD_CMD_TRIM` commands
> +- bit 6, `NBD_FLAG_SEND_DF`; defined by the `STRUCTURED_REPLY` extension;
> +  see below.
> 
>  Clients SHOULD ignore unknown flags.
> 
> @@ -349,6 +378,10 @@ of the newstyle negotiation.
> 
>      Defined by the experimental `SELECT` extension; see below.
> 
> +- `NBD_OPT_STRUCTURED_REPLY` (8)
> +
> +    Defined by the experimental `STRUCTURED_REPLY` extension; see below.
> +
>  #### Option reply types
> 
>  These values are used in the "reply type" field, sent by the server
> @@ -448,6 +481,8 @@ valid may depend on negotiation during the handshake phase.
>    set to 1 if the client requires "Force Unit Access" mode of
>    operation.  MUST NOT be set unless transmission flags included
>    `NBD_FLAG_SEND_FUA`.
> +- bit 1, `NBD_CMD_FLAG_DF`; defined by the experimental `STRUCTURED_REPLY`
> +  extension; see below
> 
>  #### Request types
> 
> @@ -456,9 +491,9 @@ The following request types exist:
>  * `NBD_CMD_READ` (0)
> 
>      A read request. Length and offset define the data to be read. The
> -    server MUST reply with a reply header, followed immediately by len
> -    bytes of data, read offset bytes into the file, unless an error
> -    condition has occurred.
> +    server MUST reply with a simple reply header, followed immediately
> +    by len bytes of data, read from offset bytes into the file, unless
> +    an error condition has occurred.
> 
>      If an error occurs, the server SHOULD set the appropriate error code
>      in the error field. The server MUST then either close the
> @@ -469,13 +504,18 @@ The following request types exist:
>      signalling no error), the server MUST immediately close the
>      connection; it MUST NOT send any further data to the client.
> 
> +    The experimental `STRUCTURED_REPLY` extension changes from a
                                                            ^ the reply format

> +    simple reply to a structured reply, in part to allow recovery
> +    after a partial read and more efficient reads of sparse files; see
> +    below.
> +
>  * `NBD_CMD_WRITE` (1)
[...]

Beyond that, LGTM.

-- 
< 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