[libvirt] [PATCH 2/2] virpci: support driver_override sysfs interface

Jim Fehlig jfehlig at suse.com
Mon Jul 11 18:23:20 UTC 2016


Currently, libvirt uses the new_id PCI sysfs interface to bind a PCI
stub driver to a PCI device. The new_id interface is known to be
buggy and racey, hence a more deterministic interface was introduced
in the 3.12 kernel - driver_override. For more details see

https://www.redhat.com/archives/libvir-list/2016-June/msg02124.html

This patch changes the stub binding/unbinding code to use the
driver_override interface if present. If not present, the new_id
interface will be used to provide compatibility with older kernels
lacking driver_override.

Signed-off-by: Jim Fehlig <jfehlig at suse.com>
---
 src/util/virpci.c | 199 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 152 insertions(+), 47 deletions(-)

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 127b3b6..3c2fc9f 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -1158,6 +1158,19 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
 
     VIR_DEBUG("Reprobing for PCI device %s", dev->name);
 
+    /* Remove driver_override if it exists */
+    VIR_FREE(path);
+    if (!(path = virPCIFile(dev->name, "driver_override")))
+        goto cleanup;
+
+    if (virFileExists(path) && virFileWriteStr(path, "\n", 0) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to remove stub driver from "
+                               "driver_override interface of PCI device '%s'"),
+                             dev->name);
+        goto cleanup;
+    }
+
     /* Trigger a re-probe of the device is not in the stub's dynamic
      * ID table. If the stub is available, but 'remove_id' isn't
      * available, then re-probing would just cause the device to be
@@ -1193,49 +1206,13 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
 
 
 static int
-virPCIDeviceBindToStub(virPCIDevicePtr dev)
+virPCIDeviceBindToStubWithNewid(virPCIDevicePtr dev,
+                                const char *stubDriverName)
 {
-    int result = -1;
-    char *stubDriverPath = NULL;
-    char *driverLink = NULL;
-    char *path = NULL; /* reused for different purposes */
-    const char *stubDriverName = NULL;
+    int ret = -1;
+    char *path = NULL;
     virErrorPtr err = NULL;
 
