Re: [Qemu-devel] [PATCH v2 1/3] libqtest: Ignore QMP events when parsing the response for HMP commands

[John Snow <jsnow@redhat.com>]

On 04/04/2017 03:31 AM, Thomas Huth wrote:
> On 03.04.2017 21:09, John Snow wrote:
>>
>>
>> On 03/30/2017 03:50 AM, Thomas Huth wrote:
>>> When running certain HMP commands (like "device_del") via QMP, we
>>> can sometimes get a QMP event in the response first, so that the
>>> "g_assert(ret)" statement in qtest_hmp() triggers and the test
>>> fails. Fix this by ignoring such QMP events while looking for the
>>> real return value from QMP.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>  tests/libqtest.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>>> index a5c3d2b..c9b2d76 100644
>>> --- a/tests/libqtest.c
>>> +++ b/tests/libqtest.c
>>> @@ -580,6 +580,12 @@ char *qtest_hmpv(QTestState *s, const char *fmt, va_list ap)
>>>                       " 'arguments': {'command-line': %s}}",
>>>                       cmd);
>>>      ret = g_strdup(qdict_get_try_str(resp, "return"));
>>> +    while (ret == NULL && qdict_get_try_str(resp, "event")) {
>>> +        /* Ignore asynchronous QMP events */
>>> +        QDECREF(resp);
>>> +        resp = qtest_qmp_receive(s);
>>> +        ret = g_strdup(qdict_get_try_str(resp, "return"));
>>> +    }
>>>      g_assert(ret);
>>>      QDECREF(resp);
>>>      g_free(cmd);
>>>
>>
>> You've probably been asked this, but can you just shove the QMP response
>> you don't want into the event queue for consumption by other calls?
> 
> Well, this is the qtest_hmpv() function, so I assume that the caller
> just wants to execute a HMP command and does not really care about QMP
> events. If you care about QMP events, you should use the qmp functions
> instead.
> 
>  Thomas
> 

I don't think it's obvious that using HMP functions should cause the QMP
stream to become faulty, though.

If someone uses an HMP function and then tries to wait on a QMP event to
confirm that some key condition has occurred (pausing or resuming, for
instance) it would not be immediately apparent from the user's POV that
this function just eats replies because it was convenient to do so.

I guess the event queue only exists in python though, so it's not as
trivial as I was thinking it would be...