qemuAgentOpen: Rework domain object refcounting

Currently, when opening an agent socket the qemuConnectAgent()
increments domain object refcounter and calls qemuAgentOpen()
where the domain object pointer is simply stored inside
_qemuAgent struct. If qemuAgentOpen() fails, then it clears @cb
member only to avoid qemuProcessHandleAgentDestroy() being called
(which decrements the domain object refcounter) and the domain
object refcounter is then decreased explicitly in
qemuConnectAgent().

The same result can be achieved with much cleaner code: increment
the refcounter inside qemuAgentOpen() and drop the dance around
@cb.

Also, the comment in qemuConnectAgent() about holding an extra
reference is not correct. The thread that called
qemuConnectAgent() already holds a reference to the domain
object. No matter how many time the object is locked and unlocked
the reference counter can't be decreased.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index d19a8b983d4e9ee06b1571e1e63a24d689cf345c..e6d9e7ac5027cc1124d576178c052b4c03fc2cd4 100644 (file)
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -160,9 +160,11 @@ qemuAgentEscapeNonPrintable(const char *text)
 static void qemuAgentDispose(void *obj)
 {
     qemuAgent *agent = obj;
+
     VIR_DEBUG("agent=%p", agent);
-    if (agent->cb && agent->cb->destroy)
-        (agent->cb->destroy)(agent, agent->vm);
+
+    if (agent->vm)
+        virObjectUnref(agent->vm);
     virCondDestroy(&agent->notify);
     g_free(agent->buffer);
     g_main_context_unref(agent->context);
@@ -671,7 +673,7 @@ qemuAgentOpen(virDomainObj *vm,
         virObjectUnref(agent);
         return NULL;
     }
-    agent->vm = vm;
+    agent->vm = virObjectRef(vm);
     agent->cb = cb;
     agent->singleSync = singleSync;
 
@@ -707,12 +709,6 @@ qemuAgentOpen(virDomainObj *vm,
     return agent;
 
  cleanup:
-    /* We don't want the 'destroy' callback invoked during
-     * cleanup from construction failure, because that can
-     * give a double-unref on virDomainObj *in the caller,
-     * so kill the callbacks now.
-     */
-    agent->cb = NULL;
     qemuAgentClose(agent);
     return NULL;
 }

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6e3d3b82e08965fa0b3a6c31ac71d4ec4331b48f..3623cf2535bfb4b7899d25e0471f57a9dcdf28c7 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -234,19 +234,12 @@ qemuConnectAgent(virQEMUDriver *driver, virDomainObj *vm)
         goto cleanup;
     }
 
-    /* Hold an extra reference because we can't allow 'vm' to be
-     * deleted while the agent is active */
-    virObjectRef(vm);
-
     agent = qemuAgentOpen(vm,
                           config->source,
                           virEventThreadGetContext(priv->eventThread),
                           &agentCallbacks,
                           virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VSERPORT_CHANGE));
 
-    if (agent == NULL)
-        virObjectUnref(vm);
-
     if (!virDomainObjIsActive(vm)) {
         qemuAgentClose(agent);
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",