Re: [PATCH v3] spec: Clarify BLOCK_STATUS reply details

[Eric Blake]

On Thu, Apr 07, 2022 at 11:37:31PM +0300, Vladimir Sementsov-Ogievskiy wrote:
> 02.04.2022 00:08, Eric Blake wrote:
> > Our docs were inconsistent on whether a NBD_REPLY_TYPE_BLOCK_STATUS
> > reply chunk can exceed the client's requested length, and silent on
> > whether the lengths must be consistent when multiple contexts were
> > negotiated.  Clarify this to match existing practice as implemented in
> > qemu-nbd.
> 
> I think by existing practice you mean only the latter?
> 
> Seems that currently we never report more information than requested on 
> BLOCK_STATUS.

nbdkit allows the last extent to exceed the client's requested length;
and that has always been an intentional design feature (if I ask for
status on 1M, but you know the entire 100M image is sparse, you should
be able to reply right away with a 100M hole, rather than making me
query 99 more times).

> 
> hope that there no existing too strict clients that supports BLOCK_STATUS 
> without REQ_ONE and check each extent to be less than original request length 
> (accordingly to old spec):)

The old spec was ambiguous.  In one place it said NO reply chunk could
exceed the client length, regardless of REQ_ONE; in another, it said
that you have to use REQ_ONE to constrain the reply length.  As
existing servers behave like the latter, that is what this patch does
to make the two places in the spec consistent with one another.

> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@openvz.org>
> 
> > Clean up some nearby grammatical errors while at it.
> > ---
> > 
> > Another round of rewording attempts, based on feedback from Rich on
> > v2.  I went ahead and pushed patch 1 and 2 of the v2 series, as they
> > were less controversial.
> > 
> >   doc/proto.md | 42 ++++++++++++++++++++++++++++--------------
> >   1 file changed, 28 insertions(+), 14 deletions(-)
> > 
> > diff --git a/doc/proto.md b/doc/proto.md
> > index 8a817d2..bacccfa 100644
> > --- a/doc/proto.md
> > +++ b/doc/proto.md
> > @@ -882,15 +882,25 @@ The procedure works as follows:
> >     server supports.
> >   - During transmission, a client can then indicate interest in metadata
> >     for a given region by way of the `NBD_CMD_BLOCK_STATUS` command,
> > -  where *offset* and *length* indicate the area of interest. The
> > -  server MUST then respond with the requested information, for all
> > -  contexts which were selected during negotiation. For every metadata
> > -  context, the server sends one set of extent chunks, where the sizes
> > -  of the extents MUST be less than or equal to the length as specified
> > -  in the request. Each extent comes with a *flags* field, the
> > -  semantics of which are defined by the metadata context.
> > -- A server MUST reply to `NBD_CMD_BLOCK_STATUS` with a structured
> > -  reply of type `NBD_REPLY_TYPE_BLOCK_STATUS`.
> > +  where *offset* and *length* indicate the area of interest. On
> > +  success, the server MUST respond with one structured reply chunk of
> > +  type `NBD_REPLY_TYPE_BLOCK_STATUS` per metadata context selected
> > +  during negotiation, where each reply chunk is a list of one or more
> > +  consecutive extents for that context.  Each extent comes with a
> > +  *flags* field, the semantics of which are defined by the metadata
> > +  context.
> > +
> > +The client's requested *length* is only a hint to the server, so the
> > +cumulative extent length contained in a chunk of the server's reply
> > +may be shorter or longer the original request.  When more than one
> > +metadata context was negotiated, the reply chunks for the different
> > +contexts of a single block status request need not have the same
> > +number of extents or cumulative extent length.
> > +
> > +In the request, the client may use the `NBD_CMD_FLAG_REQ_ONE` command
> > +flag to further constrain the server's reply so that each chunk
> > +contains exactly one extent whose length does not exceed the client's
> > +original *length*.
> > 
> >   A client MUST NOT use `NBD_CMD_BLOCK_STATUS` unless it selected a
> >   nonzero number of metadata contexts during negotiation, and used the
> > @@ -1778,8 +1788,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 is subject to further
> > -  constraints documented below. This chunk type MUST appear
> > -  exactly once per metadata ID in a structured reply.
> > +  constraints documented below.  A successful block status request MUST
> > +  have exactly one status chunk per negotiated metadata context ID.
> > 
> >     The payload starts with:
> > 
> > @@ -1801,15 +1811,19 @@ MUST initiate a hard disconnect.
> >     *length* of the final extent MAY result in a sum larger than the
> >     original requested length, if the server has that information anyway
> >     as a side effect of reporting the status of the requested region.
> > +  When multiple metadata contexts are negotiated, the reply chunks for
> > +  the different contexts need not have the same number of extents or
> > +  cumulative extent length.
> > 
> >     Even if the client did not use the `NBD_CMD_FLAG_REQ_ONE` flag in
> >     its request, the server MAY return fewer descriptors in the reply
> >     than would be required to fully specify the whole range of requested
> >     information to the client, if looking up the information would be
> >     too resource-intensive for the server, so long as at least one
> > -  extent is returned. Servers should however be aware that most
> > -  clients implementations will then simply ask for the next extent
> > -  instead.
> > +  extent is returned.  Servers should however be aware that most
> > +  client implementations will likely follow up with a request for
> > +  extent information at the first offset not covered by a
> > +  reduced-length reply.
> > 
> >   All error chunk types have bit 15 set, and begin with the same
> >   *error*, *message length*, and optional *message* fields as
> 
> 
> -- 
> Best regards,
> Vladimir
> 

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org