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

Laine Stump laine at laine.org
Thu Mar 21 22:28:41 UTC 2019


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(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 41d60277d1..18075dc48e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -5457,7 +5457,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
 {
     int idx, ret = -1;
     virDomainControllerDefPtr detach = NULL;
-    qemuDomainObjPrivatePtr priv = vm->privateData;
 
     if ((idx = virDomainControllerFind(vm->def,
                                        dev->data.controller->type,
@@ -5503,17 +5502,6 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,
     if (!async)
         qemuDomainMarkDeviceForRemoval(vm, &detach->info);
 
-    if (detach->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) {
-        int rc;
-        qemuDomainObjEnterMonitor(driver, vm);
-        rc = qemuDomainDetachExtensionDevice(priv->mon, &detach->info);
-        if (qemuDomainObjExitMonitor(driver, vm) < 0)
-            rc = -1;
-
-        if (rc < 0)
-            goto cleanup;
-    }
-
     if (qemuDomainDeleteDevice(vm, detach->info.alias) < 0)
         goto cleanup;
 
-- 
2.20.1




More information about the libvir-list mailing list