Re: [Nbd] [PATCH v2] doc: In STRUCTURED_REPLY, make error types easy to recognize
- To: Eric Blake <eblake@...696...>
- Cc: "nbd-general@lists.sourceforge.net" <nbd-general@lists.sourceforge.net>
- Subject: Re: [Nbd] [PATCH v2] doc: In STRUCTURED_REPLY, make error types easy to recognize
- From: Alex Bligh <alex@...872...>
- Date: Tue, 12 Apr 2016 11:14:26 +0100
- Message-id: <1C5342AB-1219-4391-8E9E-6001CB60A4BA@...872...>
- In-reply-to: <1460410165-421-1-git-send-email-eblake@...696...>
- References: <1460410165-421-1-git-send-email-eblake@...696...>
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: