On Thu, Jan 15, 2009 at 07:55:52PM +0100, Jim Meyering wrote:
Cole Robinson <crobinso@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 :|