qemu-devel
[Top][All Lists]
Advanced

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

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


From: Alex Bligh
Subject: Re: [Qemu-devel] [Nbd] [PATCH 3/1] doc: Propose Structured Replies extension
Date: Tue, 29 Mar 2016 18:34:11 +0100

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.

> 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_'?

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.

>>> Sure.  But keep in mind that if (when?) we add a flag for allowing the
>>> server to skip read chunks on holes, we'll have to tweak the wording to
>>> allow the server to send fewer chunks than the client's length, where
>>> the client must then assume zeroes for all chunks not received.
>> 
>> Or alternatively a chunk representing a hole. I wonder whether you
>> might be better to extend the chunk structure by 4 bytes to allow for
>> future modifications like this (e.g. NBD_CHUNK_FLAG_HOLE means
>> the chunk is full of zeroes and has no payload).
> 
> Seems reasonable (then I need to reword everything from len-8 to instead
> be len-12 when dealing with data size within the len bytes of payload).
> Network traffic-wise, I think it's better to always send the chunk
> flags, than it would be to try and make the sending of the chunk flags
> dependent on the client's choice of command flags (that is, we already
> argued that wire format should never be changed based merely on command
> flags, as it makes the server stream harder to decipher in isolation).

Absolutely. And that way if we have to add anything to the chunk (e.g.
we run out of flags!), we can use one or more bits to indicate this.

> Thanks for the good feedback, by the way; it will make v2 better.

No problem. Some time ago I rewrote chunks of the nbd test suite and
wrote the bit that tested parallel outstanding commands. At the back
of my mind is whether I should extend the test suite to test this
and how we could persuade a server to 'often fragment' so we can
test reassembly (some form of debug setting on the server like
'max fragment size' or similar I suspect).

--
Alex Bligh




Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail


reply via email to

[Prev in Thread] Current Thread [Next in Thread]