[libvirt] [PATCH] qemu: Resolve agent deadlock

[Peter Krempa]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: Digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20151026/d3434379/attachment-0001.sig>