[Qemu-devel] [PATCH v4 06/12] block/nbd: Accept SocketAddress

[Max Reitz]

Add a new option "address" to the NBD block driver which accepts a
SocketAddress.

"path", "host" and "port" are still supported as legacy options and are
mapped to their corresponding SocketAddress representation.

Signed-off-by: Max Reitz <address@hidden>
---
 block/nbd.c                   | 166 ++++++++++++++++++++++++++----------------
 tests/qemu-iotests/051.out    |   4 +-
 tests/qemu-iotests/051.pc.out |   4 +-
 3 files changed, 106 insertions(+), 68 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index cdab20f..449f94e 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -32,6 +32,9 @@
 #include "qemu/uri.h"
 #include "block/block_int.h"
 #include "qemu/module.h"
+#include "qapi-visit.h"
+#include "qapi/qmp-input-visitor.h"
+#include "qapi/qmp-output-visitor.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qint.h"
@@ -44,7 +47,8 @@ typedef struct BDRVNBDState {
     NbdClientSession client;
 
     /* For nbd_refresh_filename() */
-    char *path, *host, *port, *export, *tlscredsid;
+    SocketAddress *saddr;
+    char *export, *tlscredsid;
 } BDRVNBDState;
 
 static int nbd_parse_uri(const char *filename, QDict *options)
@@ -131,7 +135,9 @@ static bool nbd_has_filename_options_conflict(QDict 
*options, Error **errp)
         if (!strcmp(e->key, "host") ||
             !strcmp(e->key, "port") ||
             !strcmp(e->key, "path") ||
-            !strcmp(e->key, "export"))
+            !strcmp(e->key, "export") ||
+            !strcmp(e->key, "address") ||
+            !strncmp(e->key, "address.", 8))
         {
             error_setg(errp, "Option '%s' cannot be used with a file name",
                        e->key);
@@ -205,50 +211,67 @@ out:
     g_free(file);
 }
 
-static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
+static bool nbd_process_legacy_socket_options(QDict *output_options,
+                                              QemuOpts *legacy_opts,
+                                              Error **errp)
 {
-    SocketAddress *saddr;
-
-    s->path = g_strdup(qemu_opt_get(opts, "path"));
-    s->host = g_strdup(qemu_opt_get(opts, "host"));
-    s->port = g_strdup(qemu_opt_get(opts, "port"));
-
-    if (!s->path == !s->host) {
-        if (s->path) {
-            error_setg(errp, "path and host may not be used at the same time");
-        } else {
-            error_setg(errp, "one of path and host must be specified");
+    const char *path = qemu_opt_get(legacy_opts, "path");
+    const char *host = qemu_opt_get(legacy_opts, "host");
+    const char *port = qemu_opt_get(legacy_opts, "port");
+
+    if (path && host) {
+        error_setg(errp, "path and host may not be used at the same time");
+        return false;
+    } else if (path) {
+        if (port) {
+            error_setg(errp, "port may not be used without host");
+            return false;
         }
-        return NULL;
+
+        qdict_put(output_options, "address.type", qstring_from_str("unix"));
+        qdict_put(output_options, "address.data.path", qstring_from_str(path));
+    } else if (host) {
+        qdict_put(output_options, "address.type", qstring_from_str("inet"));
+        qdict_put(output_options, "address.data.host", qstring_from_str(host));
+        qdict_put(output_options, "address.data.port",
+                  qstring_from_str(port ?: stringify(NBD_DEFAULT_PORT)));
     }
-    if (s->port && !s->host) {
-        error_setg(errp, "port may not be used without host");
-        return NULL;
+
+    return true;
+}
+
+static SocketAddress *nbd_config(BDRVNBDState *s, QDict *options, Error **errp)
+{
+    SocketAddress *saddr = NULL;
+    QDict *addr = NULL;
+    QObject *crumpled_addr = NULL;
+    Visitor *iv = NULL;
+    Error *local_err = NULL;
+
+    qdict_extract_subqdict(options, &addr, "address.");
+    if (!qdict_size(addr)) {
+        error_setg(errp, "NBD server address missing");
+        goto done;
     }
 
-    saddr = g_new0(SocketAddress, 1);
+    crumpled_addr = qdict_crumple(addr, true, errp);
+    if (!crumpled_addr) {
+        goto done;
+    }
 
-    if (s->path) {
-        UnixSocketAddress *q_unix;
-        saddr->type = SOCKET_ADDRESS_KIND_UNIX;
-        q_unix = saddr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
-        q_unix->path = g_strdup(s->path);
-    } else {
-        InetSocketAddress *inet;
-
-        saddr->type = SOCKET_ADDRESS_KIND_INET;
-        inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
-        inet->host = g_strdup(s->host);
-        inet->port = g_strdup(s->port);
-        if (!inet->port) {
-            inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
-        }
+    iv = qmp_input_visitor_new(crumpled_addr, true);
+    visit_type_SocketAddress(iv, NULL, &saddr, &local_err);
+    if (local_err) {
+        error_propagate(errp, local_err);
+        goto done;
     }
 
     s->client.is_unix = saddr->type == SOCKET_ADDRESS_KIND_UNIX;
 
-    s->export = g_strdup(qemu_opt_get(opts, "export"));
-
+done:
+    QDECREF(addr);
+    qobject_decref(crumpled_addr);
+    visit_free(iv);
     return saddr;
 }
 
@@ -349,7 +372,6 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
     QemuOpts *opts = NULL;
     Error *local_err = NULL;
     QIOChannelSocket *sioc = NULL;
-    SocketAddress *saddr = NULL;
     QCryptoTLSCreds *tlscreds = NULL;
     const char *hostname = NULL;
     int ret = -EINVAL;
@@ -361,12 +383,19 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
         goto error;
     }
 
+    /* Translate @host, @port, and @path to a SocketAddress */
+    if (!nbd_process_legacy_socket_options(options, opts, errp)) {
+        goto error;
+    }
+
     /* Pop the config into our state object. Exit if invalid. */
-    saddr = nbd_config(s, opts, errp);
-    if (!saddr) {
+    s->saddr = nbd_config(s, options, errp);
+    if (!s->saddr) {
         goto error;
     }
 
+    s->export = g_strdup(qemu_opt_get(opts, "export"));
+
     s->tlscredsid = g_strdup(qemu_opt_get(opts, "tls-creds"));
     if (s->tlscredsid) {
         tlscreds = nbd_get_tls_creds(s->tlscredsid, errp);
@@ -374,17 +403,17 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
             goto error;
         }
 
-        if (saddr->type != SOCKET_ADDRESS_KIND_INET) {
+        if (s->saddr->type != SOCKET_ADDRESS_KIND_INET) {
             error_setg(errp, "TLS only supported over IP sockets");
             goto error;
         }
-        hostname = saddr->u.inet.data->host;
+        hostname = s->saddr->u.inet.data->host;
     }
 
     /* establish TCP connection, return error if it fails
      * TODO: Configurable retry-until-timeout behaviour.
      */
-    sioc = nbd_establish_connection(saddr, errp);
+    sioc = nbd_establish_connection(s->saddr, errp);
     if (!sioc) {
         ret = -ECONNREFUSED;
         goto error;
@@ -401,13 +430,10 @@ static int nbd_open(BlockDriverState *bs, QDict *options, 
int flags,
         object_unref(OBJECT(tlscreds));
     }
     if (ret < 0) {
-        g_free(s->path);
-        g_free(s->host);
-        g_free(s->port);
+        qapi_free_SocketAddress(s->saddr);
         g_free(s->export);
         g_free(s->tlscredsid);
     }
-    qapi_free_SocketAddress(saddr);
     qemu_opts_del(opts);
     return ret;
 }
@@ -429,9 +455,7 @@ static void nbd_close(BlockDriverState *bs)
 
     nbd_client_close(bs);
 
-    g_free(s->path);
-    g_free(s->host);
-    g_free(s->port);
+    qapi_free_SocketAddress(s->saddr);
     g_free(s->export);
     g_free(s->tlscredsid);
 }
@@ -458,30 +482,43 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
 {
     BDRVNBDState *s = bs->opaque;
     QDict *opts = qdict_new();
-    const char *port = s->port ?: stringify(NBD_DEFAULT_PORT);
+    QObject *saddr_qdict;
+    Visitor *ov;
+    const char *host = NULL, *port = NULL, *path = NULL;
+
+    if (s->saddr->type == SOCKET_ADDRESS_KIND_INET) {
+        const InetSocketAddress *inet = s->saddr->u.inet.data;
+        if (!inet->has_ipv4 && !inet->has_ipv6 && !inet->has_to) {
+            host = inet->host;
+            port = inet->port;
+        }
+    } else if (s->saddr->type == SOCKET_ADDRESS_KIND_UNIX) {
+        path = s->saddr->u.q_unix.data->path;
+    }
 
     qdict_put(opts, "driver", qstring_from_str("nbd"));
 
-    if (s->path && s->export) {
+    if (path && s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd+unix:///%s?socket=%s", s->export, s->path);
-    } else if (s->path && !s->export) {
+                 "nbd+unix:///%s?socket=%s", s->export, path);
+    } else if (path && !s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd+unix://?socket=%s", s->path);
-    } else if (!s->path && s->export) {
+                 "nbd+unix://?socket=%s", path);
+    } else if (host && s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s/%s", s->host, port, s->export);
-    } else if (!s->path && !s->export) {
+                 "nbd://%s:%s/%s", host, port, s->export);
+    } else if (host && !s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s", s->host, port);
+                 "nbd://%s:%s", host, port);
     }
 
-    if (s->path) {
-        qdict_put(opts, "path", qstring_from_str(s->path));
-    } else {
-        qdict_put(opts, "host", qstring_from_str(s->host));
-        qdict_put(opts, "port", qstring_from_str(port));
-    }
+    ov = qmp_output_visitor_new(&saddr_qdict);
+    visit_type_SocketAddress(ov, NULL, &s->saddr, &error_abort);
+    visit_complete(ov, &saddr_qdict);
+    assert(qobject_type(saddr_qdict) == QTYPE_QDICT);
+
+    qdict_put_obj(opts, "address", saddr_qdict);
+
     if (s->export) {
         qdict_put(opts, "export", qstring_from_str(s->export));
     }
@@ -489,6 +526,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, 
QDict *options)
         qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid));
     }
 
