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

[Jim Meyering]

"Daniel P. Berrange" <berrange at redhat.com> wrote:

> On Fri, Jan 16, 2009 at 09:24:33AM +0100, Jim Meyering wrote:
>> "Daniel P. Berrange" <berrange at redhat.com> wrote:
>> > On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote:
>> >> "Daniel P. Berrange" <berrange at redhat.com> wrote:
>> >> ...
>> >> >>   + virsh --connect qemu:///session define devs.xml
>> >> >
>> >> > Shouldn't use qemu:///session for test cases like this - this is what
>> >> > the test:///default driver is for, avoiding the fragility & danger of
>> >> > using the daemon & live hypervisor drivers.
>> >>
>> >> There's no failure with test:///default.
>> >
>> > I'm rather surprised at that - both drivers use identical XML formating
>> > routines here, so given the same config, both should fail just as badly.
>> > Is this perhaps a case where we need to run it under valgrind to make
>> > it reliably fail.
>>
>> Note that virsh itself doesn't fail, in either case.
>> It sends info to libvirtd that causes *it* to segfault,
>> so in order to trigger the bug, I have to run libvirtd.
>
> virsh SEGV's nicely enough for me
>
> $ ./src/virsh --connect test:///default
> Welcome to lt-virsh, the virtualization interactive terminal.
>
> Type:  'help' for help with commands
>        'quit' to quit
>
> virsh # define a.xml
> Domain D defined from a.xml
>
> virsh # dumpxml D
> Segmentation fault

Ah. I'd only run the define, which is enough to make
libvirtd fail when using qemu:///session.

> In fact running virsh at all is redundant - this fails nicely enough if
> it is just added as a new datafile to the qemuxml2xmltest test case.

Putting that test closer to the target code is good, so ACK,
assuming it passes "make distcheck".

However, part of my goal in running the qemu:///session server is to
get more coverage on the server side.  "make check" does very little
testing of libvirtd, currently.  So if you don't mind, I'll also add a
comparable test using qemu:///session and unix_sock_dir, so it doesn't
interfere with existing state.

Maybe you want to keep the extra disk and interface sections after all?
(the ones that you suggested I remove)
Better coverage?