On Mon, Nov 14, 2022 at 04:46:52PM -0600, Eric Blake wrote:
[...]
@@ -1370,9 +1475,10 @@ of the newstyle negotiation.
Return a list of `NBD_REP_META_CONTEXT` replies, one per context,
followed by an `NBD_REP_ACK` or an error.
- This option SHOULD NOT be requested unless structured replies have
- been negotiated first. If a client attempts to do so, a server
- MAY send `NBD_REP_ERR_INVALID`.
+ This option SHOULD NOT be requested unless structured replies or
+ extended headers have been negotiated first. If a client attempts
+ to do so, a server MAY send `NBD_REP_ERR_INVALID` or
+ `NBD_REP_ERR_EXT_HEADER_REQD`.
Is it the intent that NBD_REP_ERR_EXT_HEADER_REQD means structured
replies are not supported by this server? I think that could be
clarified here.
(this occurs twice)
[...]
+* `NBD_OPT_EXTENDED_HEADERS` (11)
+
+ The client wishes to use extended headers during the transmission
+ phase. The client MUST NOT send any additional data with the
+ option, and the server SHOULD reject a request that includes data
+ with `NBD_REP_ERR_INVALID`.
+
+ When successful, this option takes precedence over structured
+ replies. A client MAY request structured replies first, although
+ a server SHOULD support this option even if structured replies are
+ not negotiated.
+
+ It is envisioned that future extensions will add other new
+ requests that support a data payload in the request or reply. A
+ server that supports such extensions SHOULD NOT advertise those
+ extensions until the client has negotiated extended headers; and a
+ client MUST NOT make use of those extensions without first
+ enabling support for reply payloads.
+
+ The server replies with the following, or with an error permitted
+ elsewhere in this document:
+
+ - `NBD_REP_ACK`: Extended headers have been negotiated; the client
+ MUST use the 32-byte extended request header, with proper use of
+ `NBD_CMD_FLAG_PAYLOAD_LEN` for all commands sending a payload;
+ and the server MUST use the 32-byte extended reply header.
+ - For backwards compatibility, clients SHOULD be prepared to also
+ handle `NBD_REP_ERR_UNSUP`; in this case, only the compact
+ transmission headers will be used.
+
+ Note that a response of `NBD_REP_ERR_BLOCK_SIZE_REQD` does not
+ make sense in response to this command, but a server MAY fail with
+ that error for a later `NBD_OPT_GO` without a client request for
+ `NBD_INFO_BLOCK_SIZE`, since the use of extended headers provides
+ more incentive for a client to promise to obey block size
+ constraints.
+
+ If the client requests `NBD_OPT_STARTTLS` after this option, it
+ MUST renegotiate extended headers.
+
Does it make sense here to also forbid use of NBD_OPT_EXPORT_NAME? I
think the sooner we get rid of that, the better ;-)
[...]
@@ -1746,13 +1914,15 @@ unrecognized flags.
#### Structured reply types
-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, other than an *error chunk*, it
-MUST initiate a hard disconnect. A server MUST NOT send a chunk
-larger than any advertised maximum block payload size.
+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*, *content chunks*, or *status chunks*. Each type determines
+how to interpret the "length" bytes of payload. If the client
+receives an unknown or unexpected type, other than an *error chunk*,
+it MAY initiate a hard disconnect on the grounds that the client is
+uncertain whether the server handled the request as desired. A server
+MUST NOT send a chunk larger than any advertised maximum block payload
+size.
Why do we make this a MAY rather than a MUST?
Also, should this section say "structured or extended reply"? We use the
same types for both.
[...]
+* `NBD_REPLY_TYPE_BLOCK_STATUS_EXT` (6)
+
+ This chunk type is in the status chunk category. *length* MUST be
+ 8 + (a positive multiple of 16). The semantics of this chunk mirror
+ those of `NBD_REPLY_TYPE_BLOCK_STATUS`, other than the use of a
+ larger *extent length* field, added padding in each descriptor to
+ ease alignment, and the addition of a *descriptor count* field that
+ can be used for easier client processing. This chunk type MUST NOT
+ be used unless extended headers were negotiated with
+ `NBD_OPT_EXTENDED_HEADERS`.
+
+ If the *descriptor count* field contains 0, the number of subsequent
+ descriptors is determined solely by the *length* field of the reply
+ header. However, the server MAY populate the *descriptor count*
+ field with the number of descriptors that follow; when doing this,
+ the server MUST ensure that the header *length* is equal to
+ *descriptor count* * 16 + 8.
+
+ The payload starts with:
+
+ 32 bits, metadata context ID
+ 32 bits, descriptor count
+
+ and is followed by a list of one or more descriptors, each with this
+ layout:
+
+ 64 bits, length of the extent to which the status below
+ applies (unsigned, MUST be nonzero)
+ 32 bits, padding (MUST be zero)
+ 32 bits, status flags
+
+ Note that even when extended headers are in use, the client MUST be
+ prepared for the server to use either the compact or extended chunk
+ type, regardless of whether the client's hinted effect length was
+ more or less than 32 bits; but the server MUST use exactly one of
+ the two chunk types per negotiated metacontext ID.
Is this last paragraph really a good idea? I would think it makes more
sense to require the new format if we're already required to support it
on both sides anyway.
[...]
- The list of block status descriptors within the
- `NBD_REPLY_TYPE_BLOCK_STATUS` chunk represent consecutive portions
- of the file starting from specified *offset*. If the client used
I know this was in the old text (hence me mentioning it here), but this
should probably say "export" rarher than "file". NBD does not deal
(conceptually) with files...
- the `NBD_CMD_FLAG_REQ_ONE` flag, each chunk contains exactly
one
- descriptor where the *length* of the descriptor MUST NOT be
- greater than the *length* of the request; otherwise, a chunk MAY
- contain multiple descriptors, and the final descriptor MAY extend
- beyond the original requested size if the server can determine a
- larger length without additional effort. On the other hand, the
- server MAY return less data than requested. In particular, a
- server SHOULD NOT send more than 2^20 status descriptors in a
- single chunk. However the server MUST return at least one status
- descriptor, and since each status descriptor has a non-zero
- length, a client can always make progress on a successful return.
+ The list of block status descriptors within a given status chunk
+ represent consecutive portions of the file starting from specified
+ *offset*. If the client used the `NBD_CMD_FLAG_REQ_ONE` flag,
+ each chunk contains exactly one descriptor where the *length* of
+ the descriptor MUST NOT be greater than the *length* of the
+ request; otherwise, a chunk MAY contain multiple descriptors, and
+ the final descriptor MAY extend beyond the original requested size
+ if the server can determine a larger length without additional
+ effort. On the other hand, the server MAY return less data than
+ requested. In particular, a server SHOULD NOT send more than 2^20
+ status descriptors in a single chunk. However the server MUST
+ return at least one status descriptor, and since each status
+ descriptor has a non-zero length, a client can always make
+ progress on a successful return.
Other than that, no comments on this one.
--
w(a)uter.{be,co.za}
wouter(a){grep.be,fosdem.org,debian.org}
I will have a Tin-Actinium-Potassium mixture, thanks.