pci: Introduce virPCIStubDriver enumeration

This replaces the virPCIKnownStubs string array that was used
internally for stub driver validation.

Advantages:

  * possible values are well-defined
  * typos in driver names will be detected at compile time
  * avoids having several copies of the same string around
  * no error checking required when setting / getting value

The names used mirror those in the
virDomainHostdevSubsysPCIBackendType enumeration.

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index f90a1a4284f4e2a6c44422daed0e6aaa0013ff5a..1e51dcf8ba346f6f8d49c983cab8955902d8743e 100644 (file)
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2003,6 +2003,8 @@ virPCIGetVirtualFunctionIndex;
 virPCIGetVirtualFunctionInfo;
 virPCIGetVirtualFunctions;
 virPCIIsVirtualFunction;
+virPCIStubDriverTypeFromString;
+virPCIStubDriverTypeToString;
 
 
 # util/virpidfile.h

diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index d0d2848d671735c722a5791d7c8ee2526cccbcb0..d4e9c2a73b8aeb9b5812a61b9d88a2997d76080b 100644 (file)
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5070,8 +5070,7 @@ libxlNodeDeviceDetachFlags(virNodeDevicePtr dev,
         goto cleanup;
 
     if (!driverName || STREQ(driverName, "xen")) {
-        if (virPCIDeviceSetStubDriver(pci, "pciback") < 0)
-            goto cleanup;
+        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
     } else {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("unsupported driver name '%s'"), driverName);

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 5a4f3cdd17c09060669b1d8e57745f61c75e534a..e8ba3a6f54ca5c0ce33645646d8c829369df87e3 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12861,8 +12861,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
                              "supported on this system"));
             goto cleanup;
         }
-        if (virPCIDeviceSetStubDriver(pci, "vfio-pci") < 0)
-            goto cleanup;
+        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_VFIO);
     } else if (STREQ(driverName, "kvm")) {
         if (!legacy) {
             virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s",
@@ -12870,8 +12869,7 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev,
                              "supported on this system"));
             goto cleanup;
         }
-        if (virPCIDeviceSetStubDriver(pci, "pci-stub") < 0)
-            goto cleanup;
+        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_KVM);
     } else {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("unknown driver name '%s'"), driverName);

diff --git a/src/util/virhostdev.c b/src/util/virhostdev.c
index 4065535069a2c873dea6e82dbed7060968fdb489..f9072a457e38378c0d6513c15a76a8e593db5b25 100644 (file)
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -237,22 +237,13 @@ virHostdevGetPCIHostDeviceList(virDomainHostdevDefPtr *hostdevs, int nhostdevs)
         }
 
         virPCIDeviceSetManaged(dev, hostdev->managed);
-        if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-            if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0) {
-                virObjectUnref(list);
-                return NULL;
-            }
-        } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) {
-            if (virPCIDeviceSetStubDriver(dev, "pciback") < 0) {
-                virObjectUnref(list);
-                return NULL;
-            }
-        } else {
-            if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0) {
-                virObjectUnref(list);
-                return NULL;
-            }
-        }
+
+        if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
+        else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_XEN);
+        else
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_KVM);
     }
 
     return list;
@@ -574,7 +565,7 @@ virHostdevPreparePCIDevices(virHostdevManagerPtr hostdev_mgr,
     for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) {
         virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i);
         bool strict_acs_check = !!(flags & VIR_HOSTDEV_STRICT_ACS_CHECK);
-        bool usesVfio = STREQ(virPCIDeviceGetStubDriver(dev), "vfio-pci");
+        bool usesVfio = (virPCIDeviceGetStubDriver(dev) == VIR_PCI_STUB_DRIVER_VFIO);
         struct virHostdevIsPCINodeDeviceUsedData data = {hostdev_mgr, dom_name,
                                                          usesVfio};
 
@@ -745,7 +736,7 @@ virHostdevReattachPCIDevice(virPCIDevicePtr dev, virHostdevManagerPtr mgr)
     }
 
     /* Wait for device cleanup if it is qemu/kvm */
-    if (STREQ(virPCIDeviceGetStubDriver(dev), "pci-stub")) {
+    if (virPCIDeviceGetStubDriver(dev) == VIR_PCI_STUB_DRIVER_KVM) {
         int retries = 100;
         while (virPCIDeviceWaitForCleanup(dev, "kvm_assigned_device")
                && retries) {
@@ -913,19 +904,15 @@ virHostdevUpdateActivePCIDevices(virHostdevManagerPtr mgr,
             goto cleanup;
 
         virPCIDeviceSetManaged(dev, hostdev->managed);
-        if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) {
-            if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0)
-                goto cleanup;
-        } else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN) {
-            if (virPCIDeviceSetStubDriver(dev, "pciback") < 0)
-                goto cleanup;
-        } else {
-            if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0)
-                goto cleanup;
-
-        }
         virPCIDeviceSetUsedBy(dev, drv_name, dom_name);
 
+        if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO)
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
+        else if (pcisrc->backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN)
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_XEN);
+        else
+            virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_KVM);
+
         /* Setup the original states for the PCI device */
         virPCIDeviceSetUnbindFromStub(dev, hostdev->origstates.states.pci.unbind_from_stub);
         virPCIDeviceSetRemoveSlot(dev, hostdev->origstates.states.pci.remove_slot);

diff --git a/src/util/virpci.c b/src/util/virpci.c
index 21eacf55039004b89fc9d4923c9f9b6de7ae2347..9eee783a4b87d594302efea44469bf168835c731 100644 (file)
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -55,6 +55,13 @@ VIR_LOG_INIT("util.pci");
 VIR_ENUM_IMPL(virPCIELinkSpeed, VIR_PCIE_LINK_SPEED_LAST,
               "", "2.5", "5", "8")
 
+VIR_ENUM_IMPL(virPCIStubDriver, VIR_PCI_STUB_DRIVER_LAST,
+              "none",
+              "pciback", /* XEN */
+              "pci-stub", /* KVM */
+              "vfio-pci", /* VFIO */
+);
+
 struct _virPCIDevice {
     virPCIDeviceAddress address;
 
@@ -71,7 +78,8 @@ struct _virPCIDevice {
     bool          has_flr;
     bool          has_pm_reset;
     bool          managed;
-    char          *stubDriver;
+
+    virPCIStubDriver stubDriver;
 
     /* used by reattach function */
     bool          unbind_from_stub;
@@ -941,7 +949,7 @@ virPCIDeviceReset(virPCIDevicePtr dev,
     if (virPCIDeviceGetDriverPathAndName(dev, &drvPath, &drvName) < 0)
         goto cleanup;
 
-    if (STREQ_NULLABLE(drvName, "vfio-pci")) {
+    if (virPCIStubDriverTypeFromString(drvName) == VIR_PCI_STUB_DRIVER_VFIO) {
         VIR_DEBUG("Device %s is bound to vfio-pci - skip reset",
                   dev->name);
         ret = 0;
@@ -992,13 +1000,22 @@ virPCIDeviceReset(virPCIDevicePtr dev,
 
 
 static int
-virPCIProbeStubDriver(const char *driver)
+virPCIProbeStubDriver(virPCIStubDriver driver)
 {
+    const char *drvname = NULL;
     char *drvpath = NULL;
     bool probed = false;
 
+    if (driver == VIR_PCI_STUB_DRIVER_NONE ||
+        !(drvname = virPCIStubDriverTypeToString(driver))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s",
+                       _("Attempting to use unknown stub driver"));
+        return -1;
+    }
+
  recheck:
-    if ((drvpath = virPCIDriverDir(driver)) && virFileExists(drvpath)) {
+    if ((drvpath = virPCIDriverDir(drvname)) && virFileExists(drvpath)) {
         /* driver already loaded, return */
         VIR_FREE(drvpath);
         return 0;
@@ -1009,8 +1026,8 @@ virPCIProbeStubDriver(const char *driver)
     if (!probed) {
         char *errbuf = NULL;
         probed = true;
-        if ((errbuf = virKModLoad(driver, true))) {
-            VIR_WARN("failed to load driver %s: %s", driver, errbuf);
+        if ((errbuf = virKModLoad(drvname, true))) {
+            VIR_WARN("failed to load driver %s: %s", drvname, errbuf);
             VIR_FREE(errbuf);
             goto cleanup;
         }
@@ -1022,15 +1039,15 @@ virPCIProbeStubDriver(const char *driver)
     /* If we know failure was because of blacklist, let's report that;
      * otherwise, report a more generic failure message
      */
-    if (virKModIsBlacklisted(driver)) {
+    if (virKModIsBlacklisted(drvname)) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to load PCI stub module %s: "
                          "administratively prohibited"),
-                       driver);
+                       drvname);
     } else {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        _("Failed to load PCI stub module %s"),
-                       driver);
+                       drvname);
     }
 
     return -1;
@@ -1073,13 +1090,6 @@ virPCIDeviceUnbind(virPCIDevicePtr dev)
     return ret;
 }
 
-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)
 {
@@ -1087,8 +1097,6 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
     char *drvdir = NULL;
     char *path = NULL;
     char *driver = NULL;
-    const char **stubTest;
-    bool isStub = false;
 
     /* If the device is currently bound to one of the "well known"
      * stub drivers, then unbind it, otherwise ignore it.
@@ -1105,16 +1113,12 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
         goto remove_slot;
 
     /* 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)
+    if (virPCIStubDriverTypeFromString(driver) < 0 ||
+        virPCIStubDriverTypeFromString(driver) == VIR_PCI_STUB_DRIVER_NONE)
         goto remove_slot;
 
+    VIR_DEBUG("Found stub driver %s", driver);
+
     if (virPCIDeviceUnbind(dev) < 0)
         goto cleanup;
     dev->unbind_from_stub = false;
@@ -1183,9 +1187,22 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev)
     char *stubDriverPath = NULL;
     char *driverLink = NULL;
     char *path = NULL; /* reused for different purposes */
-    char *stubDriverName = dev->stubDriver;
+    const char *stubDriverName = 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;
@@ -1338,8 +1355,6 @@ virPCIDeviceDetach(virPCIDevicePtr dev,
                    virPCIDeviceList *activeDevs,
                    virPCIDeviceList *inactiveDevs)
 {
-    sa_assert(dev->stubDriver);
-
     if (virPCIProbeStubDriver(dev->stubDriver) < 0)
         return -1;
 
@@ -1622,10 +1637,9 @@ virPCIDeviceCopy(virPCIDevicePtr dev)
 
     /* shallow copy to take care of most attributes */
     *copy = *dev;
-    copy->path = copy->stubDriver = NULL;
+    copy->path = NULL;
     copy->used_by_drvname = copy->used_by_domname = NULL;
     if (VIR_STRDUP(copy->path, dev->path) < 0 ||
-        VIR_STRDUP(copy->stubDriver, dev->stubDriver) < 0 ||
         VIR_STRDUP(copy->used_by_drvname, dev->used_by_drvname) < 0 ||
         VIR_STRDUP(copy->used_by_domname, dev->used_by_domname) < 0) {
         goto error;
@@ -1645,7 +1659,6 @@ virPCIDeviceFree(virPCIDevicePtr dev)
         return;
     VIR_DEBUG("%s %s: freeing", dev->id, dev->name);
     VIR_FREE(dev->path);
-    VIR_FREE(dev->stubDriver);
     VIR_FREE(dev->used_by_drvname);
     VIR_FREE(dev->used_by_domname);
     VIR_FREE(dev);
@@ -1683,14 +1696,13 @@ virPCIDeviceGetManaged(virPCIDevicePtr dev)
     return dev->managed;
 }
 
-int
-virPCIDeviceSetStubDriver(virPCIDevicePtr dev, const char *driver)
+void
+virPCIDeviceSetStubDriver(virPCIDevicePtr dev, virPCIStubDriver driver)
 {
-    VIR_FREE(dev->stubDriver);
-    return VIR_STRDUP(dev->stubDriver, driver);
+    dev->stubDriver = driver;
 }
 
-const char *
+virPCIStubDriver
 virPCIDeviceGetStubDriver(virPCIDevicePtr dev)
 {
     return dev->stubDriver;

diff --git a/src/util/virpci.h b/src/util/virpci.h
index e628ab896eedbdd2e079eb09939f1dac9b6f7a1a..16067315d591e9aef9e450fc56044450ca104a1a 100644 (file)
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -42,6 +42,16 @@ struct _virPCIDeviceAddress {
     unsigned int function;
 };
 
+typedef enum {
+    VIR_PCI_STUB_DRIVER_NONE = 0,
+    VIR_PCI_STUB_DRIVER_XEN,
+    VIR_PCI_STUB_DRIVER_KVM,
+    VIR_PCI_STUB_DRIVER_VFIO,
+    VIR_PCI_STUB_DRIVER_LAST
+} virPCIStubDriver;
+
+VIR_ENUM_DECL(virPCIStubDriver);
+
 typedef enum {
     VIR_PCIE_LINK_SPEED_NA = 0,
     VIR_PCIE_LINK_SPEED_25,
@@ -90,10 +100,9 @@ int virPCIDeviceReset(virPCIDevicePtr dev,
 void virPCIDeviceSetManaged(virPCIDevice *dev,
                             bool managed);
 unsigned int virPCIDeviceGetManaged(virPCIDevice *dev);
-int virPCIDeviceSetStubDriver(virPCIDevicePtr dev,
-                              const char *driver)
-    ATTRIBUTE_NONNULL(2);
-const char *virPCIDeviceGetStubDriver(virPCIDevicePtr dev);
+void virPCIDeviceSetStubDriver(virPCIDevicePtr dev,
+                               virPCIStubDriver driver);
+virPCIStubDriver virPCIDeviceGetStubDriver(virPCIDevicePtr dev);
 virPCIDeviceAddressPtr virPCIDeviceGetAddress(virPCIDevicePtr dev);
 int virPCIDeviceSetUsedBy(virPCIDevice *dev,
                           const char *drv_name,

diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c
index 2eec31cf6543f7e3075d4ad5a2162f02acade2ef..95f0e4209379d2e67821e9100353b3dd59fe766b 100644 (file)
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -2366,8 +2366,7 @@ xenUnifiedNodeDeviceDetachFlags(virNodeDevicePtr dev,
         return -1;
 
     if (!driverName) {
-        if (virPCIDeviceSetStubDriver(pci, "pciback") < 0)
-            goto out;
+        virPCIDeviceSetStubDriver(pci, VIR_PCI_STUB_DRIVER_XEN);
     } else {
         virReportError(VIR_ERR_INVALID_ARG,
                        _("unknown driver name '%s'"), driverName);

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index 2a7497604e9cb5ca05b91fbdb577417d3d7dd1ba..1b306810746b8915ca9f4bb499c1e9da7b002875 100644 (file)
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -97,9 +97,10 @@ myInit(void)
     }
 
     for (i = 0; i < nhostdevs; i++) {
-        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)) ||
-            virPCIDeviceSetStubDriver(dev[i], "pci-stub") < 0)
+        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
             goto cleanup;
+
+        virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);
     }
 
     if (VIR_ALLOC(mgr) < 0)

diff --git a/tests/virpcitest.c b/tests/virpcitest.c
index 1ade3e5e1dc9678b657457f2d8e96dbeac515089..fb0c970ad85b9bbefb0145d560a9c5d80f127952 100644 (file)
--- a/tests/virpcitest.c
+++ b/tests/virpcitest.c
@@ -107,10 +107,11 @@ testVirPCIDeviceDetach(const void *oaque ATTRIBUTE_UNUSED)
     CHECK_LIST_COUNT(inactiveDevs, 0);
 
     for (i = 0; i < nDev; i++) {
-        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)) ||
-            virPCIDeviceSetStubDriver(dev[i], "pci-stub") < 0)
+        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
             goto cleanup;
 
+        virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);
+
         if (virPCIDeviceDetach(dev[i], activeDevs, inactiveDevs) < 0)
             goto cleanup;
 
@@ -147,10 +148,11 @@ testVirPCIDeviceReset(const void *opaque ATTRIBUTE_UNUSED)
     CHECK_LIST_COUNT(inactiveDevs, 0);
 
     for (i = 0; i < nDev; i++) {
-        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)) ||
-            virPCIDeviceSetStubDriver(dev[i], "pci-stub") < 0)
+        if (!(dev[i] = virPCIDeviceNew(0, 0, i + 1, 0)))
             goto cleanup;
 
+        virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);
+
         if (virPCIDeviceReset(dev[i], activeDevs, inactiveDevs) < 0)
             goto cleanup;
     }
