[Qemu-devel] [PULL 00/12] NBD patches for 2012-09-22

[Qemu-devel] [PULL 00/12] NBD patches for 2012-09-22

[Paolo Bonzini]

The following changes since commit 89c33337fd21930de671a6e34793e8b1ee257e2e:

  Remove unused CONFIG_TCG_PASS_AREG0 and dead code (2012-09-15 17:51:14 +0000)

are available in the git repository at:

  git://github.com/bonzini/qemu.git nbd-next

for you to fetch changes up to be3d30e144dbb99cd39ae4cedfb802337b5b172f:

  nbd: add nbd_export_get_blockdev (2012-09-19 14:03:15 +0200)

These patches merge the first part of the embedded NBD server.  The actual
QEMU implementation needs some refactoring of qemu-sockets, and might even
go in through Luiz's tree.

----------------------------------------------------------------
Paolo Bonzini (12):
      nbd: add more constants
      nbd: pass NBDClient to nbd_send_negotiate
      nbd: do not close BlockDriverState in nbd_export_close
      nbd: make refcount interface public
      nbd: do not leak nbd_trip coroutines when a connection is torn down
      nbd: add reference counting to NBDExport
      nbd: track clients into NBDExport
      nbd: add notification for closing an NBDExport
      qemu-nbd: rewrite termination conditions to use a state machine
      nbd: register named exports
      nbd: negotiate with named exports
      nbd: add nbd_export_get_blockdev

 nbd.c      | 396 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
 nbd.h      |  15 ++-
 qemu-nbd.c |  36 ++++--
 3 file modificati, 367 inserzioni(+), 80 rimozioni(-)
-- 
1.7.12

[Qemu-devel] [PATCH 01/12] nbd: add more constants

[Paolo Bonzini]

Avoid magic numbers and magic size computations; hide them behind
constants.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 17 ++++++++++-------
 1 file modificato, 10 inserzioni(+), 7 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 0dd60c5..8201b7a 100644
--- a/nbd.c
+++ b/nbd.c
@@ -57,9 +57,12 @@
 
 /* This is all part of the "official" NBD API */
 
+#define NBD_REQUEST_SIZE        (4 + 4 + 8 + 8 + 4)
 #define NBD_REPLY_SIZE          (4 + 4 + 8)
 #define NBD_REQUEST_MAGIC       0x25609513
 #define NBD_REPLY_MAGIC         0x67446698
+#define NBD_OPTS_MAGIC          0x49484156454F5054LL
+#define NBD_CLIENT_MAGIC        0x0000420281861253LL
 
 #define NBD_SET_SOCK            _IO(0xab, 0)
 #define NBD_SET_BLKSIZE         _IO(0xab, 1)
@@ -213,7 +216,7 @@ static int nbd_send_negotiate(int csock, off_t size, uint32_t flags)
 
     /* Negotiate
         [ 0 ..   7]   passwd   ("NBDMAGIC")
-        [ 8 ..  15]   magic    (0x00420281861253)
+        [ 8 ..  15]   magic    (NBD_CLIENT_MAGIC)
         [16 ..  23]   size
         [24 ..  27]   flags
         [28 .. 151]   reserved (0)
@@ -224,7 +227,7 @@ static int nbd_send_negotiate(int csock, off_t size, uint32_t flags)
 
     TRACE("Beginning negotiation.");
     memcpy(buf, "NBDMAGIC", 8);
-    cpu_to_be64w((uint64_t*)(buf + 8), 0x00420281861253LL);
+    cpu_to_be64w((uint64_t*)(buf + 8), NBD_CLIENT_MAGIC);
     cpu_to_be64w((uint64_t*)(buf + 16), size);
     cpu_to_be32w((uint32_t*)(buf + 24),
                  flags | NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
@@ -295,7 +298,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
         uint32_t namesize;
 
         TRACE("Checking magic (opts_magic)");
-        if (magic != 0x49484156454F5054LL) {
+        if (magic != NBD_OPTS_MAGIC) {
             LOG("Bad magic received");
             goto fail;
         }
@@ -334,7 +337,7 @@ int nbd_receive_negotiate(int csock, const char *name, uint32_t *flags,
     } else {
         TRACE("Checking magic (cli_magic)");
 
-        if (magic != 0x00420281861253LL) {
+        if (magic != NBD_CLIENT_MAGIC) {
             LOG("Bad magic received");
             goto fail;
         }
@@ -477,7 +480,7 @@ int nbd_client(int fd)
 
 ssize_t nbd_send_request(int csock, struct nbd_request *request)
 {
-    uint8_t buf[4 + 4 + 8 + 8 + 4];
+    uint8_t buf[NBD_REQUEST_SIZE];
     ssize_t ret;
 
     cpu_to_be32w((uint32_t*)buf, NBD_REQUEST_MAGIC);
@@ -504,7 +507,7 @@ ssize_t nbd_send_request(int csock, struct nbd_request *request)
 
 static ssize_t nbd_receive_request(int csock, struct nbd_request *request)
 {
-    uint8_t buf[4 + 4 + 8 + 8 + 4];
+    uint8_t buf[NBD_REQUEST_SIZE];
     uint32_t magic;
     ssize_t ret;
 
@@ -582,7 +585,7 @@ ssize_t nbd_receive_reply(int csock, struct nbd_reply *reply)
 
 static ssize_t nbd_send_reply(int csock, struct nbd_reply *reply)
 {
-    uint8_t buf[4 + 4 + 8];
+    uint8_t buf[NBD_REPLY_SIZE];
     ssize_t ret;
 
     /* Reply
-- 
1.7.12

[Qemu-devel] [PATCH 02/12] nbd: pass NBDClient to nbd_send_negotiate

[Paolo Bonzini]

We will need the NBDClient in nbd_send_negotiate to store the
export requested by the client.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 78 +++++++++++++++++++++++++++++++++++--------------------------------
 1 file modificato, 41 inserzioni(+), 37 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 8201b7a..5a3088d 100644
--- a/nbd.c
+++ b/nbd.c
@@ -78,6 +78,39 @@
 
 #define NBD_OPT_EXPORT_NAME     (1 << 0)
 
+/* Definitions for opaque data types */
+
+typedef struct NBDRequest NBDRequest;
+
+struct NBDRequest {
+    QSIMPLEQ_ENTRY(NBDRequest) entry;
+    NBDClient *client;
+    uint8_t *data;
+};
+
+struct NBDExport {
+    BlockDriverState *bs;
+    off_t dev_offset;
+    off_t size;
+    uint32_t nbdflags;
+    QSIMPLEQ_HEAD(, NBDRequest) requests;
+};
+
+struct NBDClient {
+    int refcount;
+    void (*close)(NBDClient *client);
+
+    NBDExport *exp;
+    int sock;
+
+    Coroutine *recv_coroutine;
+
+    CoMutex send_lock;
+    Coroutine *send_coroutine;
+
+    int nb_requests;
+};
+
 /* That's all folks */
 
 ssize_t nbd_wr_sync(int fd, void *buffer, size_t size, bool do_read)
@@ -209,8 +242,9 @@ int unix_socket_outgoing(const char *path)
                   Request (type == 2)
 */
 
-static int nbd_send_negotiate(int csock, off_t size, uint32_t flags)
+static int nbd_send_negotiate(NBDClient *client)
 {
+    int csock = client->sock;
     char buf[8 + 8 + 8 + 128];
     int rc;
 
@@ -228,9 +262,9 @@ static int nbd_send_negotiate(int csock, off_t size, uint32_t flags)
     TRACE("Beginning negotiation.");
     memcpy(buf, "NBDMAGIC", 8);
     cpu_to_be64w((uint64_t*)(buf + 8), NBD_CLIENT_MAGIC);
-    cpu_to_be64w((uint64_t*)(buf + 16), size);
+    cpu_to_be64w((uint64_t*)(buf + 16), client->exp->size);
     cpu_to_be32w((uint32_t*)(buf + 24),
-                 flags | NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
+                 client->exp->nbdflags | NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
                  NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
     memset(buf + 28, 0, 124);
 
@@ -613,37 +647,6 @@ static ssize_t nbd_send_reply(int csock, struct nbd_reply *reply)
 
 #define MAX_NBD_REQUESTS 16
 
-typedef struct NBDRequest NBDRequest;
-
-struct NBDRequest {
-    QSIMPLEQ_ENTRY(NBDRequest) entry;
-    NBDClient *client;
-    uint8_t *data;
-};
-
-struct NBDExport {
-    BlockDriverState *bs;
-    off_t dev_offset;
-    off_t size;
-    uint32_t nbdflags;
-    QSIMPLEQ_HEAD(, NBDRequest) requests;
-};
-
-struct NBDClient {
-    int refcount;
-    void (*close)(NBDClient *client);
-
-    NBDExport *exp;
-    int sock;
-
-    Coroutine *recv_coroutine;
-
-    CoMutex send_lock;
-    Coroutine *send_coroutine;
-
-    int nb_requests;
-};
-
 static void nbd_client_get(NBDClient *client)
 {
     client->refcount++;
@@ -977,13 +980,14 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
                           void (*close)(NBDClient *))
 {
     NBDClient *client;
-    if (nbd_send_negotiate(csock, exp->size, exp->nbdflags) < 0) {
-        return NULL;
-    }
     client = g_malloc0(sizeof(NBDClient));
     client->refcount = 1;
     client->exp = exp;
     client->sock = csock;
+    if (nbd_send_negotiate(client) < 0) {
+        g_free(client);
+        return NULL;
+    }
     client->close = close;
     qemu_co_mutex_init(&client->send_lock);
     qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
-- 
1.7.12

[Qemu-devel] [PATCH 03/12] nbd: do not close BlockDriverState in nbd_export_close

[Paolo Bonzini]

This is not desirable when embedding the NBD server inside QEMU.
Move the bdrv_close to qemu-nbd.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c      | 1 -
 qemu-nbd.c | 1 +
 2 file modificati, 1 inserzione(+). 1 rimozione(-)

diff --git a/nbd.c b/nbd.c
index 5a3088d..83200bd 100644
--- a/nbd.c
+++ b/nbd.c
@@ -721,7 +721,6 @@ void nbd_export_close(NBDExport *exp)
         g_free(first);
     }
 
-    bdrv_close(exp->bs);
     g_free(exp);
 }
 
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 1c1cf6a..23392e0 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -586,6 +586,7 @@ int main(int argc, char **argv)
     } while (!sigterm_reported && (persistent || !nbd_started || nb_fds > 0));
 
     nbd_export_close(exp);
+    bdrv_close(bs);
     if (sockpath) {
         unlink(sockpath);
     }
-- 
1.7.12

[Qemu-devel] [PATCH 04/12] nbd: make refcount interface public

[Paolo Bonzini]

After the next patch, the close callback will have to release its
reference.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 4 ++--
 nbd.h | 3 +++
 2 file modificati, 5 inserzioni(+), 2 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 83200bd..4aeb80a 100644
--- a/nbd.c
+++ b/nbd.c
@@ -647,12 +647,12 @@ static ssize_t nbd_send_reply(int csock, struct nbd_reply *reply)
 
 #define MAX_NBD_REQUESTS 16
 
-static void nbd_client_get(NBDClient *client)
+void nbd_client_get(NBDClient *client)
 {
     client->refcount++;
 }
 
-static void nbd_client_put(NBDClient *client)
+void nbd_client_put(NBDClient *client)
 {
     if (--client->refcount == 0) {
         g_free(client);
diff --git a/nbd.h b/nbd.h
index 40d58d3..a9038dc 100644
--- a/nbd.h
+++ b/nbd.h
@@ -81,7 +81,10 @@ typedef struct NBDClient NBDClient;
 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
                           off_t size, uint32_t nbdflags);
 void nbd_export_close(NBDExport *exp);
+
 NBDClient *nbd_client_new(NBDExport *exp, int csock,
                           void (*close)(NBDClient *));
+void nbd_client_get(NBDClient *client);
+void nbd_client_put(NBDClient *client);
 
 #endif
-- 
1.7.12

[Qemu-devel] [PATCH 05/12] nbd: do not leak nbd_trip coroutines when a connection is torn down

[Paolo Bonzini]

Because nbd_client_close removes the I/O handlers for the client
socket, there is no way that any suspended coroutines are restarted.
This will be a problem with the QEMU embedded NBD server, because
we will have a QMP command to forcibly close all connections with
the clients.

Instead, we can exploit the reference counting of NBDClients; shutdown the
client socket, which will make it readable and writeable.  Also call the
close callback, which will release the user's reference.  The coroutines
then will fail and exit cleanly, and release all remaining references,
until the last refcount finally triggers the closure of the client.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 33 +++++++++++++++++++++++++++------
 nbd.h |  1 +
 2 file modificati, 28 inserzioni(+), 6 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 4aeb80a..eb72f4a 100644
--- a/nbd.c
+++ b/nbd.c
@@ -109,6 +109,7 @@ struct NBDClient {
     Coroutine *send_coroutine;
 
     int nb_requests;
+    bool closing;
 };
 
 /* That's all folks */
@@ -655,19 +656,35 @@ void nbd_client_get(NBDClient *client)
 void nbd_client_put(NBDClient *client)
 {
     if (--client->refcount == 0) {
+        /* The last reference should be dropped by client->close,
+         * which is called by nbd_client_close.
+         */
+        assert(client->closing);
+
+        qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
+        close(client->sock);
+        client->sock = -1;
         g_free(client);
     }
 }
 
-static void nbd_client_close(NBDClient *client)
+void nbd_client_close(NBDClient *client)
 {
-    qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
-    close(client->sock);
-    client->sock = -1;
+    if (client->closing) {
+        return;
+    }
+
+    client->closing = true;
+
+    /* Force requests to finish.  They will drop their own references,
+     * then we'll close the socket and free the NBDClient.
+     */
+    shutdown(client->sock, 2);
+
+    /* Also tell the client, so that they release their reference.  */
     if (client->close) {
         client->close(client);
     }
-    nbd_client_put(client);
 }
 
 static NBDRequest *nbd_request_get(NBDClient *client)
@@ -810,14 +827,18 @@ out:
 static void nbd_trip(void *opaque)
 {
     NBDClient *client = opaque;
-    NBDRequest *req = nbd_request_get(client);
     NBDExport *exp = client->exp;
+    NBDRequest *req;
     struct nbd_request request;
     struct nbd_reply reply;
     ssize_t ret;
 
     TRACE("Reading request.");
+    if (client->closing) {
+        return;
+    }
 
+    req = nbd_request_get(client);
     ret = nbd_co_receive_request(req, &request);
     if (ret == -EAGAIN) {
         goto done;
diff --git a/nbd.h b/nbd.h
index a9038dc..8b84a50 100644
--- a/nbd.h
+++ b/nbd.h
@@ -84,6 +84,7 @@ void nbd_export_close(NBDExport *exp);
 
 NBDClient *nbd_client_new(NBDExport *exp, int csock,
                           void (*close)(NBDClient *));
+void nbd_client_close(NBDClient *client);
 void nbd_client_get(NBDClient *client);
 void nbd_client_put(NBDClient *client);
 
-- 
1.7.12

[Qemu-devel] [PATCH 06/12] nbd: add reference counting to NBDExport

[Paolo Bonzini]

We will use a similar two-phase destruction for NBDExport, so we need
each NBDClient to add a reference to NBDExport.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c      | 37 +++++++++++++++++++++++++++++++------
 nbd.h      |  2 ++
 qemu-nbd.c |  2 ++
 3 file modificati, 35 inserzioni(+), 6 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index eb72f4a..4922d38 100644
--- a/nbd.c
+++ b/nbd.c
@@ -89,6 +89,7 @@ struct NBDRequest {
 };
 
 struct NBDExport {
+    int refcount;
     BlockDriverState *bs;
     off_t dev_offset;
     off_t size;
@@ -664,6 +665,7 @@ void nbd_client_put(NBDClient *client)
         qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
         close(client->sock);
         client->sock = -1;
+        nbd_export_put(client->exp);
         g_free(client);
     }
 }
@@ -722,6 +724,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
 {
     NBDExport *exp = g_malloc0(sizeof(NBDExport));
     QSIMPLEQ_INIT(&exp->requests);
+    exp->refcount = 1;
     exp->bs = bs;
     exp->dev_offset = dev_offset;
     exp->nbdflags = nbdflags;
@@ -731,14 +734,34 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
 
 void nbd_export_close(NBDExport *exp)
 {
-    while (!QSIMPLEQ_EMPTY(&exp->requests)) {
-        NBDRequest *first = QSIMPLEQ_FIRST(&exp->requests);
-        QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
-        qemu_vfree(first->data);
-        g_free(first);
+    assert(exp->refcount == 1);
+
+    /* stub */
+}
+
+void nbd_export_get(NBDExport *exp)
+{
+    assert(exp->refcount > 0);
+    exp->refcount++;
+}
+
+void nbd_export_put(NBDExport *exp)
+{
+    assert(exp->refcount > 0);
+    if (exp->refcount == 1) {
+        nbd_export_close(exp);
     }
 
-    g_free(exp);
+    if (--exp->refcount == 0) {
+        while (!QSIMPLEQ_EMPTY(&exp->requests)) {
+            NBDRequest *first = QSIMPLEQ_FIRST(&exp->requests);
+            QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
+            qemu_vfree(first->data);
+            g_free(first);
+        }
+
+        g_free(exp);
+    }
 }
 
 static int nbd_can_read(void *opaque);
@@ -1011,5 +1034,7 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
     client->close = close;
     qemu_co_mutex_init(&client->send_lock);
     qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
+
+    nbd_export_get(exp);
     return client;
 }
diff --git a/nbd.h b/nbd.h
index 8b84a50..86921cd 100644
--- a/nbd.h
+++ b/nbd.h
@@ -81,6 +81,8 @@ typedef struct NBDClient NBDClient;
 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
                           off_t size, uint32_t nbdflags);
 void nbd_export_close(NBDExport *exp);
+void nbd_export_get(NBDExport *exp);
+void nbd_export_put(NBDExport *exp);
 
 NBDClient *nbd_client_new(NBDExport *exp, int csock,
                           void (*close)(NBDClient *));
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 23392e0..2a2cba3 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -586,7 +586,9 @@ int main(int argc, char **argv)
     } while (!sigterm_reported && (persistent || !nbd_started || nb_fds > 0));
 
     nbd_export_close(exp);
+    nbd_export_put(exp);
     bdrv_close(bs);
+
     if (sockpath) {
         unlink(sockpath);
     }
-- 
1.7.12

[Qemu-devel] [PATCH 07/12] nbd: track clients into NBDExport

[Paolo Bonzini]

Track the NBDClients of each NBDExport, and use it to implement
nbd_export_close.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 13 +++++++++++--
 1 file modificato, 11 inserzioni(+), 2 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 4922d38..c39692b 100644
--- a/nbd.c
+++ b/nbd.c
@@ -94,6 +94,7 @@ struct NBDExport {
     off_t dev_offset;
     off_t size;
     uint32_t nbdflags;
+    QTAILQ_HEAD(, NBDClient) clients;
     QSIMPLEQ_HEAD(, NBDRequest) requests;
 };
 
@@ -109,6 +110,7 @@ struct NBDClient {
     CoMutex send_lock;
     Coroutine *send_coroutine;
 
+    QTAILQ_ENTRY(NBDClient) next;
     int nb_requests;
     bool closing;
 };
@@ -665,6 +667,7 @@ void nbd_client_put(NBDClient *client)
         qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
         close(client->sock);
         client->sock = -1;
+        QTAILQ_REMOVE(&client->exp->clients, client, next);
         nbd_export_put(client->exp);
         g_free(client);
     }
@@ -725,6 +728,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
     NBDExport *exp = g_malloc0(sizeof(NBDExport));
     QSIMPLEQ_INIT(&exp->requests);
     exp->refcount = 1;
+    QTAILQ_INIT(&exp->clients);
     exp->bs = bs;
     exp->dev_offset = dev_offset;
     exp->nbdflags = nbdflags;
@@ -734,9 +738,13 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
 
 void nbd_export_close(NBDExport *exp)
 {
-    assert(exp->refcount == 1);
+    NBDClient *client, *next;
 
-    /* stub */
+    nbd_export_get(exp);
+    QTAILQ_FOREACH_SAFE(client, &exp->clients, next, next) {
+        nbd_client_close(client);
+    }
+    nbd_export_put(exp);
 }
 
 void nbd_export_get(NBDExport *exp)
@@ -1035,6 +1043,7 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
     qemu_co_mutex_init(&client->send_lock);
     qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
 
+    QTAILQ_INSERT_TAIL(&exp->clients, client, next);
     nbd_export_get(exp);
     return client;
 }
-- 
1.7.12

[Qemu-devel] [PATCH 08/12] nbd: add notification for closing an NBDExport

[Paolo Bonzini]

In order to exit cleanly from qemu-nbd, add a callback that triggers
when an NBDExport is closed.  In the case of qemu-nbd it will exit the
main loop.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c      | 10 +++++++++-
 nbd.h      |  3 ++-
 qemu-nbd.c |  2 +-
 3 file modificati, 12 inserzioni(+), 3 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index c39692b..2e9de70 100644
