[Qemu-devel] [PULL 0/5] NBD patches through 2018-10-03

[Qemu-devel] [PULL 0/5] NBD patches through 2018-10-03

[Eric Blake]

The following changes since commit dafd95053611aa14dda40266857608d12ddce658:

  Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-02 18:27:18 +0100)

are available in the Git repository at:

  git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-10-03

for you to fetch changes up to 7f7dfe2a53446072c136d349e3150c84d322b2bc:

  nbd/server: drop old-style negotiation (2018-10-03 15:52:32 -0500)

----------------------------------------------------------------
nbd patches for 2018-10-03

Fix bug in NBD_CMD_CACHE, drop support for oldstyle NBD server,
minor build and doc fixes

- Vladimir Sementsov-Ogievskiy: 0/2 server: drop old-style negotiation
- Eric Blake: qemu-nbd: Document --tls-creds
- Vladimir Sementsov-Ogievskiy: nbd/server: fix NBD_CMD_CACHE
- Peter Maydell: nbd: Don't take address of fields in packed structs

----------------------------------------------------------------
Eric Blake (1):
      qemu-nbd: Document --tls-creds

Peter Maydell (1):
      nbd: Don't take address of fields in packed structs

Vladimir Sementsov-Ogievskiy (3):
      nbd/server: fix NBD_CMD_CACHE
      qemu-nbd: drop old-style negotiation
      nbd/server: drop old-style negotiation

 include/block/nbd.h |  3 +-
 blockdev-nbd.c      |  3 +-
 nbd/client.c        | 44 ++++++++++++++---------------
 nbd/server.c        | 80 +++++++++++++++++++----------------------------------
 qemu-nbd.c          | 26 +++++------------
 5 files changed, 60 insertions(+), 96 deletions(-)

-- 
2.17.1

[Qemu-devel] [PULL 1/5] nbd: Don't take address of fields in packed structs

[Eric Blake]

From: Peter Maydell <peter.maydell@linaro.org>

Taking the address of a field in a packed struct is a bad idea, because
it might not be actually aligned enough for that pointer type (and
thus cause a crash on dereference on some host architectures). Newer
versions of clang warn about this. Avoid the bug by not using the
"modify in place" byte swapping functions.

This patch was produced with the following spatch script:
@@
expression E;
@@
-be16_to_cpus(&E);
+E = be16_to_cpu(E);
@@
expression E;
@@
-be32_to_cpus(&E);
+E = be32_to_cpu(E);
@@
expression E;
@@
-be64_to_cpus(&E);
+E = be64_to_cpu(E);
@@
expression E;
@@
-cpu_to_be16s(&E);
+E = cpu_to_be16(E);
@@
expression E;
@@
-cpu_to_be32s(&E);
+E = cpu_to_be32(E);
@@
expression E;
@@
-cpu_to_be64s(&E);
+E = cpu_to_be64(E);

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-Id: <20180927164200.15097-1-peter.maydell@linaro.org>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase, and squash in missed changes]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/client.c | 44 ++++++++++++++++++++++----------------------
 nbd/server.c | 24 ++++++++++++------------
 2 files changed, 34 insertions(+), 34 deletions(-)

diff --git a/nbd/client.c b/nbd/client.c
index 40b74d9761f..b4d457a19ad 100644
--- a/nbd/client.c
+++ b/nbd/client.c
@@ -117,10 +117,10 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt,
         nbd_send_opt_abort(ioc);
         return -1;
     }
-    be64_to_cpus(&reply->magic);
-    be32_to_cpus(&reply->option);
-    be32_to_cpus(&reply->type);
-    be32_to_cpus(&reply->length);
+    reply->magic = be64_to_cpu(reply->magic);
+    reply->option = be32_to_cpu(reply->option);
+    reply->type = be32_to_cpu(reply->type);
+    reply->length = be32_to_cpu(reply->length);

     trace_nbd_receive_option_reply(reply->option, nbd_opt_lookup(reply->option),
                                    reply->type, nbd_rep_lookup(reply->type),
@@ -396,7 +396,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
             return -1;
         }
         len -= sizeof(type);
-        be16_to_cpus(&type);
+        type = be16_to_cpu(type);
         switch (type) {
         case NBD_INFO_EXPORT:
             if (len != sizeof(info->size) + sizeof(info->flags)) {
@@ -410,13 +410,13 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
                 nbd_send_opt_abort(ioc);
                 return -1;
             }
-            be64_to_cpus(&info->size);
+            info->size = be64_to_cpu(info->size);
             if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
                 error_prepend(errp, "failed to read info flags: ");
                 nbd_send_opt_abort(ioc);
                 return -1;
             }
