Re: [PATCH] KVM test: Enable qemu upstream testing

Re: [PATCH] KVM test: Enable qemu upstream testing

[Michael Goldish]

----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:

> qemu upstream has slight differences regarding qemu-kvm
> on the set of flags it supports. One of the most important
> differences is that on qemu we have to set -enable-kvm
> explicitely. Take this into consideration on the base
> configuration files and enable people to test qemu upstream
> more easily.
> 
> Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> ---
>  client/tests/kvm/tests.cfg.sample      |   10 ++++++++++
>  client/tests/kvm/tests_base.cfg.sample |    6 ++++++
>  2 files changed, 16 insertions(+), 0 deletions(-)
> 
> diff --git a/client/tests/kvm/tests.cfg.sample
> b/client/tests/kvm/tests.cfg.sample
> index 74c94b4..c6a66a4 100644
> --- a/client/tests/kvm/tests.cfg.sample
> +++ b/client/tests/kvm/tests.cfg.sample
> @@ -20,11 +20,17 @@ include cdkeys.cfg
>  #cdrom.* ?<= /tmp/kvm_autotest_root/
>  #steps ?<= /tmp/kvm_autotest_root/
>  
> +# You will notice that in all test definition blocks we have 'only
> qemu-kvm'
> +# set. This means that qemu-kvm command line syntax will be used. If
> you
> +# intend to test qemu upstream, you'll have to change that to 'only
> qemu'.
>  variants:
>      - @full:
> +        only qemu-kvm
>      - @sample1:
> +        only qemu-kvm
>          only Fedora Windows
>      - @sample2:
> +        only qemu-kvm
>          only qcow2
>          only ide
>          only default
> @@ -32,9 +38,11 @@ variants:
>          only Fedora.9.* RHEL.5.* Windows
>          only rtl8139
>      - @sample3:
> +        only qemu-kvm
>          only
> qcow2.*ide.*default.*up.*Ubuntu-8.10-server.*(autotest.sleeptest)
>          only rtl8139
>      - @fc8_step:
> +        only qemu-kvm
>          only qcow2
>          only ide
>          only default
> @@ -44,6 +52,7 @@ variants:
>          only rtl8139
>          only hugepages
>      - @winXP_32_unattended:
> +        only qemu-kvm
>          only qcow2
>          only ide
>          only default
> @@ -54,6 +63,7 @@ variants:
>          only unattended_install setup boot shutdown
>          only rtl8139
>      - @fc11_kickstart:
> +        only qemu-kvm
>          only qcow2
>          only ide
>          only default
> 
> diff --git a/client/tests/kvm/tests_base.cfg.sample
> b/client/tests/kvm/tests_base.cfg.sample
> index a63cc52..b1b1539 100644
> --- a/client/tests/kvm/tests_base.cfg.sample
> +++ b/client/tests/kvm/tests_base.cfg.sample
> @@ -891,6 +891,12 @@ linux_s3:
>  
>  
>  variants:
> +    - @qemu-kvm:
> +    - qemu:
> +        extra_params += " -enable-kvm"
> +
> +
> +variants:
>      - @up:
>          no autotest.npb
>      - smp2:

Alternatively we can provide an example qemu test set in
tests.cfg.sample -- something like this:

    - @fc11_kickstart:
        only qcow2
        only ide
        only default
        ...

    - @qemu_fc11_kickstart:
        only qcow2
        only ide
        only default
        ...
        extra_params += " -enable-kvm"

Or we can allow the user to change extra_params like this
(in tests.cfg.sample, before or after the test sets):

# Uncomment this line if you would like to test upstream qemu
#extra_params += " -enable-kvm"

The problem with doing it the way you did is that there will never
be a test set that runs both qemu and qemu-kvm (sequentially).
A user will always run either qemu-kvm or qemu, not both.
Is this correct?  In that case 'qemu' is more like a flag than a
variant, because it's global and applies to the entire job.

