[Qemu-devel] [PATCH 00/13] Spice block device, ready-to-go patches

[Qemu-devel] [PATCH 00/13] 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.

thanks

Marc-André Lureau (13):
  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 session_close() idempotent
  nbd: finish any pending coroutine
  nbd: avoid uninitialized warnings

 block/Makefile.objs       |   2 +-
 block/nbd-client.c        | 384 ++++++++++++++++++++++++++++++++++++++++++++++
 block/nbd-client.h        |  50 ++++++
 block/nbd.c               | 380 ++++-----------------------------------------
 blockdev.c                |   2 +-
 include/block/coroutine.h |   6 -
 include/sysemu/blockdev.h |   2 +-
 include/sysemu/char.h     |  10 ++
 include/ui/qemu-spice.h   |   2 +
 libcacard/vscclient.c     |  10 +-
 nbd.c                     |   1 -
 qemu-char.c               |   7 +
 qemu-coroutine-lock.c     |   8 -
 qmp.c                     |   2 +-
 spice-qemu-char.c         |  11 +-
 15 files changed, 506 insertions(+), 371 deletions(-)
 create mode 100644 block/nbd-client.c
 create mode 100644 block/nbd-client.h

-- 
1.8.4.2

[Qemu-devel] [PATCH 01/13] 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] [PATCH 02/13] 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] [PATCH 03/13] 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] [PATCH 04/13] 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] [PATCH 05/13] 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] [PATCH 06/13] 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] [PATCH 07/13] 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] [PATCH 08/13] 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

Re: [Qemu-devel] [PATCH 08/13] Split nbd block client code

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> 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

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

[Qemu-devel] [PATCH 09/13] 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>
---
 nbd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/nbd.c b/nbd.c
