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

Re: [libvirt] [PATCH v2] qemu_agent: Ignore expected EOFs

On 01/22/13 15:11, Michal Privoznik wrote:

One of my previous patches (f2a4e5f176c408) tried to fix crashing
libvirtd on domain detroy. However, we need to copy pattern from
qemuProcessHandleMonitorEOF() instead of decrementing reference
counter. The rationale for this is, if qemu process is dying due
to domain being destroyed, we obtain EOF on both the monitor and
agent sockets. However, if the exit is expected, qemuProcessStop
is called, which cleans both agent and monitor sockets up. We
want qemuAgentClose() to be called iff the EOF is not expected,
so we don't leak an FD and memory.

diff to v1:
-handle race with qemuProcessHandleMonitorEOF correctly

  src/qemu/qemu_process.c | 20 ++++++++++++++++++--
  1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 2f08215..013b2ca 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -133,14 +133,30 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent,

      priv = vm->privateData;
-    if (priv->agent == agent &&
-        !virObjectUnref(priv->agent))
+    if (!priv->agent) {
+        VIR_DEBUG("Agent freed already");
+        goto unlock;
+    }

You shouls add to the commit message why this condition is needed.

+    if (priv->beingDestroyed) {
+        VIR_DEBUG("Domain is being destroyed, agent EOF is expected");
+        goto unlock;
+    }
+    if (priv->agent == agent)
          priv->agent = NULL;

Is there a possibility for the agent to be different from the saved one here?


+    return;
+    virObjectUnlock(vm);
+    qemuDriverUnlock(driver);
+    return;

ACK with the commit message amended and the question answered.


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