[Qemu-devel] [PATCH v2 0/4] Monitor: OOB related patches

[Qemu-devel] [PATCH v2 0/4] Monitor: OOB related patches

[Eric Blake]

This is patches 2, 3, 7, and 8 (with 7 rewritten) from Peter Xu's
series.  I've pushed them to git://repo.or.cz/qemu/ericb.git qapi-next
(on top of the other pending qapi patches at branch qapi); hopefully
this will be part of my pull request tomorrow for -rc1.

Eric Blake (1):
  tests: Add parameter to qtest_init_without_qmp_handshake

Peter Xu (3):
  qmp: cleanup qmp queues properly
  monitor: new parameter "x-oob"
  tests: qmp-test: add test for new "x-oob"

 tests/libqtest.h          |  7 +++-
 include/monitor/monitor.h |  1 +
 vl.c                      |  5 +++
 tests/libqtest.c          | 10 +++--
 monitor.c                 | 94 ++++++++++++++++++++++++++++++++++++-----------
 tests/qmp-test.c          | 84 +++++++++++++++++++++++++++++++++++++++++-
 6 files changed, 173 insertions(+), 28 deletions(-)

-- 
2.14.3

[Qemu-devel] [PATCH v2 1/4] qmp: cleanup qmp queues properly

[Eric Blake]

From: Peter Xu <peterx@redhat.com>

Marc-André Lureau reported that we can have this happen:

1. client1 connects, send command C1
2. client1 disconnects before getting response for C1
3. client2 connects, who might receive response of C1

However client2 should not receive remaining responses for client1.

Basically, we should clean up the request/response queue elements when:

- before a session established
- after a session is closed
- before destroying the queues

Some helpers are introduced to achieve that.  We need to make sure we're
with the lock when operating on those queues.

Reported-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-3-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: drop pointless qmp_response_free()]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 monitor.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++-----------------
 1 file changed, 53 insertions(+), 19 deletions(-)

diff --git a/monitor.c b/monitor.c
index 3b1ef34711b..8fa18936419 100644
--- a/monitor.c
+++ b/monitor.c
@@ -234,6 +234,22 @@ static struct {
     QEMUBH *qmp_respond_bh;
 } mon_global;

+struct QMPRequest {
+    /* Owner of the request */
+    Monitor *mon;
+    /* "id" field of the request */
+    QObject *id;
+    /* Request object to be handled */
+    QObject *req;
+    /*
+     * Whether we need to resume the monitor afterward.  This flag is
+     * used to emulate the old QMP server behavior that the current
+     * command must be completed before execution of the next one.
+     */
+    bool need_resume;
+};
+typedef struct QMPRequest QMPRequest;
+
 /* QMP checker flags */
 #define QMP_ACCEPT_UNKNOWNS 1

@@ -310,6 +326,38 @@ int monitor_read_password(Monitor *mon, ReadLineFunc *readline_func,
     }
 }

