Re: [Libguestfs] [PATCH v3 03/14] nbd/server: Prepare for alternate-size headers

On 15.05.23 22:53, Eric Blake wrote: > Upstream NBD now documents[1] an extension that supports 64-bit effect > lengths in requests. As part of that extension, the size of the reply > headers will change in order to permit a 64-bit length in the reply > for symmetry[2]. Additionally, where the reply header is currently > 16 bytes for simple reply, and 20 bytes for structured reply; with the > extension enabled, there will only be one structured reply type, of 32 > bytes. Since we are already wired up to use iovecs, it is easiest to > allow for this change in header size by splitting each structured > reply across two iovecs, one for the header (which will become > variable-length in a future patch according to client negotiation), > and the other for the payload, and removing the header from the > payload struct definitions. Interestingly, the client side code never > utilized the packed types, so only the server code needs to be > updated. Actually, it does, since previous patch :) But, it becomes even better now? Anyway some note somewhere is needed I think.

> > -static inline void set_be_chunk(NBDStructuredReplyChunk *chunk, uint16_t flags, > - uint16_t type, uint64_t handle, uint32_t length) > +static inline void set_be_chunk(NBDClient *client, struct iovec *iov, Suggestion: pass niov here too, and caluculate "length" as a sum of iov lengths starting from second extent automatically.

Also, worth documenting that set_be_chunk() expects half-initialized iov, with .iov_base pointing to NBDReply (IN parameter) and .iov_len unset (OUT parameter), as that's not usual practice

> + uint16_t flags, uint16_t type, > + uint64_t handle, uint32_t length) > { > + NBDStructuredReplyChunk *chunk = iov->iov_base; > + > + iov->iov_len = sizeof(*chunk); > stl_be_p(&chunk->magic, NBD_STRUCTURED_REPLY_MAGIC); > stw_be_p(&chunk->flags, flags); > stw_be_p(&chunk->type, type); [..] -- Best regards, Vladimir