[libvirt] [PATCH] Fix vm define error with back compat console device

[Daniel P. Berrange]

On Fri, Jan 16, 2009 at 02:37:04PM +0100, Jim Meyering wrote:
> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> > On Fri, Jan 16, 2009 at 12:42:35PM +0100, Jim Meyering wrote:
> >> "Daniel P. Berrange" <berrange at redhat.com> wrote:
> >>
> >>
> >> Ah. I'd only run the define, which is enough to make
> >> libvirtd fail when using qemu:///session.
> >
> > Oh, that'll be because when you run 'define' in QEMU, it parses the XML
> > and then runs dumpxml internally to save it out to disk
> 
> Exactly.  It exercises a different code path that also
> terminates in the code fixed by Cole's patch.

This bug can easily be covered by the unit test addition I proposed.
It is pointless adding multiple tests for the same bug - it just
slows down the test suite more, which is why i suggested anything
that runs with the real hypervisor drivers should be built into a
separate functional regression suite.

> > I think this is the wrong way to go about testing the real live
> > hypervisor drivers. For those we should have something that is
> > like the libvirt-CIM test suite. eg a test system which has code
> > to run all the libvirt APIs. This single test system is then run
> 
> ??
> I too would like to have it all now.
> I'm proposing what seems to me to be an easy way
> to incrementally add test cases like the one to
> exercise Cole's bug.  We need to do much more of this
> (adding tests with each bug fix), not less.

We don't have to create a functional test suite with 100% coverage
right from the start. We can incrementally build it up in just
the same way, starting with testing of the 'define' operation as
your patch does. A very small amount of boilerplate could would
let us start a useful functional test suite which can be run both
for dev trees, and real world installations by admins

 - A 'tests/regression' directory
 - Build a command / provide a shell script 'libvirt-test'
   that gets installed into /usr/bin, accepting a HV uri
   as its arg, and then running all functional tests for
   that HV driver.
 - Data files (such as test XML files) which get installed
   into /usr/share/libvirt/test to be used by test cases
 - A sub-RPM that contains these files/scripts 'libvirt-test'

Thus after install any admin can validate their deployment
by running one of

 # libvirt-test qemu:///session
 # libvirt-test qemu:///system
 # libvirt-test xen:///

as per the hypervisor they actually want to test. This will
help identify a wide range of problems, particularly where a
new upstream release of Xen / QEMU / KVM comes out which we
as libvirt developers haven't tested. We've had a long history
of being broken by Xen releases, but never had any way for
users to test if their install of libvirt actually works with
the Xen install. 

Setting up these functional test cases as things the users
can run, instead of just restircting them to use in make check
is far more useful, and does not need to be significantly more
if we build it up incrementally. The example data file you
have could be the first test case for this functional test
suite.

> > in a controlled environment once for each hypervisor URI that we
> > support. This will mean we exercise all the core codepaths for the
> > real drivers, and also validate that they are all responding in
> > the same way with consistent semantics / underling operations.
> > Adding ad-hoc test cases to leverage qemu:///session is wasted effort
> > because its creating a functional/integration test suite which is
> > neccessarily tied to QEMU driver, as opposed to being a generic
> > solution.
> 
> IMHO it's not wasted at all.
> This is a test that can be run by non-root users, and getting
> *some* coverage without requiring sudo or root is essential.
> However, the test can obviously be improved to cover more drivers.

My point is that with very little extra effort we can make something
that is far more broadly useful, both for our own dev usage, and for
end user validation.

> > or be running commands with the 'test' driver. The test driver has
> > sufficient functionality to let us functionally test core operation
> > of the libvirtd daemon too.
> 
> Some, but obviously not all.

Assuming the test driver has 100% coverage of the libvirt public 
API, it should allow complete coverage of the libvirtd daemon code
itself, since the daemon code is not driver specific in any way.
The only missing part in the test driver currently is the device
montor APIs.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|