On the other hand, there could be advantages to doing it your way,
so I'm not quite sure what's better.  What motivates me to comment
on this is that we shouldn't have a variant for every little thing
because each variants block doubles the number of dicts, slows
down parsing and lengthens the full name of all tests.  This is no
big deal, but we should be just a little picky about the variants
we define.

Re: [PATCH] KVM test: Enable qemu upstream testing

[Lucas Meneghel Rodrigues]

On Wed, 2010-01-20 at 08:24 -0500, Michael Goldish wrote:
> ----- "Lucas Meneghel Rodrigues" <lmr@redhat.com> wrote:
> 
> > qemu upstream has slight differences regarding qemu-kvm
> > on the set of flags it supports. One of the most important
> > differences is that on qemu we have to set -enable-kvm
> > explicitely. Take this into consideration on the base
> > configuration files and enable people to test qemu upstream
> > more easily.
> > 
> > Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
> > ---
> >  client/tests/kvm/tests.cfg.sample      |   10 ++++++++++
> >  client/tests/kvm/tests_base.cfg.sample |    6 ++++++
> >  2 files changed, 16 insertions(+), 0 deletions(-)
> > 
> > diff --git a/client/tests/kvm/tests.cfg.sample
> > b/client/tests/kvm/tests.cfg.sample
> > index 74c94b4..c6a66a4 100644
> > --- a/client/tests/kvm/tests.cfg.sample
> > +++ b/client/tests/kvm/tests.cfg.sample
> > @@ -20,11 +20,17 @@ include cdkeys.cfg
> >  #cdrom.* ?<= /tmp/kvm_autotest_root/
> >  #steps ?<= /tmp/kvm_autotest_root/
> >  
> > +# You will notice that in all test definition blocks we have 'only
> > qemu-kvm'
> > +# set. This means that qemu-kvm command line syntax will be used. If
> > you
> > +# intend to test qemu upstream, you'll have to change that to 'only
> > qemu'.
> >  variants:
> >      - @full:
> > +        only qemu-kvm
> >      - @sample1:
> > +        only qemu-kvm
> >          only Fedora Windows
> >      - @sample2:
> > +        only qemu-kvm
> >          only qcow2
> >          only ide
> >          only default
> > @@ -32,9 +38,11 @@ variants:
> >          only Fedora.9.* RHEL.5.* Windows
> >          only rtl8139
> >      - @sample3:
> > +        only qemu-kvm
> >          only
> > qcow2.*ide.*default.*up.*Ubuntu-8.10-server.*(autotest.sleeptest)
> >          only rtl8139
> >      - @fc8_step:
> > +        only qemu-kvm
> >          only qcow2
> >          only ide
> >          only default
> > @@ -44,6 +52,7 @@ variants:
> >          only rtl8139
> >          only hugepages
> >      - @winXP_32_unattended:
> > +        only qemu-kvm
> >          only qcow2
> >          only ide
> >          only default
> > @@ -54,6 +63,7 @@ variants:
> >          only unattended_install setup boot shutdown
> >          only rtl8139
> >      - @fc11_kickstart:
> > +        only qemu-kvm
> >          only qcow2
> >          only ide
> >          only default
> > 
> > diff --git a/client/tests/kvm/tests_base.cfg.sample
> > b/client/tests/kvm/tests_base.cfg.sample
> > index a63cc52..b1b1539 100644
> > --- a/client/tests/kvm/tests_base.cfg.sample
> > +++ b/client/tests/kvm/tests_base.cfg.sample
> > @@ -891,6 +891,12 @@ linux_s3:
> >  
> >  
> >  variants:
> > +    - @qemu-kvm:
> > +    - qemu:
> > +        extra_params += " -enable-kvm"
> > +
> > +
> > +variants:
> >      - @up:
> >          no autotest.npb
> >      - smp2:
> 
> Alternatively we can provide an example qemu test set in
> tests.cfg.sample -- something like this:
> 
>     - @fc11_kickstart:
>         only qcow2
>         only ide
>         only default
>         ...
> 
>     - @qemu_fc11_kickstart:
>         only qcow2
>         only ide
>         only default
>         ...
>         extra_params += " -enable-kvm"

