Patchwork [24/25] qmp: add NBD server commands

login
register
mail settings
Submitter Paolo Bonzini
Date Oct. 10, 2012, 2:03 p.m.
Message ID <1349877786-23514-25-git-send-email-pbonzini@redhat.com>
Download mbox | patch
Permalink /patch/190685/
State New
Headers show

Comments

Paolo Bonzini - Oct. 10, 2012, 2:03 p.m.
Adding an NBD server inside QEMU is trivial, since all the logic is
in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
The main difference is that qemu-nbd serves a single unnamed export,
while QEMU serves named exports.

Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile.objs    |   2 +-
 blockdev-nbd.c   | 119 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |  43 ++++++++++++++++++++
 qmp-commands.hx  |  16 ++++++++
 4 file modificati, 179 inserzioni(+). 1 rimozione(-)
 create mode 100644 blockdev-nbd.c
Eric Blake - Oct. 10, 2012, 8:41 p.m.
On 10/10/2012 08:03 AM, Paolo Bonzini wrote:
> Adding an NBD server inside QEMU is trivial, since all the logic is
> in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
> The main difference is that qemu-nbd serves a single unnamed export,
> while QEMU serves named exports.
> 
> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---

> +##
> +# @nbd-server-add:
> +#
> +# Export a device to QEMU's embedded NBD server.
> +#
> +# @device: Block device to be exported
> +#
> +# @writable: Whether clients should be able to write to the device via the
> +#     NBD connection (default false). #optional

Isn't the #optional designation supposed to come first, before 'Whether'?
Paolo Bonzini - Oct. 11, 2012, 1:06 p.m.
Il 10/10/2012 22:41, Eric Blake ha scritto:
> On 10/10/2012 08:03 AM, Paolo Bonzini wrote:
>> Adding an NBD server inside QEMU is trivial, since all the logic is
>> in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
>> The main difference is that qemu-nbd serves a single unnamed export,
>> while QEMU serves named exports.
>>
>> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>> ---
> 
>> +##
>> +# @nbd-server-add:
>> +#
>> +# Export a device to QEMU's embedded NBD server.
>> +#
>> +# @device: Block device to be exported
>> +#
>> +# @writable: Whether clients should be able to write to the device via the
>> +#     NBD connection (default false). #optional
> 
> Isn't the #optional designation supposed to come first, before 'Whether'?

Does it really matter with no program yet written to consume it?
Putting it at the end matches the old qmp-commands.hx format better (for
commands that do have qmp-commands.hx documentation).

Paolo
Eric Blake - Oct. 11, 2012, 1:14 p.m.
On 10/11/2012 07:06 AM, Paolo Bonzini wrote:
> Il 10/10/2012 22:41, Eric Blake ha scritto:
>> On 10/10/2012 08:03 AM, Paolo Bonzini wrote:
>>> Adding an NBD server inside QEMU is trivial, since all the logic is
>>> in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
>>> The main difference is that qemu-nbd serves a single unnamed export,
>>> while QEMU serves named exports.
>>>
>>> Acked-by: Luiz Capitulino <lcapitulino@redhat.com>
>>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
>>> ---
>>
>>> +##
>>> +# @nbd-server-add:
>>> +#
>>> +# Export a device to QEMU's embedded NBD server.
>>> +#
>>> +# @device: Block device to be exported
>>> +#
>>> +# @writable: Whether clients should be able to write to the device via the
>>> +#     NBD connection (default false). #optional
>>
>> Isn't the #optional designation supposed to come first, before 'Whether'?
> 
> Does it really matter with no program yet written to consume it?
> Putting it at the end matches the old qmp-commands.hx format better (for
> commands that do have qmp-commands.hx documentation).

