qemu-block
[Top][All Lists]
Advanced

[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: Vladimir Sementsov-Ogievskiy
Subject: Re: [Qemu-block] [PATCH 03/18] nbd: Minimal structured read for server
Date: Fri, 5 May 2017 14:30:09 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.8.0

04.05.2017 16:28, Eric Blake wrote:
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.


Ok, thank you, will add.


--
Best regards,
Vladimir




reply via email to

[Prev in Thread] Current Thread [Next in Thread]