[v4,06/12] block/nbd: Accept SocketAddress
diff mbox

Message ID 20160928205602.17275-7-mreitz@redhat.com
State New
Headers show

Commit Message

Max Reitz Sept. 28, 2016, 8:55 p.m. UTC
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 <mreitz@redhat.com>
---
 block/nbd.c                   | 166 ++++++++++++++++++++++++++----------------
 tests/qemu-iotests/051.out    |   4 +-
 tests/qemu-iotests/051.pc.out |   4 +-
 3 files changed, 106 insertions(+), 68 deletions(-)

Comments

Kevin Wolf Oct. 13, 2016, 11:42 a.m. UTC | #1
Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> 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 <mreitz@redhat.com>

Not opposed in principle to your change, but we should try to keep the
naming consistent between NBD and the other block drivers, notably the
SSH work that is currently going on.

This patch uses 'address' for the SockAddress, the proposed SSH patch
uses 'server'. I don't mind too much which one we choose, though I like
'server' a bit better. Anyway, we should choose one and stick to it in
all drivers.

>  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))

strstart()?

>          {
>              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;
> +}

If both the legacy option and the new one are given, the legacy one
takes precedence. Intentional?

Kevin
Ashijeet Acharya Oct. 14, 2016, 9:34 a.m. UTC | #2
On Thu, Oct 13, 2016 at 5:12 PM, Kevin Wolf <kwolf@redhat.com> wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> 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 <mreitz@redhat.com>
>
> Not opposed in principle to your change, but we should try to keep the
> naming consistent between NBD and the other block drivers, notably the
> SSH work that is currently going on.
>
> This patch uses 'address' for the SockAddress, the proposed SSH patch
> uses 'server'. I don't mind too much which one we choose, though I like
> 'server' a bit better. Anyway, we should choose one and stick to it in
> all drivers.
>
>>  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))
>
> strstart()?
>
>>          {
>>              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");

I am working on a similar task for the SSH block driver, and in my
'ssh_process_legacy_socket_options' I get the @host and @port fields
still pointing to NULL even after qemu_opt_get(). To clarify things a
bit more I tried to debug the same on your patch using gdb and faced
the same issue. So in your patch, ultimately the control flows
directly to the last statement i.e. 'return true;' and none of the
'if' conditions get satisfied. Reverting the patches and using the
same command-line seems to overcome the issue. Command line I used:

        $ ./bin/qemu-system-x86_64 -cdrom nbd://localhost/precise-5.7.1.iso

I can be wrong so I advise debugging yourself once and correcting me
if I am wrong.

Link to my patch:

        https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg02139.html

Thanks
Ashijeet

> Kevin
Max Reitz Oct. 15, 2016, 5:12 p.m. UTC | #3
On 13.10.2016 13:42, Kevin Wolf wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> 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 <mreitz@redhat.com>
> 
> Not opposed in principle to your change, but we should try to keep the
> naming consistent between NBD and the other block drivers, notably the
> SSH work that is currently going on.
> 
> This patch uses 'address' for the SockAddress, the proposed SSH patch
> uses 'server'. I don't mind too much which one we choose, though I like
> 'server' a bit better. Anyway, we should choose one and stick to it in
> all drivers.

OK, I'll use "server" then. I don't mind either way, so even a weak
opinion is enough to persuade me. ;-)

>>  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))
> 
> strstart()?

Yep, will use.

>>          {
>>              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;
>> +}
> 
> If both the legacy option and the new one are given, the legacy one
> takes precedence. Intentional?

No. I think it'll be easiest to just return an error when a user is
trying to use both.

Thanks,

Max

Patch
diff mbox

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