All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
To: Thomas Huth <thuth@redhat.com>
Cc: qemu-devel@nongnu.org, Markus Armbruster <armbru@redhat.com>,
	Eduardo Habkost <ehabkost@redhat.com>
Subject: Re: [Qemu-devel] [PATCH 2/3] libqtest: Add a generic function to run a callback function for every machine
Date: Tue, 28 Mar 2017 09:17:54 +0100	[thread overview]
Message-ID: <20170328081753.GA2509@work-vm> (raw)
In-Reply-To: <95bf59af-beeb-74bf-7819-f2a770b64263@redhat.com>

* Thomas Huth (thuth@redhat.com) wrote:
> On 27.03.2017 16:24, Dr. David Alan Gilbert wrote:
> > * Thomas Huth (thuth@redhat.com) 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 <thuth@redhat.com>
> > 
> > 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.

> Looks like I need to install those xen libraries here, too ...

I think it's xen-libs and xen-devel you need.

Dave

> 
>  Thomas
> 
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

  reply	other threads:[~2017-03-28  8:18 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-03-21 10:39 [Qemu-devel] [PATCH 0/3] Add a tester for HMP commands Thomas Huth
2017-03-21 10:39 ` [Qemu-devel] [PATCH 1/3] libqtest: Ignore QMP events when parsing the response " Thomas Huth
2017-03-23  8:52   ` Stefan Hajnoczi
2017-03-23  8:59     ` Thomas Huth
2017-03-21 10:39 ` [Qemu-devel] [PATCH 2/3] libqtest: Add a generic function to run a callback function for every machine Thomas Huth
2017-03-23  8:53   ` Stefan Hajnoczi
2017-03-27 14:24   ` Dr. David Alan Gilbert
2017-03-28  6:35     ` Thomas Huth
2017-03-28  8:17       ` Dr. David Alan Gilbert [this message]
2017-03-29 15:59         ` Thomas Huth
2017-03-21 10:39 ` [Qemu-devel] [PATCH 3/3] tests: Add a tester for HMP commands Thomas Huth
2017-03-23  8:56   ` Stefan Hajnoczi
2017-03-27 13:31 ` [Qemu-devel] [PATCH 0/3] " Dr. David Alan Gilbert

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170328081753.GA2509@work-vm \
    --to=dgilbert@redhat.com \
    --cc=armbru@redhat.com \
    --cc=ehabkost@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.