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

Re: [Nbd] [PATCH v2 1/2] doc: Move sections about structured reply values



Eric,

In general I think we should probably apply this and see how it
looks though I suspect fixing things is going to be an iterative
matter.

However:

> 
> @@ -355,7 +355,7 @@ S: (*length* bytes of data if the request is of type `NBD_CMD_READ`)
> Some of the major downsides of the default simple reply to
> `NBD_CMD_READ` are as follows.  First, it is not possible to support
> partial reads or early errors (the command must succeed or fail as a
> -whole, and either len bytes of data must be sent or a hard disconnect
> +whole, and either *length* bytes of data must be sent or a hard disconnect
> must be initiated, even if the failure is `EINVAL` due to bad flags).
> Second, there is no way to efficiently skip over portions of a sparse
> file that are known to contain all zeroes.  Finally, it is not

that's a nit that isn't a move. Which is fine as I found it!

> @@ -373,7 +373,7 @@ a simple reply for `NBD_CMD_READ` (even for the case of an early
> `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 string payload to present to a human user.
> +accompanied by a string 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

that's a nit that isn't a move. Which is fine as I found it!

...

> @@ -1086,10 +1088,7 @@ The following request types exist:
>     If structured replies were not negotiated, then a read request
>     MUST always be answered by a simple reply, as documented above
>     (using magic 0x67446698 `NBD_SIMPLE_REPLY_MAGIC`, and containing
> -    length bytes of data according to the client's request, although
> -    those bytes MAY be invalid if an error is returned, and a hard
> -    disconnect MUST be initiated if an error occurs after a header
> -    claiming no error).
> +    length bytes of data according to the client's request).

That's a nit that isn't a move.

'*length*' bytes of data

What's the reason for removing the fact that in simple replies you
can send invalid data (as the client will be expecting the data)?

> 
>     If an error occurs, the server SHOULD set the appropriate error
>     code in the error field. The server MUST then either initiate




-- 
Alex Bligh







Reply to: