[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

Jirka


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]