On 11 Apr 2016, at 22:29, Eric Blake <ebl...@redhat.com> wrote:

> We may add future structured error replies; making it easy
> for older clients to properly treat such new reply types as
> an error gives us a bit more flexibility on introducing new
> errors to existing commands.  Of course, good design practice
> says we should try hard to prevent the server from sending
> something the client isn't expecting, but covering the
> situation from both sides never hurts.  Also, marking error
> types resembles how NBD_REP_ERR_* has a common layout.
> 
> Signed-off-by: Eric Blake <ebl...@redhat.com>

Reviewed-by: Alex Bligh <a...@alex.org.uk>

> ---
> 
> v2: make the message length field mandatory and only 16 bits
> 
> Not done: one comment was that NBD_REPLY_TYPE_ is awkward, but
> a rename to use NBD_REPLY_CHUNK_ or any other name is best done
> separately.
> 
> Requires these patches to be applied first:
> [AB] [PATCHv8] Improve documentation for TLS
> [AB*] [PATCHv6] docs/proto.md: Clarify SHOULD / MUST / MAY etc
> 
> (*Note that this is Alex's v6 posted today based on the TLS
> patch, and not my v6 posted a couple days earlier. Proof that
> we really need to get the conflict magnet out of the way...)
> 
> doc/proto.md | 93 +++++++++++++++++++++++++++++++++++-------------------------
> 1 file changed, 54 insertions(+), 39 deletions(-)
> 
> diff --git a/doc/proto.md b/doc/proto.md
> index deb22a5..06a275d 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -1209,8 +1209,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>     These values are used in the "type" field of a structured reply.
>     Some chunk types can additionally be categorized by role, such as
>     *error chunks* or *content chunks*.  Each type determines how to
> -    interpret the "length" bytes of payload.  If the client receives
> -    an unknown or unexpected type, it MUST close the connection.
> +    interpret the "length" bytes of payload.
> 
>     - `NBD_REPLY_TYPE_NONE` (0)
> 
> @@ -1221,42 +1220,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>       chunks were sent, then this type implies that the overall client
>       request is successful.  Valid as a reply to any request.
> 
> -    - `NBD_REPLY_TYPE_ERROR` (1)
> -
> -      This chunk type is in the error chunk category.  *length* MUST
> -      be at least 4.  This chunk represents that an error occurred,
> -      and the client MAY NOT make any assumptions about partial
> -      success. This type SHOULD NOT be used more than once in a
> -      structured reply.  Valid as a reply to any request.
> -
> -      The payload is structured as:
> -
> -      32 bits: error (MUST be nonzero)  
> -      *length - 4* bytes: optional string suitable for
> -         direct display to a human being
> -
> -    - `NBD_REPLY_TYPE_ERROR_OFFSET` (2)
> -
> -      This chunk type is in the error chunk category.  *length* MUST
> -      be at least 12.  This reply represents that an error occurred at
> -      a given offset, which MUST lie within the original offset and
> -      length of the request; the client can use this offset to
> -      determine if request had any partial success.  This chunk type
> -      MAY appear multiple times in a structured reply, although the
> -      same offset SHOULD NOT be repeated.  Likewise, if content chunks
> -      were sent earlier in the structured reply, the server SHOULD NOT
> -      send multiple distinct offsets that lie within the bounds of a
> -      single content chunk.  Valid as a reply to `NBD_CMD_READ`,
> -      `NBD_CMD_WRITE`, `NBD_CMD_WRITE_ZEROES`, and `NBD_CMD_TRIM`.
> -
> -      The payload is structured as:
> -
> -      32 bits: error (MUST be non-zero)  
> -      64 bits: offset (unsigned)  
> -      *length - 12* bytes: optional string suitable for
> -         direct display to a human being
> -
> -    - `NBD_REPLY_TYPE_OFFSET_DATA` (3)
> +    - `NBD_REPLY_TYPE_OFFSET_DATA` (1)
> 
>       This chunk type is in the content chunk category.  *length* MUST
>       be at least 9.  It represents the contents of *length - 8* bytes
> @@ -1272,7 +1236,7 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>       64 bits: offset (unsigned)  
>       *length - 8* bytes: data  
> 
> -    - `NBD_REPLY_TYPE_OFFSET_HOLE` (4)
> +    - `NBD_REPLY_TYPE_OFFSET_HOLE` (2)
> 
>       This chunk type is in the content chunk category.  *length* MUST
>       be exactly 12.  It represents that the contents of *hole size*
> @@ -1289,6 +1253,57 @@ error, and alters the reply to the `NBD_CMD_READ` 
> request.
>       64 bits: offset (unsigned)  
>       32 bits: hole size (unsigned, MUST be nonzero)  
> 
> +    All error chunk types have bit 15 set, and begin with the same
> +    *error*, *message length*, and optional *message* fields as
> +    `NBD_REPLY_TYPE_ERROR`.  If non-zero, *message length* indicates
> +    that an optional error string message appears next, suitable for
> +    display to a human user.  The header *length* then covers any
> +    remaining structured fields at the end.
> +
> +    - `NBD_REPLY_TYPE_ERROR` (2^15 + 1)
> +
> +      This chunk type is in the error chunk category.  *length* MUST
> +      be at least 6.  This chunk represents that an error occurred,
> +      and the client MAY NOT make any assumptions about partial
> +      success. This type SHOULD NOT be used more than once in a
> +      structured reply.  Valid as a reply to any request.
> +
> +      The payload is structured as:
> +
> +      32 bits: error (MUST be nonzero)  
> +      16 bits: message length (no more than header *length* - 6)  
> +      *message length* bytes: optional string suitable for
> +        direct display to a human being  
> +
> +    - `NBD_REPLY_TYPE_ERROR_OFFSET` (2^15 + 2)
> +
> +      This chunk type is in the error chunk category.  *length* MUST
> +      be at least 14.  This reply represents that an error occurred at
> +      a given offset, which MUST lie within the original offset and
> +      length of the request; the client can use this offset to
> +      determine if request had any partial success.  This chunk type
> +      MAY appear multiple times in a structured reply, although the
> +      same offset SHOULD NOT be repeated.  Likewise, if content chunks
> +      were sent earlier in the structured reply, the server SHOULD NOT
> +      send multiple distinct offsets that lie within the bounds of a
> +      single content chunk.  Valid as a reply to `NBD_CMD_READ`,
> +      `NBD_CMD_WRITE`, `NBD_CMD_WRITE_ZEROES`, and `NBD_CMD_TRIM`.
> +
> +      The payload is structured as:
> +
> +      32 bits: error (MUST be non-zero)  
> +      16 bits: message length (no more than header *length* - 14)  
> +      *message length* bytes: optional string suitable for
> +         direct display to a human being  
> +      64 bits: offset (unsigned)  
> +
> +    If the client receives an unknown or unexpected type with bit 15
> +    set, it MUST consider the current reply as errored, but MAY
> +    continue transmission unless it detects that *message length* is
> +    too large to fit within the *length* specified by the header.  For
> +    all other messages with unknown or unexpected type or inconsistent
> +    contents, the client MUST close the connection.
> +
> * `NBD_OPT_STRUCTURED_REPLY`
> 
>     The client wishes to use structured replies during the
> -- 
> 2.5.5
> 
> 
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications Manager
> Applications Manager provides deep performance insights into multiple tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
> _______________________________________________
> Nbd-general mailing list
> Nbd-general@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/nbd-general
> 

-- 
Alex Bligh





------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
Nbd-general mailing list
Nbd-general@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nbd-general

Reply via email to