Re: [Qemu-devel] [Nbd] [Qemu-block] [PATCH] doc: Propose NBD_FLAG_INIT_ZEROES extension

[Eric Blake]

On 12/14/2016 02:22 AM, Wouter Verhelst wrote:
> Hi Eric,
> 
> On Tue, Dec 13, 2016 at 04:36:08PM -0600, Eric Blake wrote:
>> On 12/13/2016 06:18 AM, Wouter Verhelst wrote:
>>> On Tue, Dec 13, 2016 at 08:38:12AM +0100, Kevin Wolf wrote:
>>>> Am 12.12.2016 um 19:12 hat Wouter Verhelst geschrieben:
>>>>> I'm not opposed to this proposal, per se, but there seems to be some
>>>>> disagreement (by Kevin, for instance) on whether this extension is at
>>>>> all useful.
>>>>
>>>> FWIW, I'm not opposed to adding the flag. I don't think it can hurt, but
>>>> I wanted to ask the question so that we don't end up adding a feature
>>>> that noone actually uses. Ultimately it's your call as a maintainer
>>>> anyway how conservative you want to be with spec additions.
>>>
>>> I'm not opposed either, but I do agree with you that we shouldn't add
>>> such a feature if it doesn't end up getting used. Especially so if it
>>> burns a flag in the (16-bit) "transmission flags" field, where space is
>>> at a premium.
>>
>> No, it is NOT a "transmission flag", as it is a per-export property
>> (where we currently have 64 bits).
> 
> That may be what you meant, but the patch you sent uses a flag in the
> transmission flags field.
> 
> If you meant to use something else, you'll have to clarify what your
> patch should have been like ;-)

/me goes back and re-reads spec - I shouldn't reply to mails purely off
of memory...

Okay, I'm crossing terms here.  "Transmission flags" ARE the per-export
flags - and are sent in response to NBD_OPT_EXPORT_NAME or NBD_OPT_INFO
or NBD_OPT_GO.  And you are right that we only have 16 bits in the
current spec, but that they can differ between exports (case in point:
NBD_FLAG_READ_ONLY).  So my proposal of bit 10 as NBD_FLAG_INIT_ZEROES
is potentially in the right place - it is a per-export property (a
server may support multiple named exports for the client to choose from,
of which only a subset are known to be all zero content at the time of
export).  But your argument that 16 bits is at a premium may be worth
considering an alternative representation for the same information.

Possibility 1: when we run out of our current 16 transmission flags,
we'll need a way to extend the protocol to support more than that.  To
do that, we'd probably want a new Handshake flag
NBD_FLAG_LARGE_TRANSMISSION_FLAGS sent by the server, and which must be
answered by the client sending a new Client flag
NBD_FLAG_C_LARGE_TRANSMISSION_FLAGS, so that both sides know that all
subsequent places in the protocol that send transmission flags will now
send 64 bits instead of 16 bits (old-style negotiation cannot benefit
from this, and is permanently stuck at 16 bits, but we discourage
old-style negotiation).  We'd still want to prioritize which bits land
in the first 16 positions, for maximum compatibility with older servers
or clients (where the higher bit positions are used for data that is
more suited for optimizations, rather than controlling correct usage) -
thus NBD_FLAG_INIT_ZEROES would serve as an example to live in the
extended positions.

Possibility 2: Leverage the extension-info branch: Add a new
NBD_INFO_INIT_ZERO information datum.  NBD_BLOCK_INFO already exists to
let server and client exchange as much per-export information as they
want during handshake without burning any new transmission flags, and
with a specification that gracefully ignores unknown requests from the
client and unknown advertisements from the server.  There's the drawback
that the server can't advertise the known-zero-init optimization to
clients that don't use NBD_OPT_GO, but that's not too bad (especially if
qemu is the first client with code to actually make use of the
optimization, since I am already trying to get qemu to be one of the
first adopters of NBD_OPT_GO).

We'll probably have to eventually use something along the lines of
possibility 1 for more transmission flags for some other reason down the
road (since we have lately been adding one flag per new command/command
group as we add extensions), but I agree that it is a bit heavy for now.
 So it sounds like I should rework the proposal along the lines of
possibility 2, which also implies that we should get extension-info
ready for promotion to current.  And that means that my current proposal
to the extension-write-zeroes branch is probably not ideal, but it
reopens the question of whether extension-write-zeroes as it currently
stands is ready for promotion to stable now that qemu 2.8 implements it.

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

signature.asc
Description: OpenPGP digital signature