Re: [Qemu-devel] [Nbd] [PATCH 3/1] doc: Propose Structured Replies extension

[Eric Blake]

On 03/29/2016 11:34 AM, Alex Bligh wrote:
> 
> On 29 Mar 2016, at 16:12, Eric Blake <address@hidden> wrote:
>>>
>>> More a way of guaranteeing avoiding a fragmentation on 'simple' reads.
>>> Perhaps a 'DF' bit (don't fragment)! If the server doesn't like it, it
>>> can always error the command.
>>
>> Okay, that makes sense.  Does reusing NBD_CMD_FLAG_FUA sound reasonable
>> for this purpose, or should it be a new flag?  I guess from the
>> standpoint of client/server negotiation, we want to support this
>> don't-fragment request even if NBD_FLAG_SEND_FUA was not listed in
>> export flags, so it sounds like a new flag is best.
> 
> I think it should be separate from FUA, as there are possibly
> servers out there that support FUA but not this, but ...
> 
>> Next, should it be
>> independently negotiated, or implied by negotiating
>> NBD_FLAG_C_STRUCTURED_REPLIES?  I'm leaning towards implied - it's
>> all-or-none for use of the improved read structures.
> 
> I would agree. I think if it supports the structured reply semantics,
> it should also support 'DF'. So if you know the server supports
> structured replies, you know you can set DF on them without any
> further requirements.

Supporting DF merely transfers the burden of collection between server
and client.  I suspect that there are cases where the server does NOT
want to support DF (because it would require the server to allocate
memory to collect the data before sending a single structured read
reply), just as you have stated that there are cases where the client
has an additional burden if DF is not supported.  So for v2, I'm going
to explicitly document a DF export flag, and recommend (but not require)
that the server support it.

> 
>> I'm also leaning
>> towards the name NBD_FLAG_C_STRUCTURED_READ, since elsewhere I'm
>> documenting that negotiating this particular global flag affects only
>> the replies to NBD_CMD_READ (other commands may use structured replies,
>> but those commands will be independently negotiated).
> 
> I suspect the name is the thing that makes the least difference, and
> hence do not feel strongly at all, but:
> 
> a) Why '_C_'?

Matches existing convention on client flags; but Wouter's idea of using
NBD_OPT_ instead of global/client flags is better, so the _C_ disappears
in v2.

> 
> b) Throughout the current documentation you've called them 'structured
>    replies', not 'structured reads' and said that in the future multiple
>    commands might support them. So you should arguably call the flag
>    '*_STRUCTURED_REPLY' or change the text.

I'm changing the text, and favoring the name STRUCTURED_READ except in
the description of the transmission phase, where Structured Reply is the
header used for ANY form of reply with data (to make it more obvious
that structured read is a subset of structured replies), while at the
same time emphasizing that NBD_CMD_READ is the only command that can get
away with data in a non-structured reply, and only when structured read
was not negotiated.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature