[Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Marc-André Lureau]

Hi,

This patch series is a split-off from the Spice block device RFC.
http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02024.html

It should include only non-controversial or simple patches. The most
important change is the NBD client implementation moved to a seperate
file.

In v2:
- should address Paolo comments
- added a patch to remove unused CoQueue member

Marc-André Lureau (14):
  coroutine: remove qemu_co_queue_wait_insert_head
  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
  coroutine: remove unused CoQueue AioContext

 block/Makefile.objs       |   2 +-
 block/nbd-client.c        | 385 ++++++++++++++++++++++++++++++++++++++++++++++
 block/nbd-client.h        |  50 ++++++
 block/nbd.c               | 380 ++++-----------------------------------------
 blockdev.c                |   2 +-
 include/block/coroutine.h |   7 -
 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 +
 qemu-coroutine-lock.c     |   8 -
 qmp.c                     |   2 +-
 spice-qemu-char.c         |  11 +-
 15 files changed, 507 insertions(+), 373 deletions(-)
 create mode 100644 block/nbd-client.c
 create mode 100644 block/nbd-client.h

-- 
1.8.4.2

[Qemu-devel] [PATCHv2 01/14] coroutine: remove qemu_co_queue_wait_insert_head

[Marc-André Lureau]

qemu_co_queue_wait_insert_head() is unused in qemu code base now.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
---
 include/block/coroutine.h | 6 ------
 qemu-coroutine-lock.c     | 8 --------
 2 files changed, 14 deletions(-)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index 4d5c0cf..b122c0c 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -121,12 +121,6 @@ void qemu_co_queue_init(CoQueue *queue);
 void coroutine_fn qemu_co_queue_wait(CoQueue *queue);
 
 /**
- * Adds the current coroutine to the head of the CoQueue and transfers control to the
- * caller of the coroutine.
- */
-void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue);
-
-/**
  * Restarts the next coroutine in the CoQueue and removes it from the queue.
  *
  * Returns true if a coroutine was restarted, false if the queue is empty.
diff --git a/qemu-coroutine-lock.c b/qemu-coroutine-lock.c
index aeb33b9..e4860ae 100644
--- a/qemu-coroutine-lock.c
+++ b/qemu-coroutine-lock.c
@@ -41,14 +41,6 @@ void coroutine_fn qemu_co_queue_wait(CoQueue *queue)
     assert(qemu_in_coroutine());
 }
 
-void coroutine_fn qemu_co_queue_wait_insert_head(CoQueue *queue)
-{
-    Coroutine *self = qemu_coroutine_self();
-    QTAILQ_INSERT_HEAD(&queue->entries, self, co_queue_next);
-    qemu_coroutine_yield();
-    assert(qemu_in_coroutine());
-}
-
 /**
  * qemu_co_queue_run_restart:
  *
-- 
1.8.4.2

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

[Marc-André Lureau]

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>
---
 libcacard/vscclient.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/libcacard/vscclient.c b/libcacard/vscclient.c
index a3cb776..c413a4f 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.4.2

[Qemu-devel] [PATCHv2 03/14] spice-char: remove unused field

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.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.4.2

[Qemu-devel] [PATCHv2 04/14] qmp_change_blockdev() remove unused has_format

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.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.4.2

[Qemu-devel] [PATCHv2 05/14] include: add missing config-host.h include

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@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.4.2

[Qemu-devel] [PATCHv2 06/14] char: add qemu_chr_fe_event()

[Marc-André Lureau]

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>
---
 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.4.2

[Qemu-devel] [PATCHv2 07/14] spice-char: implement chardev port event

[Marc-André Lureau]

Wire up chardev fe_event to Spice port.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.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.4.2

[Qemu-devel] [PATCHv2 08/14] Split nbd block client code

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@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.4.2

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

[Marc-André Lureau]

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>
---
 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.4.2

[Qemu-devel] [PATCHv2 10/14] nbd: pass export name as init argument

[Marc-André Lureau]

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>
---
 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.4.2

[Qemu-devel] [PATCHv2 11/14] nbd: make nbd_client_session_close() idempotent

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@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.4.2

[Qemu-devel] [PATCHv2 12/14] nbd: finish any pending coroutine

[Marc-André Lureau]

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

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.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.4.2

[Qemu-devel] [PATCHv2 13/14] nbd: avoid uninitialized warnings

[Marc-André Lureau]

==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>
---
 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.4.2

[Qemu-devel] [PATCHv2 14/14] coroutine: remove unused CoQueue AioContext

[Marc-André Lureau]

The AioContext ctx field is apparently unused in qemu codebase since
02ffb504485.

Signed-off-by: Marc-André Lureau <marcandre.lureau@gmail.com>
---
 include/block/coroutine.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/include/block/coroutine.h b/include/block/coroutine.h
index b122c0c..a1797ae 100644
--- a/include/block/coroutine.h
+++ b/include/block/coroutine.h
@@ -105,7 +105,6 @@ bool qemu_in_coroutine(void);
  */
 typedef struct CoQueue {
     QTAILQ_HEAD(, Coroutine) entries;
-    AioContext *ctx;
 } CoQueue;
 
 /**
-- 
1.8.4.2

Re: [Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Gerd Hoffmann]

On So, 2013-12-01 at 22:23 +0100, Marc-André Lureau wrote:
> Hi,
> 
> This patch series is a split-off from the Spice block device RFC.
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02024.html
> 
> It should include only non-controversial or simple patches. The most
> important change is the NBD client implementation moved to a seperate
> file.

Well, my intention was to merge the spice bits only, not nbd which is
block stuff.  I'll happily take 1-7 as-is through the spice queue.

I can take patches 8-14 though the spice queue too, but I want an ack
from the block people for that.  Or we split the series and merge 8-14
through the block queue.  Kevin?  Stefan?

cheers,
  Gerd

Re: [Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Paolo Bonzini]

Il 02/12/2013 11:36, Gerd Hoffmann ha scritto:
> On So, 2013-12-01 at 22:23 +0100, Marc-André Lureau wrote:
>> Hi,
>>
>> This patch series is a split-off from the Spice block device RFC.
>> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02024.html
>>
>> It should include only non-controversial or simple patches. The most
>> important change is the NBD client implementation moved to a seperate
>> file.
> 
> Well, my intention was to merge the spice bits only, not nbd which is
> block stuff.  I'll happily take 1-7 as-is through the spice queue.
> 
> I can take patches 8-14 though the spice queue too, but I want an ack
> from the block people for that.  Or we split the series and merge 8-14
> through the block queue.  Kevin?  Stefan?

Patches 8-13 are NBD stuff, so

Acked-by: Paolo Bonzini <pbonzini@redhat.com>

Paolo

Re: [Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Stefan Hajnoczi]

On Mon, Dec 02, 2013 at 11:36:09AM +0100, Gerd Hoffmann wrote:
> On So, 2013-12-01 at 22:23 +0100, Marc-André Lureau wrote:
> > Hi,
> > 
> > This patch series is a split-off from the Spice block device RFC.
> > http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02024.html
> > 
> > It should include only non-controversial or simple patches. The most
> > important change is the NBD client implementation moved to a seperate
> > file.
> 
> Well, my intention was to merge the spice bits only, not nbd which is
> block stuff.  I'll happily take 1-7 as-is through the spice queue.
> 
> I can take patches 8-14 though the spice queue too, but I want an ack
> from the block people for that.  Or we split the series and merge 8-14
> through the block queue.  Kevin?  Stefan?

This series is on my radar.  I'll review what remains unmerged this week.

Stefan

Re: [Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Stefan Hajnoczi]

On Sun, Dec 01, 2013 at 10:23:33PM +0100, Marc-André Lureau wrote:
> This patch series is a split-off from the Spice block device RFC.
> http://lists.gnu.org/archive/html/qemu-devel/2013-11/msg02024.html
> 
> It should include only non-controversial or simple patches. The most
> important change is the NBD client implementation moved to a seperate
> file.
> 
> In v2:
> - should address Paolo comments
> - added a patch to remove unused CoQueue member
> 
> Marc-André Lureau (14):
>   coroutine: remove qemu_co_queue_wait_insert_head
>   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
>   coroutine: remove unused CoQueue AioContext
> 
>  block/Makefile.objs       |   2 +-
>  block/nbd-client.c        | 385 ++++++++++++++++++++++++++++++++++++++++++++++
>  block/nbd-client.h        |  50 ++++++
>  block/nbd.c               | 380 ++++-----------------------------------------
>  blockdev.c                |   2 +-
>  include/block/coroutine.h |   7 -
>  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 +
>  qemu-coroutine-lock.c     |   8 -
>  qmp.c                     |   2 +-
>  spice-qemu-char.c         |  11 +-
>  15 files changed, 507 insertions(+), 373 deletions(-)
>  create mode 100644 block/nbd-client.c
>  create mode 100644 block/nbd-client.h

Thanks, applied Patches 1 and 14 to my block tree:
https://github.com/stefanha/qemu/commits/block

Leaving the rest to Gerd and Paolo.

Stefan

Re: [Qemu-devel] [PATCHv2 00/14] Spice block device, ready-to-go patches

[Gerd Hoffmann]

  Hi,

> Thanks, applied Patches 1 and 14 to my block tree:
> https://github.com/stefanha/qemu/commits/block
> 
> Leaving the rest to Gerd and Paolo.

Added 2-13 to the spice patch queue.

cheers,
  Gerd