From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42743) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akwew-0005sL-4J for qemu-devel@nongnu.org; Tue, 29 Mar 2016 12:38:27 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1akwer-0000ke-1p for qemu-devel@nongnu.org; Tue, 29 Mar 2016 12:38:26 -0400 Received: from barbershop.grep.be ([89.106.240.122]:40082) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1akweq-0000kU-Or for qemu-devel@nongnu.org; Tue, 29 Mar 2016 12:38:20 -0400 Date: Tue, 29 Mar 2016 18:37:44 +0200 From: Wouter Verhelst Message-ID: <20160329163744.GA29988@grep.be> References: <1459173555-4890-1-git-send-email-eblake@redhat.com> <1459223796-28474-2-git-send-email-eblake@redhat.com> <55B49D68-2F63-4742-9B60-F6B428ABB3E9@alex.org.uk> <56FA8F5B.8060800@redhat.com> <88E5F63B-B036-45C7-B2FD-B555D54E88F4@alex.org.uk> <56FA9B42.2020503@redhat.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="jRHKVT23PllUwdXP" Content-Disposition: inline In-Reply-To: <56FA9B42.2020503@redhat.com> Subject: Re: [Qemu-devel] [Nbd] [PATCH 3/1] doc: Propose Structured Replies extension List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: "nbd-general@lists.sourceforge.net" , "qemu-devel@nongnu.org" , Alex Bligh --jRHKVT23PllUwdXP Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Mar 29, 2016 at 09:12:02AM -0600, Eric Blake wrote: > On 03/29/2016 08:37 AM, Alex Bligh wrote: > > Eric, > >=20 > >> I guess what I need to add is that in transmission phase, most commands > >> have exactly one response per request; but commands may document > >> scenarios where there will be multiple responses to a single request. > >> NBD_CMD_READ uses the multiple responses to make partial read and error > >> handling possible > >=20 > > Yes, this. > >=20 >=20 > >> Are you arguing that there should be a flag that controls whether reads > >> must be in-order vs. reassembled? Reassembly must happen either way, > >> the question is whether having a way to allow out-of-order for > >> efficiency, vs. defaulting to in-order for easier computation, is wort= h it. > >=20 > > No, that sounds overengineered. > >=20 > > 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. >=20 > 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. Next, should it be > independently negotiated, or implied by negotiating > NBD_FLAG_C_STRUCTURED_REPLIES? I'm leaning towards implied - it's I think it should be implied, yes. Having said that, There's only a limited number of flag bits available. We can obviously always add more flags by adding in a second flags field, but that introduces more backwards compatibility issues (would require another global flag to say "we support extended flags", which the client then has to ack, too, etc). As such, not using flag bits when we don't strictly need them is a feature. I'm not sure if this really needs to be negotiated using a flag bit. The NO_ZEROES thing was negotiated using a flag because NBD_OPT_EXPORT_NAME can't be upgraded, but that isn't the case here. This could easily be negotiated using some NBD_OPT thing: client->kernel: check whether structured replies are supported (if yes:) client->server: NBD_OPT_STRUCTURED_REPLIES server->client: NBD_REP_ACK (if supported, or NBD_REP_UNSUP if not) At this point, the server can send structured replies at leisure. We could also set a "support don't fragment" flag bit in the per-export flags field (which is a larger flags field than the global one), so that the client kernel knows it can request non-fragmented replies without requiring an extra kernel API (since per-export flags are passed to the kernel by way of the NBD_SET_FLAGS ioctl). (the spec can then possibly also say that the kernel MAY send structured replies without sending the "support don't fragment" bit, but that it then MUST NOT send fragmented replies -- although I'm not too sure whether that's a good idea) This would also get it more in line with the way the CMD_TRIM and CMD_FLUSH commands are negotiated (by way of a per-export flag). > all-or-none for use of the improved read structures. I'm also leaning > towards the name NBD_FLAG_C_STRUCTURED_READ, since elsewhere I'm That's probably a good idea too, yes (with obvious s/FLAG_C/OPT/ change as per above). > 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). Right. > >> 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. > >=20 > > 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). >=20 > 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). >=20 > Thanks for the good feedback, by the way; it will make v2 better. Regards, --=20 < ron> I mean, the main *practical* problem with C++, is there's like a doz= en people in the world who think they really understand all of its rule= s, and pretty much all of them are just lying to themselves too. -- #debian-devel, OFTC, 2016-02-12 --jRHKVT23PllUwdXP Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCAAGBQJW+q9YAAoJEMKUD5Ub3wqd7K4P/jDXTL/OCYfg3l4e0EGo5IrL esPNlpwK/4+QbwuQMZIow7y7cCLGgEtgbrbG1K7DTzfH2gqHF2VgZs+8H/pbyKYn 6agkNCEvCjMQHEOhWSm7CgfNKjRnrkVrixVuW4jEeEjZcCvMADe4bfYQJHdfpnHc ssxvX8w3bMfxGtxVauxHoO5tejeYNmtzoLKUqzXBg6uxjA9ULY1NSs7ncvajI71Q aSAal4cuS0VRKdnD0Q2ovx7shY6DhojRUFQ8kSYxfGUEYHk9qfMFyMjh9wZrj5x8 rN2i6JxnPMmkg2+3+LSH3/SnzGjLyuo+h/23Q5/DJBtrjzMjmoedwBXBNUGK1qsV PvoCBBkKSw8VPH509P6eiRVpm/tnonTiKjVRa+nbIgwccqGS5twGSHjh6YwCzXsW GRSgHt3e3NuE5dAFue6VfhHQ7SjlyiGV+omb+a7b8RYyocSgggyzZtRu3Nl44lKh /RhbeBx5LCNTWe/Uzc5Z7CD9+wV4CqG9vkf6tZMrPSy/VAKNz+5vje4tC2yMq3TZ EKu+oPBC3MlohsEOSjbouG+iwh45cK+CvPtoAsRDP3XjCGyaVVrevfxt7Ffqn1o/ SF0V4Jla5aazYgZROmZ5EoEvBJNJ5zs9H2ApxnSKnc3xGzZ+WpdVbfATApHmVRpt DppAsmNbiLtno9wuUSkX =HmoU -----END PGP SIGNATURE----- --jRHKVT23PllUwdXP--