[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] processSerialChangedEvent: Close agent monitor early



https://bugzilla.redhat.com/show_bug.cgi?id=890648

So, imagine you've issued an API that involves guest agent. For
instance, you want to query guest's IP addresses. So the API acquires
QUERY_JOB, locks the guest agent and issues the agemt command.
However, for some reason, guest agent replies to initial ping
correctly, but then crashes tragically while executing real command
(in this case guest-network-get-interfaces). Since initial ping went
well, libvirt thinks guest agent is accessible and awaits reply to the
real command. But it will never come. What will is a monitor event.
Our handler (processSerialChangedEvent) will try to acquire
MODIFY_JOB, which will fail obviously because the other thread that's
executing the API already holds a job. So the event handler exits
early, and the QUERY_JOB is never released nor ended.

I don't understand why, but qemu should in fact instead of sending a
event on the monitor close the socket. But leave that for a different
discussion. Since qemu is not doing that, we must work around it and
close and unregister the agent's FD from the even loop ourselves. This
will wake up all callers stuck in waiting for agent's reply.
Hopefully, it will make them finish the job too and even handler can
acquire its job later.

Signed-off-by: Michal Privoznik <mprivozn redhat com>
---
 src/qemu/qemu_driver.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d173aa1..becb6ae 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -4480,6 +4480,14 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
     VIR_DEBUG("Changing serial port state %s in domain %p %s",
               devAlias, vm, vm->def->name);
 
+    if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED &&
+        virDomainObjIsActive(vm) && priv->agent) {
+        /* Close agent monitor early, so that other threads
+         * waiting for the agent to reply can finish and our
+         * job we acquire below can succeed. */
+        qemuAgentClose(priv->agent);
+    }
+
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
         goto cleanup;
 
@@ -4508,7 +4516,6 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
             }
         } else {
             if (priv->agent) {
-                qemuAgentClose(priv->agent);
                 priv->agent = NULL;
                 priv->agentError = false;
             }
-- 
2.3.6


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]