[Libguestfs] [nbdkit PATCH v2 01/13] protocol: Split flags from cmd field in requests

Eric Blake eblake at redhat.com
Fri Jan 19 13:40:17 UTC 2018


Since NBD is a big-endian protocol, the upstream spec was able to
repurpose a 32-bit field with flags starting at (1<<16) OR'd into
the command into two 16-bit fields (flags first, starting at 1<<0,
then the command) for ease of documentation.  Matching that split
in our code base will also make it easier to implement smarter FUA
flag support.  This addresses one of the TODO in the nbd plugin.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 plugins/nbd/nbd.c | 39 ++++++++++++++++++++-------------------
 src/connections.c | 12 ++++++------
 src/protocol.h    | 10 +++++-----
 3 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c
index 04147f1..c9727f7 100644
--- a/plugins/nbd/nbd.c
+++ b/plugins/nbd/nbd.c
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2017 Red Hat Inc.
+ * Copyright (C) 2017-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -249,13 +249,14 @@ find_trans_by_cookie (struct handle *h, uint64_t cookie)

 /* Send a request, return 0 on success or -1 on write failure. */
 static int
-nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset,
-                 uint32_t count, uint64_t cookie, const void *buf)
+nbd_request_raw (struct handle *h, uint16_t flags, uint16_t type,
+                 uint64_t offset, uint32_t count, uint64_t cookie,
+                 const void *buf)
 {
   struct request req = {
     .magic = htobe32 (NBD_REQUEST_MAGIC),
-    /* TODO nbdkit should have a way to pass flags, separate from cmd type */
-    .type = htobe32 (type),
+    .flags = htobe16 (flags),
+    .type = htobe16 (type),
     .handle = cookie, /* Opaque to server, so endianness doesn't matter */
     .offset = htobe64 (offset),
     .count = htobe32 (count),
@@ -275,8 +276,9 @@ nbd_request_raw (struct handle *h, uint32_t type, uint64_t offset,
 /* Perform the request half of a transaction. On success, return the
    non-negative fd for reading the reply; on error return -1. */
 static int
-nbd_request_full (struct handle *h, uint32_t type, uint64_t offset,
-                  uint32_t count, const void *req_buf, void *rep_buf)
+nbd_request_full (struct handle *h, uint16_t flags, uint16_t type,
+                  uint64_t offset, uint32_t count, const void *req_buf,
+                  void *rep_buf)
 {
   int err;
   struct transaction *trans;
@@ -307,7 +309,7 @@ nbd_request_full (struct handle *h, uint32_t type, uint64_t offset,
   fd = trans->u.fds[0];
   cookie = trans->u.cookie;
   nbd_unlock (h);
-  if (nbd_request_raw (h, type, offset, count, cookie, req_buf) == 0)
+  if (nbd_request_raw (h, flags, type, offset, count, cookie, req_buf) == 0)
     return fd;
   trans = find_trans_by_cookie (h, cookie);

@@ -326,9 +328,10 @@ nbd_request_full (struct handle *h, uint32_t type, uint64_t offset,

 /* Shorthand for nbd_request_full when no extra buffers are involved. */
 static int
-nbd_request (struct handle *h, uint32_t type, uint64_t offset, uint32_t count)
+nbd_request (struct handle *h, uint16_t flags, uint16_t type, uint64_t offset,
+             uint32_t count)
 {
-  return nbd_request_full (h, type, offset, count, NULL, NULL);
+  return nbd_request_full (h, flags, type, offset, count, NULL, NULL);
 }

 /* Read a reply, and look up the fd corresponding to the transaction.
@@ -563,7 +566,7 @@ nbd_close (void *handle)
   struct handle *h = handle;

   if (!h->dead)
-    nbd_request_raw (h, NBD_CMD_DISC, 0, 0, 0, NULL);
+    nbd_request_raw (h, 0, NBD_CMD_DISC, 0, 0, 0, NULL);
   close (h->fd);
   if ((errno = pthread_join (h->reader, NULL)))
     nbdkit_debug ("failed to join reader thread: %m");
@@ -622,7 +625,7 @@ nbd_pread (void *handle, void *buf, uint32_t count, uint64_t offset)

   /* TODO Auto-fragment this if the client has a larger max transfer
      limit than the server */
-  c = nbd_request_full (h, NBD_CMD_READ, offset, count, NULL, buf);
+  c = nbd_request_full (h, 0, NBD_CMD_READ, offset, count, NULL, buf);
   return c < 0 ? c : nbd_reply (h, c);
 }

@@ -635,7 +638,7 @@ nbd_pwrite (void *handle, const void *buf, uint32_t count, uint64_t offset)

   /* TODO Auto-fragment this if the client has a larger max transfer
      limit than the server */
-  c = nbd_request_full (h, NBD_CMD_WRITE, offset, count, buf, NULL);
+  c = nbd_request_full (h, 0, NBD_CMD_WRITE, offset, count, buf, NULL);
   return c < 0 ? c : nbd_reply (h, c);
 }

@@ -644,7 +647,6 @@ static int
 nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)
 {
   struct handle *h = handle;
-  uint32_t cmd = NBD_CMD_WRITE_ZEROES;
   int c;

   if (!(h->flags & NBD_FLAG_SEND_WRITE_ZEROES)) {
@@ -653,9 +655,8 @@ nbd_zero (void *handle, uint32_t count, uint64_t offset, int may_trim)
     return -1;
   }

-  if (!may_trim)
-    cmd |= NBD_CMD_FLAG_NO_HOLE;
-  c = nbd_request (h, cmd, offset, count);
+  c = nbd_request (h, may_trim ? 0 : NBD_CMD_FLAG_NO_HOLE,
+                   NBD_CMD_WRITE_ZEROES, offset, count);
   return c < 0 ? c : nbd_reply (h, c);
 }

@@ -666,7 +667,7 @@ nbd_trim (void *handle, uint32_t count, uint64_t offset)
   struct handle *h = handle;
   int c;

-  c = nbd_request (h, NBD_CMD_TRIM, offset, count);
+  c = nbd_request (h, 0, NBD_CMD_TRIM, offset, count);
   return c < 0 ? c : nbd_reply (h, c);
 }

@@ -677,7 +678,7 @@ nbd_flush (void *handle)
   struct handle *h = handle;
   int c;

-  c = nbd_request (h, NBD_CMD_FLUSH, 0, 0);
+  c = nbd_request (h, 0, NBD_CMD_FLUSH, 0, 0);
   return c < 0 ? c : nbd_reply (h, c);
 }

diff --git a/src/connections.c b/src/connections.c
index 111a810..e700ee0 100644
--- a/src/connections.c
+++ b/src/connections.c
@@ -760,7 +760,7 @@ valid_range (struct connection *conn, uint64_t offset, uint32_t count)

 static bool
 validate_request (struct connection *conn,
-                  uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count,
+                  uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
                   uint32_t *error)
 {
   /* Readonly connection? */
@@ -865,7 +865,7 @@ get_error (struct connection *conn)
  */
 static uint32_t
 handle_request (struct connection *conn,
-                uint32_t cmd, uint32_t flags, uint64_t offset, uint32_t count,
+                uint16_t cmd, uint16_t flags, uint64_t offset, uint32_t count,
                 void *buf)
 {
   bool flush_after_command;
@@ -979,7 +979,8 @@ recv_request_send_reply (struct connection *conn)
   int r;
   struct request request;
   struct reply reply;
-  uint32_t magic, cmd, flags, count, error = 0;
+  uint16_t cmd, flags;
+  uint32_t magic, count, error = 0;
   uint64_t offset;
   CLEANUP_FREE char *buf = NULL;

@@ -1005,9 +1006,8 @@ recv_request_send_reply (struct connection *conn)
       return set_status (conn, -1);
     }

-    cmd = be32toh (request.type);
-    flags = cmd & ~NBD_CMD_MASK_COMMAND;
-    cmd &= NBD_CMD_MASK_COMMAND;
+    flags = be16toh (request.flags);
+    cmd = be16toh (request.type);

     offset = be64toh (request.offset);
     count = be32toh (request.count);
diff --git a/src/protocol.h b/src/protocol.h
index 9d9dad9..aa458a0 100644
--- a/src/protocol.h
+++ b/src/protocol.h
@@ -1,5 +1,5 @@
 /* nbdkit
- * Copyright (C) 2013 Red Hat Inc.
+ * Copyright (C) 2013-2018 Red Hat Inc.
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -112,7 +112,8 @@ struct new_handshake_finish {
 /* Request (client -> server). */
 struct request {
   uint32_t magic;               /* NBD_REQUEST_MAGIC. */
-  uint32_t type;                /* Request type. */
+  uint16_t flags;               /* Request flags. */
+  uint16_t type;                /* Request type. */
   uint64_t handle;              /* Opaque handle. */
   uint64_t offset;              /* Request offset. */
   uint32_t count;               /* Request length. */
@@ -134,9 +135,8 @@ struct reply {
 #define NBD_CMD_FLUSH             3
 #define NBD_CMD_TRIM              4
 #define NBD_CMD_WRITE_ZEROES      6
-#define NBD_CMD_MASK_COMMAND 0xffff
-#define NBD_CMD_FLAG_FUA     (1<<16)
-#define NBD_CMD_FLAG_NO_HOLE (2<<16)
+#define NBD_CMD_FLAG_FUA      (1<<0)
+#define NBD_CMD_FLAG_NO_HOLE  (1<<1)

 /* Error codes (previously errno).
  * See http://git.qemu.org/?p=qemu.git;a=commitdiff;h=ca4414804114fd0095b317785bc0b51862e62ebb
-- 
2.14.3




More information about the Libguestfs mailing list