Re: [PATCH] doc: Permit BLOCK_STATUS reply to extend beyond request
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?
--
Could you people please use IRC like normal people?!?
-- Amaya Rodrigo Sastre, trying to quiet down the buzz in the DebConf 2008
Hacklab
Reply to: