[Qemu-devel] [PATCH v4 00/12] qapi: Allow blockdev-add for NBD

[Qemu-devel] [PATCH v4 00/12] qapi: Allow blockdev-add for NBD

[Max Reitz]

This series adds blockdev-add support for NBD clients.

Good news in v4: The total diffstat changed from 443+/98- to 407+/106-.
Bad news in v4:  10/12 patches have functional differences from v3.


Patches 1, 2, 3, and 4 are minor patches with no functional relation to
this series, other than the fact that later patches will touch the code
they touch, too.

Patch 5 prepares the code for the addition of a new option prefix, which
is "address.".

Patch 6 makes the NBD client accept a SocketAddress under the "address"
option (or rather, a flattened SocketAddress QDict with its keys
prefixed by "address."). The old options "host", "port", and "path" are
supported as legacy options and translated to the respective
SocketAddress representation.

Patch 7 drops usage of "host", "port", and "path" outside of
nbd_has_filename_options_conflict(),
nbd_process_legacy_socket_options(), and nbd_refresh_filename(), making
those options nothing but legacy.

Patch 8, the goal of this series, is again not very complicated.

Patches 9, 10, and 11 are required for the iotest added in patch 12. It
will invoke qemu-nbd, so patch 9 is required. Besides qemu-nbd, it will
launch an NBD server VM concurrently to the client VM, which is why
patch 10 is required. And finally, it will test whether we can add an
NBD BDS by passing it a file descriptor, which patch 11 is needed for
(so we use the socket_scm_helper to pass sockets to qemu).

Patch 12 then adds the iotest for NBD's blockdev-add interface.


*** This series depends on the following patches: ***
- "qdict: implement a qdict_crumple method for un-flattening a dict"
  from Dan's "QAPI/QOM work for non-scalar object properties" series
- "qemu-nbd: Add --fork option" from my "iotests: Fix test 162" series


v2:
- Dropped patch 1; NBD now uses QemuOpts (as it is supposed to) for the
  legacy options, therefore we cannot just rename keys in the options
  QDict, so the function introduced in the old patch 1
  (qdict_change_key()) becomes obsolete.
- Patch 2 (prev. 3): Rebase conflicts due to the QemuOpts change
- Patch 3 (prev. 4): QemuOpts rebase conflicts
- Patch 4 (prev. 5): QemuOpts rebase conflicts
- Patch 5 (prev. 6):
  - Spell fix in the commit message
  - Break if () condition more consistent with the rest of qemu [Eric]
- Patch 6:
  - Merged previous patches 7 and 8 into a single one; this is because
    of the next point:
  - Use the QMP output visitor to generate the BDS options; this makes
    the resulting code shorter and working even for other socket types
    then 'unix' and 'inet' (i.e. 'fd');
    for this to work, however, we have to merge said two patches because
    the filename refreshing code (previously patch 7) now needs the
    SocketAddress object parsed by the code previously introduced by
    patch 8
    (and the refreshing code needs to be there before the parsing code,
    so we cannot invert the order either)
  - Rebase conflicts because of the changes in patch 5
  - The QemuOpts change means we can no longer just rename the legacy
    options in the options QDict; instead, we have to fetch them using
    the QemuOpts functions and then build a new options QDict from
    scratch.
- Patch 7 (prev. 9):
  - Dropped some changes that become unnecessary now that we use the QMP
    output visitor for nbd_refresh_filename()
- Patch 8 (prev. 10):
  - Simple rebase conflict in the QAPI schema block driver list
  - Changed "Since: 2.6" to "Since: 2.8"
- Patch 9 (prev. 11):
  - Some contextual rebase conflicts
  - iotests.py no longer sets __all__
  - Use --fork instead of a pipe to run it in the background (so we
    don't have to wait an arbitrary timespan after calling the function
    until the server is actually started)
- Patch 10 (prev. 12): Rebase conflicts due to the refactoring of the VM
  class structure
- Patch 12 (prev. 14):
  - Fixed descriptive comment (what the test does)
  - blockdev-add no longer takes an @id parameter, so use @node-name
    instead (and change the rest so it works)
  - Pack os.remove() of socket files into a try-except block


git-backport-diff against v3:

Key:
[----] : patches are identical
[####] : number of functional differences between upstream/downstream patch
[down] : patch is downstream-only
The flags [FC] indicate (F)unctional and (C)ontextual differences, respectively

001/12:[----] [-C] 'block/nbd: Drop trailing "." in error messages'
002/12:[0005] [FC] 'block/nbd: Reject port parameter without host'
003/12:[0031] [FC] 'block/nbd: Default port in nbd_refresh_filename()'
004/12:[0017] [FC] 'block/nbd: Use qdict_put()'
005/12:[0008] [FC] 'block/nbd: Add nbd_has_filename_options_conflict()'
006/12:[0193] [FC] 'block/nbd: Accept SocketAddress'
007/12:[0008] [FC] 'block/nbd: Use SocketAddress options'
008/12:[0010] [FC] 'qapi: Allow blockdev-add for NBD'
009/12:[0007] [FC] 'iotests.py: Add qemu_nbd function'
010/12:[0013] [FC] 'iotests.py: Allow concurrent qemu instances'
011/12:[----] [--] 'socket_scm_helper: Accept fd directly'
012/12:[0035] [FC] 'iotests: Add test for NBD's blockdev-add interface'


Max Reitz (12):
  block/nbd: Drop trailing "." in error messages
  block/nbd: Reject port parameter without host
  block/nbd: Default port in nbd_refresh_filename()
  block/nbd: Use qdict_put()
  block/nbd: Add nbd_has_filename_options_conflict()
  block/nbd: Accept SocketAddress
  block/nbd: Use SocketAddress options
  qapi: Allow blockdev-add for NBD
  iotests.py: Add qemu_nbd function
  iotests.py: Allow concurrent qemu instances
  socket_scm_helper: Accept fd directly
  iotests: Add test for NBD's blockdev-add interface

 block/nbd.c                            | 225 ++++++++++++++++++++-------------
 qapi/block-core.json                   |  25 +++-
 tests/qemu-iotests/051.out             |   4 +-
 tests/qemu-iotests/051.pc.out          |   4 +-
 tests/qemu-iotests/147                 | 201 +++++++++++++++++++++++++++++
 tests/qemu-iotests/147.out             |   5 +
 tests/qemu-iotests/group               |   1 +
 tests/qemu-iotests/iotests.py          |  14 +-
 tests/qemu-iotests/socket_scm_helper.c |  29 +++--
 9 files changed, 402 insertions(+), 106 deletions(-)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

-- 
2.10.0

[Qemu-devel] [PATCH v4 01/12] block/nbd: Drop trailing "." in error messages

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c                   | 4 ++--
 tests/qemu-iotests/051.out    | 4 ++--
 tests/qemu-iotests/051.pc.out | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 6bc06d6..ce7c14f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -200,9 +200,9 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
 
     if (!s->path == !s->host) {
         if (s->path) {
-            error_setg(errp, "path and host may not be used at the same time.");
+            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.");
+            error_setg(errp, "one of path and host must be specified");
         }
         return NULL;
     }
diff --git a/tests/qemu-iotests/051.out b/tests/qemu-iotests/051.out
index 408d613..9e584fe 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: one of path and host must be specified
 
 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: one of path and host must be specified
 
 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 ec6d222..6395a30 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: one of path and host must be specified
 
 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: one of path and host must be specified
 
 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

[Qemu-devel] [PATCH v4 02/12] block/nbd: Reject port parameter without host

[Max Reitz]

Currently, a port that is passed along with a UNIX socket path is
silently ignored. That is not exactly ideal, it should be an error
instead.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index ce7c14f..eaca33c 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -197,6 +197,7 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
 
     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) {
@@ -206,6 +207,10 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
         }
         return NULL;
     }
+    if (s->port && !s->host) {
+        error_setg(errp, "port may not be used without host");
+        return NULL;
+    }
 
     saddr = g_new0(SocketAddress, 1);
 
@@ -217,8 +222,6 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QemuOpts *opts, Error **errp)
     } else {
         InetSocketAddress *inet;
 
-        s->port = g_strdup(qemu_opt_get(opts, "port"));
-
         saddr->type = SOCKET_ADDRESS_KIND_INET;
         inet = saddr->u.inet.data = g_new0(InetSocketAddress, 1);
         inet->host = g_strdup(s->host);
-- 
2.10.0

[Qemu-devel] [PATCH v4 03/12] block/nbd: Default port in nbd_refresh_filename()

[Max Reitz]

Instead of not emitting the port in nbd_refresh_filename(), just set it
to the default if the user did not specify it. This makes the logic a
bit simpler.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index eaca33c..c77a969 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -444,6 +444,7 @@ 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);
 
     qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
 
@@ -453,27 +454,19 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
     } else if (s->path && !s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
                  "nbd+unix://?socket=%s", s->path);
-    } else if (!s->path && s->export && s->port) {
+    } else if (!s->path && s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s/%s", s->host, s->port, s->export);
-    } else if (!s->path && s->export && !s->port) {
+                 "nbd://%s:%s/%s", s->host, port, s->export);
+    } else if (!s->path && !s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s/%s", s->host, s->export);
-    } else if (!s->path && !s->export && s->port) {
-        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s:%s", s->host, s->port);
-    } else if (!s->path && !s->export && !s->port) {
-        snprintf(bs->exact_filename, sizeof(bs->exact_filename),
-                 "nbd://%s", s->host);
+                 "nbd://%s:%s", s->host, port);
     }
 
     if (s->path) {
         qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path)));
-    } else if (s->port) {
-        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host)));
-        qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(s->port)));
     } else {
         qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host)));
+        qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
     }
     if (s->export) {
         qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export)));
-- 
2.10.0

[Qemu-devel] [PATCH v4 04/12] block/nbd: Use qdict_put()

[Max Reitz]

Instead of inlining this nice macro (i.e. resorting to
qdict_put_obj(..., QOBJECT(...))), use it.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index c77a969..c539fb5 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -446,7 +446,7 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
     QDict *opts = qdict_new();
     const char *port = s->port ?: stringify(NBD_DEFAULT_PORT);
 
-    qdict_put_obj(opts, "driver", QOBJECT(qstring_from_str("nbd")));
+    qdict_put(opts, "driver", qstring_from_str("nbd"));
 
     if (s->path && s->export) {
         snprintf(bs->exact_filename, sizeof(bs->exact_filename),
@@ -463,17 +463,16 @@ static void nbd_refresh_filename(BlockDriverState *bs, QDict *options)
     }
 
     if (s->path) {
-        qdict_put_obj(opts, "path", QOBJECT(qstring_from_str(s->path)));
+        qdict_put(opts, "path", qstring_from_str(s->path));
     } else {
-        qdict_put_obj(opts, "host", QOBJECT(qstring_from_str(s->host)));
-        qdict_put_obj(opts, "port", QOBJECT(qstring_from_str(port)));
+        qdict_put(opts, "host", qstring_from_str(s->host));
+        qdict_put(opts, "port", qstring_from_str(port));
     }
     if (s->export) {
-        qdict_put_obj(opts, "export", QOBJECT(qstring_from_str(s->export)));
+        qdict_put(opts, "export", qstring_from_str(s->export));
     }
     if (s->tlscredsid) {
-        qdict_put_obj(opts, "tls-creds",
-                      QOBJECT(qstring_from_str(s->tlscredsid)));
+        qdict_put(opts, "tls-creds", qstring_from_str(s->tlscredsid));
     }
 
     bs->full_open_options = opts;
-- 
2.10.0

[Qemu-devel] [PATCH v4 05/12] block/nbd: Add nbd_has_filename_options_conflict()

[Max Reitz]

Right now, we have four possible options that conflict with specifying
an NBD filename, and a future patch will add another one ("address").
This future option is a nested QDict that is flattened at this point,
requiring us to test each option whether its key has an "address."
prefix. Therefore, we will then need to iterate through all options.

Adding this iteration logic now will simplify adding the new option
later. A nice side effect is that the user will not receive a long list
of five options which are not supposed to be specified with a filename,
but we can actually print the problematic option.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index c539fb5..cdab20f 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -123,6 +123,25 @@ out:
     return ret;
 }
 
+static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
+{
+    const QDictEntry *e;
+
+    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
+        if (!strcmp(e->key, "host") ||
+            !strcmp(e->key, "port") ||
+            !strcmp(e->key, "path") ||
+            !strcmp(e->key, "export"))
+        {
+            error_setg(errp, "Option '%s' cannot be used with a file name",
+                       e->key);
+            return true;
+        }
+    }
+
+    return false;
+}
+
 static void nbd_parse_filename(const char *filename, QDict *options,
                                Error **errp)
 {
@@ -131,12 +150,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
     const char *host_spec;
     const char *unixpath;
 
-    if (qdict_haskey(options, "host")
-        || qdict_haskey(options, "port")
-        || qdict_haskey(options, "path"))
-    {
-        error_setg(errp, "host/port/path and a file name may not be specified "
-                         "at the same time");
+    if (nbd_has_filename_options_conflict(options, errp)) {
         return;
     }
 
-- 
2.10.0

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

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

[Qemu-devel] [PATCH v4 07/12] block/nbd: Use SocketAddress options

[Max Reitz]

Drop the use of legacy options in favor of the SocketAddress
representation, even for internal use (i.e. for storing the result of
the filename parsing).

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 block/nbd.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/block/nbd.c b/block/nbd.c
index 449f94e..932249b 100644
--- a/block/nbd.c
+++ b/block/nbd.c
@@ -94,9 +94,13 @@ static int nbd_parse_uri(const char *filename, QDict *options)
             ret = -EINVAL;
             goto out;
         }
-        qdict_put(options, "path", qstring_from_str(qp->p[0].value));
+        qdict_put(options, "address.type", qstring_from_str("unix"));
+        qdict_put(options, "address.data.path",
+                  qstring_from_str(qp->p[0].value));
     } else {
         QString *host;
+        char *port_str;
+
         /* nbd[+tcp]://host[:port]/export */
         if (!uri->server) {
             ret = -EINVAL;
@@ -111,12 +115,12 @@ static int nbd_parse_uri(const char *filename, QDict *options)
             host = qstring_from_str(uri->server);
         }
 
-        qdict_put(options, "host", host);
-        if (uri->port) {
-            char* port_str = g_strdup_printf("%d", uri->port);
-            qdict_put(options, "port", qstring_from_str(port_str));
-            g_free(port_str);
-        }
+        qdict_put(options, "address.type", qstring_from_str("inet"));
+        qdict_put(options, "address.data.host", host);
+
+        port_str = g_strdup_printf("%d", uri->port ?: NBD_DEFAULT_PORT);
+        qdict_put(options, "address.data.port", qstring_from_str(port_str));
+        g_free(port_str);
     }
 
 out:
