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
