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
888aa4b6b9, 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 <pmores@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
This commit is contained in:
Pavel Mores 2019-12-10 15:36:08 +01:00 committed by Michal Privoznik
parent 4bccb9965d
commit d75f865fb9

View File

@ -406,6 +406,8 @@ qemuDomainObjRestoreJob(virDomainObjPtr obj,
static void static void
qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv) qemuDomainObjFreeJob(qemuDomainObjPrivatePtr priv)
{ {
qemuDomainObjResetJob(priv);
qemuDomainObjResetAsyncJob(priv);
VIR_FREE(priv->job.current); VIR_FREE(priv->job.current);
VIR_FREE(priv->job.completed); VIR_FREE(priv->job.completed);
virCondDestroy(&priv->job.cond); virCondDestroy(&priv->job.cond);
@ -2226,9 +2228,6 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
virBitmapFree(priv->migrationCaps); virBitmapFree(priv->migrationCaps);
priv->migrationCaps = NULL; priv->migrationCaps = NULL;
qemuDomainObjResetJob(priv);
qemuDomainObjResetAsyncJob(priv);
virHashRemoveAll(priv->blockjobs); virHashRemoveAll(priv->blockjobs);
virHashRemoveAll(priv->dbusVMStates); virHashRemoveAll(priv->dbusVMStates);