[Qemu-devel] [PULL 00/16] spice patch queue

[Qemu-devel] [PULL 00/16] spice patch queue

[Gerd Hoffmann]

  Hi,

Here comes the spice patch queue, with some nbd bits for upcoming
spice-nbd and the usual share of small fixes & cleanups.

please pull,
  Gerd

The following changes since commit e157b8fdd412d48eacfbb8c67d3d58780154faa3:

  Merge remote-tracking branch 'bonzini/virtio' into staging (2013-12-13 11:10:33 -0800)

are available in the git repository at:


  git://anongit.freedesktop.org/spice/qemu tags/pull-spice-1

for you to fetch changes up to 75c70e37bc4a6bdc394b4d1b163fe730abb82c72:

  spice: stop server for qxl hard reset (2013-12-16 10:12:20 +0100)

----------------------------------------------------------------
Collection of little cleanups anf bugfixes.
nbd patches in preparation of spice-nbd.

----------------------------------------------------------------
Gerd Hoffmann (4):
      spice: flip streaming video mode to off by default
      spice: move qemu_spice_display_*() from spice-graphics to spice-core
      spice: move spice_server_vm_{start,stop} calls into qemu_spice_display_*()
      spice: stop server for qxl hard reset

Marc-André Lureau (12):
      vscclient: do not add a socket watch if there is not data to send
      spice-char: remove unused field
      qmp_change_blockdev() remove unused has_format
      include: add missing config-host.h include
      char: add qemu_chr_fe_event()
      spice-char: implement chardev port event
      Split nbd block client code
      nbd: don't change socket block during negotiate
      nbd: pass export name as init argument
      nbd: make nbd_client_session_close() idempotent
      nbd: finish any pending coroutine
      nbd: avoid uninitialized warnings

 block/Makefile.objs       |   2 +-
 block/nbd-client.c        | 385 ++++++++++++++++++++++++++++++++++++++++++++++
 block/nbd-client.h        |  50 ++++++
 block/nbd.c               | 380 ++++-----------------------------------------
 blockdev.c                |   2 +-
 hw/display/qxl.c          |  10 ++
 include/sysemu/blockdev.h |   2 +-
 include/sysemu/char.h     |  10 ++
 include/ui/qemu-spice.h   |   2 +
 libcacard/vscclient.c     |  10 +-
 nbd.c                     |   2 -
 qemu-char.c               |   7 +
 qmp.c                     |   2 +-
 spice-qemu-char.c         |  11 +-
 ui/spice-core.c           |  22 ++-
 ui/spice-display.c        |  17 --
 16 files changed, 537 insertions(+), 377 deletions(-)
 create mode 100644 block/nbd-client.c
 create mode 100644 block/nbd-client.h

[Qemu-devel] [PATCH 01/16] spice: flip streaming video mode to off by default

[Gerd Hoffmann]

Video streaming detection heuristics in spice-server have problems
keeping modern desktop animations (as done by gnome shell) and real
video playback apart.  This leads to jpeg compression artefacts on
your desktop, due to spice using mjpeg to send what it thinks is
a video stream.

Turn off video detection by default to avoid these artifacts.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Alon Levy <alevy@redhat.com>
---
 ui/spice-core.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index e4d533d..9fb9544 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -776,6 +776,8 @@ void qemu_spice_init(void)
     if (str) {
         int streaming_video = parse_stream_video(str);
         spice_server_set_streaming_video(spice_server, streaming_video);
+    } else {
+        spice_server_set_streaming_video(spice_server, SPICE_STREAM_VIDEO_OFF);
     }
 
     spice_server_set_agent_mouse
-- 
1.8.3.1

