On Mon, Oct 26, 2015 at 11:06:21 -0400, John Ferlan wrote:
If we have a shutdown of a VM by a qemu agent while an agent EOF occurs at relatively the same time, it's possible that a deadlock occurs depending on what happens first.
Assume qemuProcessHandleAgentEOF runs first, with the vm->lock it clears priv->agent, then clears the vm->lock.
Couldn't we make sure, that qemuProcessHandleAgentEOF clears priv->agent if and only if it removed the last reference? [...]
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 890d8ed..a908df8 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1868,19 +1868,20 @@ qemuDomainObjEnterAgent(virDomainObjPtr obj) * Should be paired with an earlier qemuDomainObjEnterAgent() call */ void -qemuDomainObjExitAgent(virDomainObjPtr obj) +qemuDomainObjExitAgent(virDomainObjPtr obj, + qemuAgentPtr agent)
You would not need to modify this prototype ...
{ qemuDomainObjPrivatePtr priv = obj->privateData; bool hasRefs;
- hasRefs = virObjectUnref(priv->agent); + hasRefs = virObjectUnref(agent);
if (hasRefs) - virObjectUnlock(priv->agent); + virObjectUnlock(agent);
virObjectLock(obj); - VIR_DEBUG("Exited agent (agent=%p vm=%p name=%s)", - priv->agent, obj, obj->def->name); + VIR_DEBUG("Exited agent (agent=%p vm=%p name=%s hasRefs=%d)", + agent, obj, obj->def->name, hasRefs);
priv->agentStart = 0; if (!hasRefs)
void qemuDomainObjEnterRemote(virDomainObjPtr obj) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index a2cc002..b8c9ff7 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -1994,9 +1994,10 @@ static int qemuDomainShutdownFlags(virDomainPtr dom, unsigned int flags) qemuDomainSetFakeReboot(driver, vm, isReboot);
if (useAgent) { + qemuAgentPtr agent = priv->agent; qemuDomainObjEnterAgent(vm); - ret = qemuAgentShutdown(priv->agent, agentFlag); - qemuDomainObjExitAgent(vm); + ret = qemuAgentShutdown(agent, agentFlag); + qemuDomainObjExitAgent(vm, agent);
... and could avoid this rather ugly code here. The result would be IMO the same. Peter