pci: make virPCIDeviceReset more autonomous

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.

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index f24d4661e1436a13773eedc57e4158b82f1ce947..21fe47f43a4ab3a3098745cf1c2995a8d2661ee2 100644 (file)
--- 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;
@@ -1119,8 +1118,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 e05b4b37c63f19b014f974051ea4cd6f93770516..1925fe4d1a59e59445787ab57e8dfb371db3dab0 100644 (file)
--- a/src/qemu/qemu_hotplug.c
+++ b/src/qemu/qemu_hotplug.c
@@ -2492,9 +2492,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 3f8625e2bfa1d8acff722a330235ef6e30a4521b..c95c0e6326f846e65bb6a3db6f5ca8cf03fb5e6a 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -866,8 +866,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,
@@ -875,8 +877,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;
@@ -905,10 +923,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;
 }