From d75f865fb989b3e6330c78c28e1c3bf7fa28e6a5 Mon Sep 17 00:00:00 2001 From: Pavel Mores Date: Tue, 10 Dec 2019 15:36:08 +0100 Subject: [PATCH] qemu: fix concurrency crash bug in snapshot revert This commit aims to fix https://bugzilla.redhat.com/show_bug.cgi?id=1610207 The cause was apparently incorrect handling of jobs in snapshot revert code which allowed a thread executing snapshot delete to begin job while snapshot revert was still running on another thread. The snapshot delete thread then waited on a condition variable in qemuMonitorSend() while the revert thread finished, changing (and effectively corrupting) the qemuMonitor structure under the delete thread which led to its crash. The incorrect handling of jobs in revert code was due to the fact that although qemuDomainRevertToSnapshot() correctly begins a job at the start, the job was implicitly ended when qemuProcessStop() was called because the job lives in the QEMU driver's private data (qemuDomainObjPrivate) that was purged during qemuProcessStop(). This fix prevents qemuProcessStop() from clearing jobs as the idea of qemuProcessStop() clearing jobs seems wrong in the first place. It was (inadvertently) introduced in commit 888aa4b6b9db65e3db273341e79846, which is effectively reverted by the second hunk of this commit. To preserve the desired effects of the faulty commit, the first hunk is included as suggested by Michal. Signed-off-by: Pavel Mores Reviewed-by: Michal Privoznik --- src/qemu/qemu_domain.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5b82c22d96..6870c17834 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -406,6 +406,8 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj, static void qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) { + qemuDomainObjResetJob(priv); + qemuDomainObjResetAsyncJob(priv); VIR_FREE(priv->job.current); VIR_FREE(priv->job.completed); virCondDestroy(&priv->job.cond); @@ -2226,9 +2228,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv) virBitmapFree(priv->migrationCaps); priv->migrationCaps = NULL; - qemuDomainObjResetJob(priv); - qemuDomainObjResetAsyncJob(priv); - virHashRemoveAll(priv->blockjobs); virHashRemoveAll(priv->dbusVMStates);