[libvirt] [PATCH] qemu: Close the agent connection only on agent channel events

[Michal Privoznik]

On 30.06.2015 10:57, Peter Krempa wrote:
> processSerialChangedEvent processes events for all channels. Commit
> 2af51483 broke all agent interaction if a channel other than the agent
> closes since it did not check that the event actually originated from
> the guest agent channel.
> 
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1236924
> Fixes up: https://bugzilla.redhat.com/show_bug.cgi?id=890648
> ---
>  src/qemu/qemu_driver.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 2b530c8..353be31 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -4448,10 +4448,20 @@ processSerialChangedEvent(virQEMUDriverPtr driver,
> 
>      if (newstate == VIR_DOMAIN_CHR_DEVICE_STATE_DISCONNECTED &&
>          virDomainObjIsActive(vm) && priv->agent) {
> -        /* Close agent monitor early, so that other threads
> -         * waiting for the agent to reply can finish and our
> -         * job we acquire below can succeed. */
> -       qemuAgentNotifyClose(priv->agent);
> +        /* peek into the domain definition to find the channel */
> +        if (virDomainDefFindDevice(vm->def, devAlias, &dev, true) == 0 &&
> +            dev.type == VIR_DOMAIN_DEVICE_CHR &&
> +            dev.data.chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
> +            dev.data.chr->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO &&
> +            STREQ_NULLABLE(dev.data.chr->target.name, "org.qemu.guest_agent.0"))
> +            /* Close agent monitor early, so that other threads
> +             * waiting for the agent to reply can finish and our
> +             * job we acquire below can succeed. */
> +            qemuAgentNotifyClose(priv->agent);
> +
> +        /* now discard the data, since it may possibly change once we unlock
> +         * while entering the job */
> +        memset(&dev, 0, sizeof(dev));


Well, it's not used anywhere in between here and the other place where
the variable is re-initialized with another virDomainDefFindDevice()
call. But it doesn't hurt either.

>      }
> 
>      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> 

ACK and safe for the freeze.

Michal