Re: [Libguestfs] [PATCH] spec: Add NBD_OPT_EXTENDED_HEADERS

On Mon, Dec 06, 2021 at 05:00:47PM -0600, Eric Blake wrote: > On Mon, Dec 06, 2021 at 02:40:45PM +0300, Vladimir Sementsov-Ogievskiy wrote: > > > #### Simple reply message > > > > > > The simple reply message MUST be sent by the server in response to all > > > requests if structured replies have not been negotiated using > > > -`NBD_OPT_STRUCTURED_REPLY`. If structured replies have been negotiated, a simple > > > -reply MAY be used as a reply to any request other than `NBD_CMD_READ`, > > > -but only if the reply has no data payload. The message looks as > > > -follows: > > > +`NBD_OPT_STRUCTURED_REPLY`. If structured replies have been > > > +negotiated, a simple reply MAY be used as a reply to any request other > > > +than `NBD_CMD_READ`, but only if the reply has no data payload. If > > > +extended headers were not negotiated using `NBD_OPT_EXTENDED_HEADERS`, > > > +the message looks as follows: > > > > > > S: 32 bits, 0x67446698, magic (`NBD_SIMPLE_REPLY_MAGIC`; used to be > > > `NBD_REPLY_MAGIC`) > > > @@ -369,6 +398,16 @@ S: 64 bits, handle > > > S: (*length* bytes of data if the request is of type `NBD_CMD_READ` and > > > *error* is zero) > > > > > > +If extended headers were negotiated using `NBD_OPT_EXTENDED_HEADERS`, > > > +the message looks like: > > > + > > > +S: 32 bits, 0x60d12fd6, magic (`NBD_SIMPLE_REPLY_EXT_MAGIC`) > > > +S: 32 bits, error (MAY be zero) > > > +S: 64 bits, handle > > > +S: 128 bits, padding (MUST be zero) > > > +S: (*length* bytes of data if the request is of type `NBD_CMD_READ` and > > > + *error* is zero) > > > + > > > > If we go this way, let's put payload length into padding: it will help to make the protocol context-independent and less error-prone. Agreed. > Easy enough to do (the payload length will be 0 except for > NBD_CMD_READ). Indeed. > > Or, the otherway, may be just forbid the payload for simple-64bit ? What's the reason to allow 64bit requests without structured reply negotiation? > > The two happened to be orthogonal enough in my implementation. It was > easy to demonstrate either one without the other, and it IS easier to > write a client using non-structured replies (structured reads ARE > tougher than simple reads, even if it is less efficient when it comes > to reading zeros). But you are also right that we could require > structured reads prior to allowing 64-bit operations, and then have > only one supported reply type on the wire when negotiated. Wouter, > which way do you prefer? Given that I *still* haven't gotten around to implementing structured replies for nbd-server, I'd prefer not to require it, but that's not really a decent argument IMO :-) [... I haven't read this in much detail yet, intend to do that later...]

with 64-bit commands, we may want to also make it easier to let servers advertise an actual maximum size they are willing to accept for the commands in question (for example, a server may be happy with a full 64-bit block status, but still want to limit non-fast zero and cache to a smaller limit to avoid denial of service).