I'm just asking on the grounds of consistency based on observation, and
not based on an actual hard requirement of something that goes wrong if
it's out of order.  Therefore, I'm okay with your explanation, as long
as no one else can provide hard evidence for a mandatory positioning of
the marker.
Paolo Bonzini - Oct. 11, 2012, 1:22 p.m.
Il 11/10/2012 15:14, Eric Blake ha scritto:
>>>> +##
>>>> +# @nbd-server-add:
>>>> +#
>>>> +# Export a device to QEMU's embedded NBD server.
>>>> +#
>>>> +# @device: Block device to be exported
>>>> +#
>>>> +# @writable: Whether clients should be able to write to the device via the
>>>> +#     NBD connection (default false). #optional
>>>
>>> Isn't the #optional designation supposed to come first, before 'Whether'?
>>
>> Does it really matter with no program yet written to consume it?
>> Putting it at the end matches the old qmp-commands.hx format better (for
>> commands that do have qmp-commands.hx documentation).
> 
> I'm just asking on the grounds of consistency based on observation, and
> not based on an actual hard requirement of something that goes wrong if
> it's out of order.  Therefore, I'm okay with your explanation, as long
> as no one else can provide hard evidence for a mandatory positioning of
> the marker.

In fact, if such a program existed, it would be able to derive the
optional-ness of the argument from the schema, and it would make sense
for consistency to eliminate all #optional markers...

Paolo
Markus Armbruster - Oct. 19, 2012, 8:31 a.m.
Paolo Bonzini <pbonzini@redhat.com> writes:

> Adding an NBD server inside QEMU is trivial, since all the logic is
> in nbd.c and can be shared easily between qemu-nbd and QEMU itself.
> The main difference is that qemu-nbd serves a single unnamed export,
> while QEMU serves named exports.

For NBD noobs like me, a short paragraph on what's served to whom would
be useful.

Noob says code looks sane enough (for whatever that's worth).

Patch

diff --git a/Makefile.objs b/Makefile.objs
index ca67885..9eca179 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -61,7 +61,7 @@  endif
 # suppress *all* target specific code in case of system emulation, i.e. a
 # single QEMU executable should support all CPUs and machines.
 
-common-obj-y = $(block-obj-y) blockdev.o block/
+common-obj-y = $(block-obj-y) blockdev.o blockdev-nbd.o block/
 common-obj-y += net.o net/
 common-obj-y += qom/
 common-obj-y += readline.o console.o cursor.o
diff --git a/blockdev-nbd.c b/blockdev-nbd.c
new file mode 100644
index 0000000..8031813
--- /dev/null
+++ b/blockdev-nbd.c
@@ -0,0 +1,119 @@ 
+/*
+ * Serving QEMU block devices via NBD
+ *
+ * Copyright (c) 2012 Red Hat, Inc.
+ *
+ * Author: Paolo Bonzini <pbonzini@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or
+ * later.  See the COPYING file in the top-level directory.
+ */
+
+#include "blockdev.h"
+#include "hw/block-common.h"
+#include "monitor.h"
+#include "qerror.h"
+#include "sysemu.h"
+#include "qmp-commands.h"
+#include "trace.h"
+#include "nbd.h"
+#include "qemu_socket.h"
+
+static int server_fd = -1;
+
+static void nbd_accept(void *opaque)
+{
+    struct sockaddr_in addr;
+    socklen_t addr_len = sizeof(addr);
+
+    int fd = accept(server_fd, (struct sockaddr *)&addr, &addr_len);
+    if (fd >= 0) {
+        nbd_client_new(NULL, fd, nbd_client_put);
+    }
+}
+
+void qmp_nbd_server_start(SocketAddress *addr, Error **errp)
+{
+    if (server_fd != -1) {
+        error_setg(errp, "NBD server already running");
+        return;
+    }
+
+    server_fd = socket_listen(addr, errp);
+    if (server_fd != -1) {
+        qemu_set_fd_handler2(server_fd, NULL, nbd_accept, NULL, NULL);
+    }
+}
+
+/* Hook into the BlockDriverState notifiers to close the export when
+ * the file is closed.
+ */
+typedef struct NBDCloseNotifier {
+    Notifier n;
+    NBDExport *exp;
+    QTAILQ_ENTRY(NBDCloseNotifier) next;
+} NBDCloseNotifier;
+
+static QTAILQ_HEAD(, NBDCloseNotifier) close_notifiers =
+    QTAILQ_HEAD_INITIALIZER(close_notifiers);
+
+static void nbd_close_notifier(Notifier *n, void *data)
+{
+    NBDCloseNotifier *cn = DO_UPCAST(NBDCloseNotifier, n, n);
+
+    notifier_remove(&cn->n);
+    QTAILQ_REMOVE(&close_notifiers, cn, next);
+
+    nbd_export_close(cn->exp);
+    nbd_export_put(cn->exp);
+    g_free(cn);
+}
+
+static void nbd_server_put_ref(NBDExport *exp)
+{
+    BlockDriverState *bs = nbd_export_get_blockdev(exp);
+    drive_put_ref(drive_get_by_blockdev(bs));
+}
+
+void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
+                        Error **errp)
+{
+    BlockDriverState *bs;
+    NBDExport *exp;
+    NBDCloseNotifier *n;
+
+    if (nbd_export_find(device)) {
+        error_setg(errp, "NBD server already exporting device '%s'", device);
+        return;
+    }
+
+    bs = bdrv_find(device);
+    if (!bs) {
+        error_set(errp, QERR_DEVICE_NOT_FOUND, device);
+        return;
+    }
+
+    exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
+                         nbd_server_put_ref);
+
+    nbd_export_set_name(exp, device);
+    drive_get_ref(drive_get_by_blockdev(bs));
+
+    n = g_malloc0(sizeof(NBDCloseNotifier));
+    n->n.notify = nbd_close_notifier;
+    n->exp = exp;
+    bdrv_add_close_notifier(bs, &n->n);
+    QTAILQ_INSERT_TAIL(&close_notifiers, n, next);
+}
+
+void qmp_nbd_server_stop(Error **errp)
+{
+    while (!QTAILQ_EMPTY(&close_notifiers)) {
+        NBDCloseNotifier *cn = QTAILQ_FIRST(&close_notifiers);
+        nbd_close_notifier(&cn->n, nbd_export_get_blockdev(cn->exp));
+    }
+
+    qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+    close(server_fd);
+    server_fd = -1;
+}
diff --git a/qapi-schema.json b/qapi-schema.json
index d40b5fc..d0fe1ad 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -2849,3 +2849,46 @@ 
 # Since: 0.14.0
 ##
 { 'command': 'screendump', 'data': {'filename': 'str'} }
