[PATCH 2/4] 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(a)redhat.com>
---
src/qemu/qemu_agent.c | 14 +++++---------
src/qemu/qemu_process.c | 7 -------
2 files changed, 5 insertions(+), 16 deletions(-)
diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c
index 166cfaf485..8bbaa127d4 100644
--- a/src/qemu/qemu_agent.c
+++ b/src/qemu/qemu_agent.c
@@ -171,9 +171,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);
@@ -693,7 +695,7 @@ qemuAgentOpen(virDomainObj *vm,
virObjectUnref(agent);
return NULL;
}
- agent->vm = vm;
+ agent->vm = virObjectRef(vm);
agent->cb = cb;
agent->singleSync = singleSync;
@@ -729,12 +731,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 d2ea9b55fe..b1fd72d269 100644
--- 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",
--
2.32.0