--- a/nbd.c
+++ b/nbd.c
@@ -90,6 +90,8 @@ struct NBDRequest {
 
 struct NBDExport {
     int refcount;
+    void (*close)(NBDExport *exp);
+
     BlockDriverState *bs;
     off_t dev_offset;
     off_t size;
@@ -723,7 +725,8 @@ static void nbd_request_put(NBDRequest *req)
 }
 
 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
-                          off_t size, uint32_t nbdflags)
+                          off_t size, uint32_t nbdflags,
+                          void (*close)(NBDExport *))
 {
     NBDExport *exp = g_malloc0(sizeof(NBDExport));
     QSIMPLEQ_INIT(&exp->requests);
@@ -733,6 +736,7 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
     exp->dev_offset = dev_offset;
     exp->nbdflags = nbdflags;
     exp->size = size == -1 ? bdrv_getlength(bs) : size;
+    exp->close = close;
     return exp;
 }
 
@@ -761,6 +765,10 @@ void nbd_export_put(NBDExport *exp)
     }
 
     if (--exp->refcount == 0) {
+        if (exp->close) {
+            exp->close(exp);
+        }
+
         while (!QSIMPLEQ_EMPTY(&exp->requests)) {
             NBDRequest *first = QSIMPLEQ_FIRST(&exp->requests);
             QSIMPLEQ_REMOVE_HEAD(&exp->requests, entry);
diff --git a/nbd.h b/nbd.h
index 86921cd..895820b 100644
--- a/nbd.h
+++ b/nbd.h
@@ -79,7 +79,8 @@ typedef struct NBDExport NBDExport;
 typedef struct NBDClient NBDClient;
 
 NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
-                          off_t size, uint32_t nbdflags);
+                          off_t size, uint32_t nbdflags,
+                          void (*close)(NBDExport *));
 void nbd_export_close(NBDExport *exp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
diff --git a/qemu-nbd.c b/qemu-nbd.c
index 2a2cba3..8b87dea 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -546,7 +546,7 @@ int main(int argc, char **argv)
         }
     }
 
-    exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags);
+    exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, NULL);
 
     if (sockpath) {
         fd = unix_socket_incoming(sockpath);
-- 
1.7.12

[Qemu-devel] [PATCH 09/12] qemu-nbd: rewrite termination conditions to use a state machine

[Paolo Bonzini]

Use a simple state machine with the following states:

- RUNNING     => accepting connections
- TERMINATE   => main loop must call nbd_export_close/put, and not accept
  connections anymore
- TERMINATING => waiting for pending requests to finish
- TERMINATED  => the NBDExport has been closed

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-nbd.c | 37 +++++++++++++++++++++++++++----------
 1 file modificato, 27 inserzioni(+), 10 rimozioni(-)

diff --git a/qemu-nbd.c b/qemu-nbd.c
index 8b87dea..15bcd08 100644
--- a/qemu-nbd.c
+++ b/qemu-nbd.c
@@ -41,8 +41,8 @@ static NBDExport *exp;
 static int verbose;
 static char *srcpath;
 static char *sockpath;
-static bool sigterm_reported;
-static bool nbd_started;
+static int persistent = 0;
+static enum { RUNNING, TERMINATE, TERMINATING, TERMINATED } state;
 static int shared = 1;
 static int nb_fds;
 
@@ -186,7 +186,7 @@ static int find_partition(BlockDriverState *bs, int partition,
 
 static void termsig_handler(int signum)
 {
-    sigterm_reported = true;
+    state = TERMINATE;
     qemu_notify_event();
 }
 
@@ -269,10 +269,20 @@ static int nbd_can_accept(void *opaque)
     return nb_fds < shared;
 }
 
+static void nbd_export_closed(NBDExport *exp)
+{
+    assert(state == TERMINATING);
+    state = TERMINATED;
+}
+
 static void nbd_client_closed(NBDClient *client)
 {
     nb_fds--;
+    if (nb_fds == 0 && !persistent && state == RUNNING) {
+        state = TERMINATE;
+    }
     qemu_notify_event();
+    nbd_client_put(client);
 }
 
 static void nbd_accept(void *opaque)
@@ -282,7 +292,11 @@ static void nbd_accept(void *opaque)
     socklen_t addr_len = sizeof(addr);
 
     int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
-    nbd_started = true;
+    if (state >= TERMINATE) {
+        close(fd);
+        return;
+    }
+
     if (fd >= 0 && nbd_client_new(exp, fd, nbd_client_closed)) {
         nb_fds++;
     }
@@ -329,7 +343,6 @@ int main(int argc, char **argv)
     int partition = -1;
     int ret;
     int fd;
-    int persistent = 0;
     bool seen_cache = false;
 #ifdef CONFIG_LINUX_AIO
     bool seen_aio = false;
@@ -546,7 +559,7 @@ int main(int argc, char **argv)
         }
     }
 
