[Qemu-devel] [PATCH v8 23/23] tests: qmp-test: add oob test

Peter Xu posted 23 patches 7 years, 11 months ago
[Qemu-devel] [PATCH v8 23/23] tests: qmp-test: add oob test
Posted by Peter Xu 7 years, 11 months ago
Test the new OOB capability.  Here we used the new "x-oob-test" command.
Firstly, we send a lock=true and oob=false command to hang the main
thread.  Then send another lock=false and oob=true command (which will
be run inside parser this time) to free that hanged command.

Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Peter Xu <peterx@redhat.com>
---
 tests/qmp-test.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/tests/qmp-test.c b/tests/qmp-test.c
index caf3ec4a78..4c591cbc80 100644
--- a/tests/qmp-test.c
+++ b/tests/qmp-test.c
@@ -164,6 +164,70 @@ static void test_qmp_protocol(void)
     qtest_quit(qts);
 }
 
+/* Tests for Out-Of-Band support. */
+static void test_qmp_oob(void)
+{
+    QDict *resp;
+    int acks = 0;
+    const char *cmd_id;
+
+    global_qtest = qtest_init_without_qmp_handshake(common_args);
+
+    /* Ignore the greeting message. */
+    resp = qmp_receive();
+    g_assert(qdict_get_qdict(resp, "QMP"));
+    QDECREF(resp);
+
+    /* Try a fake capability, it should fail. */
+    resp = qmp("{ '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 success. */
+    resp = qmp("{ '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 = qmp("{ 'execute': 'query-cpus',"
+               "  'control': { 'run-oob': true } }");
+    g_assert(qdict_haskey(resp, "error"));
+    QDECREF(resp);
+
+    /*
+     * Firstly 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.
+     */
+    qmp_async("{ 'execute': 'x-oob-test',"
+              "  'arguments': { 'lock': true }, "
+              "  'id': 'lock-cmd'}");
+    qmp_async("{ '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 = qmp_receive();
+        cmd_id = qdict_get_str(resp, "id");
+        if (!g_strcmp0(cmd_id, "lock-cmd") ||
+            !g_strcmp0(cmd_id, "unlock-cmd")) {
+            acks++;
+        }
+        QDECREF(resp);
+    }
+
+    qtest_end();
+}
+
 static int query_error_class(const char *cmd)
 {
     static struct {
@@ -343,6 +407,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 v8 23/23] tests: qmp-test: add oob test
Posted by Eric Blake 7 years, 11 months ago
On 03/09/2018 03:00 AM, Peter Xu wrote:
> Test the new OOB capability.  Here we used the new "x-oob-test" command.
> Firstly, we send a lock=true and oob=false command to hang the main

s/Firstly/First/

> thread.  Then send another lock=false and oob=true command (which will
> be run inside parser this time) to free that hanged command.
> 
> Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> Signed-off-by: Peter Xu <peterx@redhat.com>
> ---
>   tests/qmp-test.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 65 insertions(+)
> 

> +    /* Now, enable OOB in current QMP session, it should success. */

s/success/succeed/

> +
> +    /*
> +     * Firstly send the "x-oob-test" command with lock=true and

s/Firstly/First/

> +     * 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.
> +     */
> +    qmp_async("{ 'execute': 'x-oob-test',"
> +              "  'arguments': { 'lock': true }, "
> +              "  'id': 'lock-cmd'}");
> +    qmp_async("{ '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 = qmp_receive();
> +        cmd_id = qdict_get_str(resp, "id");
> +        if (!g_strcmp0(cmd_id, "lock-cmd") ||
> +            !g_strcmp0(cmd_id, "unlock-cmd")) {
> +            acks++;
> +        }
> +        QDECREF(resp);
> +    }

Can you make the reply order deterministic?  Perhaps by having the lock 
command insert a sleep after locking but before replying, so that the 
unlock always gets to reply first?  But that can be a followup.

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

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

Re: [Qemu-devel] [PATCH v8 23/23] tests: qmp-test: add oob test
Posted by Peter Xu 7 years, 11 months ago
On Sat, Mar 10, 2018 at 08:49:42PM -0600, Eric Blake wrote:
> On 03/09/2018 03:00 AM, Peter Xu wrote:
> > Test the new OOB capability.  Here we used the new "x-oob-test" command.
> > Firstly, we send a lock=true and oob=false command to hang the main
> 
> s/Firstly/First/
> 
> > thread.  Then send another lock=false and oob=true command (which will
> > be run inside parser this time) to free that hanged command.
> > 
> > Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
> > Signed-off-by: Peter Xu <peterx@redhat.com>
> > ---
> >   tests/qmp-test.c | 65 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 65 insertions(+)
> > 
> 
> > +    /* Now, enable OOB in current QMP session, it should success. */
> 
> s/success/succeed/
> 
> > +
> > +    /*
> > +     * Firstly send the "x-oob-test" command with lock=true and
> 
> s/Firstly/First/
> 
> > +     * 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.
> > +     */
> > +    qmp_async("{ 'execute': 'x-oob-test',"
> > +              "  'arguments': { 'lock': true }, "
> > +              "  'id': 'lock-cmd'}");
> > +    qmp_async("{ '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 = qmp_receive();
> > +        cmd_id = qdict_get_str(resp, "id");
> > +        if (!g_strcmp0(cmd_id, "lock-cmd") ||
> > +            !g_strcmp0(cmd_id, "unlock-cmd")) {
> > +            acks++;
> > +        }
> > +        QDECREF(resp);
> > +    }
> 
> Can you make the reply order deterministic?  Perhaps by having the lock
> command insert a sleep after locking but before replying, so that the unlock
> always gets to reply first?  But that can be a followup.

Yes, I could.  I'm just afraid that sleep might be undeterministic too
in some extreme cases, since IMHO it'll still depend on other things
(e.g., the scheduler of host, resources/workload on the host, etc.) to
make sure the order of the two commands will be exactly what we want.

So, even if current test seems to be undetermistic on the order of
received responses, IMHO it's very good on the other hand that it is
very determinstic on the test result...

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

Thanks,

-- 
Peter Xu