@@ -189,8 +191,7 @@ testVirPCIDeviceReattach(const void *opaque ATTRIBUTE_UNUSED)
         CHECK_LIST_COUNT(activeDevs, 0);
         CHECK_LIST_COUNT(inactiveDevs, i + 1);
 
-        if (virPCIDeviceSetStubDriver(dev[i], "pci-stub") < 0)
-            goto cleanup;
+        virPCIDeviceSetStubDriver(dev[i], VIR_PCI_STUB_DRIVER_KVM);
     }
 
     CHECK_LIST_COUNT(activeDevs, 0);
@@ -248,8 +249,9 @@ testVirPCIDeviceDetachSingle(const void *opaque)
     if (!dev)
         goto cleanup;
 
-    if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0 ||
-        virPCIDeviceDetach(dev, NULL, NULL) < 0)
+    virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_KVM);
+
+    if (virPCIDeviceDetach(dev, NULL, NULL) < 0)
         goto cleanup;
 
     ret = 0;
@@ -269,8 +271,7 @@ testVirPCIDeviceDetachFail(const void *opaque)
     if (!dev)
         goto cleanup;
 
-    if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0)
-        goto cleanup;
+    virPCIDeviceSetStubDriver(dev, VIR_PCI_STUB_DRIVER_VFIO);
 
     if (virPCIDeviceDetach(dev, NULL, NULL) < 0) {
         if (virTestGetVerbose() || virTestGetDebug())
@@ -281,7 +282,7 @@ testVirPCIDeviceDetachFail(const void *opaque)
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "Attaching device %s to %s should have failed",
                        virPCIDeviceGetName(dev),
-                       virPCIDeviceGetStubDriver(dev));
+                       virPCIStubDriverTypeToString(VIR_PCI_STUB_DRIVER_VFIO));
     }
 
  cleanup: