[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] doc: Cleanups to structured reads
From: |
Alex Bligh |
Subject: |
Re: [Qemu-devel] [PATCH] doc: Cleanups to structured reads |
Date: |
Fri, 1 Apr 2016 21:05:24 +0100 |
On 1 Apr 2016, at 20:39, Eric Blake <address@hidden> wrote:
> A couple of typos, odd formatting, and missing words made it into
> the structured read spec, and several potential ambiguous situations
> were worth rewording for clarity.
>
> Signed-off-by: Eric Blake <address@hidden>
> Signed-off-by: Alex Bligh <address@hidden>
Reviewed-by: Alex Bligh <address@hidden>
I think we're there!
Alex
> ---
> doc/proto.md | 112 +++++++++++++++++++++++++++++++----------------------------
> 1 file changed, 59 insertions(+), 53 deletions(-)
>
> diff --git a/doc/proto.md b/doc/proto.md
> index 8c83382..758a661 100644
> --- a/doc/proto.md
> +++ b/doc/proto.md
> @@ -243,7 +243,7 @@ S: (*length* bytes of data if the request is of type
> `NBD_CMD_READ`)
> #### Structured reply chunk message
>
> This reply type MUST NOT be used except as documented by the
> -experimental `STRUCTURED_READ` extension; see below.
> +experimental `STRUCTURED_REPLY` extension; see below.
>
> ## Values
>
> @@ -410,8 +410,8 @@ during option haggling in the fixed newstyle negotiation.
> versions of nbd-server may send along some details about the
> export. Therefore, unless explicitly documented otherwise by a
> particular client request, this field is defined to be UTF-8
> - encoded data suitable for direct display to a human being; a
> - client SHOULD NOT assume that the data is NUL-terminated.
> + encoded data suitable for direct display to a human being; with
> + no embedded or terminating NUL characters.
>
> The experimental `SELECT` extension (see below) is a client
> request where the extra data has a definition other than a
> @@ -420,8 +420,7 @@ during option haggling in the fixed newstyle negotiation.
> There are a number of error reply types, all of which are denoted by
> having bit 31 set. All error replies MAY have some data set, in which
> case that data is an error message in UTF-8 encoding suitable for
> -display to the user, although the client MUST NOT assume that a
> -terminating NUL is present.
> +display to the user, with no embedded or terminating NUL characters.
>
> * `NBD_REP_ERR_UNSUP` (2^31 + 1)
>
> @@ -501,8 +500,8 @@ The following request types exist:
> extension was negotiated.
>
> If structured replies were not negotiated, the 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
> + with a simple reply header, followed immediately by *length* 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
> @@ -798,18 +797,23 @@ error, and alters the reply to the `NBD_CMD_READ`
> request.
> structured replies are negotiated, the server MUST use a
> structured reply for any response with a payload, and MUST NOT use
> a simple reply for `NBD_CMD_READ` (even for the case of an early
> - `EINVAL` due to bad flags).
> + `EINVAL` due to bad flags), but MAY use either a simple reply or a
> + structured reply to all other requests. The server SHOULD prefer
> + sending errors via a structured reply, as the error can then be
> + accompanied by a UTF-8 text payload to present to a human user.
>
> A structured reply MAY occupy multiple structured chunk messages
> (all with the same value for "handle"), and the
> `NBD_REPLY_FLAG_DONE` reply flag is used to identify the final
> - chunk. Relative ordering of the chunks MAY be important;
> - individual commands will document constraints on whether multiple
> - chunks may be rearranged; however, it is always safe to interleave
> - chunks of the reply to one request with messages related to other
> - requests. A server SHOULD try to minimize the number of chunks
> - sent in a reply, but MUST NOT send the final chunk if there is
> - still a possibility of detecting an error. A structured reply is
> + chunk. Unless further documented by individual requests below,
> + the chunks MAY be sent in any order, except that the chunk with
> + the flag `NBD_REPLY_FLAG_DONE` MUST be sent last. Even when a
> + command documents further constraints between chunks of one reply,
> + it is always safe to interleave chunks of that reply with messages
> + related to other requests. A server SHOULD try to minimize the
> + number of chunks sent in a reply, but MUST NOT mark a chunk as
> + final if there is still a possibility of detecting an error before
> + transmission of that chunk completes. A structured reply is
> considered successful only if it did not contain any error chunks,
> although the client MAY be able to determine partial success based
> on the chunks received.
> @@ -851,7 +855,7 @@ error, and alters the reply to the `NBD_CMD_READ` request.
> 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 SHOULD close the connection.
> + an unknown or unexpected type, it MUST close the connection.
>
> - `NBD_REPLY_TYPE_NONE` (0)
>
> @@ -874,7 +878,8 @@ error, and alters the reply to the `NBD_CMD_READ` request.
>
> 32 bits: error (MUST be nonzero)
> *length - 4* bytes: (optional UTF-8 encoded data suitable for
> - direct display to a human being, not NUL terminated)
> + direct display to a human being, with no embedded or
> + terminating NUL characters)
>
> - `NBD_REPLY_TYPE_ERROR_OFFSET` (2)
>
> @@ -895,7 +900,8 @@ error, and alters the reply to the `NBD_CMD_READ` request.
> 32 bits: error (MUST be nonzero)
> 64 bits: offset (unsigned)
> *length - 12* bytes: (optional UTF-8 encoded data suitable for
> - direct display to a human being, not NUL terminated)
> + direct display to a human being, with no embedded or
> + terminating NUL characters)
>
> - `NBD_REPLY_TYPE_OFFSET_DATA` (3)
>
> @@ -957,11 +963,8 @@ error, and alters the reply to the `NBD_CMD_READ`
> request.
> The server MUST set this transmission flag to 1 if the
> `NBD_CMD_READ` request supports the `NBD_CMD_FLAG_DF` flag, and
> MUST leave this flag clear if structured replies have not been
> - negotiated. Clients MUST NOT rely on the state of this flag prior
> - the final flags value reported by `NBD_OPT_EXPORT_NAME` or
> - experimental `NBD_OPT_GO`. Additionally, clients MUST NOT set the
> - `NBD_CMD_FLAG_DF` request flag unless this transmission flag is
> - set.
> + negotiated. Clients MUST NOT set the `NBD_CMD_FLAG_DF` request
> + flag unless this transmission flag is set.
>
> * `NBD_CMD_FLAG_DF`
>
> @@ -1000,47 +1003,50 @@ error, and alters the reply to the `NBD_CMD_READ`
> request.
> overhead, the server SHOULD use chunks with lengths and offsets as
> an integer multiple of 512 bytes, where possible (the first and
> last chunk of an unaligned read being the most obvious places for
> - an exception). The server MUST NOT content chunks that overlap
> - with any earlier content or error chunk, and MUST NOT send chunks
> - that describe data outside the offset and length of the request,
> - but MAY send the chunks in any order (the client MUST reassemble
> - content chunks into the correct order), and MAY send additional
> - data chunks even after reporting an error chunk. Note that a
> - request for more than 2^32 - 8 bytes MUST be split into at least
> - two chunks, so as not to overflow the length field of a reply
> - while still allowing space for the offset of each chunk. When no
> - error is detected, the server MUST send enough data chunks to
> - cover the entire region described by the offset and length of the
> - client's request.
> + an exception). The server MUST NOT send content chunks that
> + overlap with any earlier content or error chunk, and MUST NOT send
> + chunks that describe data outside the offset and length of the
> + request, but MAY send the content chunks in any order (the client
> + MUST reassemble content chunks into the correct order), and MAY
> + send additional content chunks even after reporting an error chunk.
> + Note that a request for more than 2^32 - 8 bytes MUST be split
> + into at least two chunks, so as not to overflow the length field
> + of a reply while still allowing space for the offset of each
> + chunk. When no error is detected, the server MUST send enough
> + data chunks to cover the entire region described by the offset and
> + length of the client's request.
>
> To minimize traffic, the server MAY use a content or error chunk
> as the final chunk by setting the `NBD_REPLY_FLAG_DONE` flag, but
> MUST NOT do so for a content chunk if it would still be possible
> to detect an error while transmitting the chunk. The
> - `NBD_REPLY_TYPE_DONE` chunk is always acceptable as the final
> + `NBD_REPLY_TYPE_NONE` chunk is always acceptable as the final
> chunk.
>
> If an error is detected, the server MUST still complete the
> - transmission of any current chunk (it SHOULD use padding bytes of
> - zero for any remaining data portion of a chunk with type
> - `NBD_REPLY_TYPE_OFFSET_DATA`), but MAY omit further content
> + transmission of any current chunk (it MUST use padding bytes which
> + SHOULD be zero, for any remaining data portion of a chunk with
> + type `NBD_REPLY_TYPE_OFFSET_DATA`), but MAY omit further content
> chunks. The server MUST include an error chunk as one of the
> subsequent chunks, but MAY defer the error reporting behind other
> queued chunks. An error chunk of type `NBD_REPLY_TYPE_ERROR`
> implies that the client MAY NOT make any assumptions about
> - validity of data chunks, and if used, SHOULD be the only error
> - chunk in the reply. On the other hand, an error chunk of type
> + validity of data chunks (whether sent before or after the error
> + chunk), and if used, SHOULD be the only error chunk in the reply.
> + On the other hand, an error chunk of type
> `NBD_REPLY_TYPE_ERROR_OFFSET` gives fine-grained information about
> - which earlier data chunk(s) encountered a failure, and MAY also be
> - sent in lieu of a data chunk; as such, a server MAY still usefully
> - follow it with further non-overlapping content chunks or with
> - error offsets for other content chunks. Generally, a server
> - SHOULD NOT mix errors with offsets with a generic error. As long
> - as all errors are accompanied by offsets, the client MAY assume
> - that any data chunks with no subsequent error offset are valid,
> - that chunks with an overlapping error offset errors are valid up
> - until the reported offset, and that portions of the read that do
> - not have a corresponding content chunk are not valid.
> + which earlier data chunk(s) encountered a failure; as such, a
> + server MAY still usefully follow it with further non-overlapping
> + content chunks or with error offsets for other content chunks.
> + The server MAY send an error chunk with no corresponding content
> + chunk, but MUST ensure that the content chunk is sent first if a
> + content and error chunk cover the same offset. Generally, a
> + server SHOULD NOT mix errors with offsets with a generic error.
> + As long as all errors are accompanied by offsets, the client MAY
> + assume that any data chunks with no subsequent error offset are
> + valid, that chunks with an overlapping error offset errors are
> + valid up until the reported offset, and that portions of the read
> + that do not have a corresponding content chunk are not valid.
>
> A client MAY close the connection if it detects that the server
> has sent invalid chunks (such as overlapping data, or not enough
> @@ -1056,8 +1062,8 @@ error, and alters the reply to the `NBD_CMD_READ`
> request.
> A server MAY reject a client's request with the error `EOVERFLOW`
> if the length is too large to send without fragmentation, in which
> case it MUST NOT send a content chunk; however, the server MUST
> - NOT use this error if the client's requested length does not
> - exceed 65,536 bytes.
> + support unfragmented reads in which the client's request length
> + does not exceed 65,536 bytes.
>
> ## About this file
>
> --
> 2.5.5
>
--
Alex Bligh