Re: [libvirt] [PATCH v2 8/8] qemu: Emit VIR_DOMAIN_EVENT_ID_DEVICE_REMOVED events

On Thu, Jul 18, 2013 at 11:44:20 +0100, Daniel Berrange wrote: > On Thu, Jul 18, 2013 at 12:03:50PM +0200, Jiri Denemark wrote: > > --- > > > > Notes: > > Version 2: > > - update qemuhotplugtest to initialize domainEventState > > > > src/qemu/qemu_hotplug.c | 30 ++++++++++++++++++++++++++++-- > > tests/qemuhotplugtest.c | 3 +++ > > 2 files changed, 31 insertions(+), 2 deletions(-) > > > > diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c > > index b1ddd92..a6d9a03 100644 > > --- a/src/qemu/qemu_hotplug.c > > +++ b/src/qemu/qemu_hotplug.c > > @@ -2234,6 +2234,7 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, > > virDomainDiskDefPtr disk) > > { > > virDomainDeviceDef dev; > > + virDomainEventPtr event; > > size_t i; > > > > VIR_DEBUG("Removing disk %s from domain %p %s", > > @@ -2241,6 +2242,10 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, > > > > virDomainAuditDisk(vm, disk->src, NULL, "detach", true); > > > > + event = virDomainEventDeviceRemovedNewFromObj(vm, disk->info.alias); > > + if (event) > > + qemuDomainEventQueue(driver, event); > > + > > for (i = 0; i < vm->def->ndisks; i++) { > > if (vm->def->disks[i] == disk) { > > virDomainDiskRemove(vm->def, i); > > @@ -2269,15 +2274,20 @@ qemuDomainRemoveDiskDevice(virQEMUDriverPtr driver, > > > > > > static void > > -qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > > +qemuDomainRemoveControllerDevice(virQEMUDriverPtr driver, > > virDomainObjPtr vm, > > virDomainControllerDefPtr controller) > > { > > + virDomainEventPtr event; > > size_t i; > > > > VIR_DEBUG("Removing controller %s from domain %p %s", > > controller->info.alias, vm, vm->def->name); > > > > + event = virDomainEventDeviceRemovedNewFromObj(vm, controller->info.alias); > > + if (event) > > + qemuDomainEventQueue(driver, event); > > + > > for (i = 0; i < vm->def->ncontrollers; i++) { > > if (vm->def->controllers[i] == controller) { > > virDomainControllerRemove(vm->def, i); > > @@ -2297,6 +2307,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, > > { > > virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); > > virNetDevVPortProfilePtr vport; > > + virDomainEventPtr event; > > size_t i; > > > > VIR_DEBUG("Removing network interface %s from domain %p %s", > > @@ -2304,6 +2315,10 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, > > > > virDomainAuditNet(vm, net, NULL, "detach", true); > > > > + event = virDomainEventDeviceRemovedNewFromObj(vm, net->info.alias); > > + if (event) > > + qemuDomainEventQueue(driver, event); > > + > > for (i = 0; i < vm->def->nnets; i++) { > > if (vm->def->nets[i] == net) { > > virDomainNetRemove(vm->def, i); > > @@ -2420,11 +2435,16 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, > > virDomainHostdevDefPtr hostdev) > > { > > virDomainNetDefPtr net = NULL; > > + virDomainEventPtr event; > > size_t i; > > > > VIR_DEBUG("Removing host device %s from domain %p %s", > > hostdev->info->alias, vm, vm->def->name); > > > > + event = virDomainEventDeviceRemovedNewFromObj(vm, hostdev->info->alias); > > + if (event) > > + qemuDomainEventQueue(driver, event); > > + > > if (hostdev->parent.type == VIR_DOMAIN_DEVICE_NET) { > > net = hostdev->parent.data.net; > > > > I'm not 100% clear on why we need to emit the event from these methods > above. > > > > @@ -2477,13 +2497,19 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, > > > > > > static void > > -qemuDomainRemoveChrDevice(virQEMUDriverPtr driver ATTRIBUTE_UNUSED, > > +qemuDomainRemoveChrDevice(virQEMUDriverPtr driver, > > virDomainObjPtr vm, > > virDomainChrDefPtr chr) > > { > > + virDomainEventPtr event; > > + > > VIR_DEBUG("Removing character device %s from domain %p %s", > > chr->info.alias, vm, vm->def->name); > > > > + event = virDomainEventDeviceRemovedNewFromObj(vm, chr->info.alias); > > + if (event) > > + qemuDomainEventQueue(driver, event); > > + > > qemuDomainChrRemove(vm->def, chr); > > virDomainChrDefFree(chr); > > } > > I would have though this is the only place where we should be emitting > the remove event. Emitting in the other methods seems to cause the > duplication of events, no ? No, all methods are specific to a single device type and since we want to emit the event for all device types, we have to emit it in each of these methods.