index f847940..3af9d17 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) {
-- 
1.8.4.2

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

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> 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>
> ---
>  nbd.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/nbd.c b/nbd.c
> index f847940..3af9d17 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) {
> 

If you remove this here, you need to remove also the matching
socket_set_nonblock,

Also, there are two callers:

- nbd.c: you can add nbd_socket_block/nonblock around
nbd_receive_negotiate in nbd_open.

- qemu-nbd.c: here the socket can remain in blocking mode.  In fact it
is blocking before the call to nbd_receive_negotiate, because
unix_connect_opts is missing a call to qemu_set_block (bug!).  I suggest
that you add the call to qemu_set_nonblock there, and add qemu_set_block
in nbd_client_thread.

Thanks,

Paolo

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

[Marc-André Lureau]

On Fri, Nov 29, 2013 at 4:22 PM, Paolo Bonzini <pbonzini@redhat.com> wrote:
> If you remove this here, you need to remove also the matching
> socket_set_nonblock,

Ok

> Also, there are two callers:
>
> - nbd.c: you can add nbd_socket_block/nonblock around
> nbd_receive_negotiate in nbd_open.
>
> - qemu-nbd.c: here the socket can remain in blocking mode.  In fact it
> is blocking before the call to nbd_receive_negotiate, because
> unix_connect_opts is missing a call to qemu_set_block (bug!).  I suggest
> that you add the call to qemu_set_nonblock there, and add qemu_set_block
> in nbd_client_thread.

Sorry, I guess I assumed wrongly that the sync nbd reader and writer
would handle all cases.

So you suggest this block/unblock: (I haven't reviewed all callers of
unix_connect_opts(), I am not sure that's what you meant) Other option
would be to move the nonblock to unix_socket_outgoing.

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/qemu-nbd.c b/qemu-nbd.c
index c26c98e..008fbf0 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -224,6 +224,7 @@ static void *nbd_client_thread(void *arg)
         goto out;
     }

+    qemu_set_block(sock);
     ret = nbd_receive_negotiate(sock, NULL, &nbdflags,
                                 &size, &blocksize);
     if (ret < 0) {
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 6b97dc1..81f2660 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -747,6 +747,8 @@ int unix_connect_opts(QemuOpts *opts, Error **errp,
         }
     } while (rc == -EINTR);

+    qemu_set_nonblock(sock);
+
     if (connect_state != NULL && QEMU_SOCKET_RC_INPROGRESS(rc)) {
         connect_state->fd = sock;
         qemu_set_fd_handler2(sock, NULL, NULL, wait_for_connect,

-- 
Marc-André Lureau

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

[Paolo Bonzini]

Il 30/11/2013 16:49, Marc-André Lureau ha scritto:
> So you suggest this block/unblock: (I haven't reviewed all callers of
> unix_connect_opts(), I am not sure that's what you meant) Other option
> would be to move the nonblock to unix_socket_outgoing.
> 
> 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);

Also

 qemu_set_nonblock(sock);

here, and remove it from nbd_receive_negotiate.

I checked again and you need not touch unix_connect_opts, nor
nbd_client_thread.

Paolo

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

[Marc-André Lureau]

----- Original Message -----
> Il 30/11/2013 16:49, Marc-André Lureau ha scritto:
> > So you suggest this block/unblock: (I haven't reviewed all callers of
> > unix_connect_opts(), I am not sure that's what you meant) Other option
> > would be to move the nonblock to unix_socket_outgoing.
> > 
> > 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);
> 
> Also
> 
>  qemu_set_nonblock(sock);
> 
> here, 


It's already a few lines below.

> and remove it from nbd_receive_negotiate.

> I checked again and you need not touch unix_connect_opts, nor
> nbd_client_thread.

Ok, I'll remove those.

[Qemu-devel] [PATCH 10/13] 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 | 10 ++++------
 block/nbd-client.h |  5 ++---
 block/nbd.c        | 13 ++++++++-----
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index 1abfc6a..e29227b 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -338,17 +338,15 @@ 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 */
-    ret = nbd_receive_negotiate(sock, client->export_name,
+    logout("session init %s\n", export);
+    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

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

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> 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 | 10 ++++------
>  block/nbd-client.h |  5 ++---
>  block/nbd.c        | 13 ++++++++-----
>  3 files changed, 14 insertions(+), 14 deletions(-)
> 
> diff --git a/block/nbd-client.c b/block/nbd-client.c
> index 1abfc6a..e29227b 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -338,17 +338,15 @@ 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 */
> -    ret = nbd_receive_negotiate(sock, client->export_name,
> +    logout("session init %s\n", export);
> +    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,
> 

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

[Qemu-devel] [PATCH 11/13] nbd: make 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 e29227b..c0ad2c2 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

Re: [Qemu-devel] [PATCH 11/13] nbd: make session_close() idempotent

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> 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 e29227b..c0ad2c2 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,
> 

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

Perhaps "make nbd_client_session_close() idempotent" in the commit message.

Paolo

[Qemu-devel] [PATCH 12/13] 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 | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/block/nbd-client.c b/block/nbd-client.c
index c0ad2c2..ad6fb01 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -32,6 +32,18 @@
 #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]) {
+            fprintf(stderr, "*nbd reply enter: %p %d\n", s, s->reply.error);
+            qemu_coroutine_enter(s->recv_coroutine[i], NULL);
+        }
+    }
+}
+
 static void nbd_reply_ready(void *opaque)
 {
     NbdClientSession *s = opaque;
@@ -67,11 +79,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)
@@ -332,6 +340,10 @@ static void nbd_teardown_connection(NbdClientSession *client)
 
     qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
     closesocket(client->sock);
+    /* finish any pending coroutines */
+    client->reply.handle = 0;
+    client->reply.error = EIO;
+    nbd_recv_coroutines_enter_all(client);
     client->sock = -1;
 }
 
-- 
1.8.4.2

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

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
>      closesocket(client->sock);
> +    /* finish any pending coroutines */
> +    client->reply.handle = 0;
> +    client->reply.error = EIO;
> +    nbd_recv_coroutines_enter_all(client);

What about using the same shutdown(2) trick as the server?  Then you can
call nbd_recv_coroutines_enter_all() while client->sock is still valid,
which I find more robust.

Paolo

[Qemu-devel] [PATCH 13/13] 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 ad6fb01..82806f1 100644
--- a/block/nbd-client.c
+++ b/block/nbd-client.c
@@ -186,11 +186,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;
 
@@ -210,11 +209,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;
@@ -276,7 +274,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;
 
@@ -284,7 +282,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;
     }
@@ -306,14 +303,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;
 
@@ -331,11 +327,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);
 
     qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
-- 
1.8.4.2

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

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> ==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 ad6fb01..82806f1 100644
> --- a/block/nbd-client.c
> +++ b/block/nbd-client.c
> @@ -186,11 +186,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;
>  
> @@ -210,11 +209,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;
> @@ -276,7 +274,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;
>  
> @@ -284,7 +282,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;
>      }
> @@ -306,14 +303,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;
>  
> @@ -331,11 +327,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);
>  
>      qemu_aio_set_fd_handler(client->sock, NULL, NULL, NULL);
> 

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

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

[Paolo Bonzini]

Il 29/11/2013 15:58, Marc-André Lureau ha scritto:
> 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.

The NBD changes require a couple small changes.  Gerd, feel free to pick
up 1-7.

Paolo