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

[Daniel P. Berrange]

On Thu, Jan 15, 2009 at 07:55:52PM +0100, Jim Meyering wrote:
> Cole Robinson <crobinso at redhat.com> wrote:
> > If you define a domain with serial devs > 0 && parallel devs >= serial
> > devs, libvirtd segfaults when trying to set up the back compat console
> > device. We were using a previous loop counter where we shouldn't. The
> > attached patch fixes this.
> >
> > Thanks,
> > Cole
> > diff --git a/src/domain_conf.c b/src/domain_conf.c
> > index 8bb51f7..890afe0 100644
> > --- a/src/domain_conf.c
> > +++ b/src/domain_conf.c
> > @@ -3320,8 +3320,9 @@ char *virDomainDefFormat(virConnectPtr conn,
> >          if (virDomainChrDefFormat(conn, &buf, def->console, "console") < 0)
> >              goto cleanup;
> >      } else if (def->nserials != 0) {
> > -        /* ..else for legacy compat duplicate the serial device as a console */
> > -        if (virDomainChrDefFormat(conn, &buf, def->serials[n], "console") < 0)
> > +        /* ..else for legacy compat duplicate the first serial device as a
> > +         * console */
> > +        if (virDomainChrDefFormat(conn, &buf, def->serials[0], "console") < 0)
> >              goto cleanup;
> >      }
> 
> Hi Cole,
> 
> ACK!
> This looks like an obviously-right fix.
> Thanks for the sample input file you provided,
> 
> I confirmed and used it to write a test case that
> demonstrates the problem.
> 
> With the following test, running this command,
> 
>     make check -C tests TESTS=define-dev-segfault VERBOSE=yes
> 
> would fail with output including this:
> 
>   + libvirtd
>   + 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.

> +# Domain definition from Cole Robinson.
> +cat <<\EOF > devs.xml || fail=1
> +<domain type='kvm'>
> +<name>D</name>
> +<uuid>aaa3ae22-fed2-bfbd-ac02-3bea3bcfad82</uuid>
> +<memory>262144</memory>
> +<currentMemory>262144</currentMemory>
> +<vcpu>1</vcpu>
> +<os>
> +<type arch='i686' machine='pc'>hvm</type>
> +<boot dev='cdrom'/>
> +</os>
> +<features>
> +<acpi/>
> +</features>
> +<clock offset='utc'/>
> +<on_poweroff>destroy</on_poweroff>
> +<on_reboot>restart</on_reboot>
> +<on_crash>destroy</on_crash>
> +<devices>
> +<emulator>/usr/bin/qemu-kvm</emulator>
> +<disk type='file' device='cdrom'>
> +  <target dev='hdc' bus='ide'/>
> +  <readonly/>
> +</disk>
> +<disk type='file' device='floppy'>
> +  <target dev='fdb' bus='fdc'/>
> +</disk>
> +<disk type='file' device='cdrom'>
> +<target dev='sda' bus='scsi'/>
> +<readonly/>
> +</disk>
> +<interface type='network'>
> +<mac address='54:52:00:6c:a0:ca'/>
> +<source network='aaaaaa'/>
> +</interface>
> +<interface type='network'>
> +<mac address='54:52:00:6c:bb:ca'/>
> +<source network='default'/>
> +</interface>
> +<serial type='pty'/>
> +<serial type='pty'/>
> +<serial type='pty'/>
> +<parallel type='pty'/>
> +<parallel type='pty'/>
> +<parallel type='pty'/>
> +<input type='mouse' bus='ps2'/>
> +<sound model='pcspk'/>
> +<sound model='es1370'/>
> +<hostdev mode='subsystem' type='usb'>
> +<source>
> +<address bus='3' device='3'/>
> +</source>
> +</hostdev>
> +<hostdev mode='subsystem' type='usb'>
> +<source>
> +<vendor id='0x0483'/>
> +<product id='0x2016'/>
> +</source>
> +</hostdev>
> +</devices>
> +</domain>
> +EOF

Can you pass that XML through xmllint -format so is has more
readable indentation - good to remove all the disk/interface/hostdev
devices too, since they're not relevant to the test in question.

> +libvirtd > log 2>&1 & pid=$!
> +sleep 1

No need for the libvirtd daemon if using the test driver.

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 :|