[libvirt] [PATCH] Use correct pci addresses during device-detach

[Michal Privoznik]

On 09.10.2015 07:58, Nitesh_Konkar wrote:
> From: Nitesh_Konkar <niteshkonkar at in.ibm.com>
> 
> The attach-device on live and persistent copies can be done independently.
> Thus devices can end up having different pci addresses in live and persistent
> copies. The detach device should try to detach the device from their respective
> addresses instead of using the same from live/persistent.
> 
> Signed-off-by:nitkon12 at linux.vnet.ibm.com
> ---
>  src/driver-nodedev.h   |  1 +
>  src/qemu/qemu_driver.c | 25 ++++++++++---------------
>  2 files changed, 11 insertions(+), 15 deletions(-)
> 
> diff --git a/src/driver-nodedev.h b/src/driver-nodedev.h
> index e846612..dea79bd 100644
> --- a/src/driver-nodedev.h
> +++ b/src/driver-nodedev.h
> @@ -59,6 +59,7 @@ typedef char *
>  typedef char *
>  (*virDrvNodeDeviceGetParent)(virNodeDevicePtr dev); 
> +

This is unrelated change.

>  typedef int
>  (*virDrvNodeDeviceNumOfCaps)(virNodeDevicePtr dev);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index f133b45..6fd58c2 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8708,23 +8708,12 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
>          !(flags & VIR_DOMAIN_AFFECT_LIVE))
>          parse_flags |= VIR_DOMAIN_DEF_PARSE_INACTIVE;
>  
> -    dev = dev_copy = virDomainDeviceDefParse(xml, vm->def,
> +    dev_copy = virDomainDeviceDefParse(xml, vm->def,
>                                               caps, driver->xmlopt,
>                                               parse_flags);
> -    if (dev == NULL)
> +    if (dev_copy == NULL)
>          goto endjob;
>  
> -    if (flags & VIR_DOMAIN_AFFECT_CONFIG &&
> -        flags & VIR_DOMAIN_AFFECT_LIVE) {
> -        /* If we are affecting both CONFIG and LIVE
> -         * create a deep copy of device as adding
> -         * to CONFIG takes one instance.
> -         */
> -        dev_copy = virDomainDeviceDefCopy(dev, vm->def, caps, driver->xmlopt);
> -        if (!dev_copy)
> -            goto endjob;
> -    }
> -
>      if (priv->qemuCaps)
>          qemuCaps = virObjectRef(priv->qemuCaps);
>      else if (!(qemuCaps = virQEMUCapsCacheLookup(driver->qemuCapsCache, vm->def->emulator)))
> @@ -8736,6 +8725,13 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
>          if (!vmdef)
>              goto endjob;
>  
> +        dev = virDomainDeviceDefParse(xml, vmdef,
> +                                             caps, driver->xmlopt,
> +                                             parse_flags);
> +        if (!dev)
> +            goto endjob;
> +
> +
>          if (virDomainDefCompatibleDevice(vmdef, dev,
>                                           VIR_DOMAIN_DEVICE_ACTION_DETACH) < 0)
>              goto endjob;
> @@ -8777,8 +8773,7 @@ static int qemuDomainDetachDeviceFlags(virDomainPtr dom, const char *xml,
>   cleanup:
>      virObjectUnref(qemuCaps);
>      virDomainDefFree(vmdef);
> -    if (dev != dev_copy)
> -        virDomainDeviceDefFree(dev_copy);
> +    virDomainDeviceDefFree(dev_copy);
>      virDomainDeviceDefFree(dev);
>      virDomainObjEndAPI(&vm);
>      virObjectUnref(caps);
> 

I don't see how this will help. I mean, if you have a device on
different addresses in live and inactive XMLs you will need to provide
two different APIs anyway. The fact that detach sometimes works even on
partial XMLs (or that some fields are unchecked) is just lack of
implementation. Users are required to pass whole device XML. So if the
device is on different addresses you should call the API two times -
each time with correct address and flag set (CONFIG or LIVE).

Michal