pci: autolearn name of stub driver, remove from arglist

virPCIDeviceReattach and virPCIDeviceUnbindFromStub (called by
virPCIDeviceReattach) had previously required the name of the stub
driver as input. This is unnecessary, because the name of the driver
the device is currently bound to can be found by looking at the link:

  /sys/bus/pci/dddd:bb:ss.ff/driver

Instead of requiring that the name of the expected stub driver name
and only unbinding if that one name is matched, we no longer take a
driver name in the arglist for either of these
functions. virPCIDeviceUnbindFromStub just compares the name of the
currently bound driver to a list of "well known" stubs (right now
contains "pci-stub" and "vfio-pci" for qemu, and "pciback" for xen),
and only performs the unbind if it's one of those devices.

This allows virsh nodedevice-reattach to work properly across a
libvirtd restart, and fixes a couple of cases where we were
erroneously still hard-coding "pci-stub" as the drive name.

For some unknown reason, virPCIDeviceReattach had been calling
modprobe on the stub driver prior to unbinding the device. This was
problematic because we no longer know the name of the stub driver in
that function. However, it is pointless to probe for the stub driver
at that time anyway - because the device is bound to the stub driver,
we are guaranteed that it is already loaded, and so that call to
modprobe has been removed.

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 296efe386de3c36210e93528fa348fa9282886ce..94928508f03aafc43768725420c4d7b5a5accbac 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -9832,7 +9832,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev)
     virPCIDeviceReattachInit(pci);
 
     if (virPCIDeviceReattach(pci, driver->activePciHostdevs,
-                             driver->inactivePciHostdevs, "pci-stub") < 0)
+                             driver->inactivePciHostdevs) < 0)
         goto out;
 
     ret = 0;

diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c
index 4d1f39d2ed41c23ddbd00f3d1e20700f1b51d765..e4af03666e0f151d3b67cd4a7b652e181031c3a2 100644 (file)
--- a/src/qemu/qemu_hostdev.c
+++ b/src/qemu/qemu_hostdev.c
@@ -609,7 +609,7 @@ reattachdevs:
         /* NB: This doesn't actually re-bind to original driver, just
          * unbinds from the stub driver
          */
-        virPCIDeviceReattach(dev, driver->activePciHostdevs, NULL, NULL);
+        virPCIDeviceReattach(dev, driver->activePciHostdevs, NULL);
     }
 
 cleanup:
@@ -869,7 +869,7 @@ void qemuReattachPciDevice(virPCIDevicePtr dev, virQEMUDriverPtr driver)
     }
 
     if (virPCIDeviceReattach(dev, driver->activePciHostdevs,
-                             driver->inactivePciHostdevs, "pci-stub") < 0) {
+                             driver->inactivePciHostdevs) < 0) {
         virErrorPtr err = virGetLastError();
         VIR_ERROR(_("Failed to re-attach PCI device: %s"),
                   err ? err->message : _("unknown error"));

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 97bba74a384f7cd3ffd36efcbe2e66a29a67313e..ba60292f15f6627f3b23c239d52004f4892bb79d 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -909,25 +909,62 @@ recheck:
     return -1;
 }
 
+static const char *virPCIKnownStubs[] = {
+    "pciback",  /* used by xen */
+    "pci-stub", /* used by kvm legacy passthrough */
+    "vfio-pci", /* used by VFIO device assignment */
+    NULL
+};
+
 static int
-virPCIDeviceUnbindFromStub(virPCIDevicePtr dev, const char *driver)
+virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
 {
     int result = -1;
     char *drvdir = NULL;
     char *path = NULL;
+    char *driver = NULL;
+    const char **stubTest;
+    bool isStub = false;
 
-    if (virPCIDriverDir(&drvdir, driver) < 0)
+    /* If the device is currently bound to one of the "well known"
+     * stub drivers, then unbind it, otherwise ignore it.
+     */
+    if (virPCIFile(&path, dev->name, "driver") < 0)
         goto cleanup;
+    /* path = "/sys/bus/pci/dddd:bb:ss.ff/driver" */
+    if (virFileIsLink(path) != 1) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Invalid device %s driver file %s is not a symlink"),
+                       dev->name, path);
+        goto cleanup;
+    }
+    if (virFileResolveLink(path, &drvdir) < 0) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unable to resolve device %s driver symlink %s"),
+                       dev->name, path);
+        goto cleanup;
+    }
+    /* drvdir = "/sys/bus/pci/drivers/${drivername}" */
+    if (!(driver = strdup(last_component(drvdir)))) {
+        virReportOOMError();
+        goto cleanup;
+    }
 
     if (!dev->unbind_from_stub)
         goto remove_slot;
 
-    /* If the device is bound to stub, unbind it.
-     */
-    if (virPCIFile(&path, dev->name, "driver") < 0)
-        goto cleanup;
+    /* If the device isn't bound to a known stub, skip the unbind. */
+    for (stubTest = virPCIKnownStubs; *stubTest != NULL; stubTest++) {
+        if (STREQ(driver, *stubTest)) {
+            isStub = true;
+            VIR_DEBUG("Found stub driver %s", *stubTest);
+            break;
+        }
+    }
+    if (!isStub)
+        goto remove_slot;
 
-    if (virFileExists(drvdir) && virFileLinkPointsTo(path, drvdir)) {
+    if (virFileExists(drvdir)) {
         if (virPCIDriverFile(&path, driver, "unbind") < 0) {
             goto cleanup;
         }
@@ -992,6 +1029,7 @@ cleanup:
 
     VIR_FREE(drvdir);
     VIR_FREE(path);
+    VIR_FREE(driver);
 
     return result;
 }
@@ -1141,7 +1179,7 @@ cleanup:
     VIR_FREE(path);
 
     if (result < 0) {
-        virPCIDeviceUnbindFromStub(dev, driver);
+        virPCIDeviceUnbindFromStub(dev);
     }
 
     return result;
@@ -1183,25 +1221,15 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
 int
 virPCIDeviceReattach(virPCIDevicePtr dev,
                      virPCIDeviceListPtr activeDevs,
-                     virPCIDeviceListPtr inactiveDevs,
-                     const char *driver)
+                     virPCIDeviceListPtr inactiveDevs)
 {
-    if (!driver && dev->stubDriver)
-        driver = dev->stubDriver;
-
-    if (virPCIProbeStubDriver(driver) < 0) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Failed to load PCI stub module %s"), driver);
-        return -1;
-    }
-
     if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Not reattaching active device %s"), dev->name);
         return -1;
     }
 
-    if (virPCIDeviceUnbindFromStub(dev, driver) < 0)
+    if (virPCIDeviceUnbindFromStub(dev) < 0)
         return -1;
 
     /* Steal the dev from list inactiveDevs */

diff --git a/src/util/virpci.h b/src/util/virpci.h
index 3911b72359a5c3ffed0e70185149f5930f0463c3..7bcadb42f6d5604d7a827c6308ac2c086002551b 100644 (file)
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -54,8 +54,7 @@ int virPCIDeviceDetach(virPCIDevicePtr dev,
                        const char *driver);
 int virPCIDeviceReattach(virPCIDevicePtr dev,
                          virPCIDeviceListPtr activeDevs,
-                         virPCIDeviceListPtr inactiveDevs,
-                         const char *driver);
+                         virPCIDeviceListPtr inactiveDevs);
 int virPCIDeviceReset(virPCIDevicePtr dev,
                       virPCIDeviceListPtr activeDevs,
                       virPCIDeviceListPtr inactiveDevs);

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 8971d4cc72a541962d6623387b0bdb138282021c..0642edb59bd47f841d8e5bf9d8b700d0faf87f7e 100644 (file)
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -2254,7 +2254,7 @@ xenUnifiedNodeDeviceReAttach(virNodeDevicePtr dev)
         goto out;
     }
 
-    if (virPCIDeviceReattach(pci, NULL, NULL, "pciback") < 0)
+    if (virPCIDeviceReattach(pci, NULL, NULL) < 0)
         goto out;
 
     ret = 0;