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

[Eric Blake]

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 <address@hidden>
> ---
>   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