[libvirt] [PATCH 01/21] qemu_hotplug: remove erroneous call to qemuDomainDetachExtensionDevice()

[Peter Krempa]

On Thu, Mar 21, 2019 at 18:28:41 -0400, Laine Stump wrote:
> qemuDomainDetachControllerDevice() calls
> qemuDomainDetachExtensionDevice() when the *controller* type is
> PCI. This is incorrect in multiple ways:
> 
> * Any code that tears down a device should be in the
>   qemuDomainRemove*Device() function (which is called after libvirt
>   gets a DEVICE_DELETED event from qemu indicating that the guest is
>   finished with the device on its end. The qemuDomainDetach*Device()
>   functions should only contain code that ensures the requested
>   operation is valid, and sends the command to qemu to initiate the
>   unplug.
> 
> * qemuDomainDetachExtensionDevice() is a function that applies to
>   devices that plug into a PCI slot, *not* necessarily PCI controllers
>   (which is what's being checked in the offending code). The proper
>   way to check for this would be to see if the DeviceInfo for the
>   controller device had a PCI address, not to check if the controller
>   is a PCI controller (the code being removed was doing the latter).
> 
> * According to commit 1d1e264f1 that added this code (and other
>   support for hotplugging zPCI devices on s390), it's not necessary to
>   explicitly detach the zPCI device when unplugging a PCI device. To
>   quote:
> 
>        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.
> 
>   and the evidence bears this out - all the other uses of
>   qemuDomainDetachExtensionDevice() (except one, which I believe is
>   also in error, and is being removed in a separate patch) are only to
>   remove the zPCI extension device in cases where it was successfully
>   added, but there was some other failure later in the hotplug process
>   (so there was no regular PCI device to remove and trigger removal of
>   the zPCI extension device).
> 
> * PCI controllers are not hot pluggable, so this is dead code
>   anyway. (The only controllers that can currently be
>   hotplugged/unplugged are SCSI controllers).
> 
> Signed-off-by: Laine Stump <laine at laine.org>
> ---
>  src/qemu/qemu_hotplug.c | 12 ------------
>  1 file changed, 12 deletions(-)

The code change looks okay to me, but I know nothing of zPCI. ACKing
this just because I don't care about zPCI would be wrong :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190322/688c1e9e/attachment-0001.sig>