-    exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, NULL);
+    exp = nbd_export_new(bs, dev_offset, fd_size, nbdflags, nbd_export_closed);
 
     if (sockpath) {
         fd = unix_socket_incoming(sockpath);
@@ -581,14 +594,18 @@ int main(int argc, char **argv)
         err(EXIT_FAILURE, "Could not chdir to root directory");
     }
 
+    state = RUNNING;
     do {
         main_loop_wait(false);
-    } while (!sigterm_reported && (persistent || !nbd_started || nb_fds > 0));
+        if (state == TERMINATE) {
+            state = TERMINATING;
+            nbd_export_close(exp);
+            nbd_export_put(exp);
+            exp = NULL;
+        }
+    } while (state != TERMINATED);
 
-    nbd_export_close(exp);
-    nbd_export_put(exp);
     bdrv_close(bs);
-
     if (sockpath) {
         unlink(sockpath);
     }
-- 
1.7.12

[Qemu-devel] [PATCH 10/12] nbd: register named exports

[Paolo Bonzini]

Add an API to register and find named exports.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 49 +++++++++++++++++++++++++++++++++++++++++++++++++
 nbd.h |  4 ++++
 2 file modificati, 53 inserzioni(+)

diff --git a/nbd.c b/nbd.c
index 2e9de70..2d2221c 100644
--- a/nbd.c
+++ b/nbd.c
@@ -93,13 +93,17 @@ struct NBDExport {
     void (*close)(NBDExport *exp);
 
     BlockDriverState *bs;
+    char *name;
     off_t dev_offset;
     off_t size;
     uint32_t nbdflags;
     QTAILQ_HEAD(, NBDClient) clients;
     QSIMPLEQ_HEAD(, NBDRequest) requests;
+    QTAILQ_ENTRY(NBDExport) next;
 };
 
+static QTAILQ_HEAD(, NBDExport) exports = QTAILQ_HEAD_INITIALIZER(exports);
+
 struct NBDClient {
     int refcount;
     void (*close)(NBDClient *client);
@@ -740,6 +744,39 @@ NBDExport *nbd_export_new(BlockDriverState *bs, off_t dev_offset,
     return exp;
 }
 
+NBDExport *nbd_export_find(const char *name)
+{
+    NBDExport *exp;
+    QTAILQ_FOREACH(exp, &exports, next) {
+        if (strcmp(name, exp->name) == 0) {
+            return exp;
+        }
+    }
+
+    return NULL;
+}
+
+void nbd_export_set_name(NBDExport *exp, const char *name)
+{
+    if (exp->name == name) {
+        return;
+    }
+
+    nbd_export_get(exp);
+    if (exp->name != NULL) {
+        g_free(exp->name);
+        exp->name = NULL;
+        QTAILQ_REMOVE(&exports, exp, next);
+        nbd_export_put(exp);
+    }
+    if (name != NULL) {
+        nbd_export_get(exp);
+        exp->name = g_strdup(name);
+        QTAILQ_INSERT_TAIL(&exports, exp, next);
+    }
+    nbd_export_put(exp);
+}
+
 void nbd_export_close(NBDExport *exp)
 {
     NBDClient *client, *next;
@@ -765,6 +802,8 @@ void nbd_export_put(NBDExport *exp)
     }
 
     if (--exp->refcount == 0) {
+        assert(exp->name == NULL);
+
         if (exp->close) {
             exp->close(exp);
         }
@@ -780,6 +819,16 @@ void nbd_export_put(NBDExport *exp)
     }
 }
 
+void nbd_export_close_all(void)
+{
+    NBDExport *exp, *next;
+
+    QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
+        nbd_export_close(exp);
+        nbd_export_set_name(exp, NULL);
+    }
+}
+
 static int nbd_can_read(void *opaque);
 static void nbd_read(void *opaque);
 static void nbd_restart_write(void *opaque);
diff --git a/nbd.h b/nbd.h
index 895820b..f0edb9c 100644
--- a/nbd.h
+++ b/nbd.h
@@ -85,6 +85,10 @@ void nbd_export_close(NBDExport *exp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
 
+NBDExport *nbd_export_find(const char *name);
+void nbd_export_set_name(NBDExport *exp, const char *name);
+void nbd_export_close_all(void);
+
 NBDClient *nbd_client_new(NBDExport *exp, int csock,
                           void (*close)(NBDClient *));
 void nbd_client_close(NBDClient *client);
-- 
1.7.12

[Qemu-devel] [PATCH 11/12] nbd: negotiate with named exports

[Paolo Bonzini]

Allow negotiation to receive the name of the requested export from
the client.  Passing a NULL export to nbd_client_new will cause
the server to send the extended negotiation header.  The exp field
is then filled during negotiation.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 169 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------
 1 file modificato, 150 inserzioni(+), 19 rimozioni(-)

diff --git a/nbd.c b/nbd.c
index 2d2221c..6f60dcf 100644
--- a/nbd.c
+++ b/nbd.c
@@ -238,11 +238,23 @@ int unix_socket_outgoing(const char *path)
     return unix_connect(path);
 }
 
-/* Basic flow
+/* Basic flow for negotiation
 
    Server         Client
-
    Negotiate
+
+   or
+
+   Server         Client
+   Negotiate #1
+                  Option
+   Negotiate #2
+
+   ----
+
+   followed by
+
+   Server         Client
                   Request
    Response
                   Request
@@ -250,20 +262,112 @@ int unix_socket_outgoing(const char *path)
                   ...
    ...
                   Request (type == 2)
+
 */
 
