The code that detaches the device from persistent definition copies the
persistent definition first so that it can easily be rolled back. The
actual detaching is then made in the copy which is assigned back on
success (if the live operation succeeded as well).

This is not the case in qemuDomainDetachDeviceAliasLiveAndConfig where
the definition was copied and put back, but the detaching happened from
the other object which was overwritten.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_driver.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index c200c5a63d..b7b318896f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8822,14 +8822,13 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver,
     if (persistentDef) {
         virDomainDeviceDef dev;

-        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
-        if (!vmdef)
+        if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt)))
             goto cleanup;

-        if (virDomainDefFindDevice(persistentDef, alias, &dev, true) < 0)
+        if (virDomainDefFindDevice(vmdef, alias, &dev, true) < 0)
             goto cleanup;

-        if (qemuDomainDetachDeviceConfig(persistentDef, &dev, caps,
+        if (qemuDomainDetachDeviceConfig(vmdef, &dev, caps,
                                          parse_flags, driver->xmlopt) < 0)
             goto cleanup;
     }
-- 
2.16.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

On 06/05/2018 09:38 AM, Peter Krempa wrote:
> The code that detaches the device from persistent definition copies the
> persistent definition first so that it can easily be rolled back. The
> actual detaching is then made in the copy which is assigned back on
> success (if the live operation succeeded as well).
> 
> This is not the case in qemuDomainDetachDeviceAliasLiveAndConfig where
> the definition was copied and put back, but the detaching happened from
> the other object which was overwritten.
> 
> Signed-off-by: Peter Krempa <pkrempa@redhat.com>
> ---
>  src/qemu/qemu_driver.c | 7 +++----
>  1 file changed, 3 insertions(+), 4 deletions(-)
> 
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index c200c5a63d..b7b318896f 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -8822,14 +8822,13 @@ qemuDomainDetachDeviceAliasLiveAndConfig(virQEMUDriverPtr driver,
>      if (persistentDef) {
>          virDomainDeviceDef dev;
> 
> -        vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt);
> -        if (!vmdef)
> +        if (!(vmdef = virDomainObjCopyPersistentDef(vm, caps, driver->xmlopt)))
>              goto cleanup;
> 
> -        if (virDomainDefFindDevice(persistentDef, alias, &dev, true) < 0)
> +        if (virDomainDefFindDevice(vmdef, alias, &dev, true) < 0)
>              goto cleanup;
> 
> -        if (qemuDomainDetachDeviceConfig(persistentDef, &dev, caps,
> +        if (qemuDomainDetachDeviceConfig(vmdef, &dev, caps,
>                                           parse_flags, driver->xmlopt) < 0)
>              goto cleanup;
>      }
> 

Ooops, yes. ACK.

Michal

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list