[libvirt] [PATCH] pci: autolearn name of stub driver, remove from arglist

Laine Stump laine at laine.org
Wed May 1 18:54:16 UTC 2013


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.
---
 src/qemu/qemu_driver.c  |  2 +-
 src/qemu/qemu_hostdev.c |  4 +--
 src/util/virpci.c       | 66 ++++++++++++++++++++++++++++++++++---------------
 src/util/virpci.h       |  3 +--
 src/xen/xen_driver.c    |  2 +-
 5 files changed, 51 insertions(+), 26 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 296efe3..9492850 100644
--- 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 4d1f39d..e4af036 100644
--- 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 97bba74..8d0dec8 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -909,25 +909,61 @@ 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 (!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. */
+    if (!(driver = strdup(last_component(drvdir))))
+        goto remove_slot;
 
-    if (virFileExists(drvdir) && virFileLinkPointsTo(path, drvdir)) {
+    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)) {
         if (virPCIDriverFile(&path, driver, "unbind") < 0) {
             goto cleanup;
         }
@@ -1141,7 +1177,7 @@ cleanup:
     VIR_FREE(path);
 
     if (result < 0) {
-        virPCIDeviceUnbindFromStub(dev, driver);
+        virPCIDeviceUnbindFromStub(dev);
     }
 
     return result;
@@ -1183,25 +1219,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 3911b72..7bcadb4 100644
--- 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 8971d4c..0642edb 100644
--- 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;
-- 
1.7.11.7




More information about the libvir-list mailing list