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

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

[Michael S. Tsirkin]

Speed up tests on host systems with kvm support.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---

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 ",
-- 
MST

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

[Paolo Bonzini]

On 13/07/2017 18:47, Michael S. Tsirkin wrote:
> Speed up tests on host systems with kvm support.
> 
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Thomas Huth <thuth@redhat.com>
> Cc: Laurent Vivier <lvivier@redhat.com>
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
> 
> 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 <pbonzini@redhat.com>

Thanks,

Paolo

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

[Thomas Huth]

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 <pbonzini@redhat.com>
>> Cc: Thomas Huth <thuth@redhat.com>
>> Cc: Laurent Vivier <lvivier@redhat.com>
>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>> ---
>>
>> 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 <pbonzini@redhat.com>

NACK.

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

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

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

[Paolo Bonzini]

> NACK.
> 
> 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.

You're right, I thought these were shut up before with Laurent's patch.
I think we should print a single message at the end, like

   qemu: -machine accel=kvm:xen: No accelerator found

Paolo

> 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
> 
> 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
> 

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

[Michael S. Tsirkin]

On Fri, Jul 14, 2017 at 02:20:33AM -0400, Paolo Bonzini wrote:
> 
> > NACK.
> > 
> > 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.
> 
> You're right, I thought these were shut up before with Laurent's patch.
> I think we should print a single message at the end, like
> 
>    qemu: -machine accel=kvm:xen: No accelerator found
> 
> Paolo

That's exactly Laurent's patch, isn't it?

> > 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
> > 
> > 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
> > 

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

[Laurent Vivier]

On 14/07/2017 16:37, Michael S. Tsirkin wrote:
> On Fri, Jul 14, 2017 at 02:20:33AM -0400, Paolo Bonzini wrote:
>>
>>> NACK.
>>>
>>> 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.
>>
>> You're right, I thought these were shut up before with Laurent's patch.
>> I think we should print a single message at the end, like
>>
>>    qemu: -machine accel=kvm:xen: No accelerator found
>>
>> Paolo
> 
> That's exactly Laurent's patch, isn't it?
> 

Do you want I send an updated and a more formal patch?

Thanks,
Laurent

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

[Michael S. Tsirkin]

On Mon, Jul 17, 2017 at 09:28:15AM +0200, Laurent Vivier wrote:
> On 14/07/2017 16:37, Michael S. Tsirkin wrote:
> > On Fri, Jul 14, 2017 at 02:20:33AM -0400, Paolo Bonzini wrote:
> >>
> >>> NACK.
> >>>
> >>> 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.
> >>
> >> You're right, I thought these were shut up before with Laurent's patch.
> >> I think we should print a single message at the end, like
> >>
> >>    qemu: -machine accel=kvm:xen: No accelerator found
> >>
> >> Paolo
> > 
> > That's exactly Laurent's patch, isn't it?
> > 
> 
> Do you want I send an updated and a more formal patch?
> 
> Thanks,
> Laurent

Yes pls.

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 <pbonzini@redhat.com>
> >> Cc: Thomas Huth <thuth@redhat.com>
> >> Cc: Laurent Vivier <lvivier@redhat.com>
> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >> ---
> >>
> >> 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 <pbonzini@redhat.com>
> 
> 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

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

[Thomas Huth]

On 14.07.2017 16:45, Michael S. Tsirkin wrote:
> 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.
[...]
>>>> 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 <pbonzini@redhat.com>
>>
>> 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.

I hope I won't scare *you* away that easily ;-)
But ok, I got the message, and will try to remember it.

>> 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.

That URL points to Paolo's "we should always only use TCG mail" ... I
guess you are rather referring to the other URL below?

 Thomas


>> 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

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

[Michael S. Tsirkin]

Speed up tests on host systems with kvm by using it.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 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 ",
-- 
MST