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

Peter Xu posted 8 patches 7 years, 10 months ago
[Qemu-devel] [PATCH for-2.12 8/8] tests: qmp-test: add test for new "x-oob"
Posted by Peter Xu 7 years, 10 months ago
Test the new OOB capability. It's mostly the reverted OOB test, but
differs in that:

- It uses the new qtest_init_with_qmp_format() to create the monitor
  with the new monitor parameter "-mon 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>
---
 tests/qmp-test.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 84 insertions(+)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index 558e83540c..4d15b0fca5 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -134,6 +134,89 @@ 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;
+    const char *qmp_params = "-chardev socket,path=%s,nowait,id=char0 "
+                             "-mon chardev=char0,mode=control,x-oob=on";
+
+    qts = qtest_init_with_qmp_format(common_args, qmp_params);
+
+    /* Ignore 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 {
@@ -318,6 +401,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 for-2.12 8/8] tests: qmp-test: add test for new "x-oob"
Posted by Eric Blake 7 years, 10 months ago
On 03/26/2018 01:39 AM, Peter Xu wrote:
> Test the new OOB capability. It's mostly the reverted OOB test, but

Helpful to mention which commit id has the reverts you are restoring. 
Looks like 4fd78ad. Are you also restoring the tests reverted in cc79760 
and a4f9092?

> differs in that:
> 
> - It uses the new qtest_init_with_qmp_format() to create the monitor
>    with the new monitor parameter "-mon 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>
> ---
>   tests/qmp-test.c | 84 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 84 insertions(+)
> 

> +++ b/tests/qmp-test.c
> @@ -134,6 +134,89 @@ 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;
> +    const char *qmp_params = "-chardev socket,path=%s,nowait,id=char0 "
> +                             "-mon chardev=char0,mode=control,x-oob=on";

Again, I'd rather fold this into 7/8 qtest_init_without_qmp_handshake by 
changing "-qmp unix:%s,nowait" into "-chardev 
socket,path=%s,nowait,id=char0 -mon chardev=char0,mode=control" for ALL 
tests (as that always works), then have a bool that decides whether we 
also append ",x-oob=on" as needed.

> +
> +    qts = qtest_init_with_qmp_format(common_args, qmp_params);
> +
> +    /* Ignore the greeting message. */

Comment is wrong - you aren't ignoring it, but...

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

...actually inspecting what it contains.

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

In the interest of getting this in -rc1, I will probably try and make 
the changes I've mentioned in this thread, and post a v2 along those 
lines; if you like my changes, I can send the pull request on my Tuesday 
morning instead of waiting for another round of back-and-forth.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org