[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCH v4] doc: Add NBD_CMD_BLOCK_STATUS extension



On 12/12/2016 02:26 PM, Wouter Verhelst wrote:

>>>  
>>>  This section describes the value and meaning of constants (other than
>>> @@ -768,8 +814,6 @@ The field has the following format:
>>>    to that command to the client. In the absense of this flag, clients
>>
>> drive-by typo fix applicable to the main branch:
>> s/absense/absence/
> 
> Isn't that a matter of en_US vs en_GB?

Not in this instance - both countries use absence.  (If it helps, I'm US
but my wife is UK, so I generally have a good feel for which words have
different spellings across the pond)


>>> then the server MUST send all the metadata
>>> +      contexts it knows about. If specified, this query string MUST
>>> +      start with a name that uniquely identifies a server
>>> +      implementation; e.g., the reference implementation that
>>> +      accompanies this document would support query strings starting
>>> +      with 'nbd-server:'
>>
>> 'nbd-server:' or 'base:' ?  [oh, I see more on this below]
> 
> Indeed. It might certainly make sense to clarify that a bit, or I could
> just decide that I take the "base:" context, and that everyone else can
> register their own names ;->

I'd be fine with the reference implementation taking 'base:' :)


>>> +
>>> +    Even if the client did not use the `NBD_CMD_FLAG_REQ_ONE` flag in
>>> +    its request, the server MAY return less descriptors in the reply
>>> +    than would be required to fully specify the whole range of requested
>>> +    information to the client, if the number of descriptors would be
>>> +    over 16 otherwise and looking up the information would be too
>>> +    resource-intensive for the server.
>>
>> Do we still want to require servers to always send 16 extents (when not
>> limited to exactly 1), or is it better to just state that as long as the
>> server sends at least one extent (so that the client can make progress),
>> then the server can shorten the reply if it is resource-intensive to
>> provide details over the entire client request?
> 
> Hrm, I wanted to drop that (I did drop another reference to that thing,
> I though), but apparently I forgot.

It looks like elsewhere you make the point that there is always at least
one extent on success, and I think that's sufficient (whether a server
stops at 1, stops at 16, or always provides as much as the client
requests, is then a quality-of-implementation decision on the server; a
client already has to be prepared for a short answer, and should also be
prepared to cache a long answer rather than repeating a query that is
redundant; and a server should tolerate a client that doesn't cache things).

> There were actually more things that I defined multiple times, and I
> thought I'd dropped them all, but apparently not so. I'll fix that up
> ASAP.
> 
> (sorry about the "noise" in that I didn't double-check everything. I'll
> do better next time, honest ;-)

That's okay.  The inter-branch diff is indeed the best way to review the
current state of the changes in relation to the master branch, rather
than trying to follow one patch at a time.  I'm grateful that you've
stepped in to try and nail down some of the wordings, and doing a qemu
proof-of-concept implementation is indeed on my plate of things to
tackle soon (well, structured replies first...)

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

Attachment: signature.asc
Description: OpenPGP digital signature


Reply to: