Re: [Libguestfs] [PATCH v2 5/6] spec: Introduce NBD_FLAG_BLOCK_STATUS_PAYLOAD

On Wed, Feb 22, 2023 at 12:05:44PM +0200, Wouter Verhelst wrote: > On Mon, Nov 14, 2022 at 04:46:54PM -0600, Eric Blake wrote: > > #### Simple reply message > > > > @@ -1232,6 +1235,19 @@ The field has the following format: > > will be faster than a regular write). Clients MUST NOT set the > > `NBD_CMD_FLAG_FAST_ZERO` request flag unless this transmission flag > > is set. > > +- bit 12, `NBD_FLAG_BLOCK_STATUS_PAYLOAD`: Indicates that the server > > + understands the use of the `NBD_CMD_FLAG_PAYLOAD_LEN` flag to > > + `NBD_CMD_BLOCK_STATUS` to allow the client to request that the > > + server filters its response to a specific subset of negotiated > > + metacontext ids passed in via a client payload, rather than the > > + default of replying to all metacontext ids. Servers MUST NOT > > + advertise this bit unless the client successfully negotiates > > + extended headers via `NBD_OPT_EXTENDED_HEADERS`, and SHOULD NOT > > + advertise this bit in response to `NBD_OPT_EXPORT_NAME` or > > + `NBD_OPT_GO` if the client does not negotiate metacontexts with > > + `NBD_OPT_SET_META_CONTEXT`; clients SHOULD NOT set the > > + `NBD_CMD_FLAG_PAYLOAD_LEN` flag for `NBD_CMD_BLOCK_STATUS` unless > > + this transmission flag is set. > > Given that we are introducing this at the same time as the extended > headers (and given that we're running out of available flags in this > 16-bit field), isn't it better to make the ability to understand > PAYLOAD_LEN be implied by extended headers? Or is there a use case for > implementing extended headers but not a payload length on > CMD_BLOCK_STATUS that I'm missing? In my proof of implementation, this was a distinct feature addition on top of 64-bit headers. It is easy to write a server that does not want to deal with a client payload on a block status request (for example, a server that only knows about one metacontext, and therefore never needs to deal with a client wanting a subset of negotiated metacontexts). Forcing a server to have to support a client-side filtering request on block status commands, merely because the server now supports 64-bit lengths, seemed like a stretch too far, which is why I decided to use a feature bit for this one.