On Thu, Jul 23, 2020 at 01:14:10PM +0300, Nikolay Shirokovskiy wrote:
We are dropping the only reference here so that the event loop thread is going to be exited synchronously. In order to avoid deadlocks we need to unlock the VM so that any handler being called can finish execution and thus even loop thread be finished too.
Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com> --- src/qemu/qemu_domain.c | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 5b22eb2..82b3d11 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1637,11 +1637,21 @@ void qemuDomainObjStopWorker(virDomainObjPtr dom) { qemuDomainObjPrivatePtr priv = dom->privateData; + virEventThread *eventThread;
- if (priv->eventThread) { - g_object_unref(priv->eventThread); - priv->eventThread = NULL; - } + if (!priv->eventThread) + return; + + /* + * We are dropping the only reference here so that the event loop thread + * is going to be exited synchronously. In order to avoid deadlocks we + * need to unlock the VM so that any handler being called can finish + * execution and thus even loop thread be finished too. + */ + eventThread = g_steal_pointer(&priv->eventThread); + virObjectUnlock(dom); + g_object_unref(eventThread); + virObjectLock(dom);
Hmm, I'm really not at all comfortable with this. I don't have confidence that qemuProcessStop() safe if we drpo & reacquire the lock half way through its cleanup process. The 'virDomainObj' is in a semi-clean state, 1/2 running 1/2 shutoff. This is the key reason I think I didn't do the synchronous thread join in the event loop. Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|