@@ -193,7 +197,8 @@ static void nbd_parse_filename(const char *filename, QDict *options,
 
     /* are we a UNIX or TCP socket? */
     if (strstart(host_spec, "unix:", &unixpath)) {
-        qdict_put(options, "path", qstring_from_str(unixpath));
+        qdict_put(options, "address.type", qstring_from_str("unix"));
+        qdict_put(options, "address.data.path", qstring_from_str(unixpath));
     } else {
         InetSocketAddress *addr = NULL;
 
@@ -202,8 +207,9 @@ static void nbd_parse_filename(const char *filename, QDict *options,
             goto out;
         }
 
-        qdict_put(options, "host", qstring_from_str(addr->host));
-        qdict_put(options, "port", qstring_from_str(addr->port));
+        qdict_put(options, "address.type", qstring_from_str("inet"));
+        qdict_put(options, "address.data.host", qstring_from_str(addr->host));
+        qdict_put(options, "address.data.port", qstring_from_str(addr->port));
         qapi_free_InetSocketAddress(addr);
     }
 
-- 
2.10.0

[Qemu-devel] [PATCH v4 08/12] qapi: Allow blockdev-add for NBD

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 qapi/block-core.json | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)

diff --git a/qapi/block-core.json b/qapi/block-core.json
index 9d797b8..54e3a6a 100644
--- a/qapi/block-core.json
+++ b/qapi/block-core.json
@@ -1708,14 +1708,15 @@
 #
 # @host_device, @host_cdrom: Since 2.1
 # @gluster: Since 2.7
+# @nbd: Since 2.8
 #
 # Since: 2.0
 ##
 { 'enum': 'BlockdevDriver',
   'data': [ 'archipelago', 'blkdebug', 'blkverify', 'bochs', 'cloop',
             'dmg', 'file', 'ftp', 'ftps', 'gluster', 'host_cdrom',
-            'host_device', 'http', 'https', 'luks', 'null-aio', 'null-co',
-            'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
+            'host_device', 'http', 'https', 'luks', 'nbd', 'null-aio',
+            'null-co', 'parallels', 'qcow', 'qcow2', 'qed', 'quorum', 'raw',
 	    'replication', 'tftp', 'vdi', 'vhdx', 'vmdk', 'vpc', 'vvfat' ] }
 
 ##
@@ -2224,6 +2225,24 @@
   'data': { 'filename': 'str' } }
 
 ##
+# @BlockdevOptionsNbd
+#
+# Driver specific block device options for NBD.
+#
+# @address:     NBD server address
+#
+# @export:      #optional export name
+#
+# @tls-creds:   #optional TLS credentials ID
+#
+# Since: 2.8
+##
+{ 'struct': 'BlockdevOptionsNbd',
+  'data': { 'address': 'SocketAddress',
+            '*export': 'str',
+            '*tls-creds': 'str' } }
+
+##
 # @BlockdevOptions
 #
 # Options for creating a block device.  Many options are available for all
@@ -2268,7 +2287,7 @@
       'https':      'BlockdevOptionsCurl',
 # TODO iscsi: Wait for structured options
       'luks':       'BlockdevOptionsLUKS',
-# TODO nbd: Should take InetSocketAddress for 'host'?
+      'nbd':        'BlockdevOptionsNbd',
 # TODO nfs: Wait for structured options
       'null-aio':   'BlockdevOptionsNull',
       'null-co':    'BlockdevOptionsNull',
-- 
2.10.0

[Qemu-devel] [PATCH v4 09/12] iotests.py: Add qemu_nbd function

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 3329bc1..5a2678f 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
 if os.environ.get('QEMU_IO_OPTIONS'):
     qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
 
+qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
+if os.environ.get('QEMU_NBD_OPTIONS'):
+    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
+
 qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
 qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
 
@@ -87,6 +91,10 @@ def qemu_io(*args):
         sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
     return subp.communicate()[0]
 
+def qemu_nbd(*args):
+    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
+    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
+
 def compare_images(img1, img2, fmt1=imgfmt, fmt2=imgfmt):
     '''Return True if two image files are identical'''
     return qemu_img('compare', '-f', fmt1,
-- 
2.10.0

[Qemu-devel] [PATCH v4 10/12] iotests.py: Allow concurrent qemu instances

[Max Reitz]

By adding an optional suffix to the files used for communication with a
VM, we can launch multiple VM instances concurrently.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/iotests.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
index 5a2678f..c589deb 100644
--- a/tests/qemu-iotests/iotests.py
+++ b/tests/qemu-iotests/iotests.py
@@ -140,8 +140,10 @@ def log(msg, filters=[]):
 class VM(qtest.QEMUQtestMachine):
     '''A QEMU VM'''
 
-    def __init__(self):
-        super(VM, self).__init__(qemu_prog, qemu_opts, test_dir=test_dir,
+    def __init__(self, path_suffix=''):
+        name = "qemu%s-%d" % (path_suffix, os.getpid())
+        super(VM, self).__init__(qemu_prog, qemu_opts, name=name,
+                                 test_dir=test_dir,
                                  socket_scm_helper=socket_scm_helper)
         if debug:
             self._debug = True
-- 
2.10.0

[Qemu-devel] [PATCH v4 11/12] socket_scm_helper: Accept fd directly

[Max Reitz]

This gives us more freedom about the fd that is passed to qemu, allowing
us to e.g. pass sockets.

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/socket_scm_helper.c | 29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/tests/qemu-iotests/socket_scm_helper.c b/tests/qemu-iotests/socket_scm_helper.c
index 80cadf4..eb76d31 100644
--- a/tests/qemu-iotests/socket_scm_helper.c
+++ b/tests/qemu-iotests/socket_scm_helper.c
@@ -60,7 +60,7 @@ static int send_fd(int fd, int fd_to_send)
 }
 
 /* Convert string to fd number. */
-static int get_fd_num(const char *fd_str)
+static int get_fd_num(const char *fd_str, bool silent)
 {
     int sock;
     char *err;
@@ -68,12 +68,16 @@ static int get_fd_num(const char *fd_str)
     errno = 0;
     sock = strtol(fd_str, &err, 10);
     if (errno) {
-        fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
-                strerror(errno));
+        if (!silent) {
+            fprintf(stderr, "Failed in strtol for socket fd, reason: %s\n",
+                    strerror(errno));
+        }
         return -1;
     }
     if (!*fd_str || *err || sock < 0) {
-        fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+        if (!silent) {
+            fprintf(stderr, "bad numerical value for socket fd '%s'\n", fd_str);
+        }
         return -1;
     }
 
@@ -104,18 +108,21 @@ int main(int argc, char **argv, char **envp)
     }
 
 
-    sock = get_fd_num(argv[1]);
+    sock = get_fd_num(argv[1], false);
     if (sock < 0) {
         return EXIT_FAILURE;
     }
 
-    /* Now only open a file in readonly mode for test purpose. If more precise
-       control is needed, use python script in file operation, which is
-       supposed to fork and exec this program. */
-    fd = open(argv[2], O_RDONLY);
+    fd = get_fd_num(argv[2], true);
     if (fd < 0) {
-        fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
-        return EXIT_FAILURE;
+        /* Now only open a file in readonly mode for test purpose. If more
+           precise control is needed, use python script in file operation, which
+           is supposed to fork and exec this program. */
+        fd = open(argv[2], O_RDONLY);
+        if (fd < 0) {
+            fprintf(stderr, "Failed to open file '%s'\n", argv[2]);
+            return EXIT_FAILURE;
+        }
     }
 
     ret = send_fd(sock, fd);
-- 
2.10.0

[Qemu-devel] [PATCH v4 12/12] iotests: Add test for NBD's blockdev-add interface

[Max Reitz]

Signed-off-by: Max Reitz <mreitz@redhat.com>
---
 tests/qemu-iotests/147     | 201 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/147.out |   5 ++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 207 insertions(+)
 create mode 100755 tests/qemu-iotests/147
 create mode 100644 tests/qemu-iotests/147.out

diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
new file mode 100755
index 0000000..61e1e6f
--- /dev/null
+++ b/tests/qemu-iotests/147
@@ -0,0 +1,201 @@
+#!/usr/bin/env python
+#
+# Test case for NBD's blockdev-add interface
+#
+# Copyright (C) 2016 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+import os
+import socket
+import stat
+import time
+import iotests
+from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
+
+NBD_PORT = 10811
+
+test_img = os.path.join(iotests.test_dir, 'test.img')
+
+class NBDBlockdevAddBase(iotests.QMPTestCase):
+    def blockdev_add_options(self, address, export=None):
+        options = { 'node-name': 'nbd-blockdev',
+                    'driver': 'raw',
+                    'file': {
+                        'driver': 'nbd',
+                        'address': address
+                    } }
+        if export is not None:
+            options['file']['export'] = export
+        return options
+
+    def client_test(self, filename, address, export=None):
+        bao = self.blockdev_add_options(address, export)
+        result = self.vm.qmp('blockdev-add', options=bao)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.vm.qmp('query-named-block-nodes')
+        for node in result['return']:
+            if node['node-name'] == 'nbd-blockdev':
+                self.assert_qmp(node, 'image/filename', filename)
+                break
+
+        result = self.vm.qmp('x-blockdev-del', node_name='nbd-blockdev')
+        self.assert_qmp(result, 'return', {})
+
+
+class QemuNBD(NBDBlockdevAddBase):
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+        self.vm = iotests.VM()
+        self.vm.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        os.remove(test_img)
+
+    def _server_up(self, *args):
+        self.assertEqual(qemu_nbd('-f', imgfmt, test_img, *args), 0)
+
+    def test_inet(self):
+        self._server_up('-p', str(NBD_PORT))
+        address = { 'type': 'inet',
+                    'data': {
+                        'host': 'localhost',
+                        'port': str(NBD_PORT)
+                    } }
+        self.client_test('nbd://localhost:%i' % NBD_PORT, address)
+
+    def test_unix(self):
+        socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
+        self._server_up('-k', socket)
+        address = { 'type': 'unix',
+                    'data': { 'path': socket } }
+        self.client_test('nbd+unix://?socket=' + socket, address)
+        try:
+            os.remove(socket)
+        except OSError:
+            pass
+
+
+class BuiltinNBD(NBDBlockdevAddBase):
+    def setUp(self):
+        qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
+        self.vm = iotests.VM()
+        self.vm.launch()
+        self.server = iotests.VM('.server')
+        self.server.add_drive_raw('if=none,id=nbd-export,' +
+                                  'file=%s,' % test_img +
+                                  'format=%s,' % imgfmt +
+                                  'cache=%s' % cachemode)
+        self.server.launch()
+
+    def tearDown(self):
+        self.vm.shutdown()
+        self.server.shutdown()
+        os.remove(test_img)
+
+    def _server_up(self, address):
+        result = self.server.qmp('nbd-server-start', addr=address)
+        self.assert_qmp(result, 'return', {})
+
+        result = self.server.qmp('nbd-server-add', device='nbd-export')
+        self.assert_qmp(result, 'return', {})
+
+    def _server_down(self):
+        result = self.server.qmp('nbd-server-stop')
+        self.assert_qmp(result, 'return', {})
+
+    def test_inet(self):
+        address = { 'type': 'inet',
+                    'data': {
+                        'host': 'localhost',
+                        'port': str(NBD_PORT)
+                    } }
+        self._server_up(address)
+        self.client_test('nbd://localhost:%i/nbd-export' % NBD_PORT,
+                         address, 'nbd-export')
+        self._server_down()
+
+    def test_inet6(self):
+        address = { 'type': 'inet',
+                    'data': {
+                        'host': '::1',
+                        'port': str(NBD_PORT),
+                        'ipv4': False,
+                        'ipv6': True
+                    } }
+        filename = 'json:{"driver": "raw", ' + \
+                         '"file": {"driver": "nbd", ' + \
+                                  '"address.data.host": "::1", ' + \
+                                  '"address.data.ipv4": false, ' + \
+                                  '"address.data.ipv6": true, ' + \
+                                  '"address.type": "inet", ' + \
+                                  '"export": "nbd-export", ' + \
+                                  '"address.data.port": "%i"}}' % NBD_PORT
+        self._server_up(address)
+        self.client_test(filename, address, 'nbd-export')
+        self._server_down()
+
+    def test_unix(self):
+        socket = os.path.join(iotests.test_dir, 'nbd.socket')
+        address = { 'type': 'unix',
+                    'data': { 'path': socket } }
+        self._server_up(address)
+        self.client_test('nbd+unix:///nbd-export?socket=' + socket,
+                         address, 'nbd-export')
+        self._server_down()
+        try:
+            os.remove(socket)
+        except OSError:
+            pass
+
+    def test_fd(self):
+        sockfile = os.path.join(iotests.test_dir, 'nbd.socket')
+        self._server_up({ 'type': 'unix',
+                          'data': { 'path': sockfile } })
+
+        sockfd = socket.socket(socket.AF_UNIX, socket.SOCK_STREAM)
+        sockfd.connect(sockfile)
+
+        result = self.vm.send_fd_scm(str(sockfd.fileno()))
+        self.assertEqual(result, 0, 'Failed to send socket FD')
+
+        result = self.vm.qmp('getfd', fdname='nbd-fifo')
+        self.assert_qmp(result, 'return', {})
+
+        filename = 'json:{"driver": "raw", ' + \
+                         '"file": {"driver": "nbd", ' + \
+                         '"address.type": "fd", ' + \
+                         '"export": "nbd-export", ' + \
+                         '"address.data.str": "nbd-fifo"}}'
+        self.client_test(filename,
+                         { 'type': 'fd',
+                           'data': { 'str': 'nbd-fifo' } },
+                         'nbd-export')
+
+        self._server_down()
+
+        try:
+            os.remove(sockfile)
+        except OSError:
+            pass
+
+
+if __name__ == '__main__':
+    # Need to support image creation
+    iotests.main(supported_fmts=['vpc', 'parallels', 'qcow', 'vdi', 'qcow2',
+                                 'vmdk', 'raw', 'vhdx', 'qed'])
+
diff --git a/tests/qemu-iotests/147.out b/tests/qemu-iotests/147.out
new file mode 100644
index 0000000..3f8a935
--- /dev/null
+++ b/tests/qemu-iotests/147.out
@@ -0,0 +1,5 @@
+......
+----------------------------------------------------------------------
+Ran 6 tests
+
+OK
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 7eb1770..d7d50cf 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -149,6 +149,7 @@
 144 rw auto quick
 145 auto quick
 146 auto quick
+147 auto
 148 rw auto quick
 149 rw auto sudo
 150 rw auto quick
-- 
2.10.0

Re: [Qemu-devel] [PATCH v4 01/12] block/nbd: Drop trailing "." in error messages

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 441 bytes --]

On 09/28/2016 03:55 PM, Max Reitz wrote:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/nbd.c                   | 4 ++--
>  tests/qemu-iotests/051.out    | 4 ++--
>  tests/qemu-iotests/051.pc.out | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 02/12] block/nbd: Reject port parameter without host

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 496 bytes --]

On 09/28/2016 03:55 PM, Max Reitz wrote:
> Currently, a port that is passed along with a UNIX socket path is
> silently ignored. That is not exactly ideal, it should be an error
> instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/nbd.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>


-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 03/12] block/nbd: Default port in nbd_refresh_filename()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 521 bytes --]

On 09/28/2016 03:55 PM, Max Reitz wrote:
> Instead of not emitting the port in nbd_refresh_filename(), just set it
> to the default if the user did not specify it. This makes the logic a
> bit simpler.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/nbd.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 04/12] block/nbd: Use qdict_put()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On 09/28/2016 03:55 PM, Max Reitz wrote:
> Instead of inlining this nice macro (i.e. resorting to
> qdict_put_obj(..., QOBJECT(...))), use it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/nbd.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

Hmm - should we have a Coccinelle script to automate this?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 05/12] block/nbd: Add nbd_has_filename_options_conflict()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 3467 bytes --]

On 09/28/2016 03:55 PM, Max Reitz wrote:
> Right now, we have four possible options that conflict with specifying
> an NBD filename, and a future patch will add another one ("address").
> This future option is a nested QDict that is flattened at this point,
> requiring us to test each option whether its key has an "address."
> prefix. Therefore, we will then need to iterate through all options.

How does the plans to add 'address.' interact with Dan's patches for
auto-nesting when parsing command line options?

https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg08238.html

I'm wondering if your patches can be made a bit smaller by basing on top
of that work (allowing the command-line user to omit 'address.' and the
parser auto-nests as a result, while the QMP form is always nested).

> 
> Adding this iteration logic now will simplify adding the new option
> later. A nice side effect is that the user will not receive a long list
> of five options which are not supposed to be specified with a filename,
> but we can actually print the problematic option.

On its own, this patch looks reasonable, but I'm a bit worried that with
all the other parsing changes going in, it may result in unused code.

> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  block/nbd.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/block/nbd.c b/block/nbd.c
> index c539fb5..cdab20f 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -123,6 +123,25 @@ out:
>      return ret;
>  }
>  
> +static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
> +{
> +    const QDictEntry *e;
> +
> +    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
> +        if (!strcmp(e->key, "host") ||
> +            !strcmp(e->key, "port") ||
> +            !strcmp(e->key, "path") ||
> +            !strcmp(e->key, "export"))
> +        {
> +            error_setg(errp, "Option '%s' cannot be used with a file name",
> +                       e->key);
> +            return true;
> +        }
> +    }

Or put another way, while manual parsing looks fine, it's even better if
we can avoid manual parsing (and having to remember to update it when
the schema changes) by letting the schema itself automatically reject
invalid option pairs, even if the automatic rejection doesn't give quite
as nice of an error message.

> +
> +    return false;
> +}
> +
>  static void nbd_parse_filename(const char *filename, QDict *options,
>                                 Error **errp)
>  {
> @@ -131,12 +150,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
>      const char *host_spec;
>      const char *unixpath;
>  
> -    if (qdict_haskey(options, "host")
> -        || qdict_haskey(options, "port")
> -        || qdict_haskey(options, "path"))
> -    {

Also, this patch looks like you are doing a bug fix mixed with code
motion - the old code manually checked for duplicates on only three
options, but the new code adds 'export' to the mix.

> -        error_setg(errp, "host/port/path and a file name may not be specified "
> -                         "at the same time");
> +    if (nbd_has_filename_options_conflict(options, errp)) {
>          return;
>      }
>  
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH v4 05/12] block/nbd: Add nbd_has_filename_options_conflict()

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 5772 bytes --]

On 03.10.2016 20:46, Eric Blake wrote:
> On 09/28/2016 03:55 PM, Max Reitz wrote:
>> Right now, we have four possible options that conflict with specifying
>> an NBD filename, and a future patch will add another one ("address").
>> This future option is a nested QDict that is flattened at this point,
>> requiring us to test each option whether its key has an "address."
>> prefix. Therefore, we will then need to iterate through all options.
> 
> How does the plans to add 'address.' interact with Dan's patches for
> auto-nesting when parsing command line options?
> 
> https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg08238.html
> 
> I'm wondering if your patches can be made a bit smaller by basing on top
> of that work (allowing the command-line user to omit 'address.' and the
> parser auto-nests as a result, while the QMP form is always nested).

It may work, but I don't think it's going to make the patches much simpler.

First, there is no "type" option in the legacy syntax, so I'll need some
way to infer the type anyway and put the correct @type argument back
into the options QDict so that the auto-nesting would work. Therefore,
nbd_process_legacy_options() would still exist, it would just lose three
qdict_put() calls (if I can lose them at all, because with the current
state of the code I'd still have to copy host, port and path from the
QemuOpts object).

Second, I'd have write more code to remove all the options that we used
from the options QDict so that the block layer does not complain about
unused options. Currently this is done by qemu_opts_absorb_qdict() for
legacy options and qdict_extract_subqdict() for @address.

Therefore, I think writing code that makes use of auto-nesting will
actually increase the number of lines.

In any case, I'll have to rebase anyway, if nothing else then for the
renaming of the input/output visitors.

>> Adding this iteration logic now will simplify adding the new option
>> later. A nice side effect is that the user will not receive a long list
>> of five options which are not supposed to be specified with a filename,
>> but we can actually print the problematic option.
> 
> On its own, this patch looks reasonable, but I'm a bit worried that with
> all the other parsing changes going in, it may result in unused code.

I'm not sure how this code would be unused. One could argue that it's
not necessary because we have the same logic elsewhere, but I think that
using that other logic will be more complicated than just translating
the legacy syntax into a SocketAddress manually.

>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  block/nbd.c | 26 ++++++++++++++++++++------
>>  1 file changed, 20 insertions(+), 6 deletions(-)
>>
>> diff --git a/block/nbd.c b/block/nbd.c
>> index c539fb5..cdab20f 100644
>> --- a/block/nbd.c
>> +++ b/block/nbd.c
>> @@ -123,6 +123,25 @@ out:
>>      return ret;
>>  }
>>  
>> +static bool nbd_has_filename_options_conflict(QDict *options, Error **errp)
>> +{
>> +    const QDictEntry *e;
>> +
>> +    for (e = qdict_first(options); e; e = qdict_next(options, e)) {
>> +        if (!strcmp(e->key, "host") ||
>> +            !strcmp(e->key, "port") ||
>> +            !strcmp(e->key, "path") ||
>> +            !strcmp(e->key, "export"))
>> +        {
>> +            error_setg(errp, "Option '%s' cannot be used with a file name",
>> +                       e->key);
>> +            return true;
>> +        }
>> +    }
> 
> Or put another way, while manual parsing looks fine, it's even better if
> we can avoid manual parsing (and having to remember to update it when
> the schema changes) by letting the schema itself automatically reject
> invalid option pairs, even if the automatic rejection doesn't give quite
> as nice of an error message.

How would the schema reject invalid option pairs? This function does not
reject invalid options pairs, it just rejects options that may not be
given when specifying the target image through a filename. I don't think
we can express that in the schema, because the schema doesn't know about
this kind of filenames (i.e. rich filenames that are translated into
options by the block driver before usage).

Other than that, there are really invalid option pairs like @host and
@path, you cannot specify both at the same time. But auto-nesting won't
solve this, as it expects the @type argument to distinguish -- which I'd
have to generate manually (at which point I would have to detect the
clash of @host and @path manually anyway).

>> +
>> +    return false;
>> +}
>> +
>>  static void nbd_parse_filename(const char *filename, QDict *options,
>>                                 Error **errp)
>>  {
>> @@ -131,12 +150,7 @@ static void nbd_parse_filename(const char *filename, QDict *options,
>>      const char *host_spec;
>>      const char *unixpath;
>>  
>> -    if (qdict_haskey(options, "host")
>> -        || qdict_haskey(options, "port")
>> -        || qdict_haskey(options, "path"))
>> -    {
> 
> Also, this patch looks like you are doing a bug fix mixed with code
> motion - the old code manually checked for duplicates on only three
> options, but the new code adds 'export' to the mix.

Well, it's not just code motion anyway. As I said in the commit message,
I'm employing a different logic anyway. I can move the fix into an own
patch, though, or add a note to the commit message, if you prefer.

Max

> 
>> -        error_setg(errp, "host/port/path and a file name may not be specified "
>> -                         "at the same time");
>> +    if (nbd_has_filename_options_conflict(options, errp)) {
>>          return;
>>      }
>>  
>>
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 01/12] block/nbd: Drop trailing "." in error messages

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 02/12] block/nbd: Reject port parameter without host

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Currently, a port that is passed along with a UNIX socket path is
> silently ignored. That is not exactly ideal, it should be an error
> instead.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 03/12] block/nbd: Default port in nbd_refresh_filename()

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Instead of not emitting the port in nbd_refresh_filename(), just set it
> to the default if the user did not specify it. This makes the logic a
> bit simpler.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 04/12] block/nbd: Use qdict_put()

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Instead of inlining this nice macro (i.e. resorting to
> qdict_put_obj(..., QOBJECT(...))), use it.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 05/12] block/nbd: Add nbd_has_filename_options_conflict()

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Right now, we have four possible options that conflict with specifying
> an NBD filename, and a future patch will add another one ("address").
> This future option is a nested QDict that is flattened at this point,
> requiring us to test each option whether its key has an "address."
> prefix. Therefore, we will then need to iterate through all options.
> 
> Adding this iteration logic now will simplify adding the new option
> later. A nice side effect is that the user will not receive a long list
> of five options which are not supposed to be specified with a filename,
> but we can actually print the problematic option.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

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

[Kevin Wolf]

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

Re: [Qemu-devel] [PATCH v4 07/12] block/nbd: Use SocketAddress options

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Drop the use of legacy options in favor of the SocketAddress
> representation, even for internal use (i.e. for storing the result of
> the filename parsing).
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 08/12] qapi: Allow blockdev-add for NBD

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 09/12] iotests.py: Add qemu_nbd function

[Kevin Wolf]

Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/iotests.py | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> index 3329bc1..5a2678f 100644
> --- a/tests/qemu-iotests/iotests.py
> +++ b/tests/qemu-iotests/iotests.py
> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>  if os.environ.get('QEMU_IO_OPTIONS'):
>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>  
> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
> +if os.environ.get('QEMU_NBD_OPTIONS'):
> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
> +
>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
>  
> @@ -87,6 +91,10 @@ def qemu_io(*args):
>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
>      return subp.communicate()[0]
>  
> +def qemu_nbd(*args):
> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))

Wouldn't it be better to always use -t, track the PID and shut it down
explicitly when the test exits?

The way you're using qemu-nbd here is fine if the test case passes, but
if it fails before we access the NBD server, the server keeps running in
the background.

Kevin

Re: [Qemu-devel] [PATCH v4 10/12] iotests.py: Allow concurrent qemu instances

[Kevin Wolf]

Am 28.09.2016 um 22:56 hat Max Reitz geschrieben:
> By adding an optional suffix to the files used for communication with a
> VM, we can launch multiple VM instances concurrently.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 11/12] socket_scm_helper: Accept fd directly

[Kevin Wolf]

Am 28.09.2016 um 22:56 hat Max Reitz geschrieben:
> This gives us more freedom about the fd that is passed to qemu, allowing
> us to e.g. pass sockets.
> 
> Signed-off-by: Max Reitz <mreitz@redhat.com>

Reviewed-by: Kevin Wolf <kwolf@redhat.com>

Re: [Qemu-devel] [PATCH v4 12/12] iotests: Add test for NBD's blockdev-add interface

[Kevin Wolf]

Am 28.09.2016 um 22:56 hat Max Reitz geschrieben:
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
>  tests/qemu-iotests/147     | 201 +++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/147.out |   5 ++
>  tests/qemu-iotests/group   |   1 +
>  3 files changed, 207 insertions(+)
>  create mode 100755 tests/qemu-iotests/147
>  create mode 100644 tests/qemu-iotests/147.out
> 
> diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
> new file mode 100755
> index 0000000..61e1e6f
> --- /dev/null
> +++ b/tests/qemu-iotests/147
> @@ -0,0 +1,201 @@
> +#!/usr/bin/env python
> +#
> +# Test case for NBD's blockdev-add interface
> +#
> +# Copyright (C) 2016 Red Hat, Inc.
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
> +#
> +
> +import os
> +import socket
> +import stat
> +import time
> +import iotests
> +from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
> +
> +NBD_PORT = 10811
> +
> +test_img = os.path.join(iotests.test_dir, 'test.img')
> +
> +class NBDBlockdevAddBase(iotests.QMPTestCase):
> +    def blockdev_add_options(self, address, export=None):
> +        options = { 'node-name': 'nbd-blockdev',
> +                    'driver': 'raw',
> +                    'file': {
> +                        'driver': 'nbd',
> +                        'address': address
> +                    } }
> +        if export is not None:
> +            options['file']['export'] = export
> +        return options
> +
> +    def client_test(self, filename, address, export=None):
> +        bao = self.blockdev_add_options(address, export)
> +        result = self.vm.qmp('blockdev-add', options=bao)

