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

[Vladimir Sementsov-Ogievskiy]

On 30.05.23 21:22, Eric Blake wrote:
> 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 at redhat.com>
>>
>>
>> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov at yandex-team.ru>
>>
>>> ---
>>>    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:

Yes, sounds good

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

-- 
Best regards,
Vladimir