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.


[1] 
https://github.com/NetworkBlockDevice/nbd/blob/extension-ext-header/doc/proto.md
as of NBD commit e6f3b94a934

[2] Note that on the surface, this is because some future server might
permit a 4G+ NBD_CMD_READ and need to reply with that much data in one
transaction.  But even though the extended reply length is widened to
64 bits, for now the NBD spec is clear that servers will not reply
with more than a maximum payload bounded by the 32-bit
NBD_INFO_BLOCK_SIZE field; allowing a client and server to mutually
agree to transactions larger than 4G would require yet another
extension.

Signed-off-by: Eric Blake <ebl...@redhat.com>


Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>

---
  include/block/nbd.h |  8 +++---
  nbd/server.c        | 64 ++++++++++++++++++++++++++++-----------------
  2 files changed, 44 insertions(+), 28 deletions(-)


[..]


-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

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to