+static int nbd_receive_options(NBDClient *client)
+{
+    int csock = client->sock;
+    char name[256];
+    uint32_t tmp, length;
+    uint64_t magic;
+    int rc;
+
+    /* Client sends:
+        [ 0 ..   3]   reserved (0)
+        [ 4 ..  11]   NBD_OPTS_MAGIC
+        [12 ..  15]   NBD_OPT_EXPORT_NAME
+        [16 ..  19]   length
+        [20 ..  xx]   export name (length bytes)
+     */
+
+    rc = -EINVAL;
+    if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+        LOG("read failed");
+        goto fail;
+    }
+    TRACE("Checking reserved");
+    if (tmp != 0) {
+        LOG("Bad reserved received");
+        goto fail;
+    }
+
+    if (read_sync(csock, &magic, sizeof(magic)) != sizeof(magic)) {
+        LOG("read failed");
+        goto fail;
+    }
+    TRACE("Checking reserved");
+    if (magic != be64_to_cpu(NBD_OPTS_MAGIC)) {
+        LOG("Bad magic received");
+        goto fail;
+    }
+
+    if (read_sync(csock, &tmp, sizeof(tmp)) != sizeof(tmp)) {
+        LOG("read failed");
+        goto fail;
+    }
+    TRACE("Checking option");
+    if (tmp != be32_to_cpu(NBD_OPT_EXPORT_NAME)) {
+        LOG("Bad option received");
+        goto fail;
+    }
+
+    if (read_sync(csock, &length, sizeof(length)) != sizeof(length)) {
+        LOG("read failed");
+        goto fail;
+    }
+    TRACE("Checking length");
+    length = be32_to_cpu(length);
+    if (length > 255) {
+        LOG("Bad length received");
+        goto fail;
+    }
+    if (read_sync(csock, name, length) != length) {
+        LOG("read failed");
+        goto fail;
+    }
+    name[length] = '\0';
+
+    client->exp = nbd_export_find(name);
+    if (!client->exp) {
+        LOG("export not found");
+        goto fail;
+    }
+
+    QTAILQ_INSERT_TAIL(&client->exp->clients, client, next);
+    nbd_export_get(client->exp);
+
+    TRACE("Option negotiation succeeded.");
+    rc = 0;
+fail:
+    return rc;
+}
+
 static int nbd_send_negotiate(NBDClient *client)
 {
     int csock = client->sock;
     char buf[8 + 8 + 8 + 128];
     int rc;
+    const int myflags = (NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
+                         NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
 
-    /* Negotiate
-        [ 0 ..   7]   passwd   ("NBDMAGIC")
-        [ 8 ..  15]   magic    (NBD_CLIENT_MAGIC)
+    /* Negotiation header without options:
+        [ 0 ..   7]   passwd       ("NBDMAGIC")
+        [ 8 ..  15]   magic        (NBD_CLIENT_MAGIC)
         [16 ..  23]   size
-        [24 ..  27]   flags
-        [28 .. 151]   reserved (0)
+        [24 ..  25]   server flags (0)
+        [24 ..  27]   export flags
+        [28 .. 151]   reserved     (0)
+
+       Negotiation header with options, part 1:
+        [ 0 ..   7]   passwd       ("NBDMAGIC")
+        [ 8 ..  15]   magic        (NBD_OPTS_MAGIC)
+        [16 ..  17]   server flags (0)
+
+       part 2 (after options are sent):
+        [18 ..  25]   size
+        [26 ..  27]   export flags
+        [28 .. 151]   reserved     (0)
      */
 
     socket_set_block(csock);
@@ -271,16 +375,39 @@ static int nbd_send_negotiate(NBDClient *client)
 
     TRACE("Beginning negotiation.");
     memcpy(buf, "NBDMAGIC", 8);
-    cpu_to_be64w((uint64_t*)(buf + 8), NBD_CLIENT_MAGIC);
-    cpu_to_be64w((uint64_t*)(buf + 16), client->exp->size);
-    cpu_to_be32w((uint32_t*)(buf + 24),
-                 client->exp->nbdflags | NBD_FLAG_HAS_FLAGS | NBD_FLAG_SEND_TRIM |
-                 NBD_FLAG_SEND_FLUSH | NBD_FLAG_SEND_FUA);
+    if (client->exp) {
+        assert ((client->exp->nbdflags & ~65535) == 0);
+        cpu_to_be64w((uint64_t*)(buf + 8), NBD_CLIENT_MAGIC);
+        cpu_to_be64w((uint64_t*)(buf + 16), client->exp->size);
+        cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags);
+    } else {
+        cpu_to_be64w((uint64_t*)(buf + 8), NBD_OPTS_MAGIC);
+    }
     memset(buf + 28, 0, 124);
 
-    if (write_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
-        LOG("write failed");
-        goto fail;
+    if (client->exp) {
+        if (write_sync(csock, buf, sizeof(buf)) != sizeof(buf)) {
+            LOG("write failed");
+            goto fail;
+        }
+    } else {
+        if (write_sync(csock, buf, 18) != 18) {
+            LOG("write failed");
+            goto fail;
+        }
+        rc = nbd_receive_options(client);
+        if (rc < 0) {
+            LOG("option negotiation failed");
+            goto fail;
+        }
+
+        assert ((client->exp->nbdflags & ~65535) == 0);
+        cpu_to_be64w((uint64_t*)(buf + 18), client->exp->size);
+        cpu_to_be16w((uint16_t*)(buf + 26), client->exp->nbdflags | myflags);
+        if (write_sync(csock, buf + 18, sizeof(buf) - 18) != sizeof(buf) - 18) {
+            LOG("write failed");
+            goto fail;
+        }
     }
 
     TRACE("Negotiation succeeded.");
@@ -673,8 +800,10 @@ void nbd_client_put(NBDClient *client)
         qemu_set_fd_handler2(client->sock, NULL, NULL, NULL, NULL);
         close(client->sock);
         client->sock = -1;
-        QTAILQ_REMOVE(&client->exp->clients, client, next);
-        nbd_export_put(client->exp);
+        if (client->exp) {
+            QTAILQ_REMOVE(&client->exp->clients, client, next);
+            nbd_export_put(client->exp);
+        }
         g_free(client);
     }
 }
