Cole Robinson <crobinso(a)redhat.com> wrote:
Jim Meyering wrote:
> Daniel Veillard <veillard(a)redhat.com> wrote:
>> On Fri, Jan 16, 2009 at 12:09:33AM +0000, Daniel P. Berrange wrote:
>>> On Thu, Jan 15, 2009 at 10:17:56PM +0100, Jim Meyering wrote:
>>>> "Daniel P. Berrange" <berrange(a)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.
>> Maybe that can be debugged separately, I would like to see the
>> patch fixed, maybe we can isolate the problem in the test driver (if
>> any) but it should not block the initial patch from being commited,
>
> Cole, can you commit your fix?
Pushed now.
Thanks!
I've pushed the following two changes
and rebased the git branch:
From 1c8099e9e4bfc4c1c3ca2f003822040d72c9cfcf Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 16 Jan 2009 18:06:33 +0000
Subject: [PATCH 1/2] tests: exercise a bug that could make virsh and libvirtd segfault
* tests/define-dev-segfault: New file.
* tests/Makefile.am (test_scripts): Add define-dev-segfault.
---
ChangeLog | 6 +++
tests/Makefile.am | 1 +
tests/define-dev-segfault | 76 +++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 83 insertions(+), 0 deletions(-)
create mode 100755 tests/define-dev-segfault
diff --git a/ChangeLog b/ChangeLog
index 7655836..536c563 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+Fri Jan 16 18:41:40 +0100 2009 Jim Meyering <meyering(a)redhat.com>
+
+ tests: exercise a bug that could make virsh and libvirtd segfault
+ * tests/define-dev-segfault: New file.
+ * tests/Makefile.am (test_scripts): Add define-dev-segfault.
+
Fri Jan 16 11:48:41 EST 2009 Cole Robinson <crobinso(a)redhat.com>
* src/domain_conf.c: Fix segfault with console device back compat.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 0486ee3..98739c5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -61,6 +61,7 @@ test_scripts += \
test_conf.sh \
cpuset \
daemon-conf \
+ define-dev-segfault \
int-overflow \
read-bufsiz \
read-non-seekable \
diff --git a/tests/define-dev-segfault b/tests/define-dev-segfault
new file mode 100755
index 0000000..4ae286f
--- /dev/null
+++ b/tests/define-dev-segfault
@@ -0,0 +1,76 @@
+#!/bin/sh
+# Exercise a bug whereby defining a valid domain could kill libvirtd.
+# The bug can also be exercised with a simple define/dumpxml pair to virsh.
+
+if test "$VERBOSE" = yes; then
+ set -x
+ 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 > D.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>
+ <serial type='pty'>
+ <target port='0'/>
+ </serial>
+ <serial type='pty'>
+ <target port='1'/>
+ </serial>
+ <serial type='pty'>
+ <target port='2'/>
+ </serial>
+ <parallel type='pty'>
+ <target port='0'/>
+ </parallel>
+ <parallel type='pty'>
+ <target port='1'/>
+ </parallel>
+ <parallel type='pty'>
+ <target port='2'/>
+ </parallel>
+ <console type='pty'>
+ <target port='0'/>
+ </console>
+ <sound model='pcspk'/>
+ <sound model='es1370'/>
+ </devices>
+</domain>
+EOF
+
+url=test:///default
+virsh --connect "$url" 'define D.xml; dumpxml D' > out 2>&1
|| fail=1
+
+cat > exp <<EOF || fail=1
+Domain D defined from D.xml
+
+$(cat D.xml)
+
+EOF
+
+compare exp out || fail=1
+
+exit $fail
--
1.6.1.258.g7ff14
From 0337ba238e69322750d3036f6794798ff1ed80e5 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering(a)redhat.com>
Date: Fri, 16 Jan 2009 18:07:24 +0000
Subject: [PATCH 2/2] tests: virsh-all and virsh-synopsis were not being run
* tests/Makefile.am (test_scripts): Add two missing backslashes.
---
ChangeLog | 5 ++++-
tests/Makefile.am | 24 ++++++++++++------------
2 files changed, 16 insertions(+), 13 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index 536c563..689ec9e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,4 +1,7 @@
-Fri Jan 16 18:41:40 +0100 2009 Jim Meyering <meyering(a)redhat.com>
+Fri Jan 16 18:44:08 +0100 2009 Jim Meyering <meyering(a)redhat.com>
+
+ tests: virsh-all and virsh-synopsis were not being run
+ * tests/Makefile.am (test_scripts): Add two missing backslashes.
tests: exercise a bug that could make virsh and libvirtd segfault
* tests/define-dev-segfault: New file.
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 98739c5..a2e5c0f 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -57,18 +57,18 @@ endif
test_scripts = domainschematest
if WITH_LIBVIRTD
-test_scripts += \
- test_conf.sh \
- cpuset \
- daemon-conf \
- define-dev-segfault \
- int-overflow \
- read-bufsiz \
- read-non-seekable \
- start \
- undefine \
- vcpupin
- virsh-all
+test_scripts += \
+ test_conf.sh \
+ cpuset \
+ daemon-conf \
+ define-dev-segfault \
+ int-overflow \
+ read-bufsiz \
+ read-non-seekable \
+ start \
+ undefine \
+ vcpupin \
+ virsh-all \
virsh-synopsis
endif
--
1.6.1.258.g7ff14