On Wed, May 30, 2018 at 10:42:01AM +0300, Vladimir Sementsov-Ogievskiy wrote:
30.05.2018 03:35, Eric Blake wrote:
When the NBD_CMD_BLOCK_STATUS extension was first discussed, the
idea of having the client's length be a hint was proposed, where
the server could reply beyond the client's request in order to
allow for fewer transactions when querying the entire disk. The
portion beyond the client's original request can only occur in
the final extent for a given context, and only if the additional
length matches the type given for the last byte actually requested
by the client.
In the meantime, qemu 2.12 was released as a first client
implementation of NBD_CMD_BLOCK_STATUS, which always sends the
NBD_CMD_FLAG_REQ_ONE flag, and which disconnects from the server
if the server's length exceeds the client request. This was
relaxed for subsequent qemu, but it means that we have to be
explicit that a server should not send extra length except when
the client is not limiting its request to exactly one extent.
Signed-off-by: Eric Blake <eblake@redhat.com>
---
I brought this topic up a few days back, but realized I never
actually posted a patch to go along with it.
doc/proto.md | 25 ++++++++++++++++++-------
1 file changed, 18 insertions(+), 7 deletions(-)
diff --git a/doc/proto.md b/doc/proto.md
index 4b73e0b..bd5f61e 100644
--- a/doc/proto.md
+++ b/doc/proto.md
@@ -1709,8 +1709,8 @@ MUST initiate a hard disconnect.
*length* MUST be 4 + (a positive integer multiple of 8). This reply
represents a series of consecutive block descriptors where the sum
- of the length fields within the descriptors MUST not be greater than
- the length of the original request. This chunk type MUST appear
+ of the length fields within the descriptors are subject to further
+ constraints documented below. This chunk type MUST appear
hmm, "the sum are ..", shouldn't it be "the sum is .." ?
exactly once per metadata ID in a structured reply.
The payload starts with:
@@ -1725,7 +1725,14 @@ MUST initiate a hard disconnect.
32 bits, status flags
If the client used the `NBD_CMD_FLAG_REQ_ONE` flag in the request,
- then every reply chunk MUST NOT contain more than one descriptor.
+ then every reply chunk MUST NOT contain more than one descriptor,
+ and the descriptor MUST NOT exceed the *length* of the request.
+ Otherwise, when the server returns N extents for a given context,
Here, may someone understand this "Otherwise" as an opposition to "one
descriptor", not to used the `NBD_CMD_FLAG_REQ_ONE` (with further discussion
about N and N-1) ? May be better to specify, like
When flag `NBD_CMD_FLAG_REQ_ONE` was not used, the sum of *length* fields
within the descriptors MAY exceed the requested length if the server has
+1. It's better to be explicit in this regard, and not allow too much
room for assumptions.
that information anyway as a side effect of reporting the status of the
requested region. However, if there more than one descriptor, the sum of
*length* fields within all descriptors but the last one MUST be smaller
than the requested length.
+ the sum of the *length* fields of the first N-1 extents MUST be
and this sum is not exist if N-1 = 0
We alreaday say that, since we're in this clause only if there N > 1,
but, details.
+ smaller than the overall *length* of the client's request, but the
+ final extent MAY exceed the requested length if the server has that
hmm, not final extent MAY exceed, but the sum of all extents... But may be
that's clear, sorry for nitpicking.
Nitpicking is good :) but I'm not sure what you mean, can you clarify?