Re: [Qemu-devel] [PATCH] tests: switch pxe and vm gen id tests to use kvm

[Michael S. Tsirkin]

On Fri, Jul 14, 2017 at 07:59:54AM +0200, Thomas Huth wrote:
> On 13.07.2017 19:00, Paolo Bonzini wrote:
> > On 13/07/2017 18:47, Michael S. Tsirkin wrote:
> >> Speed up tests on host systems with kvm support.
> >>
> >> Cc: Paolo Bonzini <address@hidden>
> >> Cc: Thomas Huth <address@hidden>
> >> Cc: Laurent Vivier <address@hidden>
> >> Signed-off-by: Michael S. Tsirkin <address@hidden>
> >> ---
> >>
> >> This is on top of my previous patch fixing vmgenid.
> >>
> >> PPC PXE test is reported to be especially slow - does anyone care to try
> >> it and measure whether this helps?
> >>
> >>  tests/pxe-test.c     | 2 +-
> >>  tests/vmgenid-test.c | 2 +-
> >>  2 files changed, 2 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/tests/pxe-test.c b/tests/pxe-test.c
> >> index 34282d3..cf6e225 100644
> >> --- a/tests/pxe-test.c
> >> +++ b/tests/pxe-test.c
> >> @@ -25,7 +25,7 @@ static void test_pxe_one(const char *params, bool ipv6)
> >>  {
> >>      char *args;
> >>  
> >> -    args = g_strdup_printf("-machine accel=tcg -nodefaults -boot order=n "
> >> +    args = g_strdup_printf("-machine accel=kvm:tcg -nodefaults -boot 
> >> order=n "
> >>                             "-netdev user,id=" NETNAME 
> >> ",tftp=./,bootfile=%s,"
> >>                             "ipv4=%s,ipv6=%s %s", disk, ipv6 ? "off" : 
> >> "on",
> >>                             ipv6 ? "on" : "off", params);
> >> diff --git a/tests/vmgenid-test.c b/tests/vmgenid-test.c
> >> index 0556ba2..6477dcc 100644
> >> --- a/tests/vmgenid-test.c
> >> +++ b/tests/vmgenid-test.c
> >> @@ -132,7 +132,7 @@ static char disk[] = "tests/vmgenid-test-disk-XXXXXX";
> >>  
> >>  static char *guid_cmd_strdup(const char *guid)
> >>  {
> >> -    return g_strdup_printf("-machine accel=tcg "
> >> +    return g_strdup_printf("-machine accel=kvm:tcg "
> >>                             "-device vmgenid,id=testvgid,guid=%s "
> >>                             "-drive id=hd0,if=none,file=%s,format=raw "
> >>                             "-device ide-hd,drive=hd0 ",
> > 
> > Acked-by: Paolo Bonzini <address@hidden>
> 
> NACK.

As a rule, don't send nacks please. Skip directly to the technical
argument. Nacks add very little to the discussion and tend to scare away
contributors. All upper case also looks like shouting. That's unlike
acks which include reviewer's name and can be recorded in git history so
people know whom to ask questions about a commit.

> It's not that simple. If you do it like this, you then suddenly get
> these ugly '"kvm" accelerator not found' messages during "make check"
> which we really should avoid.
> 
> Laurent suggested some better ideas a couple of months ago, but I think
> the common sense was rather "unit tests should always use TCG" at that
> point in time:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00400.html

Looks good. This was never posted as a patch. Would you or him care
to test it and re-post? I'll include it with my patch.

> Well, now that we've got the "--disable-tcg" configure option, we should
> maybe start to reconsider that ;-)
> 
> So I'm fine with the patch if you also include Laurent's patch to shut
> up the error message:
> 
>  https://lists.gnu.org/archive/html/qemu-devel/2016-10/msg00395.html
> 
>  Thomas