[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



Cole Robinson <crobinso redhat com> wrote:

> Jim Meyering wrote:
>> Daniel Veillard <veillard 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 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 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 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 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 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 redhat com>
+Fri Jan 16 18:44:08 +0100 2009 Jim Meyering <meyering 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


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]