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

[Lucas Meneghel Rodrigues <lmr@redhat.com>]

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.