12:55 p.m.
Cole Robinson <crobinso(a)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
./define-dev-segfault: line 84: 4882 Segmentation fault libvirtd > log 2>&1
With your patch applied, the test passes.
From b16f407a6369cf4c3a5770c4b49515b565f29b4f Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Thu, 15 Jan 2009 19:51:39 +0100
Subject: [PATCH] exercise a bug that could make libvirtd segfault
* tests/define-dev-segfault: New file.
* tests/Makefile.am (test_scripts): Add define-dev-segfault.
---
tests/Makefile.am | 1 +
tests/define-dev-segfault | 92 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 93 insertions(+), 0 deletions(-)
create mode 100755 tests/define-dev-segfault
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0486ee3..7acb745 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -59,6 +59,7 @@ test_scripts = domainschematest
if WITH_LIBVIRTD
test_scripts += \
test_conf.sh \
+ define-dev-segfault \
cpuset \
daemon-conf \
int-overflow \
diff --git a/tests/define-dev-segfault b/tests/define-dev-segfault
new file mode 100755
index 0000000..903568f
--- /dev/null
+++ b/tests/define-dev-segfault
@@ -0,0 +1,92 @@
+#!/bin/sh
+# Exercise a bug whereby defining a valid domain could kill libvirtd.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ libvirtd --version
+ virsh --version
+fi
+
+test -z "$srcdir" && srcdir=$(pwd)
+test -z "$abs_top_srcdir" && abs_top_srcdir=$(pwd)/..
+. "$srcdir/test-lib.sh"
+
+fail=0
+
+# 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
+
+libvirtd > log 2>&1 & pid=$!
+sleep 1
+
+virsh --connect qemu:///session define devs.xml > out 2>&1 \
+ || fail=1
+
+kill $pid
+
+printf 'Domain D defined from devs.xml\n\n' > exp || fail=1
+
+compare exp out || fail=1
+compare /dev/null log || fail=1
+exit $fail
--
1.6.1.233.g5b4a8