[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCH v2] doc: In STRUCTURED_REPLY, make error types easy to recognize



On 11 Apr 2016, at 22:29, Eric Blake <eblake@...696...> 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 <eblake@...696...>

Reviewed-by: Alex Bligh <alex@...872...>

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







Reply to: