[PATCHv2 2/2] pci: make virPCIDeviceReset more autonomous

[Laine Stump <laine@xxxxxxxxx>]

I recently patches the callers to virPCIDeviceReset() to not call it
if the current driver for a device was vfio-pci (since that driver
will always reset the device itself when appropriate. At the time, Dan
Berrange suggested that I could instead modify virPCIDeviceReset
to check the currently bound driver for the device, and decide
for itself whether or not to go ahead with the reset.

This patch removes the previously added checks, and replaces them with
a check down in virPCIDeviceReset(), as suggested.

The functional difference here is that previously we were deciding
based on either the hostdev configuration or the value of
stubDriverName in the virPCIDevice object, but now we are actually
comparing to the "driver" link in the device's sysfs entry
directly. In practice, both should be the same.
---
 src/qemu/qemu_hostdev.c |  6 ++----
 src/qemu/qemu_hotplug.c |  5 ++---
 src/util/virpci.c       | 27 ++++++++++++++++++++++++---
 3 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 404939e..3001d6b 100644
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -544,8 +544,7 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver,
      * can safely reset them */
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
-        if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci"))
-            continue;
+
         if (virPCIDeviceReset(dev, driver->activePciHostdevs,
                               driver->inactivePciHostdevs) < 0)
             goto reattachdevs;
@@ -1116,8 +1115,7 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver,
 
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
-        if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci"))
-            continue;
+
         if (virPCIDeviceReset(dev, driver->activePciHostdevs,
                               driver->inactivePciHostdevs) < 0) {
             virErrorPtr err = virGetLastError();
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
index 46875ad..5edd36e 100644
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2528,9 +2528,8 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver,
     if (pci) {
         activePci = virPCIDeviceListSteal(driver->activePciHostdevs, pci);
         if (activePci &&
-            (subsys->u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO ||
-             virPCIDeviceReset(activePci, driver->activePciHostdevs,
-                               driver->inactivePciHostdevs) == 0)) {
+            virPCIDeviceReset(activePci, driver->activePciHostdevs,
+                               driver->inactivePciHostdevs) == 0) {
             qemuReattachPciDevice(activePci, driver);
             ret = 0;
         } else {
diff --git a/src/util/virpci.c b/src/util/virpci.c
index fc76952..15de3f9 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -883,8 +883,10 @@ virPCIDeviceReset(virPCIDevicePtr dev,
                   virPCIDeviceList *activeDevs,
                   virPCIDeviceList *inactiveDevs)
 {
+    char *drvPath = NULL;
+    char *drvName = NULL;
     int ret = -1;
-    int fd;
+    int fd = -1;
 
     if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
@@ -892,8 +894,24 @@ virPCIDeviceReset(virPCIDevicePtr dev,
         return -1;
     }
 
+    /* If the device is currently bound to vfio-pci, ignore all
+     * requests to reset it, since the vfio-pci driver will always
+     * reset it whenever appropriate, so doing it ourselves would just
+     * be redundant.
+     */
+    if (virPCIDeviceGetDriverPathAndName(dev, &drvPath, &drvName) < 0)
+        goto cleanup;
+
+    if (STREQ_NULLABLE(drvName, "vfio-pci")) {
+        VIR_DEBUG("Device %s is bound to vfio-pci - skip reset",
+                  dev->name);
+        ret = 0;
+        goto cleanup;
+    }
+    VIR_DEBUG("Resetting device %s", dev->name);
+
     if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0)
-        return -1;
+        goto cleanup;
 
     if (virPCIDeviceInit(dev, fd) < 0)
         goto cleanup;
@@ -922,10 +940,13 @@ virPCIDeviceReset(virPCIDevicePtr dev,
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Unable to reset PCI device %s: %s"),
                        dev->name,
-                       err ? err->message : _("no FLR, PM reset or bus reset available"));
+                       err ? err->message :
+                       _("no FLR, PM reset or bus reset available"));
     }
 
 cleanup:
+    VIR_FREE(drvPath);
+    VIR_FREE(drvName);
     virPCIDeviceConfigClose(dev, fd);
     return ret;
 }
-- 
1.7.11.7

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list