On Fri, Jan 16, 2009 at 02:37:04PM +0100, Jim Meyering wrote: > "Daniel P. Berrange" <berrange@xxxxxxxxxx> wrote: > > On Fri, Jan 16, 2009 at 12:42:35PM +0100, Jim Meyering wrote: > >> "Daniel P. Berrange" <berrange@xxxxxxxxxx> 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 :| -- Libvir-list mailing list Libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list