[libvirt] [PATCH 2/2] qemu: Call virDomainDefCompatibleDevice when live add cdrom/floppy

[Jiri Denemark]

On Thu, Mar 01, 2018 at 08:03:39 -0500, John Ferlan wrote:
> Prior to calling qemuDomainChangeEjectableMedia when adding
> the cdrom/floppy to the guest, make a second pass at the
> virDomainDefCompatibleDevice with the old_disk that's being
> updated by the call.
> 
> This is similar to the qemuDomainChangeDiskLive path from
> qemuDomainUpdateDeviceLive prior to making the same call to
> qemuDomainChangeEjectableMedia when the cdrom/floppy is
> updated rather than added.
> 
> Signed-off-by: John Ferlan <jferlan at redhat.com>
> ---
>  src/qemu/qemu_hotplug.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
> index e0a5300f0..4fbc0f48c 100644
> --- a/src/qemu/qemu_hotplug.c
> +++ b/src/qemu/qemu_hotplug.c
> @@ -697,6 +697,7 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
>      size_t i;
>      virDomainDiskDefPtr disk = dev->data.disk;
>      virDomainDiskDefPtr orig_disk = NULL;
> +    virDomainDeviceDef orig_dev = { .type = dev->type };
>      int ret = -1;
>  
>      if (STRNEQ_NULLABLE(virDomainDiskGetDriver(disk), "qemu")) {
> @@ -732,6 +733,13 @@ qemuDomainAttachDeviceDiskLive(virQEMUDriverPtr driver,
>              goto cleanup;
>          }
>  
> +        /* Although called in qemuDomainAttachDeviceLiveAndConfig, now we
> +         * know the orig_disk/dev so let's make the additional check for
> +         * boot order checking */
> +        orig_dev.data.disk = orig_disk;
> +        if (virDomainDefCompatibleDevice(vm->def, dev, &orig_dev) < 0)
> +            goto cleanup;
> +
>          if (qemuDomainChangeEjectableMedia(driver, vm, orig_disk,
>                                             disk->src, false) < 0)
>              goto cleanup;

This change will have no effect. Calling virDomainDefCompatibleDevice
with oldDev != NULL makes the check less strict (the oldDev will be
ignored when checking for conflicts) so if virDomainDefCompatibleDevice
with NULL oldDev did not complain about any conflict, this second call
will not complain either. Moreover, if the new device configuration
changed more than source (e.g., it changed boot order too), the first
virDomainDefCompatibleDevice call will report conflict with the original
version of the disk even though the second one which passes non-NULL
oldDev would have ignored this conflict. Fixing this would require us to
skip the first call to virDomainDefCompatibleDevice for cdrom and floppy
devices, but I think it's actually fine if AttachDevice refuses to
update anything but the source (since the code really cares only about
the source).

That said, I don't think there's anything we need to fix here.

Jirka