Re: [Qemu-devel] [PATCH 2/3] libqtest: Add a generic function to run a callback function for every machine

[Thomas Huth]

On 28.03.2017 10:17, Dr. David Alan Gilbert wrote:
> * Thomas Huth (address@hidden) wrote:
>> On 27.03.2017 16:24, Dr. David Alan Gilbert wrote:
>>> * Thomas Huth (address@hidden) wrote:
>>>> Some tests need to run single tests for every available machine of the
>>>> current QEMU binary. To avoid code duplication, let's extract this
>>>> code that deals with 'query-machines' into a separate function.
>>>>
>>>> Signed-off-by: Thomas Huth <address@hidden>
>>>
>>> Having queued it, it's failing my test.
>>>
>>> The reason is that qom-tests.c has a blacklist which blacklists Xen;
>>> however your new generic function doesn't have the blacklist, so when
>>> you run HMP commands on all machines it tries to run it on Xen since
>>> my PC has the Xen libraries installed (so builds with Xen support) but
>>> isn't a Xen guest.
>>>
>>> It fails with:
>>> xen be core: xen be core: can't connect to xenstored
>>> can't connect to xenstored
>>> xen_init_pv: xen backend core setup failed
>>> Broken pipe
>>>
>>> I suggest you probably need to share the blacklist as well.
>>>
>>> (Unqueued)
>>>
>>> Dave
>>>
>>>> ---
>>>>  tests/libqtest.c    | 30 +++++++++++++++++
>>>>  tests/libqtest.h    |  8 +++++
>>>>  tests/pc-cpu-test.c | 95 
>>>> ++++++++++++++++++++---------------------------------
>>>>  tests/qom-test.c    | 36 ++++----------------
>>>>  4 files changed, 80 insertions(+), 89 deletions(-)
>>>>
>>>> diff --git a/tests/libqtest.c b/tests/libqtest.c
>>>> index c9b2d76..d8b8066 100644
>>>> --- a/tests/libqtest.c
>>>> +++ b/tests/libqtest.c
>>>> @@ -938,3 +938,33 @@ bool qtest_big_endian(QTestState *s)
>>>>  {
>>>>      return s->big_endian;
>>>>  }
>>>> +
>>>> +void qtest_cb_for_every_machine(void (*cb)(const char *machine))
>>>> +{
>>>> +    QDict *response, *minfo;
>>>> +    QList *list;
>>>> +    const QListEntry *p;
>>>> +    QObject *qobj;
>>>> +    QString *qstr;
>>>> +    const char *mname;
>>>> +
>>>> +    qtest_start("-machine none");
>>>> +    response = qmp("{ 'execute': 'query-machines' }");
>>>> +    g_assert(response);
>>>> +    list = qdict_get_qlist(response, "return");
>>>> +    g_assert(list);
>>>> +
>>>> +    for (p = qlist_first(list); p; p = qlist_next(p)) {
>>>> +        minfo = qobject_to_qdict(qlist_entry_obj(p));
>>>> +        g_assert(minfo);
>>>> +        qobj = qdict_get(minfo, "name");
>>>> +        g_assert(qobj);
>>>> +        qstr = qobject_to_qstring(qobj);
>>>> +        g_assert(qstr);
>>>> +        mname = qstring_get_str(qstr);
>>>> +        cb(mname);
>>>> +    }
>>>> +
>>>> +    qtest_end();
>>>> +    QDECREF(response);
>>>> +}
>> [...]
>>>> diff --git a/tests/qom-test.c b/tests/qom-test.c
>>>> index d48f890..ab0595d 100644
>>>> --- a/tests/qom-test.c
>>>> +++ b/tests/qom-test.c
>>>> @@ -107,46 +107,22 @@ static void test_machine(gconstpointer data)
>>>>      g_free((void *)machine);
>>>>  }
>>>>  
>>>> -static void add_machine_test_cases(void)
>>>> +static void add_machine_test_case(const char *mname)
>>>>  {
>>>>      const char *arch = qtest_get_arch();
>>>> -    QDict *response, *minfo;
>>>> -    QList *list;
>>>> -    const QListEntry *p;
>>>> -    QObject *qobj;
>>>> -    QString *qstr;
>>>> -    const char *mname;
>>>>  
>>>> -    qtest_start("-machine none");
>>>> -    response = qmp("{ 'execute': 'query-machines' }");
>>>> -    g_assert(response);
>>>> -    list = qdict_get_qlist(response, "return");
>>>> -    g_assert(list);
>>>> -
>>>> -    for (p = qlist_first(list); p; p = qlist_next(p)) {
>>>> -        minfo = qobject_to_qdict(qlist_entry_obj(p));
>>>> -        g_assert(minfo);
>>>> -        qobj = qdict_get(minfo, "name");
>>>> -        g_assert(qobj);
>>>> -        qstr = qobject_to_qstring(qobj);
>>>> -        g_assert(qstr);
>>>> -        mname = qstring_get_str(qstr);
>>>> -        if (!is_blacklisted(arch, mname)) {
>>>> -            char *path = g_strdup_printf("qom/%s", mname);
>>>> -            qtest_add_data_func(path, g_strdup(mname), test_machine);
>>>> -            g_free(path);
>>>> -        }
>>>> +    if (!is_blacklisted(arch, mname)) {
>>>> +        char *path = g_strdup_printf("qom/%s", mname);
>>>> +        qtest_add_data_func(path, g_strdup(mname), test_machine);
>>>> +        g_free(path);
>>>>      }
>>
>> Not sure what is going wrong here ... the "!is_blacklisted" check is
>> still here, so why is it trying to start a xen machine here?
> 
> I don't think it's this test that fails, I think it's the new one you add
> in the last patch but for the same reason.

Stupid me, I just read your reply way too fast (I just read qom-test.c
there and then somehow thought you were talking about a problem in that
file instead).

>> Looks like I need to install those xen libraries here, too ...
> 
> I think it's xen-libs and xen-devel you need.

Thanks, I can reproduce the issue now ... I'll send a v2 with a fix.

 Thomas