This needs a rebase (**bao instead of options=bao).

> +        self.assert_qmp(result, 'return', {})
> +
> +        result = self.vm.qmp('query-named-block-nodes')
> +        for node in result['return']:
> +            if node['node-name'] == 'nbd-blockdev':
> +                self.assert_qmp(node, 'image/filename', filename)
> +                break
> +
> +        result = self.vm.qmp('x-blockdev-del', node_name='nbd-blockdev')
> +        self.assert_qmp(result, 'return', {})
> +
> +
> +class QemuNBD(NBDBlockdevAddBase):
> +    def setUp(self):
> +        qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
> +        self.vm = iotests.VM()
> +        self.vm.launch()
> +
> +    def tearDown(self):
> +        self.vm.shutdown()
> +        os.remove(test_img)
> +
> +    def _server_up(self, *args):
> +        self.assertEqual(qemu_nbd('-f', imgfmt, test_img, *args), 0)
> +
> +    def test_inet(self):
> +        self._server_up('-p', str(NBD_PORT))
> +        address = { 'type': 'inet',
> +                    'data': {
> +                        'host': 'localhost',
> +                        'port': str(NBD_PORT)
> +                    } }
> +        self.client_test('nbd://localhost:%i' % NBD_PORT, address)
> +
> +    def test_unix(self):
> +        socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
> +        self._server_up('-k', socket)
> +        address = { 'type': 'unix',
> +                    'data': { 'path': socket } }
> +        self.client_test('nbd+unix://?socket=' + socket, address)
> +        try:
> +            os.remove(socket)
> +        except OSError:
> +            pass

If the test case fails, the socket file is leaked. We should probably
either create and remove it in setUp()/tearDown(), or use a try/finally
block around the test_unix code.

Kevin

Re: [Qemu-devel] [PATCH v4 00/12] qapi: Allow blockdev-add for NBD

[no-reply]

Hi,

Your series failed automatic build test. Please find the testing commands and
their output below. If you have docker installed, you can probably reproduce it
locally.

Subject: [Qemu-devel] [PATCH v4 00/12] qapi: Allow blockdev-add for NBD
Type: series
Message-id: 20160928205602.17275-1-mreitz@redhat.com

=== TEST SCRIPT BEGIN ===
#!/bin/bash
set -e
git submodule update --init dtc
# Let docker tests dump environment info
export SHOW_ENV=1
export J=16
make docker-test-quick@centos6
make docker-test-mingw@fedora
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
Switched to a new branch 'test'
db8b8fd iotests: Add test for NBD's blockdev-add interface
bc67989 socket_scm_helper: Accept fd directly
f2a2af7 iotests.py: Allow concurrent qemu instances
e86dbb6 iotests.py: Add qemu_nbd function
b5f7f81 qapi: Allow blockdev-add for NBD
1dd5e1d block/nbd: Use SocketAddress options
bbfdd26 block/nbd: Accept SocketAddress
0c908ca block/nbd: Add nbd_has_filename_options_conflict()
a534bca block/nbd: Use qdict_put()
134116f block/nbd: Default port in nbd_refresh_filename()
dfba813 block/nbd: Reject port parameter without host
28956bb block/nbd: Drop trailing "." in error messages

=== OUTPUT BEGIN ===
Submodule 'dtc' (git://git.qemu-project.org/dtc.git) registered for path 'dtc'
Cloning into 'dtc'...
Submodule path 'dtc': checked out '65cc4d2748a2c2e6f27f1cf39e07a5dbabd80ebf'
  BUILD   centos6
  ARCHIVE qemu.tgz
  ARCHIVE dtc.tgz
  COPY    RUNNER
  RUN     test-quick in centos6
Packages installed:
SDL-devel-1.2.14-7.el6_7.1.x86_64
ccache-3.1.6-2.el6.x86_64
epel-release-6-8.noarch
gcc-4.4.7-17.el6.x86_64
git-1.7.1-4.el6_7.1.x86_64
glib2-devel-2.28.8-5.el6.x86_64
libfdt-devel-1.4.0-1.el6.x86_64
make-3.81-23.el6.x86_64
package g++ is not installed
pixman-devel-0.32.8-1.el6.x86_64
tar-1.23-15.el6_8.x86_64
zlib-devel-1.2.3-29.el6.x86_64

Environment variables:
PACKAGES=libfdt-devel ccache     tar git make gcc g++     zlib-devel glib2-devel SDL-devel pixman-devel     epel-release
HOSTNAME=c2cf8161a74c
TERM=xterm
MAKEFLAGS= -j16
HISTSIZE=1000
J=16
USER=root
CCACHE_DIR=/var/tmp/ccache
EXTRA_CONFIGURE_OPTS=
V=
SHOW_ENV=1
MAIL=/var/spool/mail/root
PATH=/usr/lib/ccache:/usr/lib64/ccache:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
PWD=/
LANG=en_US.UTF-8
TARGET_LIST=
HISTCONTROL=ignoredups
SHLVL=1
HOME=/root
TEST_DIR=/tmp/qemu-test
LOGNAME=root
LESSOPEN=||/usr/bin/lesspipe.sh %s
FEATURES= dtc
DEBUG=
G_BROKEN_FILENAMES=1
CCACHE_HASHDIR=
_=/usr/bin/env

Configure options:
--enable-werror --target-list=x86_64-softmmu,aarch64-softmmu --prefix=/var/tmp/qemu-build/install
No C++ compiler available; disabling C++ specific optional code
Install prefix    /var/tmp/qemu-build/install
BIOS directory    /var/tmp/qemu-build/install/share/qemu
binary directory  /var/tmp/qemu-build/install/bin
library directory /var/tmp/qemu-build/install/lib
module directory  /var/tmp/qemu-build/install/lib/qemu
libexec directory /var/tmp/qemu-build/install/libexec
include directory /var/tmp/qemu-build/install/include
config directory  /var/tmp/qemu-build/install/etc
local state directory   /var/tmp/qemu-build/install/var
Manual directory  /var/tmp/qemu-build/install/share/man
ELF interp prefix /usr/gnemul/qemu-%M
Source path       /tmp/qemu-test/src
C compiler        cc
Host C compiler   cc
C++ compiler      
Objective-C compiler cc
ARFLAGS           rv
CFLAGS            -O2 -U_FORTIFY_SOURCE -D_FORTIFY_SOURCE=2 -g 
QEMU_CFLAGS       -I/usr/include/pixman-1    -pthread -I/usr/include/glib-2.0 -I/usr/lib64/glib-2.0/include   -fPIE -DPIE -m64 -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes -Wredundant-decls -Wall -Wundef -Wwrite-strings -Wmissing-prototypes -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels -Wmissing-include-dirs -Wempty-body -Wnested-externs -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers -Wold-style-declaration -Wold-style-definition -Wtype-limits -fstack-protector-all
LDFLAGS           -Wl,--warn-common -Wl,-z,relro -Wl,-z,now -pie -m64 -g 
make              make
install           install
python            python -B
smbd              /usr/sbin/smbd
module support    no
host CPU          x86_64
host big endian   no
target list       x86_64-softmmu aarch64-softmmu
tcg debug enabled no
gprof enabled     no
sparse enabled    no
strip binaries    yes
profiler          no
static build      no
pixman            system
SDL support       yes (1.2.14)
GTK support       no 
GTK GL support    no
VTE support       no 
TLS priority      NORMAL
GNUTLS support    no
GNUTLS rnd        no
libgcrypt         no
libgcrypt kdf     no
nettle            no 
nettle kdf        no
libtasn1          no
curses support    no
virgl support     no
curl support      no
mingw32 support   no
Audio drivers     oss
Block whitelist (rw) 
Block whitelist (ro) 
VirtFS support    no
VNC support       yes
VNC SASL support  no
VNC JPEG support  no
VNC PNG support   no
xen support       no
brlapi support    no
bluez  support    no
Documentation     no
PIE               yes
vde support       no
netmap support    no
Linux AIO support no
ATTR/XATTR support yes
Install blobs     yes
KVM support       yes
RDMA support      no
TCG interpreter   no
fdt support       yes
preadv support    yes
fdatasync         yes
madvise           yes
posix_madvise     yes
libcap-ng support no
vhost-net support yes
vhost-scsi support yes
vhost-vsock support yes
Trace backends    log
spice support     no 
rbd support       no
xfsctl support    no
smartcard support no
libusb            no
usb net redir     no
OpenGL support    no
OpenGL dmabufs    no
libiscsi support  no
libnfs support    no
build guest agent yes
QGA VSS support   no
QGA w32 disk info no
QGA MSI support   no
seccomp support   no
coroutine backend ucontext
coroutine pool    yes
debug stack usage no
GlusterFS support no
Archipelago support no
gcov              gcov
gcov enabled      no
TPM support       yes
libssh2 support   no
TPM passthrough   yes
QOM debugging     yes
lzo support       no
snappy support    no
bzip2 support     no
NUMA host support no
tcmalloc support  no
jemalloc support  no
avx2 optimization no
replication support yes
  GEN     x86_64-softmmu/config-devices.mak.tmp
  GEN     aarch64-softmmu/config-devices.mak.tmp
  GEN     config-host.h
  GEN     qemu-options.def
  GEN     qmp-commands.h
  GEN     qapi-types.h
  GEN     qapi-visit.h
  GEN     qapi-event.h
  GEN     qmp-introspect.h
  GEN     x86_64-softmmu/config-devices.mak
  GEN     module_block.h
  GEN     tests/test-qapi-types.h
  GEN     tests/test-qapi-visit.h
  GEN     tests/test-qmp-commands.h
  GEN     tests/test-qapi-event.h
  GEN     aarch64-softmmu/config-devices.mak
  GEN     tests/test-qmp-introspect.h
  GEN     trace/generated-tracers.h
  GEN     trace/generated-tcg-tracers.h
  GEN     trace/generated-helpers-wrappers.h
  GEN     trace/generated-helpers.h
  GEN     config-all-devices.mak
  CC      tests/qemu-iotests/socket_scm_helper.o
  GEN     qga/qapi-generated/qga-qapi-types.h
  GEN     qga/qapi-generated/qga-qapi-visit.h
  GEN     qga/qapi-generated/qga-qmp-commands.h
  GEN     qga/qapi-generated/qga-qapi-visit.c
  GEN     qga/qapi-generated/qga-qapi-types.c
  GEN     qga/qapi-generated/qga-qmp-marshal.c
  GEN     qmp-introspect.c
  GEN     qapi-types.c
  GEN     qapi-visit.c
  GEN     qapi-event.c
  CC      qapi/qapi-visit-core.o
  CC      qapi/qapi-dealloc-visitor.o
  CC      qapi/qmp-input-visitor.o
  CC      qapi/qmp-output-visitor.o
  CC      qapi/qmp-registry.o
  CC      qapi/qmp-dispatch.o
  CC      qapi/string-input-visitor.o
  CC      qapi/string-output-visitor.o
  CC      qapi/opts-visitor.o
  CC      qapi/qapi-clone-visitor.o
  CC      qapi/qmp-event.o
  CC      qapi/qapi-util.o
  CC      qobject/qnull.o
  CC      qobject/qint.o
  CC      qobject/qstring.o
  CC      qobject/qdict.o
  CC      qobject/qlist.o
  CC      qobject/qfloat.o
  CC      qobject/qbool.o
  CC      qobject/qjson.o
  CC      qobject/qobject.o
  CC      qobject/json-lexer.o
  CC      qobject/json-streamer.o
  CC      qobject/json-parser.o
  GEN     trace/generated-tracers.c
  CC      trace/qmp.o
  CC      trace/control.o
  CC      util/osdep.o
  CC      util/cutils.o
  CC      util/unicode.o
  CC      util/qemu-timer-common.o
  CC      util/bufferiszero.o
  CC      util/compatfd.o
  CC      util/event_notifier-posix.o
  CC      util/mmap-alloc.o
  CC      util/oslib-posix.o
  CC      util/qemu-openpty.o
  CC      util/qemu-thread-posix.o
  CC      util/envlist.o
  CC      util/memfd.o
  CC      util/path.o
  CC      util/module.o
  CC      util/bitmap.o
  CC      util/bitops.o
  CC      util/hbitmap.o
  CC      util/fifo8.o
  CC      util/acl.o
  CC      util/error.o
  CC      util/qemu-error.o
  CC      util/uri.o
  CC      util/id.o
  CC      util/iov.o
  CC      util/qemu-config.o
  CC      util/notify.o
  CC      util/qemu-sockets.o
  CC      util/qemu-option.o
  CC      util/qemu-progress.o
  CC      util/hexdump.o
  CC      util/crc32c.o
  CC      util/uuid.o
  CC      util/throttle.o
  CC      util/getauxval.o
  CC      util/readline.o
  CC      util/rfifolock.o
  CC      util/rcu.o
  CC      util/qemu-coroutine.o
  CC      util/qemu-coroutine-lock.o
  CC      util/qemu-coroutine-io.o
  CC      util/qemu-coroutine-sleep.o
  CC      util/coroutine-ucontext.o
  CC      util/buffer.o
  CC      util/timed-average.o
  CC      util/base64.o
  CC      util/log.o
  CC      util/qdist.o
  CC      util/qht.o
  CC      util/range.o
  CC      crypto/pbkdf-stub.o
  CC      stubs/arch-query-cpu-def.o
  CC      stubs/arch-query-cpu-model-expansion.o
  CC      stubs/arch-query-cpu-model-comparison.o
  CC      stubs/arch-query-cpu-model-baseline.o
  CC      stubs/bdrv-next-monitor-owned.o
  CC      stubs/blk-commit-all.o
  CC      stubs/blockdev-close-all-bdrv-states.o
  CC      stubs/clock-warp.o
  CC      stubs/cpu-get-clock.o
  CC      stubs/cpu-get-icount.o
  CC      stubs/dump.o
  CC      stubs/fdset-add-fd.o
  CC      stubs/fdset-find-fd.o
  CC      stubs/fdset-get-fd.o
  CC      stubs/fdset-remove-fd.o
  CC      stubs/gdbstub.o
  CC      stubs/get-fd.o
  CC      stubs/get-next-serial.o
  CC      stubs/get-vm-name.o
  CC      stubs/iothread-lock.o
  CC      stubs/is-daemonized.o
  CC      stubs/machine-init-done.o
  CC      stubs/migr-blocker.o
  CC      stubs/mon-is-qmp.o
  CC      stubs/mon-printf.o
  CC      stubs/monitor-init.o
  CC      stubs/notify-event.o
  CC      stubs/qtest.o
  CC      stubs/replay.o
  CC      stubs/replay-user.o
  CC      stubs/reset.o
  CC      stubs/runstate-check.o
  CC      stubs/set-fd-handler.o
  CC      stubs/slirp.o
  CC      stubs/sysbus.o
  CC      stubs/trace-control.o
  CC      stubs/uuid.o
  CC      stubs/vm-stop.o
  CC      stubs/vmstate.o
  CC      stubs/cpus.o
  CC      stubs/kvm.o
  CC      stubs/qmp_pc_dimm_device_list.o
  CC      stubs/target-monitor-defs.o
  CC      stubs/target-get-monitor-def.o
  CC      stubs/vhost.o
  CC      stubs/iohandler.o
  CC      stubs/smbios_type_38.o
  CC      stubs/ipmi.o
  CC      stubs/pc_madt_cpu_entry.o
  CC      contrib/ivshmem-client/ivshmem-client.o
  CC      contrib/ivshmem-client/main.o
  CC      contrib/ivshmem-server/ivshmem-server.o
  CC      qemu-nbd.o
  CC      contrib/ivshmem-server/main.o
  CC      async.o
  CC      thread-pool.o
  CC      block.o
  CC      blockjob.o
  CC      main-loop.o
  CC      iohandler.o
  CC      qemu-timer.o
  CC      aio-posix.o
  CC      qemu-io-cmds.o
  CC      replication.o
  CC      block/raw_bsd.o
  CC      block/qcow.o
  CC      block/vdi.o
  CC      block/vmdk.o
  CC      block/cloop.o
  CC      block/bochs.o
  CC      block/vpc.o
  CC      block/vvfat.o
  CC      block/dmg.o
  CC      block/qcow2.o
  CC      block/qcow2-refcount.o
  CC      block/qcow2-cluster.o
  CC      block/qcow2-snapshot.o
  CC      block/qcow2-cache.o
  CC      block/qed.o
  CC      block/qed-gencb.o
  CC      block/qed-l2-cache.o
  CC      block/qed-table.o
  CC      block/qed-cluster.o
  CC      block/qed-check.o
  CC      block/vhdx.o
  CC      block/vhdx-endian.o
  CC      block/vhdx-log.o
  CC      block/quorum.o
  CC      block/parallels.o
  CC      block/blkdebug.o
  CC      block/blkverify.o
  CC      block/block-backend.o
  CC      block/blkreplay.o
  CC      block/snapshot.o
  CC      block/qapi.o
  CC      block/raw-posix.o
  CC      block/null.o
  CC      block/mirror.o
  CC      block/commit.o
  CC      block/io.o
  CC      block/throttle-groups.o
  CC      block/nbd.o
  CC      block/nbd-client.o
  CC      block/sheepdog.o
  CC      block/accounting.o
  CC      block/dirty-bitmap.o
  CC      block/write-threshold.o
  CC      block/backup.o
  CC      block/replication.o
  CC      block/crypto.o
  CC      nbd/server.o
  CC      nbd/client.o
  CC      nbd/common.o
/tmp/qemu-test/src/block/nbd.c: In function ‘nbd_config’:
/tmp/qemu-test/src/block/nbd.c:263: warning: implicit declaration of function ‘qdict_crumple’
/tmp/qemu-test/src/block/nbd.c:263: warning: nested extern declaration of ‘qdict_crumple’
/tmp/qemu-test/src/block/nbd.c:263: warning: assignment makes pointer from integer without a cast
  CC      crypto/init.o
  CC      crypto/hash.o
  CC      crypto/hash-glib.o
  CC      crypto/desrfb.o
  CC      crypto/aes.o
  CC      crypto/tlscreds.o
  CC      crypto/cipher.o
  CC      crypto/tlscredsanon.o
  CC      crypto/tlscredsx509.o
  CC      crypto/tlssession.o
  CC      crypto/secret.o
  CC      crypto/random-platform.o
  CC      crypto/pbkdf.o
  CC      crypto/ivgen.o
  CC      crypto/ivgen-essiv.o
  CC      crypto/ivgen-plain.o
  CC      crypto/ivgen-plain64.o
  CC      crypto/afsplit.o
  CC      crypto/xts.o
  CC      crypto/block.o
  CC      crypto/block-qcow.o
  CC      crypto/block-luks.o
  CC      io/channel.o
  CC      io/channel-buffer.o
  CC      io/channel-command.o
  CC      io/channel-file.o
  CC      io/channel-socket.o
  CC      io/channel-tls.o
  CC      io/channel-watch.o
  CC      io/channel-websock.o
  CC      io/channel-util.o
  CC      io/task.o
  CC      qom/object.o
  CC      qom/container.o
  CC      qom/qom-qobject.o
  CC      qom/object_interfaces.o
  GEN     qemu-img-cmds.h
  CC      qemu-io.o
  CC      qemu-bridge-helper.o
  CC      blockdev.o
  CC      blockdev-nbd.o
  CC      iothread.o
  CC      qdev-monitor.o
  CC      device-hotplug.o
  CC      os-posix.o
  CC      qemu-char.o
  CC      page_cache.o
  CC      accel.o
  CC      bt-host.o
  CC      bt-vhci.o
  CC      dma-helpers.o
  CC      vl.o
  CC      tpm.o
  CC      device_tree.o
  GEN     qmp-marshal.c
  CC      qmp.o
  CC      hmp.o
  CC      tcg-runtime.o
  CC      cpus-common.o
  CC      audio/audio.o
  CC      audio/noaudio.o
  CC      audio/wavaudio.o
  CC      audio/mixeng.o
  CC      audio/sdlaudio.o
  CC      audio/ossaudio.o
  CC      audio/wavcapture.o
  CC      backends/rng.o
  CC      backends/rng-egd.o
  CC      backends/msmouse.o
  CC      backends/rng-random.o
  CC      backends/testdev.o
  CC      backends/tpm.o
  CC      backends/hostmem.o
  CC      backends/hostmem-ram.o
  CC      backends/hostmem-file.o
  CC      block/stream.o
  CC      disas/arm.o
  CC      disas/i386.o
  CC      fsdev/qemu-fsdev-dummy.o
  CC      fsdev/qemu-fsdev-opts.o
  CC      hw/acpi/core.o
  CC      hw/acpi/piix4.o
  CC      hw/acpi/pcihp.o
  CC      hw/acpi/ich9.o
  CC      hw/acpi/tco.o
  CC      hw/acpi/cpu_hotplug.o
  CC      hw/acpi/memory_hotplug.o
  CC      hw/acpi/memory_hotplug_acpi_table.o
  CC      hw/acpi/cpu.o
  CC      hw/acpi/acpi_interface.o
  CC      hw/acpi/bios-linker-loader.o
  CC      hw/acpi/aml-build.o
  CC      hw/acpi/ipmi.o
  CC      hw/audio/sb16.o
  CC      hw/audio/es1370.o
  CC      hw/audio/ac97.o
  CC      hw/audio/fmopl.o
  CC      hw/audio/adlib.o
  CC      hw/audio/gus.o
  CC      hw/audio/gusemu_hal.o
  CC      hw/audio/gusemu_mixer.o
  CC      hw/audio/cs4231a.o
  CC      hw/audio/intel-hda.o
  CC      hw/audio/hda-codec.o
  CC      hw/audio/pcspk.o
  CC      hw/audio/wm8750.o
  CC      hw/audio/pl041.o
  CC      hw/audio/lm4549.o
  CC      hw/audio/marvell_88w8618.o
  CC      hw/block/block.o
  CC      hw/block/cdrom.o
  CC      hw/block/hd-geometry.o
  CC      hw/block/fdc.o
  CC      hw/block/m25p80.o
  CC      hw/block/nand.o
  CC      hw/block/pflash_cfi01.o
  CC      hw/block/pflash_cfi02.o
  CC      hw/block/ecc.o
  CC      hw/block/onenand.o
  CC      hw/block/nvme.o
  CC      hw/bt/core.o
  CC      hw/bt/l2cap.o
  CC      hw/bt/hci.o
  CC      hw/bt/sdp.o
  CC      hw/bt/hid.o
  CC      hw/bt/hci-csr.o
  CC      hw/char/ipoctal232.o
  CC      hw/char/parallel.o
  CC      hw/char/pl011.o
  CC      hw/char/serial.o
  CC      hw/char/serial-isa.o
  CC      hw/char/serial-pci.o
  CC      hw/char/virtio-console.o
  CC      hw/char/debugcon.o
  CC      hw/char/cadence_uart.o
  CC      hw/char/imx_serial.o
  CC      hw/core/qdev.o
  CC      hw/core/qdev-properties.o
  CC      hw/core/bus.o
  CC      hw/core/fw-path-provider.o
  CC      hw/core/irq.o
  CC      hw/core/hotplug.o
  CC      hw/core/ptimer.o
  CC      hw/core/sysbus.o
  CC      hw/core/machine.o
  CC      hw/core/null-machine.o
  CC      hw/core/loader.o
  CC      hw/core/qdev-properties-system.o
  CC      hw/core/register.o
  CC      hw/core/or-irq.o
  CC      hw/core/platform-bus.o
  CC      hw/display/ads7846.o
  CC      hw/display/cirrus_vga.o
  CC      hw/display/pl110.o
  CC      hw/display/ssd0303.o
  CC      hw/display/ssd0323.o
  CC      hw/display/vga-pci.o
  CC      hw/display/vga-isa.o
  CC      hw/display/vmware_vga.o
  CC      hw/display/blizzard.o
  CC      hw/display/exynos4210_fimd.o
  CC      hw/display/framebuffer.o
  CC      hw/display/tc6393xb.o
  CC      hw/dma/pl080.o
  CC      hw/dma/pl330.o
  CC      hw/dma/i8257.o
  CC      hw/dma/xlnx-zynq-devcfg.o
  CC      hw/gpio/max7310.o
  CC      hw/gpio/pl061.o
  CC      hw/gpio/zaurus.o
  CC      hw/gpio/gpio_key.o
  CC      hw/i2c/core.o
  CC      hw/i2c/smbus.o
  CC      hw/i2c/smbus_eeprom.o
  CC      hw/i2c/versatile_i2c.o
  CC      hw/i2c/i2c-ddc.o
  CC      hw/i2c/smbus_ich9.o
  CC      hw/i2c/pm_smbus.o
  CC      hw/i2c/bitbang_i2c.o
  CC      hw/i2c/exynos4210_i2c.o
  CC      hw/i2c/imx_i2c.o
  CC      hw/i2c/aspeed_i2c.o
  CC      hw/ide/core.o
  CC      hw/ide/atapi.o
  CC      hw/ide/qdev.o
  CC      hw/ide/pci.o
  CC      hw/ide/isa.o
  CC      hw/ide/piix.o
  CC      hw/ide/microdrive.o
  CC      hw/ide/ahci.o
  CC      hw/ide/ich.o
  CC      hw/input/hid.o
  CC      hw/input/lm832x.o
  CC      hw/input/pckbd.o
  CC      hw/input/pl050.o
  CC      hw/input/ps2.o
  CC      hw/input/stellaris_input.o
  CC      hw/input/tsc2005.o
  CC      hw/input/vmmouse.o
  CC      hw/input/virtio-input.o
  CC      hw/input/virtio-input-hid.o
  CC      hw/input/virtio-input-host.o
  CC      hw/intc/i8259_common.o
  CC      hw/intc/i8259.o
  CC      hw/intc/pl190.o
  CC      hw/intc/imx_avic.o
  CC      hw/intc/realview_gic.o
  CC      hw/intc/arm_gic_common.o
  CC      hw/intc/ioapic_common.o
  CC      hw/intc/arm_gic.o
  CC      hw/intc/arm_gicv2m.o
  CC      hw/intc/arm_gicv3_common.o
  CC      hw/intc/arm_gicv3.o
  CC      hw/intc/arm_gicv3_dist.o
  CC      hw/intc/arm_gicv3_redist.o
  CC      hw/intc/arm_gicv3_its_common.o
  CC      hw/intc/intc.o
  CC      hw/ipack/ipack.o
  CC      hw/ipack/tpci200.o
  CC      hw/ipmi/ipmi.o
  CC      hw/ipmi/ipmi_bmc_sim.o
  CC      hw/ipmi/ipmi_bmc_extern.o
  CC      hw/ipmi/isa_ipmi_kcs.o
  CC      hw/ipmi/isa_ipmi_bt.o
  CC      hw/isa/isa-bus.o
  CC      hw/isa/apm.o
  CC      hw/mem/pc-dimm.o
  CC      hw/mem/nvdimm.o
  CC      hw/misc/applesmc.o
  CC      hw/misc/max111x.o
  CC      hw/misc/tmp105.o
  CC      hw/misc/debugexit.o
  CC      hw/misc/sga.o
  CC      hw/misc/pc-testdev.o
  CC      hw/misc/pci-testdev.o
  CC      hw/misc/arm_integrator_debug.o
  CC      hw/misc/arm_l2x0.o
  CC      hw/misc/a9scu.o
  CC      hw/misc/arm11scu.o
  CC      hw/net/ne2000.o
  CC      hw/net/eepro100.o
  CC      hw/net/pcnet-pci.o
  CC      hw/net/pcnet.o
  CC      hw/net/e1000.o
  CC      hw/net/e1000x_common.o
  CC      hw/net/net_tx_pkt.o
  CC      hw/net/net_rx_pkt.o
  CC      hw/net/e1000e.o
  CC      hw/net/e1000e_core.o
  CC      hw/net/rtl8139.o
  CC      hw/net/vmxnet3.o
  CC      hw/net/smc91c111.o
  CC      hw/net/lan9118.o
  CC      hw/net/ne2000-isa.o
  CC      hw/net/xgmac.o
  CC      hw/net/allwinner_emac.o
  CC      hw/net/imx_fec.o
  CC      hw/net/cadence_gem.o
  CC      hw/net/rocker/rocker.o
  CC      hw/net/stellaris_enet.o
  CC      hw/net/rocker/rocker_fp.o
  CC      hw/net/rocker/rocker_desc.o
  CC      hw/net/rocker/rocker_world.o
  CC      hw/net/rocker/rocker_of_dpa.o
  CC      hw/nvram/eeprom93xx.o
  CC      hw/nvram/fw_cfg.o
  CC      hw/pci-bridge/pci_bridge_dev.o
  CC      hw/pci-bridge/pci_expander_bridge.o
  CC      hw/pci-bridge/xio3130_upstream.o
  CC      hw/pci-bridge/ioh3420.o
  CC      hw/pci-bridge/xio3130_downstream.o
  CC      hw/pci-bridge/i82801b11.o
  CC      hw/pci-host/pam.o
  CC      hw/pci-host/versatile.o
  CC      hw/pci-host/piix.o
  CC      hw/pci-host/q35.o
  CC      hw/pci-host/gpex.o
  CC      hw/pci/pci.o
  CC      hw/pci/pci_bridge.o
  CC      hw/pci/msix.o
  CC      hw/pci/msi.o
  CC      hw/pci/shpc.o
  CC      hw/pci/slotid_cap.o
  CC      hw/pci/pci_host.o
  CC      hw/pci/pcie_host.o
  CC      hw/pci/pcie.o
  CC      hw/pci/pcie_aer.o
  CC      hw/pci/pcie_port.o
  CC      hw/pci/pci-stub.o
  CC      hw/pcmcia/pcmcia.o
  CC      hw/scsi/scsi-disk.o
  CC      hw/scsi/scsi-generic.o
  CC      hw/scsi/scsi-bus.o
  CC      hw/scsi/lsi53c895a.o
  CC      hw/scsi/mptsas.o
  CC      hw/scsi/mptconfig.o
/tmp/qemu-test/src/hw/nvram/fw_cfg.c: In function ‘fw_cfg_dma_transfer’:
/tmp/qemu-test/src/hw/nvram/fw_cfg.c:330: warning: ‘read’ may be used uninitialized in this function
  CC      hw/scsi/mptendian.o
  CC      hw/scsi/megasas.o
  CC      hw/scsi/vmw_pvscsi.o
  CC      hw/scsi/esp.o
  CC      hw/scsi/esp-pci.o
  CC      hw/sd/pl181.o
  CC      hw/sd/ssi-sd.o
  CC      hw/sd/sd.o
  CC      hw/sd/core.o
  CC      hw/sd/sdhci.o
  CC      hw/smbios/smbios.o
  CC      hw/smbios/smbios_type_38.o
  CC      hw/ssi/pl022.o
  CC      hw/ssi/ssi.o
  CC      hw/ssi/xilinx_spips.o
  CC      hw/ssi/stm32f2xx_spi.o
  CC      hw/ssi/aspeed_smc.o
  CC      hw/timer/arm_timer.o
  CC      hw/timer/arm_mptimer.o
  CC      hw/timer/a9gtimer.o
  CC      hw/timer/cadence_ttc.o
  CC      hw/timer/ds1338.o
  CC      hw/timer/hpet.o
  CC      hw/timer/i8254_common.o
  CC      hw/timer/i8254.o
  CC      hw/timer/pl031.o
  CC      hw/timer/twl92230.o
  CC      hw/timer/imx_epit.o
  CC      hw/timer/imx_gpt.o
  CC      hw/timer/stm32f2xx_timer.o
  CC      hw/timer/aspeed_timer.o
  CC      hw/tpm/tpm_tis.o
  CC      hw/tpm/tpm_passthrough.o
  CC      hw/tpm/tpm_util.o
  CC      hw/usb/core.o
  CC      hw/usb/combined-packet.o
  CC      hw/usb/bus.o
  CC      hw/usb/libhw.o
  CC      hw/usb/desc.o
  CC      hw/usb/desc-msos.o
  CC      hw/usb/hcd-uhci.o
  CC      hw/usb/hcd-ohci.o
  CC      hw/usb/hcd-ehci.o
  CC      hw/usb/hcd-ehci-pci.o
  CC      hw/usb/hcd-ehci-sysbus.o
  CC      hw/usb/hcd-xhci.o
  CC      hw/usb/hcd-musb.o
  CC      hw/usb/dev-hub.o
  CC      hw/usb/dev-hid.o
  CC      hw/usb/dev-wacom.o
  CC      hw/usb/dev-storage.o
  CC      hw/usb/dev-uas.o
  CC      hw/usb/dev-audio.o
  CC      hw/usb/dev-serial.o
  CC      hw/usb/dev-network.o
  CC      hw/usb/dev-bluetooth.o
  CC      hw/usb/dev-smartcard-reader.o
  CC      hw/usb/dev-mtp.o
  CC      hw/usb/host-stub.o
  CC      hw/virtio/virtio-rng.o
  CC      hw/virtio/virtio-pci.o
  CC      hw/virtio/virtio-bus.o
  CC      hw/virtio/virtio-mmio.o
  CC      hw/watchdog/watchdog.o
  CC      hw/watchdog/wdt_i6300esb.o
  CC      hw/watchdog/wdt_ib700.o
  CC      migration/migration.o
  CC      migration/socket.o
  CC      migration/fd.o
  CC      migration/exec.o
  CC      migration/tls.o
  CC      migration/vmstate.o
  CC      migration/qemu-file.o
  CC      migration/qemu-file-channel.o
  CC      migration/xbzrle.o
  CC      migration/postcopy-ram.o
  CC      migration/qjson.o
  CC      migration/block.o
  CC      net/net.o
  CC      net/queue.o
  CC      net/checksum.o
  CC      net/util.o
  CC      net/hub.o
  CC      net/socket.o
  CC      net/dump.o
  CC      net/eth.o
  CC      net/l2tpv3.o
  CC      net/tap.o
  CC      net/vhost-user.o
  CC      net/tap-linux.o
  CC      net/slirp.o
  CC      net/filter.o
  CC      net/filter-buffer.o
  CC      net/filter-mirror.o
  CC      net/colo-compare.o
  CC      net/colo.o
  CC      net/filter-rewriter.o
  CC      qom/cpu.o
  CC      replay/replay.o
  CC      replay/replay-internal.o
  CC      replay/replay-events.o
  CC      replay/replay-input.o
  CC      replay/replay-time.o
  CC      replay/replay-char.o
  CC      replay/replay-snapshot.o
  CC      slirp/cksum.o
  CC      slirp/if.o
  CC      slirp/ip_icmp.o
  CC      slirp/ip6_icmp.o
  CC      slirp/ip6_input.o
/tmp/qemu-test/src/replay/replay-internal.c: In function ‘replay_put_array’:
/tmp/qemu-test/src/replay/replay-internal.c:65: warning: ignoring return value of ‘fwrite’, declared with attribute warn_unused_result
  CC      slirp/ip6_output.o
  CC      slirp/ip_input.o
  CC      slirp/ip_output.o
  CC      slirp/dnssearch.o
  CC      slirp/dhcpv6.o
  CC      slirp/slirp.o
  CC      slirp/mbuf.o
  CC      slirp/misc.o
  CC      slirp/sbuf.o
  CC      slirp/tcp_input.o
  CC      slirp/socket.o
  CC      slirp/tcp_subr.o
  CC      slirp/tcp_output.o
  CC      slirp/tcp_timer.o
  CC      slirp/udp.o
  CC      slirp/udp6.o
  CC      slirp/bootp.o
  CC      slirp/tftp.o
  CC      slirp/arp_table.o
  CC      slirp/ndp_table.o
  CC      ui/keymaps.o
  CC      ui/cursor.o
  CC      ui/console.o
  CC      ui/qemu-pixman.o
  CC      ui/input.o
  CC      ui/input-keymap.o
  CC      ui/input-legacy.o
  CC      ui/input-linux.o
  CC      ui/sdl.o
  CC      ui/sdl_zoom.o
  CC      ui/x_keymap.o
  CC      ui/vnc.o
  CC      ui/vnc-enc-zlib.o
/tmp/qemu-test/src/slirp/tcp_input.c: In function ‘tcp_input’:
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_p’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_len’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_tos’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_id’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_off’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_ttl’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_sum’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_src.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:219: warning: ‘save_ip.ip_dst.s_addr’ may be used uninitialized in this function
/tmp/qemu-test/src/slirp/tcp_input.c:220: warning: ‘save_ip6.ip_nh’ may be used uninitialized in this function
  CC      ui/vnc-enc-hextile.o
  CC      ui/vnc-enc-tight.o
  CC      ui/vnc-palette.o
  CC      ui/vnc-enc-zrle.o
  CC      ui/vnc-auth-vencrypt.o
  CC      ui/vnc-ws.o
  CC      ui/vnc-jobs.o
  LINK    tests/qemu-iotests/socket_scm_helper
  CC      qga/commands.o
  AS      optionrom/multiboot.o
  AS      optionrom/linuxboot.o
  CC      qga/guest-agent-command-state.o
  CC      optionrom/linuxboot_dma.o
cc: unrecognized option '-no-integrated-as'
cc: unrecognized option '-no-integrated-as'
  CC      qga/main.o
  AS      optionrom/kvmvapic.o
  BUILD   optionrom/multiboot.img
  BUILD   optionrom/linuxboot.img
  BUILD   optionrom/linuxboot_dma.img
  BUILD   optionrom/kvmvapic.img
  CC      qga/commands-posix.o
  BUILD   optionrom/multiboot.raw
  BUILD   optionrom/linuxboot.raw
  BUILD   optionrom/linuxboot_dma.raw
  CC      qga/channel-posix.o
  BUILD   optionrom/kvmvapic.raw
  SIGN    optionrom/multiboot.bin
  SIGN    optionrom/linuxboot.bin
  CC      qga/qapi-generated/qga-qapi-types.o
  SIGN    optionrom/linuxboot_dma.bin
  CC      qga/qapi-generated/qga-qapi-visit.o
  SIGN    optionrom/kvmvapic.bin
  CC      qga/qapi-generated/qga-qmp-marshal.o
  CC      qmp-introspect.o
  CC      qapi-types.o
  CC      qapi-visit.o
  CC      qapi-event.o
  AR      libqemustub.a
  CC      qemu-img.o
  CC      qmp-marshal.o
  CC      trace/generated-tracers.o
  AR      libqemuutil.a
  LINK    qemu-ga
  LINK    ivshmem-client
  LINK    ivshmem-server
  LINK    qemu-nbd
  LINK    qemu-img
  LINK    qemu-io
  LINK    qemu-bridge-helper
block/nbd.o: In function `nbd_config':
/tmp/qemu-test/src/block/nbd.c:263: undefined reference to `qdict_crumple'
collect2: ld returned 1 exit status
make: *** [qemu-nbd] Error 1
make: *** Waiting for unfinished jobs....
block/nbd.o: In function `nbd_config':
/tmp/qemu-test/src/block/nbd.c:263: undefined reference to `qdict_crumple'
collect2: ld returned 1 exit status
make: *** [qemu-img] Error 1
block/nbd.o: In function `nbd_config':
/tmp/qemu-test/src/block/nbd.c:263: undefined reference to `qdict_crumple'
collect2: ld returned 1 exit status
make: *** [qemu-io] Error 1
  GEN     aarch64-softmmu/hmp-commands.h
  GEN     aarch64-softmmu/config-target.h
  GEN     aarch64-softmmu/hmp-commands-info.h
  CC      aarch64-softmmu/exec.o
  CC      aarch64-softmmu/translate-all.o
  CC      aarch64-softmmu/cpu-exec.o
  CC      aarch64-softmmu/translate-common.o
  CC      aarch64-softmmu/cpu-exec-common.o
  CC      aarch64-softmmu/tcg/tcg.o
  CC      aarch64-softmmu/tcg/tcg-op.o
  CC      aarch64-softmmu/tcg/optimize.o
  CC      aarch64-softmmu/tcg/tcg-common.o
  CC      aarch64-softmmu/fpu/softfloat.o
  CC      aarch64-softmmu/disas.o
  GEN     aarch64-softmmu/gdbstub-xml.c
  CC      aarch64-softmmu/kvm-stub.o
  CC      aarch64-softmmu/arch_init.o
  CC      aarch64-softmmu/cpus.o
  GEN     x86_64-softmmu/hmp-commands.h
  CC      aarch64-softmmu/monitor.o
  GEN     x86_64-softmmu/hmp-commands-info.h
  GEN     x86_64-softmmu/config-target.h
  CC      aarch64-softmmu/gdbstub.o
  CC      x86_64-softmmu/exec.o
  CC      x86_64-softmmu/translate-all.o
  CC      aarch64-softmmu/balloon.o
  CC      x86_64-softmmu/cpu-exec.o
  CC      aarch64-softmmu/ioport.o
  CC      aarch64-softmmu/numa.o
  CC      aarch64-softmmu/qtest.o
  CC      aarch64-softmmu/bootdevice.o
  CC      x86_64-softmmu/translate-common.o
  CC      aarch64-softmmu/memory.o
  CC      x86_64-softmmu/cpu-exec-common.o
  CC      aarch64-softmmu/cputlb.o
  CC      x86_64-softmmu/tcg/tcg.o
  CC      aarch64-softmmu/memory_mapping.o
  CC      aarch64-softmmu/dump.o
  CC      aarch64-softmmu/migration/ram.o
  CC      x86_64-softmmu/tcg/tcg-op.o
  CC      x86_64-softmmu/tcg/optimize.o
  CC      x86_64-softmmu/tcg/tcg-common.o
  CC      aarch64-softmmu/migration/savevm.o
  CC      x86_64-softmmu/fpu/softfloat.o
  CC      aarch64-softmmu/xen-common-stub.o
  CC      x86_64-softmmu/disas.o
  CC      aarch64-softmmu/xen-hvm-stub.o
  CC      aarch64-softmmu/hw/adc/stm32f2xx_adc.o
  CC      x86_64-softmmu/arch_init.o
  CC      x86_64-softmmu/cpus.o
  CC      x86_64-softmmu/monitor.o
  CC      x86_64-softmmu/gdbstub.o
  CC      aarch64-softmmu/hw/block/virtio-blk.o
  CC      aarch64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      aarch64-softmmu/hw/char/exynos4210_uart.o
  CC      aarch64-softmmu/hw/char/omap_uart.o
  CC      aarch64-softmmu/hw/char/digic-uart.o
  CC      aarch64-softmmu/hw/char/stm32f2xx_usart.o
  CC      x86_64-softmmu/balloon.o
  CC      x86_64-softmmu/ioport.o
  CC      aarch64-softmmu/hw/char/bcm2835_aux.o
  CC      aarch64-softmmu/hw/char/virtio-serial-bus.o
  CC      aarch64-softmmu/hw/core/nmi.o
  CC      aarch64-softmmu/hw/core/generic-loader.o
  CC      x86_64-softmmu/numa.o
  CC      aarch64-softmmu/hw/cpu/arm11mpcore.o
  CC      x86_64-softmmu/qtest.o
  CC      aarch64-softmmu/hw/cpu/realview_mpcore.o
  CC      x86_64-softmmu/bootdevice.o
  CC      x86_64-softmmu/kvm-all.o
  CC      x86_64-softmmu/memory.o
  CC      aarch64-softmmu/hw/cpu/a9mpcore.o
  CC      aarch64-softmmu/hw/cpu/a15mpcore.o
  CC      aarch64-softmmu/hw/cpu/core.o
  CC      aarch64-softmmu/hw/display/omap_dss.o
  CC      x86_64-softmmu/cputlb.o
  CC      x86_64-softmmu/memory_mapping.o
  CC      x86_64-softmmu/dump.o
  CC      aarch64-softmmu/hw/display/omap_lcdc.o
  CC      aarch64-softmmu/hw/display/pxa2xx_lcd.o
  CC      aarch64-softmmu/hw/display/bcm2835_fb.o
  CC      aarch64-softmmu/hw/display/vga.o
  CC      x86_64-softmmu/migration/ram.o
  CC      x86_64-softmmu/migration/savevm.o
  CC      aarch64-softmmu/hw/display/virtio-gpu.o
  CC      x86_64-softmmu/xen-common-stub.o
  CC      x86_64-softmmu/xen-hvm-stub.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-3d.o
  CC      x86_64-softmmu/hw/acpi/nvdimm.o
  CC      x86_64-softmmu/hw/block/virtio-blk.o
  CC      aarch64-softmmu/hw/display/virtio-gpu-pci.o
  CC      x86_64-softmmu/hw/block/dataplane/virtio-blk.o
  CC      aarch64-softmmu/hw/display/dpcd.o
  CC      x86_64-softmmu/hw/char/virtio-serial-bus.o
  CC      x86_64-softmmu/hw/core/nmi.o
  CC      aarch64-softmmu/hw/display/xlnx_dp.o
  CC      aarch64-softmmu/hw/dma/xlnx_dpdma.o
  CC      x86_64-softmmu/hw/core/generic-loader.o
  CC      aarch64-softmmu/hw/dma/omap_dma.o
  CC      x86_64-softmmu/hw/cpu/core.o
  CC      x86_64-softmmu/hw/display/vga.o
  CC      aarch64-softmmu/hw/dma/soc_dma.o
  CC      aarch64-softmmu/hw/dma/pxa2xx_dma.o
  CC      aarch64-softmmu/hw/dma/bcm2835_dma.o
  CC      x86_64-softmmu/hw/display/virtio-gpu.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-3d.o
  CC      aarch64-softmmu/hw/gpio/omap_gpio.o
  CC      x86_64-softmmu/hw/display/virtio-gpu-pci.o
  CC      aarch64-softmmu/hw/gpio/imx_gpio.o
  CC      aarch64-softmmu/hw/i2c/omap_i2c.o
  CC      aarch64-softmmu/hw/input/pxa2xx_keypad.o
  CC      aarch64-softmmu/hw/input/tsc210x.o
  CC      aarch64-softmmu/hw/intc/armv7m_nvic.o
  CC      x86_64-softmmu/hw/display/virtio-vga.o
  CC      x86_64-softmmu/hw/intc/apic.o
  CC      x86_64-softmmu/hw/intc/apic_common.o
  CC      aarch64-softmmu/hw/intc/exynos4210_gic.o
  CC      aarch64-softmmu/hw/intc/exynos4210_combiner.o
  CC      aarch64-softmmu/hw/intc/omap_intc.o
  CC      x86_64-softmmu/hw/intc/ioapic.o
  CC      x86_64-softmmu/hw/isa/lpc_ich9.o
  CC      aarch64-softmmu/hw/intc/bcm2835_ic.o
  CC      aarch64-softmmu/hw/intc/bcm2836_control.o
  CC      aarch64-softmmu/hw/intc/allwinner-a10-pic.o
  CC      aarch64-softmmu/hw/intc/aspeed_vic.o
  CC      aarch64-softmmu/hw/intc/arm_gicv3_cpuif.o
  CC      aarch64-softmmu/hw/misc/ivshmem.o
  CC      aarch64-softmmu/hw/misc/arm_sysctl.o
  CC      aarch64-softmmu/hw/misc/cbus.o
  CC      x86_64-softmmu/hw/misc/vmport.o
  CC      x86_64-softmmu/hw/misc/ivshmem.o
  CC      aarch64-softmmu/hw/misc/exynos4210_pmu.o
  CC      aarch64-softmmu/hw/misc/imx_ccm.o
  CC      aarch64-softmmu/hw/misc/imx31_ccm.o
  CC      aarch64-softmmu/hw/misc/imx25_ccm.o
  CC      aarch64-softmmu/hw/misc/imx6_ccm.o
  CC      aarch64-softmmu/hw/misc/imx6_src.o
  CC      aarch64-softmmu/hw/misc/mst_fpga.o
  CC      x86_64-softmmu/hw/misc/pvpanic.o
  CC      x86_64-softmmu/hw/misc/edu.o
  CC      x86_64-softmmu/hw/misc/hyperv_testdev.o
  CC      aarch64-softmmu/hw/misc/omap_clk.o
  CC      aarch64-softmmu/hw/misc/omap_gpmc.o
  CC      x86_64-softmmu/hw/net/virtio-net.o
  CC      x86_64-softmmu/hw/net/vhost_net.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi.o
  CC      x86_64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      aarch64-softmmu/hw/misc/omap_l4.o
  CC      x86_64-softmmu/hw/scsi/vhost-scsi.o
  CC      aarch64-softmmu/hw/misc/omap_sdrc.o
  CC      aarch64-softmmu/hw/misc/omap_tap.o
  CC      aarch64-softmmu/hw/misc/bcm2835_mbox.o
  CC      aarch64-softmmu/hw/misc/bcm2835_property.o
  CC      aarch64-softmmu/hw/misc/zynq_slcr.o
  CC      x86_64-softmmu/hw/timer/mc146818rtc.o
  CC      aarch64-softmmu/hw/misc/zynq-xadc.o
  CC      x86_64-softmmu/hw/vfio/common.o
  CC      aarch64-softmmu/hw/misc/stm32f2xx_syscfg.o
  CC      x86_64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/hw/misc/edu.o
  CC      x86_64-softmmu/hw/vfio/pci-quirks.o
  CC      aarch64-softmmu/hw/misc/auxbus.o
  CC      aarch64-softmmu/hw/misc/aspeed_scu.o
  CC      aarch64-softmmu/hw/misc/aspeed_sdmc.o
  CC      aarch64-softmmu/hw/net/virtio-net.o
  CC      x86_64-softmmu/hw/vfio/platform.o
  CC      aarch64-softmmu/hw/net/vhost_net.o
  CC      x86_64-softmmu/hw/vfio/calxeda-xgmac.o
  CC      x86_64-softmmu/hw/vfio/amd-xgbe.o
  CC      x86_64-softmmu/hw/vfio/spapr.o
  CC      x86_64-softmmu/hw/virtio/virtio.o
  CC      x86_64-softmmu/hw/virtio/virtio-balloon.o
  CC      aarch64-softmmu/hw/pcmcia/pxa2xx.o
  CC      x86_64-softmmu/hw/virtio/vhost.o
  CC      x86_64-softmmu/hw/virtio/vhost-backend.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi.o
  CC      x86_64-softmmu/hw/virtio/vhost-user.o
  CC      aarch64-softmmu/hw/scsi/virtio-scsi-dataplane.o
  CC      aarch64-softmmu/hw/scsi/vhost-scsi.o
  CC      x86_64-softmmu/hw/virtio/vhost-vsock.o
  CC      x86_64-softmmu/hw/i386/multiboot.o
  CC      x86_64-softmmu/hw/i386/pc.o
  CC      x86_64-softmmu/hw/i386/pc_piix.o
  CC      x86_64-softmmu/hw/i386/pc_q35.o
  CC      aarch64-softmmu/hw/sd/omap_mmc.o
  CC      aarch64-softmmu/hw/sd/pxa2xx_mmci.o
  CC      aarch64-softmmu/hw/ssi/omap_spi.o
  CC      x86_64-softmmu/hw/i386/pc_sysfw.o
  CC      x86_64-softmmu/hw/i386/x86-iommu.o
  CC      aarch64-softmmu/hw/ssi/imx_spi.o
  CC      aarch64-softmmu/hw/timer/exynos4210_mct.o
  CC      x86_64-softmmu/hw/i386/intel_iommu.o
  CC      aarch64-softmmu/hw/timer/exynos4210_pwm.o
  CC      aarch64-softmmu/hw/timer/exynos4210_rtc.o
  CC      x86_64-softmmu/hw/i386/amd_iommu.o
  CC      aarch64-softmmu/hw/timer/omap_gptimer.o
  CC      aarch64-softmmu/hw/timer/omap_synctimer.o
  CC      aarch64-softmmu/hw/timer/pxa2xx_timer.o
  CC      x86_64-softmmu/hw/i386/kvmvapic.o
  CC      aarch64-softmmu/hw/timer/digic-timer.o
/tmp/qemu-test/src/hw/i386/pc_piix.c: In function ‘igd_passthrough_isa_bridge_create’:
/tmp/qemu-test/src/hw/i386/pc_piix.c:1046: warning: ‘pch_rev_id’ may be used uninitialized in this function
  CC      aarch64-softmmu/hw/usb/tusb6010.o
  CC      x86_64-softmmu/hw/i386/acpi-build.o
  CC      aarch64-softmmu/hw/timer/allwinner-a10-pit.o
  CC      x86_64-softmmu/hw/i386/pci-assign-load-rom.o
  CC      x86_64-softmmu/hw/i386/kvm/clock.o
  CC      aarch64-softmmu/hw/vfio/common.o
  CC      x86_64-softmmu/hw/i386/kvm/apic.o
  CC      x86_64-softmmu/hw/i386/kvm/i8259.o
  CC      x86_64-softmmu/hw/i386/kvm/ioapic.o
  CC      aarch64-softmmu/hw/vfio/pci.o
  CC      aarch64-softmmu/hw/vfio/platform.o
  CC      aarch64-softmmu/hw/vfio/pci-quirks.o
  CC      x86_64-softmmu/hw/i386/kvm/i8254.o
  CC      x86_64-softmmu/hw/i386/kvm/pci-assign.o
  CC      aarch64-softmmu/hw/vfio/calxeda-xgmac.o
  CC      aarch64-softmmu/hw/vfio/amd-xgbe.o
  CC      x86_64-softmmu/target-i386/translate.o
  CC      x86_64-softmmu/target-i386/helper.o
  CC      aarch64-softmmu/hw/vfio/spapr.o
  CC      aarch64-softmmu/hw/virtio/vhost.o
  CC      aarch64-softmmu/hw/virtio/virtio.o
  CC      aarch64-softmmu/hw/virtio/virtio-balloon.o
  CC      x86_64-softmmu/target-i386/cpu.o
  CC      aarch64-softmmu/hw/virtio/vhost-backend.o
  CC      x86_64-softmmu/target-i386/bpt_helper.o
  CC      x86_64-softmmu/target-i386/excp_helper.o
  CC      aarch64-softmmu/hw/virtio/vhost-user.o
  CC      aarch64-softmmu/hw/virtio/vhost-vsock.o
/tmp/qemu-test/src/hw/i386/acpi-build.c: In function ‘build_append_pci_bus_devices’:
/tmp/qemu-test/src/hw/i386/acpi-build.c:472: warning: ‘notify_method’ may be used uninitialized in this function
  CC      x86_64-softmmu/target-i386/fpu_helper.o
  CC      aarch64-softmmu/hw/arm/boot.o
  CC      x86_64-softmmu/target-i386/cc_helper.o
  CC      x86_64-softmmu/target-i386/int_helper.o
  CC      aarch64-softmmu/hw/arm/collie.o
  CC      aarch64-softmmu/hw/arm/exynos4_boards.o
  CC      x86_64-softmmu/target-i386/svm_helper.o
  CC      x86_64-softmmu/target-i386/smm_helper.o
  CC      x86_64-softmmu/target-i386/misc_helper.o
  CC      x86_64-softmmu/target-i386/mem_helper.o
  CC      aarch64-softmmu/hw/arm/gumstix.o
  CC      x86_64-softmmu/target-i386/seg_helper.o
  CC      x86_64-softmmu/target-i386/mpx_helper.o
  CC      x86_64-softmmu/target-i386/gdbstub.o
  CC      x86_64-softmmu/target-i386/machine.o
  CC      x86_64-softmmu/target-i386/arch_memory_mapping.o
  CC      x86_64-softmmu/target-i386/arch_dump.o
  CC      aarch64-softmmu/hw/arm/highbank.o
  CC      aarch64-softmmu/hw/arm/digic_boards.o
  CC      aarch64-softmmu/hw/arm/integratorcp.o
  CC      aarch64-softmmu/hw/arm/mainstone.o
  CC      aarch64-softmmu/hw/arm/musicpal.o
  CC      x86_64-softmmu/target-i386/monitor.o
  CC      x86_64-softmmu/target-i386/kvm.o
  CC      x86_64-softmmu/target-i386/hyperv.o
  CC      aarch64-softmmu/hw/arm/nseries.o
  CC      aarch64-softmmu/hw/arm/omap_sx1.o
  CC      aarch64-softmmu/hw/arm/palm.o
  GEN     trace/generated-helpers.c
  CC      aarch64-softmmu/hw/arm/realview.o
  CC      x86_64-softmmu/trace/control-target.o
  CC      aarch64-softmmu/hw/arm/spitz.o
  CC      aarch64-softmmu/hw/arm/stellaris.o
  CC      aarch64-softmmu/hw/arm/tosa.o
  CC      aarch64-softmmu/hw/arm/versatilepb.o
  CC      aarch64-softmmu/hw/arm/vexpress.o
  CC      aarch64-softmmu/hw/arm/virt.o
  CC      aarch64-softmmu/hw/arm/xilinx_zynq.o
  CC      x86_64-softmmu/trace/generated-helpers.o
  CC      aarch64-softmmu/hw/arm/z2.o
  CC      aarch64-softmmu/hw/arm/virt-acpi-build.o
  CC      aarch64-softmmu/hw/arm/netduino2.o
  CC      aarch64-softmmu/hw/arm/sysbus-fdt.o
  CC      aarch64-softmmu/hw/arm/armv7m.o
  CC      aarch64-softmmu/hw/arm/exynos4210.o
  CC      aarch64-softmmu/hw/arm/pxa2xx.o
  CC      aarch64-softmmu/hw/arm/pxa2xx_gpio.o
  CC      aarch64-softmmu/hw/arm/pxa2xx_pic.o
  CC      aarch64-softmmu/hw/arm/digic.o
  CC      aarch64-softmmu/hw/arm/omap1.o
  CC      aarch64-softmmu/hw/arm/omap2.o
  CC      aarch64-softmmu/hw/arm/strongarm.o
  CC      aarch64-softmmu/hw/arm/allwinner-a10.o
  CC      aarch64-softmmu/hw/arm/cubieboard.o
  CC      aarch64-softmmu/hw/arm/bcm2835_peripherals.o
  CC      aarch64-softmmu/hw/arm/bcm2836.o
  CC      aarch64-softmmu/hw/arm/raspi.o
  CC      aarch64-softmmu/hw/arm/stm32f205_soc.o
  CC      aarch64-softmmu/hw/arm/xlnx-zynqmp.o
  CC      aarch64-softmmu/hw/arm/xlnx-ep108.o
  CC      aarch64-softmmu/hw/arm/fsl-imx25.o
  CC      aarch64-softmmu/hw/arm/imx25_pdk.o
  CC      aarch64-softmmu/hw/arm/fsl-imx31.o
  CC      aarch64-softmmu/hw/arm/kzm.o
  CC      aarch64-softmmu/hw/arm/fsl-imx6.o
  CC      aarch64-softmmu/hw/arm/sabrelite.o
  CC      aarch64-softmmu/hw/arm/aspeed_soc.o
  CC      aarch64-softmmu/hw/arm/aspeed.o
  CC      aarch64-softmmu/target-arm/arm-semi.o
  CC      aarch64-softmmu/target-arm/machine.o
  CC      aarch64-softmmu/target-arm/psci.o
  CC      aarch64-softmmu/target-arm/arch_dump.o
  CC      aarch64-softmmu/target-arm/monitor.o
  CC      aarch64-softmmu/target-arm/kvm-stub.o
  CC      aarch64-softmmu/target-arm/translate.o
  CC      aarch64-softmmu/target-arm/op_helper.o
  CC      aarch64-softmmu/target-arm/helper.o
  CC      aarch64-softmmu/target-arm/cpu.o
  CC      aarch64-softmmu/target-arm/neon_helper.o
  CC      aarch64-softmmu/target-arm/iwmmxt_helper.o
  CC      aarch64-softmmu/target-arm/gdbstub.o
  CC      aarch64-softmmu/target-arm/cpu64.o
  CC      aarch64-softmmu/target-arm/translate-a64.o
  CC      aarch64-softmmu/target-arm/helper-a64.o
  CC      aarch64-softmmu/target-arm/gdbstub64.o
  CC      aarch64-softmmu/target-arm/crypto_helper.o
  CC      aarch64-softmmu/target-arm/arm-powerctl.o
  GEN     trace/generated-helpers.c
  CC      aarch64-softmmu/trace/control-target.o
  CC      aarch64-softmmu/gdbstub-xml.o
  CC      aarch64-softmmu/trace/generated-helpers.o
  LINK    x86_64-softmmu/qemu-system-x86_64
../block/nbd.o: In function `nbd_config':
/tmp/qemu-test/src/block/nbd.c:263: undefined reference to `qdict_crumple'
collect2: ld returned 1 exit status
make[1]: *** [qemu-system-x86_64] Error 1
make: *** [subdir-x86_64-softmmu] Error 2
/tmp/qemu-test/src/target-arm/translate-a64.c: In function ‘handle_shri_with_rndacc’:
/tmp/qemu-test/src/target-arm/translate-a64.c:6333: warning: ‘tcg_src_hi’ may be used uninitialized in this function
/tmp/qemu-test/src/target-arm/translate-a64.c: In function ‘disas_simd_scalar_two_reg_misc’:
/tmp/qemu-test/src/target-arm/translate-a64.c:8060: warning: ‘rmode’ may be used uninitialized in this function
  LINK    aarch64-softmmu/qemu-system-aarch64
../block/nbd.o: In function `nbd_config':
/tmp/qemu-test/src/block/nbd.c:263: undefined reference to `qdict_crumple'
collect2: ld returned 1 exit status
make[1]: *** [qemu-system-aarch64] Error 1
make: *** [subdir-aarch64-softmmu] Error 2
tests/docker/Makefile.include:107: recipe for target 'docker-run-test-quick@centos6' failed
make: *** [docker-run-test-quick@centos6] Error 2
=== OUTPUT END ===

Test command exited with code: 2


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@freelists.org

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

[Ashijeet Acharya]

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

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

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 4815 bytes --]

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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 09/12] iotests.py: Add qemu_nbd function

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 1747 bytes --]