[Qemu-devel] [PATCH 02/16] vscclient: do not add a socket watch if there is not data to send

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Fixes the following error:
** (process:780): CRITICAL **: do_socket_send: assertion
`socket_to_send->len != 0' failed

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 libcacard/vscclient.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index f1d46d3..24f7088 100644
--- a/libcacard/vscclient.c
+++ b/libcacard/vscclient.c
@@ -58,7 +58,7 @@ static QemuMutex socket_to_send_lock;
 static guint socket_tag;
 
 static void
-update_socket_watch(gboolean out);
+update_socket_watch(void);
 
 static gboolean
 do_socket_send(GIOChannel *source,
@@ -80,7 +80,7 @@ do_socket_send(GIOChannel *source,
     g_byte_array_remove_range(socket_to_send, 0, bw);
 
     if (socket_to_send->len == 0) {
-        update_socket_watch(FALSE);
+        update_socket_watch();
         return FALSE;
     }
     return TRUE;
@@ -89,7 +89,7 @@ do_socket_send(GIOChannel *source,
 static gboolean
 socket_prepare_sending(gpointer user_data)
 {
-    update_socket_watch(TRUE);
+    update_socket_watch();
 
     return FALSE;
 }
@@ -440,8 +440,10 @@ do_socket(GIOChannel *source,
 }
 
 static void
-update_socket_watch(gboolean out)
+update_socket_watch(void)
 {
+    gboolean out = socket_to_send->len > 0;
+
     if (socket_tag != 0) {
         g_source_remove(socket_tag);
     }
-- 
1.8.3.1

[Qemu-devel] [PATCH 03/16] spice-char: remove unused field

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 spice-qemu-char.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index 6d147a7..e074d9e 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -11,7 +11,6 @@
 typedef struct SpiceCharDriver {
     CharDriverState*      chr;
     SpiceCharDeviceInstance     sin;
-    char                  *subtype;
     bool                  active;
     bool                  blocked;
     const uint8_t         *datapos;
-- 
1.8.3.1

[Qemu-devel] [PATCH 04/16] qmp_change_blockdev() remove unused has_format

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 blockdev.c                | 2 +-
 include/sysemu/blockdev.h | 2 +-
 qmp.c                     | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/blockdev.c b/blockdev.c
index 44755e1..6a85961 100644
--- a/blockdev.c
+++ b/blockdev.c
@@ -1523,7 +1523,7 @@ static void qmp_bdrv_open_encrypted(BlockDriverState *bs, const char *filename,
 }
 
 void qmp_change_blockdev(const char *device, const char *filename,
-                         bool has_format, const char *format, Error **errp)
+                         const char *format, Error **errp)
 {
     BlockDriverState *bs;
     BlockDriver *drv = NULL;
diff --git a/include/sysemu/blockdev.h b/include/sysemu/blockdev.h
index 1082091..134712b 100644
--- a/include/sysemu/blockdev.h
+++ b/include/sysemu/blockdev.h
@@ -64,7 +64,7 @@ DriveInfo *drive_init(QemuOpts *arg, BlockInterfaceType block_default_type);
 DriveInfo *add_init_drive(const char *opts);
 
 void qmp_change_blockdev(const char *device, const char *filename,
-                         bool has_format, const char *format, Error **errp);
+                         const char *format, Error **errp);
 void do_commit(Monitor *mon, const QDict *qdict);
 int do_drive_del(Monitor *mon, const QDict *qdict, QObject **ret_data);
 #endif
diff --git a/qmp.c b/qmp.c
index 4c149b3..1d7a04d 100644
--- a/qmp.c
+++ b/qmp.c
@@ -400,7 +400,7 @@ void qmp_change(const char *device, const char *target,
     if (strcmp(device, "vnc") == 0) {
         qmp_change_vnc(target, has_arg, arg, err);
     } else {
-        qmp_change_blockdev(device, target, has_arg, arg, err);
+        qmp_change_blockdev(device, target, arg, err);
     }
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 05/16] include: add missing config-host.h include

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/ui/qemu-spice.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 86c75c7..a93b4b2 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -18,6 +18,8 @@
 #ifndef QEMU_SPICE_H
 #define QEMU_SPICE_H
 
+#include "config-host.h"
+
 #ifdef CONFIG_SPICE
 
 #include <spice.h>
-- 
1.8.3.1

[Qemu-devel] [PATCH 06/16] char: add qemu_chr_fe_event()

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Teach the chardev frontend to send event. This is used by the Spice port
chardev currently.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 include/sysemu/char.h | 10 ++++++++++
 qemu-char.c           |  7 +++++++
 2 files changed, 17 insertions(+)

diff --git a/include/sysemu/char.h b/include/sysemu/char.h
index ad101d9..b81a6ff 100644
--- a/include/sysemu/char.h
+++ b/include/sysemu/char.h
@@ -69,6 +69,7 @@ struct CharDriverState {
     void (*chr_accept_input)(struct CharDriverState *chr);
     void (*chr_set_echo)(struct CharDriverState *chr, bool echo);
     void (*chr_set_fe_open)(struct CharDriverState *chr, int fe_open);
+    void (*chr_fe_event)(struct CharDriverState *chr, int event);
     void *opaque;
     char *label;
     char *filename;
@@ -138,6 +139,15 @@ void qemu_chr_fe_set_echo(struct CharDriverState *chr, bool echo);
 void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open);
 
 /**
+ * @qemu_chr_fe_event:
+ *
+ * Send an event from the front end to the back end.
+ *
+ * @event the event to send
+ */
+void qemu_chr_fe_event(CharDriverState *s, int event);
+
+/**
  * @qemu_chr_fe_printf:
  *
  * Write to a character backend using a printf style interface.
diff --git a/qemu-char.c b/qemu-char.c
index e00f84c..418dc69 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -3353,6 +3353,13 @@ void qemu_chr_fe_set_open(struct CharDriverState *chr, int fe_open)
     }
 }
 
+void qemu_chr_fe_event(struct CharDriverState *chr, int event)
+{
+    if (chr->chr_fe_event) {
+        chr->chr_fe_event(chr, event);
+    }
+}
+
 int qemu_chr_fe_add_watch(CharDriverState *s, GIOCondition cond,
                           GIOFunc func, void *user_data)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH 07/16] spice-char: implement chardev port event

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@gmail.com>

Wire up chardev fe_event to Spice port.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 spice-qemu-char.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/spice-qemu-char.c b/spice-qemu-char.c
index e074d9e..16439c5 100644
--- a/spice-qemu-char.c
+++ b/spice-qemu-char.c
@@ -222,6 +222,15 @@ static void spice_chr_set_fe_open(struct CharDriverState *chr, int fe_open)
     }
 }
 
+static void spice_chr_fe_event(struct CharDriverState *chr, int event)
+{
+#if SPICE_SERVER_VERSION >= 0x000c02
+    SpiceCharDriver *s = chr->opaque;
+
+    spice_server_port_event(&s->sin, event);
+#endif
+}
+
 static void print_allowed_subtypes(void)
 {
     const char** psubtype;
@@ -255,6 +264,7 @@ static CharDriverState *chr_open(const char *subtype)
     chr->chr_close = spice_chr_close;
     chr->chr_set_fe_open = spice_chr_set_fe_open;
     chr->explicit_be_open = true;
+    chr->chr_fe_event = spice_chr_fe_event;
 
     QLIST_INSERT_HEAD(&spice_chars, s, next);
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 08/16] Split nbd block client code

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/Makefile.objs |   2 +-
 block/nbd-client.c  | 372 +++++++++++++++++++++++++++++++++++++++++++++++++++
 block/nbd-client.h  |  51 +++++++
 block/nbd.c         | 373 ++++------------------------------------------------
 4 files changed, 452 insertions(+), 346 deletions(-)
 create mode 100644 block/nbd-client.c
 create mode 100644 block/nbd-client.h

diff --git a/block/Makefile.objs b/block/Makefile.objs
index f43ecbc..4e8c91e 100644
--- a/block/Makefile.objs
+++ b/block/Makefile.objs
@@ -10,7 +10,7 @@ block-obj-$(CONFIG_POSIX) += raw-posix.o
 block-obj-$(CONFIG_LINUX_AIO) += linux-aio.o
 
 ifeq ($(CONFIG_POSIX),y)
-block-obj-y += nbd.o sheepdog.o
+block-obj-y += nbd.o nbd-client.o sheepdog.o
 block-obj-$(CONFIG_LIBISCSI) += iscsi.o
 block-obj-$(CONFIG_CURL) += curl.o
 block-obj-$(CONFIG_RBD) += rbd.o
diff --git a/block/nbd-client.c b/block/nbd-client.c
new file mode 100644
index 0000000..1abfc6a
--- /dev/null
+++ b/block/nbd-client.c
@@ -0,0 +1,372 @@
+/*
+ * QEMU Block driver for  NBD
+ *
+ * Copyright (C) 2008 Bull S.A.S.
+ *     Author: Laurent Vivier <Laurent.Vivier@bull.net>
+ *
+ * Some parts:
+ *    Copyright (C) 2007 Anthony Liguori <anthony@codemonkey.ws>
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "nbd-client.h"
+#include "qemu/sockets.h"
+
+#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
+#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
+
+static void nbd_reply_ready(void *opaque)
+{
+    NbdClientSession *s = opaque;
+    uint64_t i;
+    int ret;
+
+    if (s->reply.handle == 0) {
+        /* No reply already in flight.  Fetch a header.  It is possible
+         * that another thread has done the same thing in parallel, so
+         * the socket is not readable anymore.
+         */
+        ret = nbd_receive_reply(s->sock, &s->reply);
+        if (ret == -EAGAIN) {
+            return;
+        }
+        if (ret < 0) {
+            s->reply.handle = 0;
+            goto fail;
+        }
+    }
+
+    /* There's no need for a mutex on the receive side, because the
+     * handler acts as a synchronization point and ensures that only
+     * one coroutine is called until the reply finishes.  */
+    i = HANDLE_TO_INDEX(s, s->reply.handle);
+    if (i >= MAX_NBD_REQUESTS) {
+        goto fail;
+    }
+
+    if (s->recv_coroutine[i]) {
+        qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+        return;
+    }
+
+fail:
+    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+        if (s->recv_coroutine[i]) {
+            qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+        }
+    }
+}
+
+static void nbd_restart_write(void *opaque)
+{
+    NbdClientSession *s = opaque;
+
+    qemu_coroutine_enter(s->send_coroutine, NULL);
+}
+
+static int nbd_co_send_request(NbdClientSession *s,
+    struct nbd_request *request,
+    QEMUIOVector *qiov, int offset)
+{
+    int rc, ret;
+
+    qemu_co_mutex_lock(&s->send_mutex);
+    s->send_coroutine = qemu_coroutine_self();
+    qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write, s);
+    if (qiov) {
+        if (!s->is_unix) {
+            socket_set_cork(s->sock, 1);
+        }
+        rc = nbd_send_request(s->sock, request);
+        if (rc >= 0) {
+            ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
+                                offset, request->len);
+            if (ret != request->len) {
+                rc = -EIO;
+            }
+        }
+        if (!s->is_unix) {
+            socket_set_cork(s->sock, 0);
+        }
+    } else {
+        rc = nbd_send_request(s->sock, request);
+    }
+    qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL, s);
+    s->send_coroutine = NULL;
+    qemu_co_mutex_unlock(&s->send_mutex);
+    return rc;
+}
+
+static void nbd_co_receive_reply(NbdClientSession *s,
+    struct nbd_request *request, struct nbd_reply *reply,
+    QEMUIOVector *qiov, int offset)
+{
+    int ret;
+
+    /* Wait until we're woken up by the read handler.  TODO: perhaps
+     * peek at the next reply and avoid yielding if it's ours?  */
+    qemu_coroutine_yield();
+    *reply = s->reply;
+    if (reply->handle != request->handle) {
+        reply->error = EIO;
+    } else {
+        if (qiov && reply->error == 0) {
+            ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov,
+                                offset, request->len);
+            if (ret != request->len) {
+                reply->error = EIO;
+            }
+        }
+
+        /* Tell the read handler to read another header.  */
+        s->reply.handle = 0;
+    }
+}
+
+static void nbd_coroutine_start(NbdClientSession *s,
+   struct nbd_request *request)
+{
+    int i;
+
+    /* Poor man semaphore.  The free_sema is locked when no other request
+     * can be accepted, and unlocked after receiving one reply.  */
+    if (s->in_flight >= MAX_NBD_REQUESTS - 1) {
+        qemu_co_mutex_lock(&s->free_sema);
+        assert(s->in_flight < MAX_NBD_REQUESTS);
+    }
+    s->in_flight++;
+
+    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+        if (s->recv_coroutine[i] == NULL) {
+            s->recv_coroutine[i] = qemu_coroutine_self();
+            break;
+        }
+    }
+
+    assert(i < MAX_NBD_REQUESTS);
+    request->handle = INDEX_TO_HANDLE(s, i);
+}
+
+static void nbd_coroutine_end(NbdClientSession *s,
+    struct nbd_request *request)
+{
+    int i = HANDLE_TO_INDEX(s, request->handle);
+    s->recv_coroutine[i] = NULL;
+    if (s->in_flight-- == MAX_NBD_REQUESTS) {
+        qemu_co_mutex_unlock(&s->free_sema);
+    }
+}
+
+static int nbd_co_readv_1(NbdClientSession *client, int64_t sector_num,
+                          int nb_sectors, QEMUIOVector *qiov,
+                          int offset)
+{
+    struct nbd_request request;
+    struct nbd_reply reply;
+    ssize_t ret;
+
+    request.type = NBD_CMD_READ;
+    request.from = sector_num * 512;
+    request.len = nb_sectors * 512;
+
+    nbd_coroutine_start(client, &request);
+    ret = nbd_co_send_request(client, &request, NULL, 0);
+    if (ret < 0) {
+        reply.error = -ret;
+    } else {
+        nbd_co_receive_reply(client, &request, &reply, qiov, offset);
+    }
+    nbd_coroutine_end(client, &request);
+    return -reply.error;
+
+}
+
+static int nbd_co_writev_1(NbdClientSession *client, int64_t sector_num,
+                           int nb_sectors, QEMUIOVector *qiov,
+                           int offset)
+{
+    struct nbd_request request;
+    struct nbd_reply reply;
+    ssize_t ret;
+
+    request.type = NBD_CMD_WRITE;
+    if (!bdrv_enable_write_cache(client->bs) &&
+        (client->nbdflags & NBD_FLAG_SEND_FUA)) {
+        request.type |= NBD_CMD_FLAG_FUA;
+    }
+
+    request.from = sector_num * 512;
+    request.len = nb_sectors * 512;
+
+    nbd_coroutine_start(client, &request);
+    ret = nbd_co_send_request(client, &request, qiov, offset);
+    if (ret < 0) {
+        reply.error = -ret;
+    } else {
+        nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+    }
+    nbd_coroutine_end(client, &request);
+    return -reply.error;
+}
+
+/* qemu-nbd has a limit of slightly less than 1M per request.  Try to
+ * remain aligned to 4K. */
+#define NBD_MAX_SECTORS 2040
+
+int nbd_client_session_co_readv(NbdClientSession *client, int64_t sector_num,
+    int nb_sectors, QEMUIOVector *qiov)
+{
+    int offset = 0;
+    int ret;
+    while (nb_sectors > NBD_MAX_SECTORS) {
+        ret = nbd_co_readv_1(client, sector_num,
+                             NBD_MAX_SECTORS, qiov, offset);
+        if (ret < 0) {
+            return ret;
+        }
+        offset += NBD_MAX_SECTORS * 512;
+        sector_num += NBD_MAX_SECTORS;
+        nb_sectors -= NBD_MAX_SECTORS;
+    }
+    return nbd_co_readv_1(client, sector_num, nb_sectors, qiov, offset);
+}
+
+int nbd_client_session_co_writev(NbdClientSession *client, int64_t sector_num,
+                                 int nb_sectors, QEMUIOVector *qiov)
+{
+    int offset = 0;
+    int ret;
+    while (nb_sectors > NBD_MAX_SECTORS) {
+        ret = nbd_co_writev_1(client, sector_num,
+                              NBD_MAX_SECTORS, qiov, offset);
+        if (ret < 0) {
+            return ret;
+        }
+        offset += NBD_MAX_SECTORS * 512;
+        sector_num += NBD_MAX_SECTORS;
+        nb_sectors -= NBD_MAX_SECTORS;
+    }
+    return nbd_co_writev_1(client, sector_num, nb_sectors, qiov, offset);
+}
+
+int nbd_client_session_co_flush(NbdClientSession *client)
+{
+    struct nbd_request request;
+    struct nbd_reply reply;
+    ssize_t ret;
+
+    if (!(client->nbdflags & NBD_FLAG_SEND_FLUSH)) {
+        return 0;
+    }
+
+    request.type = NBD_CMD_FLUSH;
+    if (client->nbdflags & NBD_FLAG_SEND_FUA) {
+        request.type |= NBD_CMD_FLAG_FUA;
+    }
+
+    request.from = 0;
+    request.len = 0;
+
+    nbd_coroutine_start(client, &request);
+    ret = nbd_co_send_request(client, &request, NULL, 0);
+    if (ret < 0) {
+        reply.error = -ret;
+    } else {
+        nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+    }
+    nbd_coroutine_end(client, &request);
+    return -reply.error;
+}
+
+int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
+    int nb_sectors)
+{
+    struct nbd_request request;
+    struct nbd_reply reply;
+    ssize_t ret;
+
+    if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
+        return 0;
+    }
+    request.type = NBD_CMD_TRIM;
+    request.from = sector_num * 512;
+    request.len = nb_sectors * 512;
+
+    nbd_coroutine_start(client, &request);
+    ret = nbd_co_send_request(client, &request, NULL, 0);
+    if (ret < 0) {
+        reply.error = -ret;
+    } else {
+        nbd_co_receive_reply(client, &request, &reply, NULL, 0);
+    }
+    nbd_coroutine_end(client, &request);
+    return -reply.error;
+
+}
+
+static void nbd_teardown_connection(NbdClientSession *client)
+{
+    struct nbd_request request;
+
+    request.type = NBD_CMD_DISC;
+    request.from = 0;
+    request.len = 0;
+    nbd_send_request(client->sock, &request);
+
+    qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
+    closesocket(client->sock);
+    client->sock = -1;
+}
+
+void nbd_client_session_close(NbdClientSession *client)
+{
+    nbd_teardown_connection(client);
+    g_free(client->export_name);
+    client->export_name = NULL;
+}
+
+int nbd_client_session_init(NbdClientSession *client,
+    BlockDriverState *bs, int sock)
+{
+    int ret;
+
+    /* NBD handshake */
+    ret = nbd_receive_negotiate(sock, client->export_name,
+                                &client->nbdflags, &client->size,
+                                &client->blocksize);
+    if (ret < 0) {
+        logout("Failed to negotiate with the NBD server\n");
+        closesocket(sock);
+        return ret;
+    }
+
+    qemu_co_mutex_init(&client->send_mutex);
+    qemu_co_mutex_init(&client->free_sema);
+    client->bs = bs;
+    client->sock = sock;
+
+    /* Now that we're connected, set the socket to be non-blocking and
+     * kick the reply mechanism.  */
+    qemu_set_nonblock(sock);
+    qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL, client);
+
+    logout("Established connection with NBD server\n");
+    return 0;
+}
diff --git a/block/nbd-client.h b/block/nbd-client.h
new file mode 100644
index 0000000..c271236
--- /dev/null
+++ b/block/nbd-client.h
@@ -0,0 +1,51 @@
+#ifndef NBD_CLIENT_H
+#define NBD_CLIENT_H
+
+#include "qemu-common.h"
+#include "block/nbd.h"
+#include "block/block_int.h"
+
+/* #define DEBUG_NBD */
+
+#if defined(DEBUG_NBD)
+#define logout(fmt, ...) \
+    fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
+#else
+#define logout(fmt, ...) ((void)0)
+#endif
+
+#define MAX_NBD_REQUESTS    16
+
+typedef struct NbdClientSession {
+    int sock;
+    uint32_t nbdflags;
+    off_t size;
+    size_t blocksize;
+
+    CoMutex send_mutex;
+    CoMutex free_sema;
+    Coroutine *send_coroutine;
+    int in_flight;
+
+    Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
+    struct nbd_reply reply;
+
+    char *export_name; /* An NBD server may export several devices */
+    bool is_unix;
+
+    BlockDriverState *bs;
+} NbdClientSession;
+
+int nbd_client_session_init(NbdClientSession *client,
+                            BlockDriverState *bs, int sock);
+void nbd_client_session_close(NbdClientSession *client);
+
+int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
+                                  int nb_sectors);
+int nbd_client_session_co_flush(NbdClientSession *client);
+int nbd_client_session_co_writev(NbdClientSession *client, int64_t sector_num,
+                                 int nb_sectors, QEMUIOVector *qiov);
+int nbd_client_session_co_readv(NbdClientSession *client, int64_t sector_num,
+                                int nb_sectors, QEMUIOVector *qiov);
+
+#endif /* NBD_CLIENT_H */
diff --git a/block/nbd.c b/block/nbd.c
index c8deeee..be75ba0 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -26,8 +26,7 @@
  * THE SOFTWARE.
  */
 
-#include "qemu-common.h"
-#include "block/nbd.h"
+#include "block/nbd-client.h"
 #include "qemu/uri.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
@@ -40,37 +39,9 @@
 
 #define EN_OPTSTR ":exportname="
 
-/* #define DEBUG_NBD */
-
-#if defined(DEBUG_NBD)
-#define logout(fmt, ...) \
-                fprintf(stderr, "nbd\t%-24s" fmt, __func__, ##__VA_ARGS__)
-#else
-#define logout(fmt, ...) ((void)0)
-#endif
-
-#define MAX_NBD_REQUESTS	16
-#define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
-#define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
-
 typedef struct BDRVNBDState {
-    int sock;
-    uint32_t nbdflags;
-    off_t size;
-    size_t blocksize;
-
-    CoMutex send_mutex;
-    CoMutex free_sema;
-    Coroutine *send_coroutine;
-    int in_flight;
-
-    Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
-    struct nbd_reply reply;
-
-    bool is_unix;
+    NbdClientSession client;
     QemuOpts *socket_opts;
-
-    char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
 static int nbd_parse_uri(const char *filename, QDict *options)
@@ -227,9 +198,9 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
                           "be used at the same time.");
             return -EINVAL;
         }
-        s->is_unix = true;
+        s->client.is_unix = true;
     } else if (qdict_haskey(options, "host")) {
-        s->is_unix = false;
+        s->client.is_unix = false;
     } else {
         return -EINVAL;
     }
@@ -247,162 +218,20 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
         qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT);
     }
 
-    s->export_name = g_strdup(qdict_get_try_str(options, "export"));
-    if (s->export_name) {
+    s->client.export_name = g_strdup(qdict_get_try_str(options, "export"));
+    if (s->client.export_name) {
         qdict_del(options, "export");
     }
 
     return 0;
 }
 
-
-static void nbd_coroutine_start(BDRVNBDState *s, struct nbd_request *request)
-{
-    int i;
-
-    /* Poor man semaphore.  The free_sema is locked when no other request
-     * can be accepted, and unlocked after receiving one reply.  */
-    if (s->in_flight >= MAX_NBD_REQUESTS - 1) {
-        qemu_co_mutex_lock(&s->free_sema);
-        assert(s->in_flight < MAX_NBD_REQUESTS);
-    }
-    s->in_flight++;
-
-    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
-        if (s->recv_coroutine[i] == NULL) {
-            s->recv_coroutine[i] = qemu_coroutine_self();
-            break;
-        }
-    }
-
-    assert(i < MAX_NBD_REQUESTS);
-    request->handle = INDEX_TO_HANDLE(s, i);
-}
-
-static void nbd_reply_ready(void *opaque)
-{
-    BDRVNBDState *s = opaque;
-    uint64_t i;
-    int ret;
-
-    if (s->reply.handle == 0) {
-        /* No reply already in flight.  Fetch a header.  It is possible
-         * that another thread has done the same thing in parallel, so
-         * the socket is not readable anymore.
-         */
-        ret = nbd_receive_reply(s->sock, &s->reply);
-        if (ret == -EAGAIN) {
-            return;
-        }
-        if (ret < 0) {
-            s->reply.handle = 0;
-            goto fail;
-        }
-    }
-
-    /* There's no need for a mutex on the receive side, because the
-     * handler acts as a synchronization point and ensures that only
-     * one coroutine is called until the reply finishes.  */
-    i = HANDLE_TO_INDEX(s, s->reply.handle);
-    if (i >= MAX_NBD_REQUESTS) {
-        goto fail;
-    }
-
-    if (s->recv_coroutine[i]) {
-        qemu_coroutine_enter(s->recv_coroutine[i], NULL);
-        return;
-    }
-
-fail:
-    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
-        if (s->recv_coroutine[i]) {
-            qemu_coroutine_enter(s->recv_coroutine[i], NULL);
-        }
-    }
-}
-
-static void nbd_restart_write(void *opaque)
-{
-    BDRVNBDState *s = opaque;
-    qemu_coroutine_enter(s->send_coroutine, NULL);
-}
-
-static int nbd_co_send_request(BDRVNBDState *s, struct nbd_request *request,
-                               QEMUIOVector *qiov, int offset)
-{
-    int rc, ret;
-
-    qemu_co_mutex_lock(&s->send_mutex);
-    s->send_coroutine = qemu_coroutine_self();
-    qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, nbd_restart_write, s);
-    if (qiov) {
-        if (!s->is_unix) {
-            socket_set_cork(s->sock, 1);
-        }
-        rc = nbd_send_request(s->sock, request);
-        if (rc >= 0) {
-            ret = qemu_co_sendv(s->sock, qiov->iov, qiov->niov,
-                                offset, request->len);
-            if (ret != request->len) {
-                rc = -EIO;
-            }
-        }
-        if (!s->is_unix) {
-            socket_set_cork(s->sock, 0);
-        }
-    } else {
-        rc = nbd_send_request(s->sock, request);
-    }
-    qemu_aio_set_fd_handler(s->sock, nbd_reply_ready, NULL, s);
-    s->send_coroutine = NULL;
-    qemu_co_mutex_unlock(&s->send_mutex);
-    return rc;
-}
-
-static void nbd_co_receive_reply(BDRVNBDState *s, struct nbd_request *request,
-                                 struct nbd_reply *reply,
-                                 QEMUIOVector *qiov, int offset)
-{
-    int ret;
-
-    /* Wait until we're woken up by the read handler.  TODO: perhaps
-     * peek at the next reply and avoid yielding if it's ours?  */
-    qemu_coroutine_yield();
-    *reply = s->reply;
-    if (reply->handle != request->handle) {
-        reply->error = EIO;
-    } else {
-        if (qiov && reply->error == 0) {
-            ret = qemu_co_recvv(s->sock, qiov->iov, qiov->niov,
-                                offset, request->len);
-            if (ret != request->len) {
-                reply->error = EIO;
-            }
-        }
-
-        /* Tell the read handler to read another header.  */
-        s->reply.handle = 0;
-    }
-}
-
-static void nbd_coroutine_end(BDRVNBDState *s, struct nbd_request *request)
-{
-    int i = HANDLE_TO_INDEX(s, request->handle);
-    s->recv_coroutine[i] = NULL;
-    if (s->in_flight-- == MAX_NBD_REQUESTS) {
-        qemu_co_mutex_unlock(&s->free_sema);
-    }
-}
-
 static int nbd_establish_connection(BlockDriverState *bs)
 {
     BDRVNBDState *s = bs->opaque;
     int sock;
-    int ret;
-    off_t size;
-    size_t blocksize;
 
-    if (s->is_unix) {
+    if (s->client.is_unix) {
         sock = unix_socket_outgoing(qemu_opt_get(s->socket_opts, "path"));
     } else {
         sock = tcp_socket_outgoing_opts(s->socket_opts);
@@ -417,50 +246,14 @@ static int nbd_establish_connection(BlockDriverState *bs)
         return -errno;
     }
 
-    /* NBD handshake */
-    ret = nbd_receive_negotiate(sock, s->export_name, &s->nbdflags, &size,
-                                &blocksize);
-    if (ret < 0) {
-        logout("Failed to negotiate with the NBD server\n");
-        closesocket(sock);
-        return ret;
-    }
-
-    /* Now that we're connected, set the socket to be non-blocking and
-     * kick the reply mechanism.  */
-    qemu_set_nonblock(sock);
-    qemu_aio_set_fd_handler(sock, nbd_reply_ready, NULL, s);
-
-    s->sock = sock;
-    s->size = size;
-    s->blocksize = blocksize;
-
-    logout("Established connection with NBD server\n");
-    return 0;
-}
-
-static void nbd_teardown_connection(BlockDriverState *bs)
-{
-    BDRVNBDState *s = bs->opaque;
-    struct nbd_request request;
-
-    request.type = NBD_CMD_DISC;
-    request.from = 0;
-    request.len = 0;
-    nbd_send_request(s->sock, &request);
-
-    qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL);
-    closesocket(s->sock);
+    return sock;
 }
 
 static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
     BDRVNBDState *s = bs->opaque;
-    int result;
-
-    qemu_co_mutex_init(&s->send_mutex);
-    qemu_co_mutex_init(&s->free_sema);
+    int result, sock;
 
     /* Pop the config into our state object. Exit if invalid. */
     result = nbd_config(s, options);
@@ -471,172 +264,62 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     /* establish TCP connection, return error if it fails
      * TODO: Configurable retry-until-timeout behaviour.
      */
-    result = nbd_establish_connection(bs);
-
-    return result;
-}
-
-static int nbd_co_readv_1(BlockDriverState *bs, int64_t sector_num,
-                          int nb_sectors, QEMUIOVector *qiov,
-                          int offset)
-{
-    BDRVNBDState *s = bs->opaque;
-    struct nbd_request request;
-    struct nbd_reply reply;
-    ssize_t ret;
-
-    request.type = NBD_CMD_READ;
-    request.from = sector_num * 512;
-    request.len = nb_sectors * 512;
-
-    nbd_coroutine_start(s, &request);
-    ret = nbd_co_send_request(s, &request, NULL, 0);
-    if (ret < 0) {
-        reply.error = -ret;
-    } else {
-        nbd_co_receive_reply(s, &request, &reply, qiov, offset);
-    }
-    nbd_coroutine_end(s, &request);
-    return -reply.error;
-
-}
-
-static int nbd_co_writev_1(BlockDriverState *bs, int64_t sector_num,
-                           int nb_sectors, QEMUIOVector *qiov,
-                           int offset)
-{
-    BDRVNBDState *s = bs->opaque;
-    struct nbd_request request;
-    struct nbd_reply reply;
-    ssize_t ret;
-
-    request.type = NBD_CMD_WRITE;
-    if (!bdrv_enable_write_cache(bs) && (s->nbdflags & NBD_FLAG_SEND_FUA)) {
-        request.type |= NBD_CMD_FLAG_FUA;
+    sock = nbd_establish_connection(bs);
+    if (sock < 0) {
+        return sock;
     }
 
-    request.from = sector_num * 512;
-    request.len = nb_sectors * 512;
-
-    nbd_coroutine_start(s, &request);
-    ret = nbd_co_send_request(s, &request, qiov, offset);
-    if (ret < 0) {
-        reply.error = -ret;
-    } else {
-        nbd_co_receive_reply(s, &request, &reply, NULL, 0);
-    }
-    nbd_coroutine_end(s, &request);
-    return -reply.error;
+    /* NBD handshake */
+    return nbd_client_session_init(&s->client, bs, sock);
 }
 
-/* qemu-nbd has a limit of slightly less than 1M per request.  Try to
- * remain aligned to 4K. */
-#define NBD_MAX_SECTORS 2040
-
 static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
                         int nb_sectors, QEMUIOVector *qiov)
 {
-    int offset = 0;
-    int ret;
-    while (nb_sectors > NBD_MAX_SECTORS) {
-        ret = nbd_co_readv_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
-        if (ret < 0) {
-            return ret;
-        }
-        offset += NBD_MAX_SECTORS * 512;
-        sector_num += NBD_MAX_SECTORS;
-        nb_sectors -= NBD_MAX_SECTORS;
-    }
-    return nbd_co_readv_1(bs, sector_num, nb_sectors, qiov, offset);
+    BDRVNBDState *s = bs->opaque;
+
+    return nbd_client_session_co_readv(&s->client, sector_num,
+                                       nb_sectors, qiov);
 }
 
 static int nbd_co_writev(BlockDriverState *bs, int64_t sector_num,
                          int nb_sectors, QEMUIOVector *qiov)
 {
-    int offset = 0;
-    int ret;
-    while (nb_sectors > NBD_MAX_SECTORS) {
-        ret = nbd_co_writev_1(bs, sector_num, NBD_MAX_SECTORS, qiov, offset);
-        if (ret < 0) {
-            return ret;
-        }
-        offset += NBD_MAX_SECTORS * 512;
-        sector_num += NBD_MAX_SECTORS;
-        nb_sectors -= NBD_MAX_SECTORS;
-    }
-    return nbd_co_writev_1(bs, sector_num, nb_sectors, qiov, offset);
+    BDRVNBDState *s = bs->opaque;
+
+    return nbd_client_session_co_writev(&s->client, sector_num,
+                                        nb_sectors, qiov);
 }
 
 static int nbd_co_flush(BlockDriverState *bs)
 {
     BDRVNBDState *s = bs->opaque;
-    struct nbd_request request;
-    struct nbd_reply reply;
-    ssize_t ret;
-
-    if (!(s->nbdflags & NBD_FLAG_SEND_FLUSH)) {
-        return 0;
-    }
 
-    request.type = NBD_CMD_FLUSH;
-    if (s->nbdflags & NBD_FLAG_SEND_FUA) {
-        request.type |= NBD_CMD_FLAG_FUA;
-    }
-
-    request.from = 0;
-    request.len = 0;
-
-    nbd_coroutine_start(s, &request);
-    ret = nbd_co_send_request(s, &request, NULL, 0);
-    if (ret < 0) {
-        reply.error = -ret;
-    } else {
-        nbd_co_receive_reply(s, &request, &reply, NULL, 0);
-    }
-    nbd_coroutine_end(s, &request);
-    return -reply.error;
+    return nbd_client_session_co_flush(&s->client);
 }
 
 static int nbd_co_discard(BlockDriverState *bs, int64_t sector_num,
                           int nb_sectors)
 {
     BDRVNBDState *s = bs->opaque;
-    struct nbd_request request;
-    struct nbd_reply reply;
-    ssize_t ret;
 
-    if (!(s->nbdflags & NBD_FLAG_SEND_TRIM)) {
-        return 0;
-    }
-    request.type = NBD_CMD_TRIM;
-    request.from = sector_num * 512;
-    request.len = nb_sectors * 512;
-
-    nbd_coroutine_start(s, &request);
-    ret = nbd_co_send_request(s, &request, NULL, 0);
-    if (ret < 0) {
-        reply.error = -ret;
-    } else {
-        nbd_co_receive_reply(s, &request, &reply, NULL, 0);
-    }
-    nbd_coroutine_end(s, &request);
-    return -reply.error;
+    return nbd_client_session_co_discard(&s->client, sector_num,
+                                         nb_sectors);
 }
 
 static void nbd_close(BlockDriverState *bs)
 {
     BDRVNBDState *s = bs->opaque;
-    g_free(s->export_name);
-    qemu_opts_del(s->socket_opts);
 
-    nbd_teardown_connection(bs);
+    qemu_opts_del(s->socket_opts);
+    nbd_client_session_close(&s->client);
 }
 
 static int64_t nbd_getlength(BlockDriverState *bs)
 {
     BDRVNBDState *s = bs->opaque;
 
-    return s->size;
+    return s->client.size;
 }
 
 static BlockDriver bdrv_nbd = {
-- 
1.8.3.1

[Qemu-devel] [PATCH 09/16] nbd: don't change socket block during negotiate

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

The caller might handle non-blocking using coroutine. Leave the choice
to the caller to use a blocking or non-blocking negotiate.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/nbd-client.c | 1 +
 nbd.c              | 2 --
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1abfc6a..693110d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -348,6 +348,7 @@ int nbd_client_session_init(NbdClientSession *client,
     int ret;
 
     /* NBD handshake */
+    qemu_set_block(sock);
     ret = nbd_receive_negotiate(sock, client->export_name,
                                 &client->nbdflags, &client->size,
                                 &client->blocksize);
diff --git a/nbd.c b/nbd.c
index f847940..030f56b 100644
--- a/nbd.c
+++ b/nbd.c
@@ -443,7 +443,6 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
 
     TRACE("Receiving negotiation.");
 
-    qemu_set_block(csock);
     rc = -EINVAL;
 
     if (read_sync(csock, buf, 8) != 8) {
@@ -558,7 +557,6 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
     rc = 0;
 
 fail:
-    qemu_set_nonblock(csock);
     return rc;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 10/16] nbd: pass export name as init argument

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

There is no need to keep the export name around, and it seems a better
fit as an argument in the init() call.

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/nbd-client.c |  9 ++++-----
 block/nbd-client.h |  5 ++---
 block/nbd.c        | 13 ++++++++-----
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 693110d..3dfae1d 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -338,18 +338,17 @@ static void nbd_teardown_connection(NbdClientSession *client)
 void nbd_client_session_close(NbdClientSession *client)
 {
     nbd_teardown_connection(client);
-    g_free(client->export_name);
-    client->export_name = NULL;
 }
 
-int nbd_client_session_init(NbdClientSession *client,
-    BlockDriverState *bs, int sock)
+int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
+    int sock, const char *export)
 {
     int ret;
 
     /* NBD handshake */
+    logout("session init %s\n", export);
     qemu_set_block(sock);
-    ret = nbd_receive_negotiate(sock, client->export_name,
+    ret = nbd_receive_negotiate(sock, export,
                                 &client->nbdflags, &client->size,
                                 &client->blocksize);
     if (ret < 0) {
diff --git a/block/nbd-client.h b/block/nbd-client.h
index c271236..f2a6337 100644
--- a/block/nbd-client.h
+++ b/block/nbd-client.h
@@ -30,14 +30,13 @@ typedef struct NbdClientSession {
     Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
     struct nbd_reply reply;
 
-    char *export_name; /* An NBD server may export several devices */
     bool is_unix;
 
     BlockDriverState *bs;
 } NbdClientSession;
 
-int nbd_client_session_init(NbdClientSession *client,
-                            BlockDriverState *bs, int sock);
+int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
+                            int sock, const char *export_name);
 void nbd_client_session_close(NbdClientSession *client);
 
 int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
diff --git a/block/nbd.c b/block/nbd.c
index be75ba0..4455a13 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -188,7 +188,7 @@ out:
     g_free(file);
 }
 
-static int nbd_config(BDRVNBDState *s, QDict *options)
+static int nbd_config(BDRVNBDState *s, QDict *options, char **export)
 {
     Error *local_err = NULL;
 
@@ -218,8 +218,8 @@ static int nbd_config(BDRVNBDState *s, QDict *options)
         qemu_opt_set_number(s->socket_opts, "port", NBD_DEFAULT_PORT);
     }
 
-    s->client.export_name = g_strdup(qdict_get_try_str(options, "export"));
-    if (s->client.export_name) {
+    *export = g_strdup(qdict_get_try_str(options, "export"));
+    if (*export) {
         qdict_del(options, "export");
     }
 
@@ -253,10 +253,11 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
     BDRVNBDState *s = bs->opaque;
+    char *export = NULL;
     int result, sock;
 
     /* Pop the config into our state object. Exit if invalid. */
-    result = nbd_config(s, options);
+    result = nbd_config(s, options, &export);
     if (result != 0) {
         return result;
     }
@@ -270,7 +271,9 @@ static int nbd_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* NBD handshake */
-    return nbd_client_session_init(&s->client, bs, sock);
+    result = nbd_client_session_init(&s->client, bs, sock, export);
+    g_free(export);
+    return result;
 }
 
 static int nbd_co_readv(BlockDriverState *bs, int64_t sector_num,
-- 
1.8.3.1

[Qemu-devel] [PATCH 11/16] nbd: make nbd_client_session_close() idempotent

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/nbd-client.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 3dfae1d..9156093 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -337,7 +337,12 @@ static void nbd_teardown_connection(NbdClientSession *client)
 
 void nbd_client_session_close(NbdClientSession *client)
 {
+    if (!client->bs) {
+        return;
+    }
+
     nbd_teardown_connection(client);
+    client->bs = NULL;
 }
 
 int nbd_client_session_init(NbdClientSession *client, BlockDriverState *bs,
-- 
1.8.3.1

[Qemu-devel] [PATCH 12/16] nbd: finish any pending coroutine

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@gmail.com>

Make sure all pending coroutines are finished when closing the session.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/nbd-client.c | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 9156093..1973cf0 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -32,6 +32,17 @@
 #define HANDLE_TO_INDEX(bs, handle) ((handle) ^ ((uint64_t)(intptr_t)bs))
 #define INDEX_TO_HANDLE(bs, index)  ((index)  ^ ((uint64_t)(intptr_t)bs))
 
+static void nbd_recv_coroutines_enter_all(NbdClientSession *s)
+{
+    int i;
+
+    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
+        if (s->recv_coroutine[i]) {
+            qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+        }
+    }
+}
+
 static void nbd_reply_ready(void *opaque)
 {
     NbdClientSession *s = opaque;
@@ -67,11 +78,7 @@ static void nbd_reply_ready(void *opaque)
     }
 
 fail:
-    for (i = 0; i < MAX_NBD_REQUESTS; i++) {
-        if (s->recv_coroutine[i]) {
-            qemu_coroutine_enter(s->recv_coroutine[i], NULL);
-        }
-    }
+    nbd_recv_coroutines_enter_all(s);
 }
 
 static void nbd_restart_write(void *opaque)
@@ -330,6 +337,10 @@ static void nbd_teardown_connection(NbdClientSession *client)
     request.len = 0;
     nbd_send_request(client->sock, &request);
 
+    /* finish any pending coroutines */
+    shutdown(client->sock, 2);
+    nbd_recv_coroutines_enter_all(client);
+
     qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
     closesocket(client->sock);
     client->sock = -1;
-- 
1.8.3.1

[Qemu-devel] [PATCH 13/16] nbd: avoid uninitialized warnings

[Gerd Hoffmann]

From: Marc-André Lureau <marcandre.lureau@gmail.com>

==15815== Thread 1:
==15815== Syscall param socketcall.sendto(msg) points to uninitialised byte(s)
==15815==    at 0x65AD5CB: send (send.c:31)
==15815==    by 0x37F84B: nbd_wr_sync (nbd.c:145)
==15815==    by 0x37F94B: write_sync (nbd.c:186)
==15815==    by 0x380FA9: nbd_send_request (nbd.c:681)
==15815==    by 0x1C4A2D: nbd_teardown_connection (nbd-client.c:337)
==15815==    by 0x1C4AD8: nbd_client_session_close (nbd-client.c:354)
==15815==    by 0x1ED2D8: close_socketpair (spicebd.c:132)
==15815==    by 0x1EE265: spice_close (spicebd.c:457)
==15815==    by 0x1ACBF6: bdrv_close (block.c:1519)
==15815==    by 0x1AD804: bdrv_delete (block.c:1772)
==15815==    by 0x1B4136: bdrv_unref (block.c:4476)
==15815==    by 0x1ACCE0: bdrv_close (block.c:1541)
==15815==  Address 0x7feffef98 is on thread 1's stack

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
Acked-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 block/nbd-client.c | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1973cf0..0922b78 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -185,11 +185,10 @@ static int nbd_co_readv_1(NbdClientSession *client, int64_t sector_num,
                           int nb_sectors, QEMUIOVector *qiov,
                           int offset)
 {
-    struct nbd_request request;
+    struct nbd_request request = { .type = NBD_CMD_READ };
     struct nbd_reply reply;
     ssize_t ret;
 
-    request.type = NBD_CMD_READ;
     request.from = sector_num * 512;
     request.len = nb_sectors * 512;
 
@@ -209,11 +208,10 @@ static int nbd_co_writev_1(NbdClientSession *client, int64_t sector_num,
                            int nb_sectors, QEMUIOVector *qiov,
                            int offset)
 {
-    struct nbd_request request;
+    struct nbd_request request = { .type = NBD_CMD_WRITE };
     struct nbd_reply reply;
     ssize_t ret;
 
-    request.type = NBD_CMD_WRITE;
     if (!bdrv_enable_write_cache(client->bs) &&
         (client->nbdflags & NBD_FLAG_SEND_FUA)) {
         request.type |= NBD_CMD_FLAG_FUA;
@@ -275,7 +273,7 @@ int nbd_client_session_co_writev(NbdClientSession *client, int64_t sector_num,
 
 int nbd_client_session_co_flush(NbdClientSession *client)
 {
-    struct nbd_request request;
+    struct nbd_request request = { .type = NBD_CMD_FLUSH };
     struct nbd_reply reply;
     ssize_t ret;
 
@@ -283,7 +281,6 @@ int nbd_client_session_co_flush(NbdClientSession *client)
         return 0;
     }
 
-    request.type = NBD_CMD_FLUSH;
     if (client->nbdflags & NBD_FLAG_SEND_FUA) {
         request.type |= NBD_CMD_FLAG_FUA;
     }
@@ -305,14 +302,13 @@ int nbd_client_session_co_flush(NbdClientSession *client)
 int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
     int nb_sectors)
 {
-    struct nbd_request request;
+    struct nbd_request request = { .type = NBD_CMD_TRIM };
     struct nbd_reply reply;
     ssize_t ret;
 
     if (!(client->nbdflags & NBD_FLAG_SEND_TRIM)) {
         return 0;
     }
-    request.type = NBD_CMD_TRIM;
     request.from = sector_num * 512;
     request.len = nb_sectors * 512;
 
@@ -330,11 +326,12 @@ int nbd_client_session_co_discard(NbdClientSession *client, int64_t sector_num,
 
 static void nbd_teardown_connection(NbdClientSession *client)
 {
-    struct nbd_request request;
+    struct nbd_request request = {
+        .type = NBD_CMD_DISC,
+        .from = 0,
+        .len = 0
+    };
 
-    request.type = NBD_CMD_DISC;
-    request.from = 0;
-    request.len = 0;
     nbd_send_request(client->sock, &request);
 
     /* finish any pending coroutines */
-- 
1.8.3.1

[Qemu-devel] [PATCH 14/16] spice: move qemu_spice_display_*() from spice-graphics to spice-core

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-core.c    | 16 ++++++++++++++++
 ui/spice-display.c | 17 -----------------
 2 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 9fb9544..0c48156 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -47,6 +47,7 @@ static const char *auth = "spice";
 static char *auth_passwd;
 static time_t auth_expires = TIME_MAX;
 static int spice_migration_completed;
+static int spice_display_is_running;
 int using_spice = 0;
 
 static QemuThread me;
@@ -904,6 +905,21 @@ int qemu_spice_display_add_client(int csock, int skipauth, int tls)
     }
 }
 
+void qemu_spice_display_start(void)
+{
+    spice_display_is_running = true;
+}
+
+void qemu_spice_display_stop(void)
+{
+    spice_display_is_running = false;
+}
+
+int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
+{
+    return spice_display_is_running;
+}
+
 static void spice_register_config(void)
 {
     qemu_add_opts(&qemu_spice_opts);
diff --git a/ui/spice-display.c b/ui/spice-display.c
index f23a318..9bb42f1 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -126,23 +126,6 @@ void qemu_spice_wakeup(SimpleSpiceDisplay *ssd)
     spice_qxl_wakeup(&ssd->qxl);
 }
 
-static int spice_display_is_running;
-
-void qemu_spice_display_start(void)
-{
-    spice_display_is_running = true;
-}
-
-void qemu_spice_display_stop(void)
-{
-    spice_display_is_running = false;
-}
-
-int qemu_spice_display_is_running(SimpleSpiceDisplay *ssd)
-{
-    return spice_display_is_running;
-}
-
 static void qemu_spice_create_one_update(SimpleSpiceDisplay *ssd,
                                          QXLRect *rect)
 {
-- 
1.8.3.1

[Qemu-devel] [PATCH 15/16] spice: move spice_server_vm_{start, stop} calls into qemu_spice_display_*()

[Gerd Hoffmann]

So calling spice server to start/stop the worker goes
hand in hand with the status variable update.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 ui/spice-core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/spice-core.c b/ui/spice-core.c
index 0c48156..4cce3b3 100644
--- a/ui/spice-core.c
+++ b/ui/spice-core.c
@@ -623,9 +623,7 @@ static void vm_change_state_handler(void *opaque, int running,
 {
     if (running) {
         qemu_spice_display_start();
-        spice_server_vm_start(spice_server);
     } else {
-        spice_server_vm_stop(spice_server);
         qemu_spice_display_stop();
     }
 }
@@ -908,10 +906,12 @@ int qemu_spice_display_add_client(int csock, int skipauth, int tls)
 void qemu_spice_display_start(void)
 {
     spice_display_is_running = true;
+    spice_server_vm_start(spice_server);
 }
 
 void qemu_spice_display_stop(void)
 {
+    spice_server_vm_stop(spice_server);
     spice_display_is_running = false;
 }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 16/16] spice: stop server for qxl hard reset

[Gerd Hoffmann]

Hard reset can happen at any time.  We should be able to put qxl into a
known-good state no matter what.  Stop spice server thread for reset so
it can't be confused by fetching stale commands lingering around in the
rings while we reset is ongoing.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/qxl.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index efdefd6..e4f172e 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1144,8 +1144,14 @@ static void qxl_soft_reset(PCIQXLDevice *d)
 
 static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
 {
+    bool startstop = qemu_spice_display_is_running(&d->ssd);
+
     trace_qxl_hard_reset(d->id, loadvm);
 
+    if (startstop) {
+        qemu_spice_display_stop();
+    }
+
     qxl_spice_reset_cursor(d);
     qxl_spice_reset_image_cache(d);
     qxl_reset_surfaces(d);
@@ -1159,6 +1165,10 @@ static void qxl_hard_reset(PCIQXLDevice *d, int loadvm)
     }
     qemu_spice_create_host_memslot(&d->ssd);
     qxl_soft_reset(d);
+
+    if (startstop) {
+        qemu_spice_display_start();
+    }
 }
 
 static void qxl_reset_handler(DeviceState *dev)
-- 
1.8.3.1