+    qdict_flatten(opts);
     bs->full_open_options = opts;
 }
 
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 9e584fe..42bf416 100644
--- a/tests/qemu-iotests/051.out
+++ b/tests/qemu-iotests/051.out
@@ -222,7 +222,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified
+QEMU_PROG: -drive driver=nbd: NBD server address missing
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -231,7 +231,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
+QEMU_PROG: -drive file.driver=nbd: NBD server address missing
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
diff --git a/tests/qemu-iotests/051.pc.out b/tests/qemu-iotests/051.pc.out
index 6395a30..603bb76 100644
--- a/tests/qemu-iotests/051.pc.out
+++ b/tests/qemu-iotests/051.pc.out
@@ -316,7 +316,7 @@ Testing: -drive driver=file
 QEMU_PROG: -drive driver=file: The 'file' block driver requires a file name
 
 Testing: -drive driver=nbd
-QEMU_PROG: -drive driver=nbd: one of path and host must be specified
+QEMU_PROG: -drive driver=nbd: NBD server address missing
 
 Testing: -drive driver=raw
 QEMU_PROG: -drive driver=raw: Can't use 'raw' as a block driver for the 
protocol level
@@ -325,7 +325,7 @@ Testing: -drive file.driver=file
 QEMU_PROG: -drive file.driver=file: The 'file' block driver requires a file 
name
 
 Testing: -drive file.driver=nbd
-QEMU_PROG: -drive file.driver=nbd: one of path and host must be specified
+QEMU_PROG: -drive file.driver=nbd: NBD server address missing
 
 Testing: -drive file.driver=raw
 QEMU_PROG: -drive file.driver=raw: Can't use 'raw' as a block driver for the 
protocol level
-- 
2.10.0