Bug#720303: Let's also fix #717893
Hi,
I just had a chance to go through my mail backlog and found Ferenc fixed
another crash in libvirtd. An updated debdiff is attached.
Cheers,
-- Guido
diff --git a/debian/changelog b/debian/changelog
index 5ead189..27adc06 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,23 @@
+libvirt (0.9.12-11+deb7u3) wheezy-proposed-updates; urgency=low
+
+ * [9c12e5a] Fix race condition when destroying guests.
+ Closes: #717893
+ Thanks to Ferenc Wágner for sorting this out.
+
+ -- Guido Günther <agx@sigxcpu.org> Wed, 21 Aug 2013 19:30:54 +0200
+
+libvirt (0.9.12-11+deb7u2) wheezy-proposed-updates; urgency=low
+
+ [ Guido Günther ]
+ * [5bc00df] Make sure qemu.conf isn't world readable by default
+ since the user might add passwords to it. (Closes: #710537)
+
+ [ Ferenc Wagner ]
+ * [ce7ef48] Fix libvirtd crash when destroying a domain with attached console
+ (Closes: #710517)
+
+ -- Guido Günther <agx@sigxcpu.org> Tue, 20 Aug 2013 10:37:16 +0200
+
libvirt (0.9.12-11+deb7u1) wheezy-proposed-updates; urgency=low
[ Guido Günther ]
diff --git a/debian/libvirt-bin.postinst b/debian/libvirt-bin.postinst
index 53119a2..510bee7 100644
--- a/debian/libvirt-bin.postinst
+++ b/debian/libvirt-bin.postinst
@@ -61,6 +61,8 @@ add_statoverrides()
SANLOCK_DIR="/var/lib/libvirt/sanlock"
+ QEMU_CONF="/etc/libvirt/qemu.conf"
+
for dir in ${ROOT_DIRS}; do
if ! dpkg-statoverride --list "${dir}" >/dev/null 2>&1; then
chown root:root "${dir}"
@@ -79,6 +81,11 @@ add_statoverrides()
chown root:root "${SANLOCK_DIR}"
chmod 0700 "${SANLOCK_DIR}"
fi
+
+ if ! dpkg-statoverride --list "${QEMU_CONF}" >/dev/null 2>&1; then
+ chown root:root "${QEMU_CONF}"
+ chmod 0600 "${QEMU_CONF}"
+ fi
}
diff --git a/debian/patches/series b/debian/patches/series
index 5e1cbe7..60bcbee 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -20,3 +20,5 @@ rpc-Fix-crash-on-error-paths-of-message-dispatching.patch
qemu-Add-support-for-no-user-config.patch
debian/Allow-xen-toolstack-to-find-it-s-binaries.patch
fix-leak-virStorageBackendLogicalMakeVol.patch
+upstream/Fix-libvirtd-crash-when-destroying-a-domain-with-att.patch
+upstream/Fix-race-condition-when-destroying-guests.patch
diff --git a/debian/patches/upstream/Fix-libvirtd-crash-when-destroying-a-domain-with-att.patch b/debian/patches/upstream/Fix-libvirtd-crash-when-destroying-a-domain-with-att.patch
new file mode 100644
index 0000000..62899fa
--- /dev/null
+++ b/debian/patches/upstream/Fix-libvirtd-crash-when-destroying-a-domain-with-att.patch
@@ -0,0 +1,72 @@
+From: Peter Krempa <pkrempa@redhat.com>
+Date: Sat, 8 Jun 2013 15:20:45 +0200
+Subject: Fix libvirtd crash when destroying a domain with attached console
+
+Origin: upstream
+Bug: https://bugzilla.redhat.com/show_bug.cgi?id=969497
+Applied-Upstream: ba226d334acbc49f6751b430e0c4e00f69eef6bf and 45edefc7a7bcbec988f54331ff37fc32e4bc2718
+Last-Update: 2013-06-07
+
+This is two upstream commits squashed:
+
+commit ba226d334acbc49f6751b430e0c4e00f69eef6bf
+Author: Peter Krempa <pkrempa@redhat.com>
+Date: Fri Jul 27 14:50:54 2012 +0200
+
+ conf: Remove callback from stream when freeing entries in console hash
+
+ When a domain has a active console connection and is destroyed the
+ callback is called on private data that no longer exist causing a
+ segfault.
+
+commit 45edefc7a7bcbec988f54331ff37fc32e4bc2718
+Author: Peter Krempa <pkrempa@redhat.com>
+Date: Fri Aug 3 11:20:29 2012 +0200
+
+ conf: Remove console stream callback only when freeing console helper
+
+ Commit ba226d334acbc49f6751b430e0c4e00f69eef6bf tried to fix crash of
+ the daemon when a domain with an open console was destroyed. The fix was
+ wrong as it tried to remove the callback also when the stream was
+ aborted, where at that point the fd stream driver was already freed and
+ removed.
+
+ This patch clears the callbacks with a helper right before the hash is
+ freed, so that it doesn't interfere with other codepaths where the
+ stream object is freed.
+
+---
+ src/conf/virconsole.c | 13 +++++++++++++
+ 1 file changed, 13 insertions(+)
+
+diff --git a/src/conf/virconsole.c b/src/conf/virconsole.c
+index 443d80d..01f1c84 100644
+--- a/src/conf/virconsole.c
++++ b/src/conf/virconsole.c
+@@ -290,6 +290,18 @@ error:
+ }
+
+ /**
++ * Helper to clear stream callbacks when freeing the hash
++ */
++static void virConsoleFreeClearCallbacks(void *payload,
++ const void *name ATTRIBUTE_UNUSED,
++ void *data ATTRIBUTE_UNUSED)
++{
++ virStreamPtr st = payload;
++
++ virFDStreamSetInternalCloseCb(st, NULL, NULL, NULL);
++}
++
++/**
+ * Free structures for handling open console streams.
+ *
+ * @cons Pointer to the private structure.
+@@ -300,6 +312,7 @@ void virConsoleFree(virConsolesPtr cons)
+ return;
+
+ virMutexLock(&cons->lock);
++ virHashForEach(cons->hash, virConsoleFreeClearCallbacks, NULL);
+ virHashFree(cons->hash);
+ virMutexUnlock(&cons->lock);
+ virMutexDestroy(&cons->lock);
diff --git a/debian/patches/upstream/Fix-race-condition-when-destroying-guests.patch b/debian/patches/upstream/Fix-race-condition-when-destroying-guests.patch
new file mode 100644
index 0000000..121a511
--- /dev/null
+++ b/debian/patches/upstream/Fix-race-condition-when-destroying-guests.patch
@@ -0,0 +1,55 @@
+From: =?UTF-8?q?Ferenc=20W=C3=A1gner?= <wferi@niif.hu>
+Date: Wed, 21 Aug 2013 18:58:06 +0200
+Subject: Fix race condition when destroying guests
+
+Backport of 81621f3e6e45e8681cc18ae49404736a0e772a11 and
+f1b4021b38f9485c50d386af6f682ecfc8025af5 to fix a race (resulting in a
+segfault) when destroying domains.
+---
+ src/qemu/qemu_driver.c | 19 +++++++++++++------
+ 1 file changed, 13 insertions(+), 6 deletions(-)
+
+diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
+index 0053ed1..c0b4707 100644
+--- a/src/qemu/qemu_driver.c
++++ b/src/qemu/qemu_driver.c
+@@ -1827,6 +1827,12 @@ qemuDomainDestroyFlags(virDomainPtr dom,
+
+ qemuDomainSetFakeReboot(driver, vm, false);
+
++
++ /* We need to prevent monitor EOF callback from doing our work (and sending
++ * misleading events) while the vm is unlocked inside BeginJob/ProcessKill API
++ */
++ priv->beingDestroyed = true;
++
+ /* Although qemuProcessStop does this already, there may
+ * be an outstanding job active. We want to make sure we
+ * can kill the process even if a job is active. Killing
+@@ -1834,19 +1840,20 @@ qemuDomainDestroyFlags(virDomainPtr dom,
+ */
+ if (flags & VIR_DOMAIN_DESTROY_GRACEFUL) {
+ if (qemuProcessKill(driver, vm, 0) < 0) {
++ priv->beingDestroyed = false;
+ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
+ _("failed to kill qemu process with SIGTERM"));
+ goto cleanup;
+ }
+ } else {
+- ignore_value(qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE));
++ if (qemuProcessKill(driver, vm, VIR_QEMU_PROCESS_KILL_FORCE) < 0) {
++ priv->beingDestroyed = false;
++ qemuReportError(VIR_ERR_OPERATION_FAILED, "%s",
++ _("failed to kill qemu process with SIGTERM"));
++ goto cleanup;
++ }
+ }
+
+- /* We need to prevent monitor EOF callback from doing our work (and sending
+- * misleading events) while the vm is unlocked inside BeginJob API
+- */
+- priv->beingDestroyed = true;
+-
+ if (qemuDomainObjBeginJobWithDriver(driver, vm, QEMU_JOB_DESTROY) < 0)
+ goto cleanup;
+
Reply to: