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@xxxxxxxxxx>
> ---
> 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)
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
