From 77404a4f0ff85fbc0a2c24afc736ffbd8b82ef4d Mon Sep 17 00:00:00 2001

Message-Id: <77404a4f0ff85fbc0a2c24afc736ffbd8b82ef4d@dist-git>

From: Laine Stump <laine@laine.org>

Date: Mon, 8 Apr 2019 10:57:31 +0200

Subject: [PATCH] qemu_hotplug: remove erroneous call to

 qemuDomainDetachExtensionDevice()

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

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@laine.org>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

Reviewed-by: Boris Fiuczynski <fiuczy@linux.ibm.com>

(cherry picked from commit 143291698358f5a1e693c768893d89038420af25)

https://bugzilla.redhat.com/show_bug.cgi?id=1508149

Conflicts:

  * src/qemu/qemu_hotplug.c

    + the code dealing with entering and exiting the monitor is

      quite a bit different because we don't have backported

      qemuDomainDeleteDevice() downstream

      - missing 4cd13478ac33

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

Message-Id: <20190408085732.28684-15-abologna@redhat.com>

Reviewed-by: Laine Stump <laine@redhat.com>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

---

 src/qemu/qemu_hotplug.c | 6 ------

 1 file changed, 6 deletions(-)

diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c

index 8394efa739..f16213c6e0 100644

--- a/src/qemu/qemu_hotplug.c

+++ b/src/qemu/qemu_hotplug.c

@@ -5077,17 +5077,11 @@ int qemuDomainDetachControllerDevice(virQEMUDriverPtr driver,

         qemuDomainMarkDeviceForRemoval(vm, &detach->info);

     qemuDomainObjEnterMonitor(driver, vm);

-    if (detach->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&

-        qemuDomainDetachExtensionDevice(priv->mon, &detach->info)) {

-        goto exit_monitor;

-    }

-

     if (qemuMonitorDelDevice(priv->mon, detach->info.alias)) {

         ignore_value(qemuDomainObjExitMonitor(driver, vm));

         goto cleanup;

     }

- exit_monitor:

     if (qemuDomainObjExitMonitor(driver, vm) < 0)

         goto cleanup;

-- 

2.22.0