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
Attachment:
signature.asc
Description: Digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list