@@ -1100,7 +1229,9 @@ NBDClient *nbd_client_new(NBDExport *exp, int csock,
     qemu_co_mutex_init(&client->send_lock);
     qemu_set_fd_handler2(csock, nbd_can_read, nbd_read, NULL, client);
 
-    QTAILQ_INSERT_TAIL(&exp->clients, client, next);
-    nbd_export_get(exp);
+    if (exp) {
+        QTAILQ_INSERT_TAIL(&exp->clients, client, next);
+        nbd_export_get(exp);
+    }
     return client;
 }
-- 
1.7.12

[Qemu-devel] [PATCH 12/12] nbd: add nbd_export_get_blockdev

[Paolo Bonzini]

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 nbd.c | 7 ++++++-
 nbd.h | 2 ++
 2 file modificati, 8 inserzioni(+). 1 rimozione(-)

diff --git a/nbd.c b/nbd.c
index 6f60dcf..57edfde 100644
--- a/nbd.c
+++ b/nbd.c
@@ -914,6 +914,7 @@ void nbd_export_close(NBDExport *exp)
     QTAILQ_FOREACH_SAFE(client, &exp->clients, next, next) {
         nbd_client_close(client);
     }
+    nbd_export_set_name(exp, NULL);
     nbd_export_put(exp);
 }
 
@@ -948,13 +949,17 @@ void nbd_export_put(NBDExport *exp)
     }
 }
 
+BlockDriverState *nbd_export_get_blockdev(NBDExport *exp)
+{
+    return exp->bs;
+}
+
 void nbd_export_close_all(void)
 {
     NBDExport *exp, *next;
 
     QTAILQ_FOREACH_SAFE(exp, &exports, next, next) {
         nbd_export_close(exp);
-        nbd_export_set_name(exp, NULL);
     }
 }
 
diff --git a/nbd.h b/nbd.h
index f0edb9c..344f05b 100644
--- a/nbd.h
+++ b/nbd.h
@@ -85,6 +85,8 @@ void nbd_export_close(NBDExport *exp);
 void nbd_export_get(NBDExport *exp);
 void nbd_export_put(NBDExport *exp);
 
+BlockDriverState *nbd_export_get_blockdev(NBDExport *exp);
+
 NBDExport *nbd_export_find(const char *name);
 void nbd_export_set_name(NBDExport *exp, const char *name);
 void nbd_export_close_all(void);
-- 
1.7.12

Re: [Qemu-devel] [PULL 00/12] NBD patches for 2012-09-22

[Anthony Liguori]

Paolo Bonzini <pbonzini@redhat.com> writes:

> The following changes since commit 89c33337fd21930de671a6e34793e8b1ee257e2e:
>
>   Remove unused CONFIG_TCG_PASS_AREG0 and dead code (2012-09-15 17:51:14 +0000)
>
> are available in the git repository at:
>
>   git://github.com/bonzini/qemu.git nbd-next
>
> for you to fetch changes up to be3d30e144dbb99cd39ae4cedfb802337b5b172f:
>
>   nbd: add nbd_export_get_blockdev (2012-09-19 14:03:15 +0200)
>
> These patches merge the first part of the embedded NBD server.  The actual
> QEMU implementation needs some refactoring of qemu-sockets, and might even
> go in through Luiz's tree.

Pulled. Thanks.

Regards,

Anthony Liguori

>
> ----------------------------------------------------------------
> Paolo Bonzini (12):
>       nbd: add more constants
>       nbd: pass NBDClient to nbd_send_negotiate
>       nbd: do not close BlockDriverState in nbd_export_close
>       nbd: make refcount interface public
>       nbd: do not leak nbd_trip coroutines when a connection is torn down
>       nbd: add reference counting to NBDExport
>       nbd: track clients into NBDExport
>       nbd: add notification for closing an NBDExport
>       qemu-nbd: rewrite termination conditions to use a state machine
>       nbd: register named exports
>       nbd: negotiate with named exports
>       nbd: add nbd_export_get_blockdev
>
>  nbd.c      | 396 ++++++++++++++++++++++++++++++++++++++++++++++++++-----------
>  nbd.h      |  15 ++-
>  qemu-nbd.c |  36 ++++--
>  3 file modificati, 367 inserzioni(+), 80 rimozioni(-)
> -- 
> 1.7.12