Indeed, I agree. I chose to make another block because some times it's
desirable to test both qemu-kvm and qemu (see below):

> Or we can allow the user to change extra_params like this
> (in tests.cfg.sample, before or after the test sets):
> 
> # Uncomment this line if you would like to test upstream qemu
> #extra_params += " -enable-kvm"
> 
> The problem with doing it the way you did is that there will never
> be a test set that runs both qemu and qemu-kvm (sequentially).
> A user will always run either qemu-kvm or qemu, not both.
> Is this correct?  In that case 'qemu' is more like a flag than a
> variant, because it's global and applies to the entire job.

Not quite, on our upstream job I've enabled qemu-kvm and qemu test on
the same job, that's why I think it's a good idea.

> On the other hand, there could be advantages to doing it your way,
> so I'm not quite sure what's better.  What motivates me to comment
> on this is that we shouldn't have a variant for every little thing
> because each variants block doubles the number of dicts, slows
> down parsing and lengthens the full name of all tests.  This is no
> big deal, but we should be just a little picky about the variants
> we define.

I agree, in this case it's worth to keep it as a variants block.

[PATCH] KVM test: Enable qemu upstream testing

[Lucas Meneghel Rodrigues]

qemu upstream has slight differences regarding qemu-kvm
on the set of flags it supports. One of the most important
differences is that on qemu we have to set -enable-kvm
explicitely. Take this into consideration on the base
configuration files and enable people to test qemu upstream
more easily.

Signed-off-by: Lucas Meneghel Rodrigues <lmr@redhat.com>
---
 client/tests/kvm/tests.cfg.sample      |   10 ++++++++++
 client/tests/kvm/tests_base.cfg.sample |    6 ++++++
 2 files changed, 16 insertions(+), 0 deletions(-)

diff --git a/client/tests/kvm/tests.cfg.sample b/client/tests/kvm/tests.cfg.sample
index 74c94b4..c6a66a4 100644
--- a/client/tests/kvm/tests.cfg.sample
+++ b/client/tests/kvm/tests.cfg.sample
@@ -20,11 +20,17 @@ include cdkeys.cfg
 #cdrom.* ?<= /tmp/kvm_autotest_root/
 #steps ?<= /tmp/kvm_autotest_root/
 
+# You will notice that in all test definition blocks we have 'only qemu-kvm'
+# set. This means that qemu-kvm command line syntax will be used. If you
+# intend to test qemu upstream, you'll have to change that to 'only qemu'.
 variants:
     - @full:
+        only qemu-kvm
     - @sample1:
+        only qemu-kvm
         only Fedora Windows
     - @sample2:
+        only qemu-kvm
         only qcow2
         only ide
         only default
@@ -32,9 +38,11 @@ variants:
         only Fedora.9.* RHEL.5.* Windows
         only rtl8139
     - @sample3:
+        only qemu-kvm
         only qcow2.*ide.*default.*up.*Ubuntu-8.10-server.*(autotest.sleeptest)
         only rtl8139
     - @fc8_step:
+        only qemu-kvm
         only qcow2
         only ide
         only default
@@ -44,6 +52,7 @@ variants:
         only rtl8139
         only hugepages
     - @winXP_32_unattended:
+        only qemu-kvm
         only qcow2
         only ide
         only default
@@ -54,6 +63,7 @@ variants:
         only unattended_install setup boot shutdown
         only rtl8139
     - @fc11_kickstart:
+        only qemu-kvm
         only qcow2
         only ide
         only default
diff --git a/client/tests/kvm/tests_base.cfg.sample b/client/tests/kvm/tests_base.cfg.sample
index a63cc52..b1b1539 100644
--- a/client/tests/kvm/tests_base.cfg.sample
+++ b/client/tests/kvm/tests_base.cfg.sample
@@ -891,6 +891,12 @@ linux_s3:
 
 
 variants:
+    - @qemu-kvm:
+    - qemu:
+        extra_params += " -enable-kvm"
+
+
+variants:
     - @up:
         no autotest.npb
     - smp2:
-- 
1.6.5.2