[libvirt] [PATCH 2/2] Emit VIR_DOMAIN_EVENT_ID_DEVICE_ADDED in the QEMU driver

[John Ferlan]

On 04/04/2015 01:16 PM, Ján Tomko wrote:
> Only for devices that have an alias.
> ---
>  src/qemu/qemu_driver.c  | 17 ++++++++++++++++-
>  src/qemu/qemu_hotplug.c |  5 +++++
>  2 files changed, 21 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6132674..c13f22b 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7586,17 +7586,20 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>  {
>      virQEMUDriverPtr driver = dom->conn->privateData;
>      int ret = -1;
> +    const char *alias = NULL;
>  
>      switch ((virDomainDeviceType) dev->type) {
>      case VIR_DOMAIN_DEVICE_DISK:
>          qemuDomainObjCheckDiskTaint(driver, vm, dev->data.disk, -1);
>          ret = qemuDomainAttachDeviceDiskLive(dom->conn, driver, vm, dev);
> +        alias = dev->data.disk->info.alias;
>          if (!ret)
>              dev->data.disk = NULL;
>          break;
>  
>      case VIR_DOMAIN_DEVICE_CONTROLLER:
>          ret = qemuDomainAttachDeviceControllerLive(driver, vm, dev);
> +        alias = dev->data.controller->info.alias;

The one concern I'd have for all of these is - if (ret != 0) - is there
any path that free's anything along the way that you're using a pointer
in the alias fetching?

Additionally of course, since the only way to print the alias is if (ret
== 0) later, one could point out that setting it when ret != 0 is
pointless; however, at least if ret == 0, you should be able to assume
no one as deleted the alias!

Perhaps it's best to only get the alias if (!ret)

Your call if you want to add a "note" for case VIR_DOMAIN_DEVICE_MEMORY
that the event is elicited inside the call since the call consumes
dev->data.memory and hence the alias.

I think with the alias setting inside !ret I'd feel comfortable giving
an ACK - although I suspect in the other case it's not deleted until
after this call exits

John

>          if (!ret)
>              dev->data.controller = NULL;
>          break;
> @@ -7612,6 +7615,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>          qemuDomainObjCheckNetTaint(driver, vm, dev->data.net, -1);
>          ret = qemuDomainAttachNetDevice(dom->conn, driver, vm,
>                                          dev->data.net);
> +        alias = dev->data.net->info.alias;
>          if (!ret)
>              dev->data.net = NULL;
>          break;
> @@ -7620,6 +7624,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>          qemuDomainObjCheckHostdevTaint(driver, vm, dev->data.hostdev, -1);
>          ret = qemuDomainAttachHostDevice(dom->conn, driver, vm,
>                                           dev->data.hostdev);
> +        alias = dev->data.hostdev->info->alias;
>          if (!ret)
>              dev->data.hostdev = NULL;
>          break;
> @@ -7627,6 +7632,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_REDIRDEV:
>          ret = qemuDomainAttachRedirdevDevice(driver, vm,
>                                               dev->data.redirdev);
> +        alias = dev->data.redirdev->info.alias;
>          if (!ret)
>              dev->data.redirdev = NULL;
>          break;
> @@ -7634,6 +7640,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_CHR:
>          ret = qemuDomainAttachChrDevice(driver, vm,
>                                          dev->data.chr);
> +        alias = dev->data.chr->info.alias;
>          if (!ret)
>              dev->data.chr = NULL;
>          break;
> @@ -7641,6 +7648,7 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_RNG:
>          ret = qemuDomainAttachRNGDevice(driver, vm,
>                                          dev->data.rng);
> +        alias = dev->data.rng->info.alias;
>          if (!ret)
>              dev->data.rng = NULL;
>          break;
> @@ -7673,8 +7681,15 @@ qemuDomainAttachDeviceLive(virDomainObjPtr vm,
>          break;
>      }
>  
> -    if (ret == 0)
> +    if (ret == 0) {
>          ret = qemuDomainUpdateDeviceList(driver, vm, QEMU_ASYNC_JOB_NONE);
> +        if (alias) {
> +            virObjectEventPtr event;
> +            event = virDomainEventDeviceAddedNewFromObj(vm, alias);
> +            if (event)
> +                qemuDomainEventQueue(driver, event);
> +        }
> +    }
>  
>      return ret;
>  }
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index 2f0549e..f07c54d 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -1726,6 +1726,7 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>      char *objalias = NULL;
>      const char *backendType;
>      virJSONValuePtr props = NULL;
> +    virObjectEventPtr event;
>      int id;
>      int ret = -1;
>  
> @@ -1769,6 +1770,10 @@ qemuDomainAttachMemory(virQEMUDriverPtr driver,
>          goto cleanup;
>      }
>  
> +    event = virDomainEventDeviceAddedNewFromObj(vm, objalias);
> +    if (event)
> +        qemuDomainEventQueue(driver, event);
> +
>      /* mem is consumed by vm->def */
>      mem = NULL;
>  
>