External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-02-22 03:12:22 +00:00
Code Issues Packages Projects Releases Wiki Activity

qemu: Use virEventThreadStop() in qemuProcessStop()

Browse Source 
Currently, qemuProcessStop() unlocks given domain object right in
the middle of cleanup process. This is dangerous because there
might be another thread which is executing virDomainObjListAdd().
And since the domain object is on the list of domain objects AND
by the time qemuProcessStop() unlocks it the object is also
marked as inactive, the other thread acquires the lock and
switches vm->def pointer.

The unlocking of domain object is needed though, to allow even
processing thread finish its queue. Well, the processing can be
done before any cleanup is attempted.

Therefore, use freshly introduced virEventThreadStop() to join
the event thread and drop lock/unlock from the middle of
qemuProcessStop().

Now, there's a comment being removed that mentions
qemuDomainObjStopWorker() and why it has to be called only after
the domain is marked as dead. This comment is no longed
applicable because call to qemuDomainObjStopWorker() is removed
also. Moreover, priv->beingDestroyed is set to true before
unlocking the domain object, thus any event processing callback
is going to see the domain being destroyed and can chose to
either exit early or finish processing event.

Fixes: 3865410e7f67ca4ec66e9a905e75f452762a97f0
Resolves: https://issues.redhat.com/browse/RHEL-49607
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
This commit is contained in:
Michal Privoznik 2024-07-25 09:51:45 +02:00
parent 7aca235d8d
commit 0888784f38
1 changed files with 18 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

22
src/qemu/qemu_process.c
View File

@ -8696,6 +8696,20 @@ void qemuProcessStop(virQEMUDriver *driver,
VIR_QEMU_PROCESS_KILL_FORCE|
VIR_QEMU_PROCESS_KILL_NOCHECK));
/* By unlocking the domain object the events processing thread is allowed
* to finish its job. Unlocking must happen before resetting vm->def->id as
* the global domain object list code depends on it (and it can't actually
* check 'priv->beingDestroyed as that's private). */
if (priv->eventThread) {
/* Explicitly set priv->beingDestroyed. While it's done in
* qemuProcessBeginStopJob(), qemuProcessStop() is called from places
* where stop job is not acquired. */
priv->beingDestroyed = true;
virObjectUnlock(vm);
virEventThreadStop(priv->eventThread);
virObjectLock(vm);
}
if (priv->agent) {
g_clear_pointer(&priv->agent, qemuAgentClose);
}
@ -8727,13 +8741,13 @@ void qemuProcessStop(virQEMUDriver *driver,
vm->def->id = -1;
priv->beingDestroyed = false;
/* No unlocking of @vm after this point until whole cleanup is done. */
/* Wake up anything waiting on domain condition */
virDomainObjBroadcast(vm);
/* IMPORTANT: qemuDomainObjStopWorker() unlocks @vm in order to prevent
* deadlocks with the per-VM event loop thread. This MUST be done after
* marking the VM as dead */
qemuDomainObjStopWorker(vm);
if (priv->eventThread)
g_object_unref(g_steal_pointer(&priv->eventThread));
if (!!g_atomic_int_dec_and_test(&driver->nactive) && driver->inhibitCallback)
driver->inhibitCallback(false, driver->inhibitOpaque);