Merge remote-tracking branch 'remotes/ericb/tags/pull-nbd-2017-07-17' into staging

nbd patches for 2017-07-17

- Eric Blake: nbd: Fix iotests failure due to changed client error message
- Eric Blake: [0/2] NBD fixes before softfreeze

# gpg: Signature made Mon 17 Jul 2017 23:12:20 BST
# gpg:                using RSA key 0xA7A16B4A2527436A
# gpg: Good signature from "Eric Blake <eblake@redhat.com>"
# gpg:                 aka "Eric Blake (Free Software Programmer) <ebb9@byu.net>"
# gpg:                 aka "[jpeg image of size 6874]"
# Primary key fingerprint: 71C2 CC22 B1C4 6029 27D2  F3AA A7A1 6B4A 2527 436A

* remotes/ericb/tags/pull-nbd-2017-07-17:
  nbd: Fix server reply to NBD_OPT_EXPORT_NAME of older clients
  nbd: Trace client command being sent
  nbd: Fix iotests failure due to changed client error message

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

6 files changed, 25 insertions, 17 deletions

diff --git a/nbd/client.c b/nbd/client.c
index c3ee9f36b1..509ed5e4ba 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -232,8 +232,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
         break;
 
     case NBD_REP_ERR_UNKNOWN:
-        error_setg(errp, "Requested export not available for option %" PRIx32
-                   " (%s)", reply->option, nbd_opt_lookup(reply->option));
+        error_setg(errp, "Requested export not available");
         break;
 
     case NBD_REP_ERR_SHUTDOWN:
@@ -253,7 +252,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, nbd_opt_reply *reply,
     }
 
     if (msg) {
-        error_append_hint(errp, "%s\n", msg);
+        error_append_hint(errp, "server reported: %s\n", msg);
     }
 
  cleanup:
@@ -902,7 +901,8 @@ ssize_t nbd_send_request(QIOChannel *ioc, NBDRequest *request)
     uint8_t buf[NBD_REQUEST_SIZE];
 
     trace_nbd_send_request(request->from, request->len, request->handle,
-                           request->flags, request->type);
+                           request->flags, request->type,
+                           nbd_cmd_lookup(request->type));
 
     stl_be_p(buf, NBD_REQUEST_MAGIC);
     stw_be_p(buf + 4, request->flags);
diff --git a/nbd/nbd-internal.h b/nbd/nbd-internal.h
index 4065bc68ac..396ddb4d3e 100644
--- a/nbd/nbd-internal.h
+++ b/nbd/nbd-internal.h
@@ -38,9 +38,13 @@
  */
 
 /* Size of all NBD_OPT_*, without payload */
-#define NBD_REQUEST_SIZE        (4 + 2 + 2 + 8 + 8 + 4)
+#define NBD_REQUEST_SIZE            (4 + 2 + 2 + 8 + 8 + 4)
 /* Size of all NBD_REP_* sent in answer to most NBD_OPT_*, without payload */
-#define NBD_REPLY_SIZE          (4 + 4 + 8)
+#define NBD_REPLY_SIZE              (4 + 4 + 8)
+/* Size of reply to NBD_OPT_EXPORT_NAME */
+#define NBD_REPLY_EXPORT_NAME_SIZE  (8 + 2 + 124)
+/* Size of oldstyle negotiation */
+#define NBD_OLDSTYLE_NEGOTIATE_SIZE (8 + 8 + 8 + 4 + 124)
 
 #define NBD_REQUEST_MAGIC       0x25609513
 #define NBD_REPLY_MAGIC         0x67446698
diff --git a/nbd/server.c b/nbd/server.c
index 49ed57455c..82a78bf439 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -283,12 +283,16 @@ static int nbd_negotiate_handle_export_name(NBDClient *client, uint32_t length,
                                             Error **errp)
 {
     char name[NBD_MAX_NAME_SIZE + 1];
-    char buf[8 + 4 + 124] = "";
+    char buf[NBD_REPLY_EXPORT_NAME_SIZE] = "";
     size_t len;
     int ret;
 
     /* Client sends:
         [20 ..  xx]   export name (length bytes)
+       Server replies:
+        [ 0 ..   7]   size
+        [ 8 ..   9]   export flags
+        [10 .. 133]   reserved     (0) [unless no_zeroes]
      */
     trace_nbd_negotiate_handle_export_name();
     if (length >= sizeof(name)) {
@@ -800,22 +804,21 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
  */
 static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
 {
-    char buf[8 + 8 + 8 + 128];
+    char buf[NBD_OLDSTYLE_NEGOTIATE_SIZE] = "";
     int ret;
     const uint16_t myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
                               NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA |
                               NBD_FLAG_SEND_WRITE_ZEROES);
     bool oldStyle;
 
-    /* Old style negotiation header without options
+    /* Old style negotiation header, no room for options
         [ 0 ..   7]   passwd       ("NBDMAGIC")
         [ 8 ..  15]   magic        (NBD_CLIENT_MAGIC)
         [16 ..  23]   size
-        [24 ..  25]   server flags (0)
-        [26 ..  27]   export flags
+        [24 ..  27]   export flags (zero-extended)
         [28 .. 151]   reserved     (0)
 
-       New style negotiation header with options
+       New style negotiation header, client can send options
         [ 0 ..   7]   passwd       ("NBDMAGIC")
         [ 8 ..  15]   magic        (NBD_OPTS_MAGIC)
         [16 ..  17]   server flags (0)
@@ -825,7 +828,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
     qio_channel_set_blocking(client->ioc, false, NULL);
 
     trace_nbd_negotiate_begin();
-    memset(buf, 0, sizeof(buf));
     memcpy(buf, "NBDMAGIC", 8);
 
     oldStyle = client->exp != NULL && !client->tlscreds;
@@ -834,7 +836,7 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
                                       client->exp->nbdflags | myflags);
         stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
         stq_be_p(buf + 16, client->exp->size);
-        stw_be_p(buf + 26, client->exp->nbdflags | myflags);
+        stl_be_p(buf + 24, client->exp->nbdflags | myflags);
 
         if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
             error_prepend(errp, "write failed: ");
diff --git a/nbd/trace-events b/nbd/trace-events
index f5024d85a1..d7c7746ea8 100644
--- a/nbd/trace-events
+++ b/nbd/trace-events
@@ -28,7 +28,7 @@ nbd_client_loop(void) "Doing NBD loop"
 nbd_client_loop_ret(int ret, const char *error) "NBD loop returned %d: %s"
 nbd_client_clear_queue(void) "Clearing NBD queue"
 nbd_client_clear_socket(void) "Clearing NBD socket"
-nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = %" PRIx16 ", .type = %" PRIu16 " }"
+nbd_send_request(uint64_t from, uint32_t len, uint64_t handle, uint16_t flags, uint16_t type, const char *name) "Sending request to server: { .from = %" PRIu64", .len = %" PRIu32 ", .handle = %" PRIu64 ", .flags = %" PRIx16 ", .type = %" PRIu16 " (%s) }"
 nbd_receive_reply(uint32_t magic, int32_t error, uint64_t handle) "Got reply: { magic = 0x%" PRIx32 ", .error = % " PRId32 ", handle = %" PRIu64" }"
 
 # nbd/server.c
diff --git a/tests/qemu-iotests/140.out b/tests/qemu-iotests/140.out
index 0689b2b41c..7295b3d975 100644
--- a/tests/qemu-iotests/140.out
+++ b/tests/qemu-iotests/140.out
@@ -8,7 +8,8 @@ wrote 65536/65536 bytes at offset 0
 read 65536/65536 bytes at offset 0
 64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 {"return": {}}
-can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: No export with name 'drv' available
+can't open device nbd+unix:///drv?socket=TEST_DIR/nbd: Requested export not available
+server reported: export 'drv' not present
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
 *** done
diff --git a/tests/qemu-iotests/143.out b/tests/qemu-iotests/143.out
index 0978b8985a..1c7fb45543 100644
--- a/tests/qemu-iotests/143.out
+++ b/tests/qemu-iotests/143.out
@@ -1,7 +1,8 @@
 QA output created by 143
 {"return": {}}
 {"return": {}}
-can't open device nbd+unix:///no_such_export?socket=TEST_DIR/nbd: No export with name 'no_such_export' available
+can't open device nbd+unix:///no_such_export?socket=TEST_DIR/nbd: Requested export not available
+server reported: export 'no_such_export' not present
 {"return": {}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN", "data": {"guest": false}}
 *** done