[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH 03/18] nbd: Minimal structured read for server
From: |
Eric Blake |
Subject: |
Re: [Qemu-block] [PATCH 03/18] nbd: Minimal structured read for server |
Date: |
Thu, 4 May 2017 08:28:32 -0500 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.0 |
On 05/04/2017 05:58 AM, Vladimir Sementsov-Ogievskiy wrote:
>>> @@ -70,6 +70,25 @@ struct NBDSimpleReply {
>>> };
>>> typedef struct NBDSimpleReply NBDSimpleReply;
>>> +typedef struct NBDStructuredReplyChunk {
>>> + uint32_t magic;
>>> + uint16_t flags;
>>> + uint16_t type;
>>> + uint64_t handle;
>>> + uint32_t length;
>>> +} QEMU_PACKED NBDStructuredReplyChunk;
>>> +
>>> +typedef struct NBDStructuredRead {
>>> + NBDStructuredReplyChunk h;
>>> + uint64_t offset;
>>> +} QEMU_PACKED NBDStructuredRead;
>>> +
>>> +typedef struct NBDStructuredError {
>>> + NBDStructuredReplyChunk h;
>>> + uint32_t error;
>>> + uint16_t message_length;
>>> +} QEMU_PACKED NBDStructuredError;
>> Definitely a subset of all types added in the NBD protocol extension,
>> but reasonable for a minimal implementation. Might be worth adding
>> comments to the types...
>
> Hmm, for me their names looks descriptive enough, but my sight may be
> biased.. What kind of comments do you want?
I guess I was thinking of existing structs in include/block/nbd.h:
/* Handshake phase structs - this struct is passed on the wire */
struct nbd_option {
uint64_t magic; /* NBD_OPTS_MAGIC */
uint32_t option; /* NBD_OPT_* */
uint32_t length;
} QEMU_PACKED;
typedef struct nbd_option nbd_option;
and compared to:
/* Transmission phase structs
*
* Note: these are _NOT_ the same as the network representation of an NBD
* request and reply!
*/
struct NBDRequest {
uint64_t handle;
uint64_t from;
uint32_t len;
uint16_t flags; /* NBD_CMD_FLAG_* */
uint16_t type; /* NBD_CMD_* */
};
typedef struct NBDRequest NBDRequest;
where the comments make it obvious whether QEMU_PACKED matters because
we are using the struct to directly map bytes read/written on the wire.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature