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

[Peter Krempa]

On Tue, Jun 30, 2015 at 11:52:35 +0200, Michal Privoznik wrote:
> 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.

I want be super sure in this case that somebody does not optimize that
out later in some way.

> 
> >      }
> > 
> >      if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
> > 
> 
> ACK and safe for the freeze.

Pushed; Thanks.

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/20150630/a7ae8c26/attachment-0001.sig>