[libvirt] [PATCH 01/21] qemu_hotplug: remove erroneous call to qemuDomainDetachExtensionDevice()
Ján Tomko
jtomko at redhat.com
Fri Mar 22 11:57:51 UTC 2019
On Thu, Mar 21, 2019 at 06:28:41PM -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
You would have had me convinced at '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(-)
>
Reviewed-by: Ján Tomko <jtomko at redhat.com>
Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20190322/385b473b/attachment-0001.sig>
More information about the libvir-list
mailing list