-            be16_to_cpus(&info->flags);
+            info->flags = be16_to_cpu(info->flags);
             trace_nbd_receive_negotiate_size_flags(info->size, info->flags);
             break;

@@ -433,7 +433,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
                 nbd_send_opt_abort(ioc);
                 return -1;
             }
-            be32_to_cpus(&info->min_block);
+            info->min_block = be32_to_cpu(info->min_block);
             if (!is_power_of_2(info->min_block)) {
                 error_setg(errp, "server minimum block size %" PRIu32
                            " is not a power of two", info->min_block);
@@ -447,7 +447,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
                 nbd_send_opt_abort(ioc);
                 return -1;
             }
-            be32_to_cpus(&info->opt_block);
+            info->opt_block = be32_to_cpu(info->opt_block);
             if (!is_power_of_2(info->opt_block) ||
                 info->opt_block < info->min_block) {
                 error_setg(errp, "server preferred block size %" PRIu32
@@ -461,7 +461,7 @@ static int nbd_opt_go(QIOChannel *ioc, const char *wantname,
                 nbd_send_opt_abort(ioc);
                 return -1;
             }
-            be32_to_cpus(&info->max_block);
+            info->max_block = be32_to_cpu(info->max_block);
             if (info->max_block < info->min_block) {
                 error_setg(errp, "server maximum block size %" PRIu32
                            " is not valid", info->max_block);
@@ -668,7 +668,7 @@ static int nbd_negotiate_simple_meta_context(QIOChannel *ioc,
         if (nbd_read(ioc, &received_id, sizeof(received_id), errp) < 0) {
             return -1;
         }
-        be32_to_cpus(&received_id);
+        received_id = be32_to_cpu(received_id);

         reply.length -= sizeof(received_id);
         name = g_malloc(reply.length + 1);
@@ -872,13 +872,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
             error_prepend(errp, "Failed to read export length: ");
             goto fail;
         }
-        be64_to_cpus(&info->size);
+        info->size = be64_to_cpu(info->size);

         if (nbd_read(ioc, &info->flags, sizeof(info->flags), errp) < 0) {
             error_prepend(errp, "Failed to read export flags: ");
             goto fail;
         }
-        be16_to_cpus(&info->flags);
+        info->flags = be16_to_cpu(info->flags);
     } else if (magic == NBD_CLIENT_MAGIC) {
         uint32_t oldflags;

@@ -895,13 +895,13 @@ int nbd_receive_negotiate(QIOChannel *ioc, const char *name,
             error_prepend(errp, "Failed to read export length: ");
             goto fail;
         }
-        be64_to_cpus(&info->size);
+        info->size = be64_to_cpu(info->size);

         if (nbd_read(ioc, &oldflags, sizeof(oldflags), errp) < 0) {
             error_prepend(errp, "Failed to read export flags: ");
             goto fail;
         }
-        be32_to_cpus(&oldflags);
+        oldflags = be32_to_cpu(oldflags);
         if (oldflags & ~0xffff) {
             error_setg(errp, "Unexpected export flags %0x" PRIx32, oldflags);
             goto fail;
@@ -1080,8 +1080,8 @@ static int nbd_receive_simple_reply(QIOChannel *ioc, NBDSimpleReply *reply,
         return ret;
     }

-    be32_to_cpus(&reply->error);
-    be64_to_cpus(&reply->handle);
+    reply->error = be32_to_cpu(reply->error);
+    reply->handle = be64_to_cpu(reply->handle);

     return 0;
 }
@@ -1105,10 +1105,10 @@ static int nbd_receive_structured_reply_chunk(QIOChannel *ioc,
         return ret;
     }

-    be16_to_cpus(&chunk->flags);
-    be16_to_cpus(&chunk->type);
-    be64_to_cpus(&chunk->handle);
-    be32_to_cpus(&chunk->length);
+    chunk->flags = be16_to_cpu(chunk->flags);
+    chunk->type = be16_to_cpu(chunk->type);
+    chunk->handle = be64_to_cpu(chunk->handle);
+    chunk->length = be32_to_cpu(chunk->length);

     return 0;
 }
@@ -1128,7 +1128,7 @@ int nbd_receive_reply(QIOChannel *ioc, NBDReply *reply, Error **errp)
         return ret;
     }

-    be32_to_cpus(&reply->magic);
+    reply->magic = be32_to_cpu(reply->magic);

     switch (reply->magic) {
     case NBD_SIMPLE_REPLY_MAGIC:
diff --git a/nbd/server.c b/nbd/server.c
index c3dd402b45e..98d0fa25158 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -333,7 +333,7 @@ static int nbd_opt_read_name(NBDClient *client, char *name, uint32_t *length,
     if (ret <= 0) {
         return ret;
     }
-    cpu_to_be32s(&len);
+    len = cpu_to_be32(len);

     if (len > NBD_MAX_NAME_SIZE) {
         return nbd_opt_invalid(client, errp,
@@ -486,7 +486,7 @@ static int nbd_negotiate_send_info(NBDClient *client,
     if (rc < 0) {
         return rc;
     }
-    cpu_to_be16s(&info);
+    info = cpu_to_be16(info);
     if (nbd_write(client->ioc, &info, sizeof(info), errp) < 0) {
         return -EIO;
     }
@@ -551,14 +551,14 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags,
     if (rc <= 0) {
         return rc;
     }
-    be16_to_cpus(&requests);
+    requests = be16_to_cpu(requests);
     trace_nbd_negotiate_handle_info_requests(requests);
     while (requests--) {
         rc = nbd_opt_read(client, &request, sizeof(request), errp);
         if (rc <= 0) {
             return rc;
         }
-        be16_to_cpus(&request);
+        request = be16_to_cpu(request);
         trace_nbd_negotiate_handle_info_request(request,
                                                 nbd_info_lookup(request));
         /* We care about NBD_INFO_NAME and NBD_INFO_BLOCK_SIZE;
@@ -618,9 +618,9 @@ static int nbd_negotiate_handle_info(NBDClient *client, uint16_t myflags,
     /* maximum - At most 32M, but smaller as appropriate. */
     sizes[2] = MIN(blk_get_max_transfer(exp->blk), NBD_MAX_BUFFER_SIZE);
     trace_nbd_negotiate_handle_info_block_size(sizes[0], sizes[1], sizes[2]);
-    cpu_to_be32s(&sizes[0]);
-    cpu_to_be32s(&sizes[1]);
-    cpu_to_be32s(&sizes[2]);
+    sizes[0] = cpu_to_be32(sizes[0]);
+    sizes[1] = cpu_to_be32(sizes[1]);
+    sizes[2] = cpu_to_be32(sizes[2]);
     rc = nbd_negotiate_send_info(client, NBD_INFO_BLOCK_SIZE,
                                  sizeof(sizes), sizes, errp);
     if (rc < 0) {
@@ -904,7 +904,7 @@ static int nbd_negotiate_meta_query(NBDClient *client,
     if (ret <= 0) {
         return ret;
     }
-    cpu_to_be32s(&len);
+    len = cpu_to_be32(len);

     if (len < ns_len) {
         trace_nbd_negotiate_meta_query_skip("length too short");
@@ -971,7 +971,7 @@ static int nbd_negotiate_meta_queries(NBDClient *client,
     if (ret <= 0) {
         return ret;
     }
-    cpu_to_be32s(&nb_queries);
+    nb_queries = cpu_to_be32(nb_queries);
     trace_nbd_negotiate_meta_context(nbd_opt_lookup(client->opt),
                                      export_name, nb_queries);

@@ -1049,7 +1049,7 @@ static int nbd_negotiate_options(NBDClient *client, uint16_t myflags,
         error_prepend(errp, "read failed: ");
         return -EIO;
     }
-    be32_to_cpus(&flags);
+    flags = be32_to_cpu(flags);
     trace_nbd_negotiate_options_flags(flags);
     if (flags & NBD_FLAG_C_FIXED_NEWSTYLE) {
         fixedNewstyle = true;
@@ -1900,8 +1900,8 @@ static int blockstatus_to_extents(BlockDriverState *bs, uint64_t offset,
     extents_end = extent + 1;

     for (extent = extents; extent < extents_end; extent++) {
-        cpu_to_be32s(&extent->flags);
-        cpu_to_be32s(&extent->length);
+        extent->flags = cpu_to_be32(extent->flags);
+        extent->length = cpu_to_be32(extent->length);
     }

     *bytes -= remaining_bytes;
-- 
2.17.1

[Qemu-devel] [PULL 2/5] nbd/server: fix NBD_CMD_CACHE

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

We should not go to structured-read branch on CACHE command, fix that.

Bug introduced in bc37b06a5cde24 "nbd/server: introduce NBD_CMD_CACHE"
with the whole feature and affects 3.0.0 release.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
CC: qemu-stable@nongnu.org
Message-Id: <20181003144738.70670-1-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: commit message typo fix]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 nbd/server.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/nbd/server.c b/nbd/server.c
index 98d0fa25158..4fb247b1166 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -2177,7 +2177,8 @@ static coroutine_fn int nbd_do_cmd_read(NBDClient *client, NBDRequest *request,
     }

     if (client->structured_reply && !(request->flags & NBD_CMD_FLAG_DF) &&
-        request->len) {
+        request->len && request->type != NBD_CMD_CACHE)
+    {
         return nbd_co_send_sparse_read(client, request->handle, request->from,
                                        data, request->len, errp);
     }
-- 
2.17.1

[Qemu-devel] [PULL 3/5] qemu-nbd: Document --tls-creds

[Eric Blake]

Commit 145614a1 introduced --tls-creds and documented it in
qemu-nbd.texi, but forgot to document it in 'qemu-nbd --help'.

Signed-off-by: Eric Blake <eblake@redhat.com>
Message-Id: <20181003180426.602765-1-eblake@redhat.com>
Reviewed-by: John Snow <jsnow@redhat.com>
---
 qemu-nbd.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 51b9d38c727..66e023f7fa4 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -94,6 +94,7 @@ static void usage(const char *name)
 "General purpose options:\n"
 "  --object type,id=ID,...   define an object such as 'secret' for providing\n"
 "                            passwords and/or encryption keys\n"
+"  --tls-creds=ID            use id of an earlier --object to provide TLS\n"
 "  -T, --trace [[enable=]<pattern>][,events=<file>][,file=<file>]\n"
 "                            specify tracing options\n"
 "  --fork                    fork off the server process and exit the parent\n"
-- 
2.17.1

[Qemu-devel] [PULL 4/5] qemu-nbd: drop old-style negotiation

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

Use new-style negotiation always, with default "" (empty) export name
if it is not specified with '-x' option.

qemu as client can manage either style since 2.6.0, commit 69b49502d8

For comparison:

nbd 3.10 dropped oldstyle long ago (Mar 2015):
https://github.com/NetworkBlockDevice/nbd/commit/36940193

nbdkit 1.3 switched its default to newstyle (Jan 2018):
https://github.com/libguestfs/nbdkit/commit/b2a8aecc
https://github.com/libguestfs/nbdkit/commit/8158e773

Furthermore, if a client that only speaks oldstyle still needs to
communicate to qemu, nbdkit remains available to perform the
translation between the two protocols.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20181003170228.95973-2-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: enhance commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 qemu-nbd.c | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 66e023f7fa4..6aaebe7d938 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -56,7 +56,6 @@
 #define MBR_SIZE 512

 static NBDExport *exp;
-static bool newproto;
 static int verbose;
 static char *srcpath;
 static SocketAddress *saddr;
@@ -84,8 +83,8 @@ static void usage(const char *name)
 "  -e, --shared=NUM          device can be shared by NUM clients (default '1')\n"
 "  -t, --persistent          don't exit on the last connection\n"
 "  -v, --verbose             display extra debugging information\n"
-"  -x, --export-name=NAME    expose export by name\n"
-"  -D, --description=TEXT    with -x, also export a human-readable description\n"
+"  -x, --export-name=NAME    expose export by name (default is empty string)\n"
+"  -D, --description=TEXT    export a human-readable description\n"
 "\n"
 "Exposing part of the image:\n"
 "  -o, --offset=OFFSET       offset into the image\n"
@@ -355,8 +354,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,

     nb_fds++;
     nbd_update_server_watch();
-    nbd_client_new(newproto ? NULL : exp, cioc,
-                   tlscreds, NULL, nbd_client_closed);
+    nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed);
 }

 static void nbd_update_server_watch(void)
@@ -550,7 +548,7 @@ int main(int argc, char **argv)
     Error *local_err = NULL;
     BlockdevDetectZeroesOptions detect_zeroes = BLOCKDEV_DETECT_ZEROES_OPTIONS_OFF;
     QDict *options = NULL;
-    const char *export_name = NULL;
+    const char *export_name = ""; /* Default export name */
     const char *export_description = NULL;
     const char *tlscredsid = NULL;
     bool imageOpts = false;
@@ -809,11 +807,6 @@ int main(int argc, char **argv)
             error_report("TLS is not supported with a host device");
             exit(EXIT_FAILURE);
         }
-        if (!export_name) {
-            /* Set the default NBD protocol export name, since
-             * we *must* use new style protocol for TLS */
-            export_name = "";
-        }
         tlscreds = nbd_get_tls_creds(tlscredsid, &local_err);
         if (local_err) {
             error_report("Failed to get TLS creds %s",
@@ -1014,14 +1007,8 @@ int main(int argc, char **argv)
         error_report_err(local_err);
         exit(EXIT_FAILURE);
     }
-    if (export_name) {
-        nbd_export_set_name(exp, export_name);
-        nbd_export_set_description(exp, export_description);
-        newproto = true;
-    } else if (export_description) {
-        error_report("Export description requires an export name");
-        exit(EXIT_FAILURE);
-    }
+    nbd_export_set_name(exp, export_name);
+    nbd_export_set_description(exp, export_description);

     if (device) {
         int ret;
-- 
2.17.1

[Qemu-devel] [PULL 5/5] nbd/server: drop old-style negotiation

[Eric Blake]

From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

After the previous commit, nbd_client_new's first parameter is always
NULL. Let's drop it with all corresponding old-style negotiation code
path which is unreachable now.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
Message-Id: <20181003170228.95973-3-vsementsov@virtuozzo.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: re-wrap short line]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/block/nbd.h |  3 +--
 blockdev-nbd.c      |  3 +--
 nbd/server.c        | 53 +++++++++++++--------------------------------
 qemu-nbd.c          |  2 +-
 4 files changed, 18 insertions(+), 43 deletions(-)

diff --git a/include/block/nbd.h b/include/block/nbd.h
index 4638c839f51..0129d1a4b46 100644
--- a/include/block/nbd.h
+++ b/include/block/nbd.h
@@ -308,8 +308,7 @@ void nbd_export_set_name(NBDExport *exp, const char *name);
 void nbd_export_set_description(NBDExport *exp, const char *description);
 void nbd_export_close_all(void);

-void nbd_client_new(NBDExport *exp,
-                    QIOChannelSocket *sioc,
+void nbd_client_new(QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
                     void (*close_fn)(NBDClient *, bool));
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
index 1ef11041a73..1d170c80b82 100644
--- a/blockdev-nbd.c
+++ b/blockdev-nbd.c
@@ -36,8 +36,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,
                        gpointer opaque)
 {
     qio_channel_set_name(QIO_CHANNEL(cioc), "nbd-server");
-    nbd_client_new(NULL, cioc,
-                   nbd_server->tlscreds, NULL,
+    nbd_client_new(cioc, nbd_server->tlscreds, NULL,
                    nbd_blockdev_client_closed);
 }

diff --git a/nbd/server.c b/nbd/server.c
index 4fb247b1166..a1eda0114fa 100644
--- a/nbd/server.c
+++ b/nbd/server.c
@@ -1253,7 +1253,6 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
     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 | NBD_FLAG_SEND_CACHE);
-    bool oldStyle;

     /* Old style negotiation header, no room for options
         [ 0 ..   7]   passwd       ("NBDMAGIC")
@@ -1274,33 +1273,19 @@ static coroutine_fn int nbd_negotiate(NBDClient *client, Error **errp)
     trace_nbd_negotiate_begin();
     memcpy(buf, "NBDMAGIC", 8);

-    oldStyle = client->exp != NULL && !client->tlscreds;
-    if (oldStyle) {
-        trace_nbd_negotiate_old_style(client->exp->size,
-                                      client->exp->nbdflags | myflags);
-        stq_be_p(buf + 8, NBD_CLIENT_MAGIC);
-        stq_be_p(buf + 16, client->exp->size);
-        stl_be_p(buf + 24, client->exp->nbdflags | myflags);
+    stq_be_p(buf + 8, NBD_OPTS_MAGIC);
+    stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);

-        if (nbd_write(client->ioc, buf, sizeof(buf), errp) < 0) {
-            error_prepend(errp, "write failed: ");
-            return -EINVAL;
-        }
-    } else {
-        stq_be_p(buf + 8, NBD_OPTS_MAGIC);
-        stw_be_p(buf + 16, NBD_FLAG_FIXED_NEWSTYLE | NBD_FLAG_NO_ZEROES);
-
-        if (nbd_write(client->ioc, buf, 18, errp) < 0) {
-            error_prepend(errp, "write failed: ");
-            return -EINVAL;
-        }
-        ret = nbd_negotiate_options(client, myflags, errp);
-        if (ret != 0) {
-            if (ret < 0) {
-                error_prepend(errp, "option negotiation failed: ");
-            }
-            return ret;
+    if (nbd_write(client->ioc, buf, 18, errp) < 0) {
+        error_prepend(errp, "write failed: ");
+        return -EINVAL;
+    }
+    ret = nbd_negotiate_options(client, myflags, errp);
+    if (ret != 0) {
+        if (ret < 0) {
+            error_prepend(errp, "option negotiation failed: ");
         }
+        return ret;
     }

     assert(!client->optlen);
@@ -2396,13 +2381,8 @@ static void nbd_client_receive_next_request(NBDClient *client)
 static coroutine_fn void nbd_co_client_start(void *opaque)
 {
     NBDClient *client = opaque;
-    NBDExport *exp = client->exp;
     Error *local_err = NULL;

-    if (exp) {
-        nbd_export_get(exp);
-        QTAILQ_INSERT_TAIL(&exp->clients, client, next);
-    }
     qemu_co_mutex_init(&client->send_lock);

     if (nbd_negotiate(client, &local_err)) {
@@ -2417,13 +2397,11 @@ static coroutine_fn void nbd_co_client_start(void *opaque)
 }

 /*
- * Create a new client listener on the given export @exp, using the
- * given channel @sioc.  Begin servicing it in a coroutine.  When the
- * connection closes, call @close_fn with an indication of whether the
- * client completed negotiation.
+ * Create a new client listener using the given channel @sioc.
+ * Begin servicing it in a coroutine.  When the connection closes, call
+ * @close_fn with an indication of whether the client completed negotiation.
  */
-void nbd_client_new(NBDExport *exp,
-                    QIOChannelSocket *sioc,
+void nbd_client_new(QIOChannelSocket *sioc,
                     QCryptoTLSCreds *tlscreds,
                     const char *tlsaclname,
                     void (*close_fn)(NBDClient *, bool))
@@ -2433,7 +2411,6 @@ void nbd_client_new(NBDExport *exp,

     client = g_new0(NBDClient, 1);
     client->refcount = 1;
-    client->exp = exp;
     client->tlscreds = tlscreds;
     if (tlscreds) {
         object_ref(OBJECT(client->tlscreds));
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 6aaebe7d938..e76fe3082ab 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -354,7 +354,7 @@ static void nbd_accept(QIONetListener *listener, QIOChannelSocket *cioc,

     nb_fds++;
     nbd_update_server_watch();
-    nbd_client_new(NULL, cioc, tlscreds, NULL, nbd_client_closed);
+    nbd_client_new(cioc, tlscreds, NULL, nbd_client_closed);
 }

 static void nbd_update_server_watch(void)
-- 
2.17.1

Re: [Qemu-devel] [PULL 0/5] NBD patches through 2018-10-03

[Eric Blake]

On 10/3/18 4:22 PM, Eric Blake wrote:
> The following changes since commit dafd95053611aa14dda40266857608d12ddce658:
> 
>    Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream' into staging (2018-10-02 18:27:18 +0100)
> 
> are available in the Git repository at:
> 
>    git://repo.or.cz/qemu/ericb.git tags/pull-nbd-2018-10-03
> 
> for you to fetch changes up to 7f7dfe2a53446072c136d349e3150c84d322b2bc:
> 
>    nbd/server: drop old-style negotiation (2018-10-03 15:52:32 -0500)
> 
> ----------------------------------------------------------------
> nbd patches for 2018-10-03
> 
> Fix bug in NBD_CMD_CACHE, drop support for oldstyle NBD server,
> minor build and doc fixes
> 
> - Vladimir Sementsov-Ogievskiy: 0/2 server: drop old-style negotiation
> - Eric Blake: qemu-nbd: Document --tls-creds
> - Vladimir Sementsov-Ogievskiy: nbd/server: fix NBD_CMD_CACHE
> - Peter Maydell: nbd: Don't take address of fields in packed structs

NACK; a second bug in NBD_CMD_CACHE was found, I'll post a v2 series to 
include that fix as well.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org