[libvirt] [PATCH v3 10/12] qemu: Add hotpluging support for PCI devices on S390 guests

[Andrea Bolognani]

On Tue, 2018-08-07 at 17:10 +0800, Yi Min Zhao wrote:
> This commit adds hotplug support for PCI devices on S390 guests.
> There's no need to implement hot unplug for zPCI as QEMU implements
> an unplug callback which will unplug both PCI and zPCI device in a
> cascaded way.

It looks like you ended up implementing explicit hot unplug at
least for controllers. I think perhaps it would be a good idea
to implement it for all devices instead of relying on QEMU's
own unplug cascading so that we have more control over the whole
process.

> @@ -669,8 +737,13 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver,
>      if (qemuHotplugDiskSourceAttach(priv->mon, diskdata) < 0)
>          goto exit_monitor;
>  
> -    if (qemuMonitorAddDevice(priv->mon, devstr) < 0)
> +    if (qemuDomainAttachExtensionDevice(priv->mon, &disk->info) < 0)
> +        goto exit_monitor;
> +
> +    if (qemuMonitorAddDevice(priv->mon, devstr) < 0) {
> +        ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info));
>          goto exit_monitor;
> +    }
>  
>      if (qemuDomainObjExitMonitor(driver, vm) < 0) {
>          ret = -2;

So, I'm very much not familiar with the hotplug code and seeing
changes to stuff like qemuDomainAttachDiskGeneric() makes me a bit
uncomfortable :)

I can't spot anything obviously wrong in your changes, but I think
perhaps you might want to enter and exit the monitor separately
for the zpci device and for the virtio device? I'm not sure that's
useful at all, but network devices for example seems to follow
that pattern... It would be great if someone with more experience
in this area could provide a review.

-- 
Andrea Bolognani / Red Hat / Virtualization