-    /* Check the device is configured to use one of the known stub drivers */
-    if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("No stub driver configured for PCI device %s"),
-                       dev->name);
-        return -1;
-    } else if (!(stubDriverName = virPCIStubDriverTypeToString(dev->stubDriver))) {
-        virReportError(VIR_ERR_INTERNAL_ERROR,
-                       _("Unknown stub driver configured for PCI device %s"),
-                       dev->name);
-        return -1;
-    }
-
-    if (!(stubDriverPath = virPCIDriverDir(stubDriverName))  ||
-        !(driverLink = virPCIFile(dev->name, "driver")))
-        goto cleanup;
-
-    if (virFileExists(driverLink)) {
-        if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
-            /* The device is already bound to the correct driver */
-            VIR_DEBUG("Device %s is already bound to %s",
-                      dev->name, stubDriverName);
-            dev->unbind_from_stub = true;
-            dev->remove_slot = true;
-            result = 0;
-            goto cleanup;
-        }
-        /*
-         * If the device is bound to a driver that is not the stub,  we'll
-         * need to reprobe later
-         */
-        dev->reprobe = true;
-    }
-
     /* Add the PCI device ID to the stub's dynamic ID table;
      * this is needed to allow us to bind the device to the stub.
      * Note: if the device is not currently bound to any driver,
@@ -1283,7 +1260,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev)
     }
     dev->unbind_from_stub = true;
 
-    result = 0;
+    ret = 0;
 
  remove_id:
     err = virSaveLastError();
@@ -1299,7 +1276,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev)
                      "cannot be probed again.", dev->id, stubDriverName);
         }
         dev->reprobe = false;
-        result = -1;
+        ret = -1;
         goto cleanup;
     }
 
@@ -1314,11 +1291,143 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev)
                      "cannot be probed again.", dev->id, stubDriverName);
         }
         dev->reprobe = false;
-        result = -1;
+        ret = -1;
         goto cleanup;
     }
 
  cleanup:
+    VIR_FREE(path);
+
+    if (err)
+        virSetError(err);
+    virFreeError(err);
+
+    return ret;
+}
+
+
+static int
+virPCIDeviceBindToStubWithOverride(virPCIDevicePtr dev,
+                                   const char *stubDriverName)
+{
+    int ret = -1;
+    char *path = NULL;
+
+    /*
+     * Add stub to the device's driver_override, falling back to
+     * adding the device ID to the stub's dynamic ID table.
+     */
+    if (!(path = virPCIFile(dev->name, "driver_override")))
+        return -1;
+
+    if (virFileWriteStr(path, stubDriverName, 0) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to add stub driver '%s' to "
+                               "driver_override interface of PCI device '%s'"),
+                             stubDriverName, dev->name);
+        goto cleanup;
+    }
+
+    if (virPCIDeviceUnbind(dev) < 0)
+        goto cleanup;
+
+    /* Xen's pciback.ko wants you to use new_slot first */
+    VIR_FREE(path);
+    if (!(path = virPCIDriverFile(stubDriverName, "new_slot")))
+        goto cleanup;
+
+    if (virFileExists(path) && virFileWriteStr(path, dev->name, 0) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to add slot for "
+                               "PCI device '%s' to %s"),
+                             dev->name, stubDriverName);
+        goto cleanup;
+    }
+    dev->remove_slot = true;
+
+    if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) {
+        virReportSystemError(errno,
+                             _("Failed to trigger a re-probe for PCI device '%s'"),
+                             dev->name);
+        goto cleanup;
+    }
+
+    /*
+     * Device is now bound to the stub. Set reprobe so it will be re-bound
+     * when unbinding from the stub.
+     */
+    dev->reprobe = true;
+    dev->unbind_from_stub = true;
+
+    ret = 0;
+
+ cleanup:
+    VIR_FREE(path);
+    return ret;
+}
+
+
+static int
+virPCIDeviceBindToStub(virPCIDevicePtr dev)
+{
+    int result = -1;
+    char *stubDriverPath = NULL;
+    char *driverLink = NULL;
+    char *path = NULL; /* reused for different purposes */
+    const char *stubDriverName = NULL;
+
+    /* Check the device is configured to use one of the known stub drivers */
+    if (dev->stubDriver == VIR_PCI_STUB_DRIVER_NONE) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("No stub driver configured for PCI device %s"),
+                       dev->name);
+        return -1;
+    } else if (!(stubDriverName = virPCIStubDriverTypeToString(dev->stubDriver))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       _("Unknown stub driver configured for PCI device %s"),
+                       dev->name);
+        return -1;
+    }
+
+    if (!(stubDriverPath = virPCIDriverDir(stubDriverName))  ||
+        !(driverLink = virPCIFile(dev->name, "driver")))
+        goto cleanup;
+
+    if (virFileExists(driverLink)) {
+        if (virFileLinkPointsTo(driverLink, stubDriverPath)) {
+            /* The device is already bound to the correct driver */
+            VIR_DEBUG("Device %s is already bound to %s",
+                      dev->name, stubDriverName);
+            dev->unbind_from_stub = true;
+            dev->remove_slot = true;
+            result = 0;
+            goto cleanup;
+        }
+        /*
+         * If the device is bound to a driver that is not the stub,  we'll
+         * need to reprobe later
+         */
+        dev->reprobe = true;
+    }
+
+    /*
+     * Add stub to the device's driver_override, falling back to
+     * adding the device ID to the stub's dynamic ID table.
+     */
+    if (!(path = virPCIFile(dev->name, "driver_override")))
+        goto cleanup;
+
+    if (virFileExists(path)) {
+        if (virPCIDeviceBindToStubWithOverride(dev, stubDriverName) < 0)
+            goto cleanup;
+    } else {
+        if (virPCIDeviceBindToStubWithNewid(dev, stubDriverName) < 0)
+            goto cleanup;
+    }
+
+    result = 0;
+
+ cleanup:
     VIR_FREE(stubDriverPath);
     VIR_FREE(driverLink);
     VIR_FREE(path);
@@ -1326,10 +1435,6 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev)
     if (result < 0)
         virPCIDeviceUnbindFromStub(dev);
 
-    if (err)
-        virSetError(err);
-    virFreeError(err);
-
     return result;
 }
 
-- 
2.1.4




More information about the libvir-list mailing list