+
+##
+# @nbd-server-start:
+#
+# Start an NBD server listening on the given host and port.  Block
+# devices can then be exported using @nbd-server-add.  The NBD
+# server will present them as named exports; for example, another
+# QEMU instance could refer to them as "nbd:HOST:PORT:exportname=NAME".
+#
+# @addr: Address on which to listen.
+#
+# Returns: error if the server is already running.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-start',
+  'data': { 'addr': 'SocketAddress' } }
+
+##
+# @nbd-server-add:
+#
+# Export a device to QEMU's embedded NBD server.
+#
+# @device: Block device to be exported
+#
+# @writable: Whether clients should be able to write to the device via the
+#     NBD connection (default false). #optional
+#
+# Returns: error if the device is already marked for export.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-add', 'data': {'device': 'str', '*writable': 'bool'} }
+
+##
+# @nbd-server-stop:
+#
+# Stop QEMU's embedded NBD server, and unregister all devices previously
+# added via @nbd-server-add.
+#
+# Since: 1.3.0
+##
+{ 'command': 'nbd-server-stop' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 2f8477e..8b40902 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2543,6 +2543,22 @@  EQMP
     },
 
     {
+        .name       = "nbd-server-start",
+        .args_type  = "addr:q",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_start,
+    },
+    {
+        .name       = "nbd-server-add",
+        .args_type  = "device:B,writable:b?",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_add,
+    },
+    {
+        .name       = "nbd-server-stop",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_nbd_server_stop,
+    },
+
+    {
         .name       = "change-vnc-password",
         .args_type  = "password:s",
         .mhandler.cmd_new = qmp_marshal_input_change_vnc_password,