Re: [Libguestfs] [PATCH v3 05/14] nbd: Add types for extended headers

On Tue, May 30, 2023 at 04:23:46PM +0300, Vladimir Sementsov-Ogievskiy wrote: > On 15.05.23 22:53, Eric Blake wrote: >> Add the constants and structs necessary for later patches to start >> implementing the NBD_OPT_EXTENDED_HEADERS extension in both the client >> and server, matching recent commit e6f3b94a934] in the upstream nbd >> project. This patch does not change any existing behavior, but merely >> sets the stage. >> >> This patch does not change the status quo that neither the client nor >> server use a packed-struct representation for the request header. >> >> Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; > > > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov(a)yandex-team.ru&gt; > >> --- >> docs/interop/nbd.txt | 1 + >> include/block/nbd.h | 74 ++++++++++++++++++++++++++++++++------------ >> nbd/common.c | 10 +++++- >> 3 files changed, 65 insertions(+), 20 deletions(-) >> >> diff --git a/docs/interop/nbd.txt b/docs/interop/nbd.txt >> index f5ca25174a6..abaf4c28a96 100644 >> --- a/docs/interop/nbd.txt >> +++ b/docs/interop/nbd.txt >> @@ -69,3 +69,4 @@ NBD_CMD_BLOCK_STATUS for "qemu:dirty-bitmap:", NBD_CMD_CACHE >> NBD_CMD_FLAG_FAST_ZERO >> * 5.2: NBD_CMD_BLOCK_STATUS for "qemu:allocation-depth" >> * 7.1: NBD_FLAG_CAN_MULTI_CONN for shareable writable exports >> +* 8.1: NBD_OPT_EXTENDED_HEADERS >> diff --git a/include/block/nbd.h b/include/block/nbd.h >> index 50626ab2744..d753fb8006f 100644 >> --- a/include/block/nbd.h >> +++ b/include/block/nbd.h >> @@ -87,13 +87,24 @@ typedef struct NBDStructuredReplyChunk { >> uint32_t length; /* length of payload */ >> } QEMU_PACKED NBDStructuredReplyChunk; >> > > [..] > >> -/* Extent chunk for NBD_REPLY_TYPE_BLOCK_STATUS */ >> +/* Extent array for NBD_REPLY_TYPE_BLOCK_STATUS */ > > Why? NBDExtent is one extent, not extent array. It's not the entire chunk either, because that also includes the header and the metacontext id that are not part of the extent array. Maybe 'Extent array element', which matches our wire layout of:

<- chunk -> <- hdr -><- payload -> ... id <- array -> ext[0] ext[1]... > >> typedef struct NBDExtent { >> uint32_t length; >> uint32_t flags; /* NBD_STATE_* */ >> } QEMU_PACKED NBDExtent; >>