+static void qmp_request_free(QMPRequest *req)
+{
+    qobject_decref(req->id);
+    qobject_decref(req->req);
+    g_free(req);
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_req_queue_locked(Monitor *mon)
+{
+    while (!g_queue_is_empty(mon->qmp.qmp_requests)) {
+        qmp_request_free(g_queue_pop_head(mon->qmp.qmp_requests));
+    }
+}
+
+/* Must with the mon->qmp.qmp_queue_lock held */
+static void monitor_qmp_cleanup_resp_queue_locked(Monitor *mon)
+{
+    while (!g_queue_is_empty(mon->qmp.qmp_responses)) {
+        qobject_decref(g_queue_pop_head(mon->qmp.qmp_responses));
+    }
+}
+
+static void monitor_qmp_cleanup_queues(Monitor *mon)
+{
+    qemu_mutex_lock(&mon->qmp.qmp_queue_lock);
+    monitor_qmp_cleanup_req_queue_locked(mon);
+    monitor_qmp_cleanup_resp_queue_locked(mon);
+    qemu_mutex_unlock(&mon->qmp.qmp_queue_lock);
+}
+
+
 static void monitor_flush_locked(Monitor *mon);

 static gboolean monitor_unblocked(GIOChannel *chan, GIOCondition cond,
@@ -701,6 +749,8 @@ static void monitor_data_destroy(Monitor *mon)
     QDECREF(mon->outbuf);
     qemu_mutex_destroy(&mon->out_lock);
     qemu_mutex_destroy(&mon->qmp.qmp_queue_lock);
+    monitor_qmp_cleanup_req_queue_locked(mon);
+    monitor_qmp_cleanup_resp_queue_locked(mon);
     g_queue_free(mon->qmp.qmp_requests);
     g_queue_free(mon->qmp.qmp_responses);
 }
@@ -4009,22 +4059,6 @@ static void monitor_qmp_respond(Monitor *mon, QObject *rsp,
     qobject_decref(rsp);
 }

-struct QMPRequest {
-    /* Owner of the request */
-    Monitor *mon;
-    /* "id" field of the request */
-    QObject *id;
-    /* Request object to be handled */
-    QObject *req;
-    /*
-     * Whether we need to resume the monitor afterward.  This flag is
-     * used to emulate the old QMP server behavior that the current
-     * command must be completed before execution of the next one.
-     */
-    bool need_resume;
-};
-typedef struct QMPRequest QMPRequest;
-
 /*
  * Dispatch one single QMP request. The function will free the req_obj
  * and objects inside it before return.
@@ -4191,9 +4225,7 @@ static void handle_qmp_command(JSONMessageParser *parser, GQueue *tokens)
             qapi_event_send_command_dropped(id,
                                             COMMAND_DROP_REASON_QUEUE_FULL,
                                             &error_abort);
-            qobject_decref(id);
-            qobject_decref(req);
-            g_free(req_obj);
+            qmp_request_free(req_obj);
             return;
         }
     }
@@ -4327,6 +4359,7 @@ static void monitor_qmp_event(void *opaque, int event)

     switch (event) {
     case CHR_EVENT_OPENED:
+        monitor_qmp_cleanup_queues(mon);
         mon->qmp.commands = &qmp_cap_negotiation_commands;
         monitor_qmp_caps_reset(mon);
         data = get_qmp_greeting(mon);
@@ -4335,6 +4368,7 @@ static void monitor_qmp_event(void *opaque, int event)
         mon_refcount++;
         break;
     case CHR_EVENT_CLOSED:
+        monitor_qmp_cleanup_queues(mon);
         json_message_parser_destroy(&mon->qmp.parser);
         json_message_parser_init(&mon->qmp.parser, handle_qmp_command);
         mon_refcount--;
-- 
2.14.3

[Qemu-devel] [PATCH v2 2/4] monitor: new parameter "x-oob"

[Eric Blake]

From: Peter Xu <peterx@redhat.com>

Add new parameter to optionally enable Out-Of-Band for a QMP server.

An example command line:

  ./qemu-system-x86_64 -chardev stdio,id=char0 \
                       -mon chardev=char0,mode=control,x-oob=on

By default, Out-Of-Band is off.

It is not allowed if either MUX or non-QMP is detected, since
Out-Of-Band is currently only for QMP, and non-MUX chardev backends.

Note that the client STILL has to request 'oob' during qmp_capabilities;
in part because the x-oob command line option may disappear in the
future if we decide the capabilities negotiation is sufficient.

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-4-peterx@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[eblake: enhance commit message]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 include/monitor/monitor.h |  1 +
 vl.c                      |  5 +++++
 monitor.c                 | 22 ++++++++++++++++++++--
 3 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 0cb0538a315..d6ab70cae23 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -13,6 +13,7 @@ extern Monitor *cur_mon;
 #define MONITOR_USE_READLINE  0x02
 #define MONITOR_USE_CONTROL   0x04
 #define MONITOR_USE_PRETTY    0x08
+#define MONITOR_USE_OOB       0x10

 bool monitor_cur_is_qmp(void);

diff --git a/vl.c b/vl.c
index c81cc866079..5fd01bd5f69 100644
--- a/vl.c
+++ b/vl.c
@@ -2404,6 +2404,11 @@ static int mon_init_func(void *opaque, QemuOpts *opts, Error **errp)
     if (qemu_opt_get_bool(opts, "pretty", 0))
         flags |= MONITOR_USE_PRETTY;

+    /* OOB is off by default */
+    if (qemu_opt_get_bool(opts, "x-oob", 0)) {
+        flags |= MONITOR_USE_OOB;
+    }
+
     chardev = qemu_opt_get(opts, "chardev");
     chr = qemu_chr_find(chardev);
     if (chr == NULL) {
diff --git a/monitor.c b/monitor.c
index 8fa18936419..e11724eec24 100644
--- a/monitor.c
+++ b/monitor.c
@@ -36,6 +36,7 @@
 #include "net/slirp.h"
 #include "chardev/char-fe.h"
 #include "chardev/char-io.h"
+#include "chardev/char-mux.h"
 #include "ui/qemu-spice.h"
 #include "sysemu/numa.h"
 #include "monitor/monitor.h"
@@ -4563,12 +4564,26 @@ static void monitor_qmp_setup_handlers_bh(void *opaque)
 void monitor_init(Chardev *chr, int flags)
 {
     Monitor *mon = g_malloc(sizeof(*mon));
+    bool use_readline = flags & MONITOR_USE_READLINE;
+    bool use_oob = flags & MONITOR_USE_OOB;

-    monitor_data_init(mon, false, false);
+    if (use_oob) {
+        if (CHARDEV_IS_MUX(chr)) {
+            error_report("Monitor Out-Of-Band is not supported with "
+                         "MUX typed chardev backend");
+            exit(1);
+        }
+        if (use_readline) {
+            error_report("Monitor Out-Of-band is only supported by QMP");
+            exit(1);
+        }
+    }
+
+    monitor_data_init(mon, false, use_oob);

     qemu_chr_fe_init(&mon->chr, chr, &error_abort);
     mon->flags = flags;
-    if (flags & MONITOR_USE_READLINE) {
+    if (use_readline) {
         mon->rs = readline_init(monitor_readline_printf,
                                 monitor_readline_flush,
                                 mon,
@@ -4664,6 +4679,9 @@ QemuOptsList qemu_mon_opts = {
         },{
             .name = "pretty",
             .type = QEMU_OPT_BOOL,
+        },{
+            .name = "x-oob",
+            .type = QEMU_OPT_BOOL,
         },
         { /* end of list */ }
     },
-- 
2.14.3

[Qemu-devel] [PATCH v2 3/4] tests: Add parameter to qtest_init_without_qmp_handshake

[Eric Blake]

Allow callers to choose whether to allow OOB support during a test;
for now, all existing callers pass false, but the next patch will
add a new caller.  Also, rewrite the monitor setup to be generic
(using the -qmp shorthand is insufficient for honoring the parameter).

Based on an idea by Peter Xu, in <20180326063901.27425-8-peterx@redhat.com>

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/libqtest.h |  7 +++++--
 tests/libqtest.c | 10 ++++++----
 tests/qmp-test.c |  2 +-
 3 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/tests/libqtest.h b/tests/libqtest.h
index 811169453ab..cbe8df44730 100644
--- a/tests/libqtest.h
+++ b/tests/libqtest.h
@@ -56,11 +56,14 @@ QTestState *qtest_init(const char *extra_args);

 /**
  * qtest_init_without_qmp_handshake:
- * @extra_args: other arguments to pass to QEMU.
+ * @use_oob: true to have the server advertise OOB support
+ * @extra_args: other arguments to pass to QEMU.  CAUTION: these
+ * arguments are subject to word splitting and shell evaluation.
  *
  * Returns: #QTestState instance.
  */
-QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
+QTestState *qtest_init_without_qmp_handshake(bool use_oob,
+                                             const char *extra_args);

 /**
  * qtest_quit:
diff --git a/tests/libqtest.c b/tests/libqtest.c
index 200b2b9e92a..6f33a37667c 100644
--- a/tests/libqtest.c
+++ b/tests/libqtest.c
@@ -166,7 +166,8 @@ static const char *qtest_qemu_binary(void)
     return qemu_bin;
 }

-QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
+QTestState *qtest_init_without_qmp_handshake(bool use_oob,
+                                             const char *extra_args)
 {
     QTestState *s;
     int sock, qmpsock, i;
@@ -199,12 +200,13 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
         command = g_strdup_printf("exec %s "
                                   "-qtest unix:%s,nowait "
                                   "-qtest-log %s "
-                                  "-qmp unix:%s,nowait "
+                                  "-chardev socket,path=%s,nowait,id=char0 "
+                                  "-mon chardev=char0,mode=control%s "
                                   "-machine accel=qtest "
                                   "-display none "
                                   "%s", qemu_binary, socket_path,
                                   getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
-                                  qmp_socket_path,
+                                  qmp_socket_path, use_oob ? ",x-oob=on" : "",
                                   extra_args ?: "");
         execlp("/bin/sh", "sh", "-c", command, NULL);
         exit(1);
@@ -239,7 +241,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)

 QTestState *qtest_init(const char *extra_args)
 {
-    QTestState *s = qtest_init_without_qmp_handshake(extra_args);
+    QTestState *s = qtest_init_without_qmp_handshake(false, extra_args);

     /* Read the QMP greeting and then do the handshake */
     qtest_qmp_discard_response(s, "");
diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 8de99a4727e..2134d95db97 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -81,7 +81,7 @@ static void test_qmp_protocol(void)
     QList *capabilities;
     QTestState *qts;

-    qts = qtest_init_without_qmp_handshake(common_args);
+    qts = qtest_init_without_qmp_handshake(false, common_args);

     /* Test greeting */
     resp = qtest_qmp_receive(qts);
-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 3/4] tests: Add parameter to qtest_init_without_qmp_handshake

[Marc-André Lureau]

On Tue, Mar 27, 2018 at 3:36 AM, Eric Blake <eblake@redhat.com> wrote:
> Allow callers to choose whether to allow OOB support during a test;
> for now, all existing callers pass false, but the next patch will
> add a new caller.  Also, rewrite the monitor setup to be generic
> (using the -qmp shorthand is insufficient for honoring the parameter).
>
> Based on an idea by Peter Xu, in <20180326063901.27425-8-peterx@redhat.com>
>
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/libqtest.h |  7 +++++--
>  tests/libqtest.c | 10 ++++++----
>  tests/qmp-test.c |  2 +-
>  3 files changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/tests/libqtest.h b/tests/libqtest.h
> index 811169453ab..cbe8df44730 100644
> --- a/tests/libqtest.h
> +++ b/tests/libqtest.h
> @@ -56,11 +56,14 @@ QTestState *qtest_init(const char *extra_args);
>
>  /**
>   * qtest_init_without_qmp_handshake:
> - * @extra_args: other arguments to pass to QEMU.
> + * @use_oob: true to have the server advertise OOB support
> + * @extra_args: other arguments to pass to QEMU.  CAUTION: these
> + * arguments are subject to word splitting and shell evaluation.
>   *
>   * Returns: #QTestState instance.
>   */
> -QTestState *qtest_init_without_qmp_handshake(const char *extra_args);
> +QTestState *qtest_init_without_qmp_handshake(bool use_oob,
> +                                             const char *extra_args);
>
>  /**
>   * qtest_quit:
> diff --git a/tests/libqtest.c b/tests/libqtest.c
> index 200b2b9e92a..6f33a37667c 100644
> --- a/tests/libqtest.c
> +++ b/tests/libqtest.c
> @@ -166,7 +166,8 @@ static const char *qtest_qemu_binary(void)
>      return qemu_bin;
>  }
>
> -QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
> +QTestState *qtest_init_without_qmp_handshake(bool use_oob,
> +                                             const char *extra_args)
>  {
>      QTestState *s;
>      int sock, qmpsock, i;
> @@ -199,12 +200,13 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>          command = g_strdup_printf("exec %s "
>                                    "-qtest unix:%s,nowait "
>                                    "-qtest-log %s "
> -                                  "-qmp unix:%s,nowait "
> +                                  "-chardev socket,path=%s,nowait,id=char0 "
> +                                  "-mon chardev=char0,mode=control%s "
>                                    "-machine accel=qtest "
>                                    "-display none "
>                                    "%s", qemu_binary, socket_path,
>                                    getenv("QTEST_LOG") ? "/dev/fd/2" : "/dev/null",
> -                                  qmp_socket_path,
> +                                  qmp_socket_path, use_oob ? ",x-oob=on" : "",
>                                    extra_args ?: "");
>          execlp("/bin/sh", "sh", "-c", command, NULL);
>          exit(1);
> @@ -239,7 +241,7 @@ QTestState *qtest_init_without_qmp_handshake(const char *extra_args)
>
>  QTestState *qtest_init(const char *extra_args)
>  {
> -    QTestState *s = qtest_init_without_qmp_handshake(extra_args);
> +    QTestState *s = qtest_init_without_qmp_handshake(false, extra_args);
>
>      /* Read the QMP greeting and then do the handshake */
>      qtest_qmp_discard_response(s, "");
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index 8de99a4727e..2134d95db97 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -81,7 +81,7 @@ static void test_qmp_protocol(void)
>      QList *capabilities;
>      QTestState *qts;
>
> -    qts = qtest_init_without_qmp_handshake(common_args);
> +    qts = qtest_init_without_qmp_handshake(false, common_args);
>
>      /* Test greeting */
>      resp = qtest_qmp_receive(qts);
> --
> 2.14.3
>

[Qemu-devel] [PATCH v2 4/4] tests: qmp-test: add test for new "x-oob"

[Eric Blake]

From: Peter Xu <peterx@redhat.com>

Test the new OOB capability. It's mostly the reverted OOB test
(see commit 4fd78ad7), but differs in that:

- It uses the new qtest_init_without_qmp_handshake() parameter to
  create the monitor with "x-oob"
- Squashed the capability tests on greeting message
- Don't use qtest_global any more, instead use self-maintained
  QTestState, which is the trend

Signed-off-by: Peter Xu <peterx@redhat.com>
Message-Id: <20180326063901.27425-9-peterx@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
[eblake: rebase to qtest_init changes]
Signed-off-by: Eric Blake <eblake@redhat.com>
---
 tests/qmp-test.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 82 insertions(+)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 2134d95db97..772058fc4c4 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -135,6 +135,87 @@ static void test_qmp_protocol(void)
     qtest_quit(qts);
 }

+/* Tests for Out-Of-Band support. */
+static void test_qmp_oob(void)
+{
+    QTestState *qts;
+    QDict *resp, *q;
+    int acks = 0;
+    const QListEntry *entry;
+    QList *capabilities;
+    QString *qstr;
+    const char *cmd_id;
+
+    qts = qtest_init_without_qmp_handshake(true, common_args);
+
+    /* Check the greeting message. */
+    resp = qtest_qmp_receive(qts);
+    q = qdict_get_qdict(resp, "QMP");
+    g_assert(q);
+    capabilities = qdict_get_qlist(q, "capabilities");
+    g_assert(capabilities && !qlist_empty(capabilities));
+    entry = qlist_first(capabilities);
+    g_assert(entry);
+    qstr = qobject_to(QString, entry->value);
+    g_assert(qstr);
+    g_assert_cmpstr(qstring_get_str(qstr), ==, "oob");
+    QDECREF(resp);
+
+    /* Try a fake capability, it should fail. */
+    resp = qtest_qmp(qts,
+                     "{ 'execute': 'qmp_capabilities', "
+                     "  'arguments': { 'enable': [ 'cap-does-not-exist' ] } }");
+    g_assert(qdict_haskey(resp, "error"));
+    QDECREF(resp);
+
+    /* Now, enable OOB in current QMP session, it should succeed. */
+    resp = qtest_qmp(qts,
+                     "{ 'execute': 'qmp_capabilities', "
+                     "  'arguments': { 'enable': [ 'oob' ] } }");
+    g_assert(qdict_haskey(resp, "return"));
+    QDECREF(resp);
+
+    /*
+     * Try any command that does not support OOB but with OOB flag. We
+     * should get failure.
+     */
+    resp = qtest_qmp(qts,
+                     "{ 'execute': 'query-cpus',"
+                     "  'control': { 'run-oob': true } }");
+    g_assert(qdict_haskey(resp, "error"));
+    QDECREF(resp);
+
+    /*
+     * First send the "x-oob-test" command with lock=true and
+     * oob=false, it should hang the dispatcher and main thread;
+     * later, we send another lock=false with oob=true to continue
+     * that thread processing.  Finally we should receive replies from
+     * both commands.
+     */
+    qtest_async_qmp(qts,
+                    "{ 'execute': 'x-oob-test',"
+                    "  'arguments': { 'lock': true }, "
+                    "  'id': 'lock-cmd'}");
+    qtest_async_qmp(qts,
+                    "{ 'execute': 'x-oob-test', "
+                    "  'arguments': { 'lock': false }, "
+                    "  'control': { 'run-oob': true }, "
+                    "  'id': 'unlock-cmd' }");
+
+    /* Ignore all events.  Wait for 2 acks */
+    while (acks < 2) {
+        resp = qtest_qmp_receive(qts);
+        cmd_id = qdict_get_str(resp, "id");
+        if (!g_strcmp0(cmd_id, "lock-cmd") ||
+            !g_strcmp0(cmd_id, "unlock-cmd")) {
+            acks++;
+        }
+        QDECREF(resp);
+    }
+
+    qtest_quit(qts);
+}
+
 static int query_error_class(const char *cmd)
 {
     static struct {
@@ -319,6 +400,7 @@ int main(int argc, char *argv[])
     g_test_init(&argc, &argv, NULL);

     qtest_add_func("qmp/protocol", test_qmp_protocol);
+    qtest_add_func("qmp/oob", test_qmp_oob);
     qmp_schema_init(&schema);
     add_query_tests(&schema);

-- 
2.14.3

Re: [Qemu-devel] [PATCH v2 4/4] tests: qmp-test: add test for new "x-oob"

[Marc-André Lureau]

On Tue, Mar 27, 2018 at 3:36 AM, Eric Blake <eblake@redhat.com> wrote:
> From: Peter Xu <peterx@redhat.com>
>
> Test the new OOB capability. It's mostly the reverted OOB test
> (see commit 4fd78ad7), but differs in that:
>
> - It uses the new qtest_init_without_qmp_handshake() parameter to
>   create the monitor with "x-oob"
> - Squashed the capability tests on greeting message
> - Don't use qtest_global any more, instead use self-maintained
>   QTestState, which is the trend
>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> Message-Id: <20180326063901.27425-9-peterx@redhat.com>
> Reviewed-by: Eric Blake <eblake@redhat.com>
> [eblake: rebase to qtest_init changes]
> Signed-off-by: Eric Blake <eblake@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  tests/qmp-test.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
>
> diff --git a/tests/qmp-test.c b/tests/qmp-test.c
> index 2134d95db97..772058fc4c4 100644
> --- a/tests/qmp-test.c
> +++ b/tests/qmp-test.c
> @@ -135,6 +135,87 @@ static void test_qmp_protocol(void)
>      qtest_quit(qts);
>  }
>
> +/* Tests for Out-Of-Band support. */
> +static void test_qmp_oob(void)
> +{
> +    QTestState *qts;
> +    QDict *resp, *q;
> +    int acks = 0;
> +    const QListEntry *entry;
> +    QList *capabilities;
> +    QString *qstr;
> +    const char *cmd_id;
> +
> +    qts = qtest_init_without_qmp_handshake(true, common_args);
> +
> +    /* Check the greeting message. */
> +    resp = qtest_qmp_receive(qts);
> +    q = qdict_get_qdict(resp, "QMP");
> +    g_assert(q);
> +    capabilities = qdict_get_qlist(q, "capabilities");
> +    g_assert(capabilities && !qlist_empty(capabilities));
> +    entry = qlist_first(capabilities);
> +    g_assert(entry);
> +    qstr = qobject_to(QString, entry->value);
> +    g_assert(qstr);
> +    g_assert_cmpstr(qstring_get_str(qstr), ==, "oob");
> +    QDECREF(resp);
> +
> +    /* Try a fake capability, it should fail. */
> +    resp = qtest_qmp(qts,
> +                     "{ 'execute': 'qmp_capabilities', "
> +                     "  'arguments': { 'enable': [ 'cap-does-not-exist' ] } }");
> +    g_assert(qdict_haskey(resp, "error"));
> +    QDECREF(resp);
> +
> +    /* Now, enable OOB in current QMP session, it should succeed. */
> +    resp = qtest_qmp(qts,
> +                     "{ 'execute': 'qmp_capabilities', "
> +                     "  'arguments': { 'enable': [ 'oob' ] } }");
> +    g_assert(qdict_haskey(resp, "return"));
> +    QDECREF(resp);
> +
> +    /*
> +     * Try any command that does not support OOB but with OOB flag. We
> +     * should get failure.
> +     */
> +    resp = qtest_qmp(qts,
> +                     "{ 'execute': 'query-cpus',"
> +                     "  'control': { 'run-oob': true } }");
> +    g_assert(qdict_haskey(resp, "error"));
> +    QDECREF(resp);
> +
> +    /*
> +     * First send the "x-oob-test" command with lock=true and
> +     * oob=false, it should hang the dispatcher and main thread;
> +     * later, we send another lock=false with oob=true to continue
> +     * that thread processing.  Finally we should receive replies from
> +     * both commands.
> +     */
> +    qtest_async_qmp(qts,
> +                    "{ 'execute': 'x-oob-test',"
> +                    "  'arguments': { 'lock': true }, "
> +                    "  'id': 'lock-cmd'}");
> +    qtest_async_qmp(qts,
> +                    "{ 'execute': 'x-oob-test', "
> +                    "  'arguments': { 'lock': false }, "
> +                    "  'control': { 'run-oob': true }, "
> +                    "  'id': 'unlock-cmd' }");
> +
> +    /* Ignore all events.  Wait for 2 acks */
> +    while (acks < 2) {
> +        resp = qtest_qmp_receive(qts);
> +        cmd_id = qdict_get_str(resp, "id");
> +        if (!g_strcmp0(cmd_id, "lock-cmd") ||
> +            !g_strcmp0(cmd_id, "unlock-cmd")) {
> +            acks++;
> +        }
> +        QDECREF(resp);
> +    }
> +
> +    qtest_quit(qts);
> +}
> +
>  static int query_error_class(const char *cmd)
>  {
>      static struct {
> @@ -319,6 +400,7 @@ int main(int argc, char *argv[])
>      g_test_init(&argc, &argv, NULL);
>
>      qtest_add_func("qmp/protocol", test_qmp_protocol);
> +    qtest_add_func("qmp/oob", test_qmp_oob);
>      qmp_schema_init(&schema);
>      add_query_tests(&schema);
>
> --
> 2.14.3
>

Re: [Qemu-devel] [PATCH v2 0/4] Monitor: OOB related patches

[Peter Xu]

On Mon, Mar 26, 2018 at 08:36:16PM -0500, Eric Blake wrote:
> This is patches 2, 3, 7, and 8 (with 7 rewritten) from Peter Xu's
> series.  I've pushed them to git://repo.or.cz/qemu/ericb.git qapi-next
> (on top of the other pending qapi patches at branch qapi); hopefully
> this will be part of my pull request tomorrow for -rc1.
> 
> Eric Blake (1):
>   tests: Add parameter to qtest_init_without_qmp_handshake
> 
> Peter Xu (3):
>   qmp: cleanup qmp queues properly
>   monitor: new parameter "x-oob"
>   tests: qmp-test: add test for new "x-oob"
> 
>  tests/libqtest.h          |  7 +++-
>  include/monitor/monitor.h |  1 +
>  vl.c                      |  5 +++
>  tests/libqtest.c          | 10 +++--
>  monitor.c                 | 94 ++++++++++++++++++++++++++++++++++++-----------
>  tests/qmp-test.c          | 84 +++++++++++++++++++++++++++++++++++++++++-
>  6 files changed, 173 insertions(+), 28 deletions(-)

All of them look good to me.

Thanks Eric!

-- 
Peter Xu

Re: [Qemu-devel] [PATCH v2 0/4] Monitor: OOB related patches

[Christian Borntraeger]

On 03/27/2018 03:36 AM, Eric Blake wrote:
> This is patches 2, 3, 7, and 8 (with 7 rewritten) from Peter Xu's
> series.  I've pushed them to git://repo.or.cz/qemu/ericb.git qapi-next

that branch seems to work fine. (no make check or iotest regression on s390)





> (on top of the other pending qapi patches at branch qapi); hopefully
> this will be part of my pull request tomorrow for -rc1.
> 
> Eric Blake (1):
>   tests: Add parameter to qtest_init_without_qmp_handshake
> 
> Peter Xu (3):
>   qmp: cleanup qmp queues properly
>   monitor: new parameter "x-oob"
>   tests: qmp-test: add test for new "x-oob"
> 
>  tests/libqtest.h          |  7 +++-
>  include/monitor/monitor.h |  1 +
>  vl.c                      |  5 +++
>  tests/libqtest.c          | 10 +++--
>  monitor.c                 | 94 ++++++++++++++++++++++++++++++++++++-----------
>  tests/qmp-test.c          | 84 +++++++++++++++++++++++++++++++++++++++++-
>  6 files changed, 173 insertions(+), 28 deletions(-)
>