Merge remote-tracking branch 'bonzini/nbd-next' into staging

* bonzini/nbd-next:
  nbd: fixes to read-only handling
  hmp: add NBD server commands
  nbd: disallow nbd-server-add before nbd-server-start
  nbd: force read-only export for read-only devices
  nbd: fix nbd_server_stop crash when no server was running
  nbd: accept URIs
  nbd: accept relative path to Unix socket
  qemu-nbd: initialize main loop before block layer
Signed-off-by: NAnthony Liguori <aliguori@us.ibm.com>

block/nbd.c

@@ -28,6 +28,7 @@
 
 #include "qemu-common.h"
 #include "nbd.h"
+#include "uri.h"
 #include "block_int.h"
 #include "module.h"
 #include "qemu_socket.h"
@@ -55,7 +56,6 @@ typedef struct BDRVNBDState {
     uint32_t nbdflags;
     off_t size;
     size_t blocksize;
-    char *export_name; /* An NBD server may export several devices */
 
     CoMutex send_mutex;
     CoMutex free_sema;
@@ -65,13 +65,75 @@ typedef struct BDRVNBDState {
     Coroutine *recv_coroutine[MAX_NBD_REQUESTS];
     struct nbd_reply reply;
 
-    /* If it begins with  '/', this is a UNIX domain socket. Otherwise,
-     * it's a string of the form <hostname|ip4|\[ip6\]>:port
-     */
+    int is_unix;
     char *host_spec;
+    char *export_name; /* An NBD server may export several devices */
 } BDRVNBDState;
 
-static int nbd_config(BDRVNBDState *s, const char *filename, int flags)
+static int nbd_parse_uri(BDRVNBDState *s, const char *filename)
+{
+    URI *uri;
+    const char *p;
+    QueryParams *qp = NULL;
+    int ret = 0;
+
+    uri = uri_parse(filename);
+    if (!uri) {
+        return -EINVAL;
+    }
+
+    /* transport */
+    if (!strcmp(uri->scheme, "nbd")) {
+        s->is_unix = false;
+    } else if (!strcmp(uri->scheme, "nbd+tcp")) {
+        s->is_unix = false;
+    } else if (!strcmp(uri->scheme, "nbd+unix")) {
+        s->is_unix = true;
+    } else {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    p = uri->path ? uri->path : "/";
+    p += strspn(p, "/");
+    if (p[0]) {
+        s->export_name = g_strdup(p);
+    }
+
+    qp = query_params_parse(uri->query);
+    if (qp->n > 1 || (s->is_unix && !qp->n) || (!s->is_unix && qp->n)) {
+        ret = -EINVAL;
+        goto out;
+    }
+
+    if (s->is_unix) {
+        /* nbd+unix:///export?socket=path */
+        if (uri->server || uri->port || strcmp(qp->p[0].name, "socket")) {
+            ret = -EINVAL;
+            goto out;
+        }
+        s->host_spec = g_strdup(qp->p[0].value);
+    } else {
+        /* nbd[+tcp]://host:port/export */
+        if (!uri->server) {
+            ret = -EINVAL;
+            goto out;
+        }
+        if (!uri->port) {
+            uri->port = NBD_DEFAULT_PORT;
+        }
+        s->host_spec = g_strdup_printf("%s:%d", uri->server, uri->port);
+    }
+
+out:
+    if (qp) {
+        query_params_free(qp);
+    }
+    uri_free(uri);
+    return ret;
+}
+
+static int nbd_config(BDRVNBDState *s, const char *filename)
 {
     char *file;
     char *export_name;
@@ -79,6 +141,10 @@ static int nbd_config(BDRVNBDState *s, const char *filename, int flags)
     const char *unixpath;
     int err = -EINVAL;
 
+    if (strstr(filename, "://")) {
+        return nbd_parse_uri(s, filename);
+    }
+
     file = g_strdup(filename);
 
     export_name = strstr(file, EN_OPTSTR);
@@ -98,11 +164,10 @@ static int nbd_config(BDRVNBDState *s, const char *filename, int flags)
 
     /* are we a UNIX or TCP socket? */
     if (strstart(host_spec, "unix:", &unixpath)) {
-        if (unixpath[0] != '/') { /* We demand  an absolute path*/
-            goto out;
-        }
+        s->is_unix = true;
         s->host_spec = g_strdup(unixpath);
     } else {
+        s->is_unix = false;
         s->host_spec = g_strdup(host_spec);
     }
 
@@ -262,7 +327,7 @@ static int nbd_establish_connection(BlockDriverState *bs)
     off_t size;
     size_t blocksize;
 
-    if (s->host_spec[0] == '/') {
+    if (s->is_unix) {
         sock = unix_socket_outgoing(s->host_spec);
     } else {
         sock = tcp_socket_outgoing_spec(s->host_spec);
@@ -320,7 +385,7 @@ static int nbd_open(BlockDriverState *bs, const char* filename, int flags)
     qemu_co_mutex_init(&s->free_sema);
 
     /* Pop the config into our state object. Exit if invalid. */
-    result = nbd_config(s, filename, flags);
+    result = nbd_config(s, filename);
     if (result != 0) {
         return result;
     }
@@ -498,6 +563,33 @@ static int64_t nbd_getlength(BlockDriverState *bs)
 
 static BlockDriver bdrv_nbd = {
     .format_name         = "nbd",
+    .protocol_name       = "nbd",
+    .instance_size       = sizeof(BDRVNBDState),
+    .bdrv_file_open      = nbd_open,
+    .bdrv_co_readv       = nbd_co_readv,
+    .bdrv_co_writev      = nbd_co_writev,
+    .bdrv_close          = nbd_close,
+    .bdrv_co_flush_to_os = nbd_co_flush,
+    .bdrv_co_discard     = nbd_co_discard,
+    .bdrv_getlength      = nbd_getlength,
+};
+
+static BlockDriver bdrv_nbd_tcp = {
+    .format_name         = "nbd",
+    .protocol_name       = "nbd+tcp",
+    .instance_size       = sizeof(BDRVNBDState),
+    .bdrv_file_open      = nbd_open,
+    .bdrv_co_readv       = nbd_co_readv,
+    .bdrv_co_writev      = nbd_co_writev,
+    .bdrv_close          = nbd_close,
+    .bdrv_co_flush_to_os = nbd_co_flush,
+    .bdrv_co_discard     = nbd_co_discard,
+    .bdrv_getlength      = nbd_getlength,
+};
+
+static BlockDriver bdrv_nbd_unix = {
+    .format_name         = "nbd",
+    .protocol_name       = "nbd+unix",
     .instance_size       = sizeof(BDRVNBDState),
     .bdrv_file_open      = nbd_open,
     .bdrv_co_readv       = nbd_co_readv,
@@ -506,12 +598,13 @@ static BlockDriver bdrv_nbd = {
     .bdrv_co_flush_to_os = nbd_co_flush,
     .bdrv_co_discard     = nbd_co_discard,
     .bdrv_getlength      = nbd_getlength,
-    .protocol_name       = "nbd",
 };
 
 static void bdrv_nbd_init(void)
 {
     bdrv_register(&bdrv_nbd);
+    bdrv_register(&bdrv_nbd_tcp);
+    bdrv_register(&bdrv_nbd_unix);
 }
 
 block_init(bdrv_nbd_init);

blockdev-nbd.c

@@ -82,6 +82,11 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
     NBDExport *exp;
     NBDCloseNotifier *n;
 
+    if (server_fd == -1) {
+        error_setg(errp, "NBD server not running");
+        return;
+    }
+
     if (nbd_export_find(device)) {
         error_setg(errp, "NBD server already exporting device '%s'", device);
         return;
@@ -93,6 +98,13 @@ void qmp_nbd_server_add(const char *device, bool has_writable, bool writable,
         return;
     }
 
+    if (!has_writable) {
+        writable = true;
+    }
+    if (bdrv_is_read_only(bs)) {
+        writable = false;
+    }
+
     exp = nbd_export_new(bs, 0, -1, writable ? 0 : NBD_FLAG_READ_ONLY,
                          nbd_server_put_ref);
 
@@ -113,7 +125,9 @@ void qmp_nbd_server_stop(Error **errp)
         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;
+    if (server_fd != -1) {
+        qemu_set_fd_handler2(server_fd, NULL, NULL, NULL, NULL);
+        close(server_fd);
+        server_fd = -1;
+    }
 }

hmp-commands.hx

@@ -1310,6 +1310,51 @@ Remove all matches from the access control list, and set the default
 policy back to @code{deny}.
 ETEXI
 
+    {
+        .name       = "nbd_server_start",
+        .args_type  = "all:-a,writable:-w,uri:s",
+        .params     = "nbd_server_start [-a] [-w] host:port",
+        .help       = "serve block devices on the given host and port",
+        .mhandler.cmd = hmp_nbd_server_start,
+    },
+STEXI
+@item nbd_server_start @var{host}:@var{port}
+@findex nbd_server_start
+Start an NBD server on the given host and/or port.  If the @option{-a}
+option is included, all of the virtual machine's block devices that
+have an inserted media on them are automatically exported; in this case,
+the @option{-w} option makes the devices writable too.
+ETEXI
+
+    {
+        .name       = "nbd_server_add",
+        .args_type  = "writable:-w,device:B",
+        .params     = "nbd_server_add [-w] device",
+        .help       = "export a block device via NBD",
+        .mhandler.cmd = hmp_nbd_server_add,
+    },
+STEXI
+@item nbd_server_add @var{device}
+@findex nbd_server_add
+Export a block device through QEMU's NBD server, which must be started
+beforehand with @command{nbd_server_start}.  The @option{-w} option makes the
+exported device writable too.
+ETEXI
+
+    {
+        .name       = "nbd_server_stop",
+        .args_type  = "",
+        .params     = "nbd_server_stop",
+        .help       = "stop serving block devices using the NBD protocol",
+        .mhandler.cmd = hmp_nbd_server_stop,
+    },
+STEXI
+@item nbd_server_stop
+@findex nbd_server_stop
+Stop the QEMU embedded NBD server.
+ETEXI
+
+
 #if defined(TARGET_I386)
 
     {

hmp.c

@@ -18,6 +18,7 @@
 #include "qemu-option.h"
 #include "qemu-timer.h"
 #include "qmp-commands.h"
+#include "qemu_socket.h"
 #include "monitor.h"
 #include "console.h"
 
@@ -1259,3 +1260,78 @@ void hmp_screen_dump(Monitor *mon, const QDict *qdict)
     qmp_screendump(filename, &err);
     hmp_handle_error(mon, &err);
 }
+
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict)
+{
+    const char *uri = qdict_get_str(qdict, "uri");
+    int writable = qdict_get_try_bool(qdict, "writable", 0);
+    int all = qdict_get_try_bool(qdict, "all", 0);
+    Error *local_err = NULL;
+    BlockInfoList *block_list, *info;
+    SocketAddress *addr;
+
+    if (writable && !all) {
+        error_setg(&local_err, "-w only valid together with -a");
+        goto exit;
+    }
+
+    /* First check if the address is valid and start the server.  */
+    addr = socket_parse(uri, &local_err);
+    if (local_err != NULL) {
+        goto exit;
+    }
+
+    qmp_nbd_server_start(addr, &local_err);
+    qapi_free_SocketAddress(addr);
+    if (local_err != NULL) {
+        goto exit;
+    }
+
+    if (!all) {
+        return;
+    }
+
+    /* Then try adding all block devices.  If one fails, close all and
+     * exit.
+     */
+    block_list = qmp_query_block(NULL);
+
+    for (info = block_list; info; info = info->next) {
+        if (!info->value->has_inserted) {
+            continue;
+        }
+
+        qmp_nbd_server_add(info->value->device, true, writable, &local_err);
+
+        if (local_err != NULL) {
+            qmp_nbd_server_stop(NULL);
+            break;
+        }
+    }
+
+    qapi_free_BlockInfoList(block_list);
+
+exit:
+    hmp_handle_error(mon, &local_err);
+}
+
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict)
+{
+    const char *device = qdict_get_str(qdict, "device");
+    int writable = qdict_get_try_bool(qdict, "writable", 0);
+    Error *local_err = NULL;
+
+    qmp_nbd_server_add(device, true, writable, &local_err);
+
+    if (local_err != NULL) {
+        hmp_handle_error(mon, &local_err);
+    }
+}
+
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict)
+{
+    Error *errp = NULL;
+
+    qmp_nbd_server_stop(&errp);
+    hmp_handle_error(mon, &errp);
+}

hmp.h

@@ -77,5 +77,8 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_send_key(Monitor *mon, const QDict *qdict);
 void hmp_screen_dump(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_start(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_add(Monitor *mon, const QDict *qdict);
+void hmp_nbd_server_stop(Monitor *mon, const QDict *qdict);
 
 #endif

nbd.c

@@ -596,24 +596,23 @@ int nbd_init(int fd, int csock, uint32_t flags, off_t size, size_t blocksize)
         return -serrno;
     }
 
-    if (flags & NBD_FLAG_READ_ONLY) {
-        int read_only = 1;
-        TRACE("Setting readonly attribute");
-
-        if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
+    if (ioctl(fd, NBD_SET_FLAGS, flags) < 0) {
+        if (errno == ENOTTY) {
+            int read_only = (flags & NBD_FLAG_READ_ONLY) != 0;
+            TRACE("Setting readonly attribute");
+
+            if (ioctl(fd, BLKROSET, (unsigned long) &read_only) < 0) {
+                int serrno = errno;
+                LOG("Failed setting read-only attribute");
+                return -serrno;
+            }
+        } else {
             int serrno = errno;
-            LOG("Failed setting read-only attribute");
+            LOG("Failed setting flags");
             return -serrno;
         }
     }
 
-    if (ioctl(fd, NBD_SET_FLAGS, flags) < 0
-        && errno != ENOTTY) {
-        int serrno = errno;
-        LOG("Failed setting flags");
-        return -serrno;
-    }
-
     TRACE("Negotiation ended");
 
     return 0;

qemu-doc.texi

@@ -610,14 +610,14 @@ QEMU can access directly to block device exported using the Network Block Device
 protocol.
 
 @example
-qemu-system-i386 linux.img -hdb nbd:my_nbd_server.mydomain.org:1024
+qemu-system-i386 linux.img -hdb nbd://my_nbd_server.mydomain.org:1024/
 @end example
 
 If the NBD server is located on the same host, you can use an unix socket instead
 of an inet socket:
 
 @example
-qemu-system-i386 linux.img -hdb nbd:unix:/tmp/my_socket
+qemu-system-i386 linux.img -hdb nbd+unix://?socket=/tmp/my_socket
 @end example
 
 In this case, the block device must be exported using qemu-nbd:
@@ -631,17 +631,26 @@ The use of qemu-nbd allows to share a disk between several guests:
 qemu-nbd --socket=/tmp/my_socket --share=2 my_disk.qcow2
 @end example
 
+@noindent
 and then you can use it with two guests:
 @example
-qemu-system-i386 linux1.img -hdb nbd:unix:/tmp/my_socket
-qemu-system-i386 linux2.img -hdb nbd:unix:/tmp/my_socket
+qemu-system-i386 linux1.img -hdb nbd+unix://?socket=/tmp/my_socket
+qemu-system-i386 linux2.img -hdb nbd+unix://?socket=/tmp/my_socket
 @end example
 
-If the nbd-server uses named exports (since NBD 2.9.18), you must use the
-"exportname" option:
+If the nbd-server uses named exports (supported since NBD 2.9.18, or with QEMU's
+own embedded NBD server), you must specify an export name in the URI:
 @example
-qemu-system-i386 -cdrom nbd:localhost:exportname=debian-500-ppc-netinst
-qemu-system-i386 -cdrom nbd:localhost:exportname=openSUSE-11.1-ppc-netinst
+qemu-system-i386 -cdrom nbd://localhost/debian-500-ppc-netinst
+qemu-system-i386 -cdrom nbd://localhost/openSUSE-11.1-ppc-netinst
+@end example
+
+The URI syntax for NBD is supported since QEMU 1.3.  An alternative syntax is
+also available.  Here are some example of the older syntax:
+@example
+qemu-system-i386 linux.img -hdb nbd:my_nbd_server.mydomain.org:1024
+qemu-system-i386 linux2.img -hdb nbd:unix:/tmp/my_socket
+qemu-system-i386 -cdrom nbd:localhost:10809:exportname=debian-500-ppc-netinst
 @end example
 
 @node disk_images_sheepdog

qemu-nbd.c

@@ -539,6 +539,7 @@ int main(int argc, char **argv)
         snprintf(sockpath, 128, SOCKET_PATH, basename(device));
     }
 
+    qemu_init_main_loop();
     bdrv_init();
     atexit(bdrv_close_all);
 
@@ -584,7 +585,6 @@ int main(int argc, char **argv)
         memset(&client_thread, 0, sizeof(client_thread));
     }
 
-    qemu_init_main_loop();
     qemu_set_fd_handler2(fd, nbd_can_accept, nbd_accept, NULL,
                          (void *)(uintptr_t)fd);