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

[Peter Xu <peterx@redhat.com>]

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