[libvirt PATCH] qemu: ensure domain event thread is always stopped

Daniel P. Berrangé posted 1 patch 4 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20200320111454.2701635-1-berrange@redhat.com
src/qemu/qemu_process.c | 12 +-----------
1 file changed, 1 insertion(+), 11 deletions(-)
[libvirt PATCH] qemu: ensure domain event thread is always stopped
Posted by Daniel P. Berrangé 4 years, 1 month ago
In previous commit:

  commit e6afacb0feabd9bf58331aa0e5259a35378be74e
  Author: Daniel P. Berrangé <berrange@redhat.com>
  Date:   Wed Feb 12 12:26:11 2020 +0000

    qemu: start/stop an event loop thread for domains

A bogus comment was added claiming we didn't need to shutdown the
event thread in the qemuProcessStop method, because this would be
done in the monitor EOF callback. This was wrong because the EOF
callback only runs in the case of a QEMU crash or a guest initiated
clean shutdown & poweroff.  In the case where the libvirt admin
calls virDomainDestroy, the EOF callback never fires because we
have already unregistered the event callbacks. We must thus always
attempt to stop the event thread in qemuProcessStop.

Reported-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/qemu/qemu_process.c | 12 +-----------
 1 file changed, 1 insertion(+), 11 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e8401030a2..fcdde76aaa 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7398,17 +7398,7 @@ void qemuProcessStop(virQEMUDriverPtr driver,
         priv->monConfig = NULL;
     }
 
-    /*
-     * We cannot stop the event thread at this time. When
-     * we are in this code, we may not yet have processed the
-     * STOP event or EOF from the monitor. So the event loop
-     * may have pending input that we need to process still.
-     * The qemuProcessHandleMonitorEOF method will kill
-     * the event thread because at that point we don't
-     * expect any more I/O from the QEMU monitor. We are
-     * assuming we don't need to get any more events from the
-     * QEMU agent at that time.
-     */
+    qemuDomainObjStopWorker(vm);
 
     /* Remove the master key */
     qemuDomainMasterKeyRemove(priv);
-- 
2.24.1

Re: [libvirt PATCH] qemu: ensure domain event thread is always stopped
Posted by Daniel Henrique Barboza 4 years, 1 month ago

On 3/20/20 8:14 AM, Daniel P. Berrangé wrote:
> In previous commit:
> 
>    commit e6afacb0feabd9bf58331aa0e5259a35378be74e
>    Author: Daniel P. Berrangé <berrange@redhat.com>
>    Date:   Wed Feb 12 12:26:11 2020 +0000
> 
>      qemu: start/stop an event loop thread for domains
> 
> A bogus comment was added claiming we didn't need to shutdown the
> event thread in the qemuProcessStop method, because this would be
> done in the monitor EOF callback. This was wrong because the EOF
> callback only runs in the case of a QEMU crash or a guest initiated
> clean shutdown & poweroff.  In the case where the libvirt admin
> calls virDomainDestroy, the EOF callback never fires because we
> have already unregistered the event callbacks. We must thus always
> attempt to stop the event thread in qemuProcessStop.
> 
> Reported-by: Peter Krempa <pkrempa@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---


Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>