[libvirt] [PATCH v2 2/3] conf: Reintroduce action to virDomainDefCompatibleDevice

John Ferlan jferlan at redhat.com
Tue Jun 26 14:30:12 UTC 2018



On 06/26/2018 06:21 AM, Michal Privoznik wrote:
> This was lost in c57f3fd2f8999d17e01. But now we are going to
> need it again.

Looks like that commit also "lost" the compatible device check on detach
too (e.g. ACTION_DETACH) without explanation.  Whether at the time it
then became the last consumer of virDomainDeviceAction is another question.

So, since you're bringing this back to life - should those checks be
reinstated into the Detach code (lxc, qemu)? If not, surely
ACTION_DETACH then still isn't used.

Can I assume you tested bz1439991 with the new condition?

The restoration of what's been done thus far looks fine, just need some
more details...

John

> 
> Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/conf/domain_conf.c |  3 ++-
>  src/conf/domain_conf.h |  3 ++-
>  src/lxc/lxc_driver.c   |  9 ++++++---
>  src/qemu/qemu_driver.c | 24 ++++++++++++++++--------
>  4 files changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index d8cb7f37f3..93cfca351c 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -28205,7 +28205,8 @@ virDomainDeviceInfoCheckBootIndex(virDomainDefPtr def ATTRIBUTE_UNUSED,
>  int
>  virDomainDefCompatibleDevice(virDomainDefPtr def,
>                               virDomainDeviceDefPtr dev,
> -                             virDomainDeviceDefPtr oldDev)
> +                             virDomainDeviceDefPtr oldDev,
> +                             virDomainDeviceAction action ATTRIBUTE_UNUSED)
>  {
>      virDomainCompatibleDeviceData data = {
>          .newInfo = virDomainDeviceGetInfo(dev),
> diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> index 0924fc4f3c..f33405e097 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -3106,7 +3106,8 @@ typedef enum {
>  
>  int virDomainDefCompatibleDevice(virDomainDefPtr def,
>                                   virDomainDeviceDefPtr dev,
> -                                 virDomainDeviceDefPtr oldDev);
> +                                 virDomainDeviceDefPtr oldDev,
> +                                 virDomainDeviceAction action);
>  
>  void virDomainRNGDefFree(virDomainRNGDefPtr def);
>  
> diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
> index cfb431488d..850b12726b 100644
> --- a/src/lxc/lxc_driver.c
> +++ b/src/lxc/lxc_driver.c
> @@ -3549,7 +3549,8 @@ lxcDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>              goto cleanup;
>  
>          oldDev.data.net = vmdef->nets[idx];
> -        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
> +                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>              return -1;
>  
>          virDomainNetDefFree(vmdef->nets[idx]);
> @@ -4785,7 +4786,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
>          if (!vmdef)
>              goto endjob;
>  
> -        if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
> +                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
>              goto endjob;
>  
>          if ((ret = lxcDomainAttachDeviceConfig(vmdef, dev)) < 0)
> @@ -4793,7 +4795,8 @@ static int lxcDomainAttachDeviceFlags(virDomainPtr dom,
>      }
>  
>      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
> +        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
> +                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
>              goto endjob;
>  
>          if ((ret = lxcDomainAttachDeviceLive(dom->conn, driver, vm, dev_copy)) < 0)
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index e3fb4919a5..8c0c681c4d 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -7885,7 +7885,8 @@ qemuDomainChangeDiskLive(virDomainObjPtr vm,
>      }
>  
>      oldDev.data.disk = orig_disk;
> -    if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
> +    if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
> +                                     VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>          goto cleanup;
>  
>      if (!qemuDomainDiskChangeSupported(disk, orig_disk))
> @@ -7943,7 +7944,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_GRAPHICS:
>          if ((idx = qemuDomainFindGraphicsIndex(vm->def, dev->data.graphics)) >= 0) {
>              oldDev.data.graphics = vm->def->graphics[idx];
> -            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
> +            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
> +                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>                  return -1;
>          }
>  
> @@ -7953,7 +7955,8 @@ qemuDomainUpdateDeviceLive(virDomainObjPtr vm,
>      case VIR_DOMAIN_DEVICE_NET:
>          if ((idx = virDomainNetFindIdx(vm->def, dev->data.net)) >= 0) {
>              oldDev.data.net = vm->def->nets[idx];
> -            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev) < 0)
> +            if (virDomainDefCompatibleDevice(vm->def, dev, &oldDev,
> +                                             VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>                  return -1;
>          }
>  
> @@ -8406,7 +8409,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>          }
>  
>          oldDev.data.disk = vmdef->disks[pos];
> -        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
> +                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>              return -1;
>  
>          virDomainDiskDefFree(vmdef->disks[pos]);
> @@ -8425,7 +8429,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>          }
>  
>          oldDev.data.graphics = vmdef->graphics[pos];
> -        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
> +                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>              return -1;
>  
>          virDomainGraphicsDefFree(vmdef->graphics[pos]);
> @@ -8439,7 +8444,8 @@ qemuDomainUpdateDeviceConfig(virDomainDefPtr vmdef,
>              return -1;
>  
>          oldDev.data.net = vmdef->nets[pos];
> -        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, &oldDev,
> +                                         VIR_DOMAIN_DEVICE_ACTION_UPDATE) < 0)
>              return -1;
>  
>          virDomainNetDefFree(vmdef->nets[pos]);
> @@ -8530,7 +8536,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>          if (!vmdef)
>              goto cleanup;
>  
> -        if (virDomainDefCompatibleDevice(vmdef, dev, NULL) < 0)
> +        if (virDomainDefCompatibleDevice(vmdef, dev, NULL,
> +                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
>              goto cleanup;
>          if ((ret = qemuDomainAttachDeviceConfig(vmdef, dev, caps,
>                                                  parse_flags,
> @@ -8539,7 +8546,8 @@ qemuDomainAttachDeviceLiveAndConfig(virDomainObjPtr vm,
>      }
>  
>      if (flags & VIR_DOMAIN_AFFECT_LIVE) {
> -        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL) < 0)
> +        if (virDomainDefCompatibleDevice(vm->def, dev_copy, NULL,
> +                                         VIR_DOMAIN_DEVICE_ACTION_ATTACH) < 0)
>              goto cleanup;
>  
>          if ((ret = qemuDomainAttachDeviceLive(vm, dev_copy, driver)) < 0)
> 




More information about the libvir-list mailing list