On 13.10.2016 15:11, Kevin Wolf wrote:
> Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/iotests.py | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>>
>> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
>> index 3329bc1..5a2678f 100644
>> --- a/tests/qemu-iotests/iotests.py
>> +++ b/tests/qemu-iotests/iotests.py
>> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
>>  if os.environ.get('QEMU_IO_OPTIONS'):
>>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
>>  
>> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
>> +if os.environ.get('QEMU_NBD_OPTIONS'):
>> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
>> +
>>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
>>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
>>  
>> @@ -87,6 +91,10 @@ def qemu_io(*args):
>>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
>>      return subp.communicate()[0]
>>  
>> +def qemu_nbd(*args):
>> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
>> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
> 
> Wouldn't it be better to always use -t, track the PID and shut it down
> explicitly when the test exits?

Probably. It's a lot more complicated, though. I'll see what I can do
but I'm not sure if I can do a lot before 2.8.

Max

> The way you're using qemu-nbd here is fine if the test case passes, but
> if it fails before we access the NBD server, the server keeps running in
> the background.
> 
> Kevin


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 12/12] iotests: Add test for NBD's blockdev-add interface

[Max Reitz]

[-- Attachment #1: Type: text/plain, Size: 4376 bytes --]

On 13.10.2016 15:26, Kevin Wolf wrote:
> Am 28.09.2016 um 22:56 hat Max Reitz geschrieben:
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>  tests/qemu-iotests/147     | 201 +++++++++++++++++++++++++++++++++++++++++++++
>>  tests/qemu-iotests/147.out |   5 ++
>>  tests/qemu-iotests/group   |   1 +
>>  3 files changed, 207 insertions(+)
>>  create mode 100755 tests/qemu-iotests/147
>>  create mode 100644 tests/qemu-iotests/147.out
>>
>> diff --git a/tests/qemu-iotests/147 b/tests/qemu-iotests/147
>> new file mode 100755
>> index 0000000..61e1e6f
>> --- /dev/null
>> +++ b/tests/qemu-iotests/147
>> @@ -0,0 +1,201 @@
>> +#!/usr/bin/env python
>> +#
>> +# Test case for NBD's blockdev-add interface
>> +#
>> +# Copyright (C) 2016 Red Hat, Inc.
>> +#
>> +# This program is free software; you can redistribute it and/or modify
>> +# it under the terms of the GNU General Public License as published by
>> +# the Free Software Foundation; either version 2 of the License, or
>> +# (at your option) any later version.
>> +#
>> +# This program is distributed in the hope that it will be useful,
>> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
>> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> +# GNU General Public License for more details.
>> +#
>> +# You should have received a copy of the GNU General Public License
>> +# along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> +#
>> +
>> +import os
>> +import socket
>> +import stat
>> +import time
>> +import iotests
>> +from iotests import cachemode, imgfmt, qemu_img, qemu_nbd
>> +
>> +NBD_PORT = 10811
>> +
>> +test_img = os.path.join(iotests.test_dir, 'test.img')
>> +
>> +class NBDBlockdevAddBase(iotests.QMPTestCase):
>> +    def blockdev_add_options(self, address, export=None):
>> +        options = { 'node-name': 'nbd-blockdev',
>> +                    'driver': 'raw',
>> +                    'file': {
>> +                        'driver': 'nbd',
>> +                        'address': address
>> +                    } }
>> +        if export is not None:
>> +            options['file']['export'] = export
>> +        return options
>> +
>> +    def client_test(self, filename, address, export=None):
>> +        bao = self.blockdev_add_options(address, export)
>> +        result = self.vm.qmp('blockdev-add', options=bao)
> 
> This needs a rebase (**bao instead of options=bao).
> 
>> +        self.assert_qmp(result, 'return', {})
>> +
>> +        result = self.vm.qmp('query-named-block-nodes')
>> +        for node in result['return']:
>> +            if node['node-name'] == 'nbd-blockdev':
>> +                self.assert_qmp(node, 'image/filename', filename)
>> +                break
>> +
>> +        result = self.vm.qmp('x-blockdev-del', node_name='nbd-blockdev')
>> +        self.assert_qmp(result, 'return', {})
>> +
>> +
>> +class QemuNBD(NBDBlockdevAddBase):
>> +    def setUp(self):
>> +        qemu_img('create', '-f', iotests.imgfmt, test_img, '64k')
>> +        self.vm = iotests.VM()
>> +        self.vm.launch()
>> +
>> +    def tearDown(self):
>> +        self.vm.shutdown()
>> +        os.remove(test_img)
>> +
>> +    def _server_up(self, *args):
>> +        self.assertEqual(qemu_nbd('-f', imgfmt, test_img, *args), 0)
>> +
>> +    def test_inet(self):
>> +        self._server_up('-p', str(NBD_PORT))
>> +        address = { 'type': 'inet',
>> +                    'data': {
>> +                        'host': 'localhost',
>> +                        'port': str(NBD_PORT)
>> +                    } }
>> +        self.client_test('nbd://localhost:%i' % NBD_PORT, address)
>> +
>> +    def test_unix(self):
>> +        socket = os.path.join(iotests.test_dir, 'qemu-nbd.socket')
>> +        self._server_up('-k', socket)
>> +        address = { 'type': 'unix',
>> +                    'data': { 'path': socket } }
>> +        self.client_test('nbd+unix://?socket=' + socket, address)
>> +        try:
>> +            os.remove(socket)
>> +        except OSError:
>> +            pass
> 
> If the test case fails, the socket file is leaked. We should probably
> either create and remove it in setUp()/tearDown(), or use a try/finally
> block around the test_unix code.

Yes, will do, thanks.

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 480 bytes --]

Re: [Qemu-devel] [PATCH v4 09/12] iotests.py: Add qemu_nbd function

[Kevin Wolf]

[-- Attachment #1: Type: text/plain, Size: 1830 bytes --]

Am 15.10.2016 um 19:17 hat Max Reitz geschrieben:
> On 13.10.2016 15:11, Kevin Wolf wrote:
> > Am 28.09.2016 um 22:55 hat Max Reitz geschrieben:
> >> Signed-off-by: Max Reitz <mreitz@redhat.com>
> >> ---
> >>  tests/qemu-iotests/iotests.py | 8 ++++++++
> >>  1 file changed, 8 insertions(+)
> >>
> >> diff --git a/tests/qemu-iotests/iotests.py b/tests/qemu-iotests/iotests.py
> >> index 3329bc1..5a2678f 100644
> >> --- a/tests/qemu-iotests/iotests.py
> >> +++ b/tests/qemu-iotests/iotests.py
> >> @@ -39,6 +39,10 @@ qemu_io_args = [os.environ.get('QEMU_IO_PROG', 'qemu-io')]
> >>  if os.environ.get('QEMU_IO_OPTIONS'):
> >>      qemu_io_args += os.environ['QEMU_IO_OPTIONS'].strip().split(' ')
> >>  
> >> +qemu_nbd_args = [os.environ.get('QEMU_NBD_PROG', 'qemu-nbd')]
> >> +if os.environ.get('QEMU_NBD_OPTIONS'):
> >> +    qemu_nbd_args += os.environ['QEMU_NBD_OPTIONS'].strip().split(' ')
> >> +
> >>  qemu_prog = os.environ.get('QEMU_PROG', 'qemu')
> >>  qemu_opts = os.environ.get('QEMU_OPTIONS', '').strip().split(' ')
> >>  
> >> @@ -87,6 +91,10 @@ def qemu_io(*args):
> >>          sys.stderr.write('qemu-io received signal %i: %s\n' % (-exitcode, ' '.join(args)))
> >>      return subp.communicate()[0]
> >>  
> >> +def qemu_nbd(*args):
> >> +    '''Run qemu-nbd in daemon mode and return the parent's exit code'''
> >> +    return subprocess.call(qemu_nbd_args + ['--fork'] + list(args))
> > 
> > Wouldn't it be better to always use -t, track the PID and shut it down
> > explicitly when the test exits?
> 
> Probably. It's a lot more complicated, though. I'll see what I can do
> but I'm not sure if I can do a lot before 2.8.

In that case, I'd prefer to have this series in 2.8 and improve the test
case later, so don't let this stop you from sending the next version.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]