[libvirt] [PATCH 03/10] pci: Introduce virPCIStubDriver enumeration

Andrea Bolognani abologna at redhat.com
Wed Dec 2 17:39:14 UTC 2015


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.
---
 src/libvirt_private.syms |  2 ++
 src/libxl/libxl_driver.c |  3 +-
 src/qemu/qemu_driver.c   |  6 ++--
 src/util/virhostdev.c    | 43 +++++++++-------------------
 src/util/virpci.c        | 74 ++++++++++++++++++++++++++++--------------------
 src/util/virpci.h        | 16 ++++++++---
 src/xen/xen_driver.c     |  3 +-
 tests/virhostdevtest.c   |  5 ++--
 tests/virpcitest.c       | 33 ++++++++++++++-------
 9 files changed, 100 insertions(+), 85 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index dd085c3..c1fd9f6 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1999,6 +1999,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 35d7fae..c28b884 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -4972,8 +4972,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 ae1d8e7..91fb1c4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12860,8 +12860,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",
@@ -12869,8 +12868,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 de029a0..33a45cb 100644
--- a/src/util/virhostdev.c
+++ b/src/util/virhostdev.c
@@ -237,22 +237,12 @@ 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 +564,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};
 
@@ -749,7 +739,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) {
@@ -917,17 +907,12 @@ 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;
-
-        }
+        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);
         virPCIDeviceSetUsedBy(dev, drv_name, dom_name);
 
         /* Setup the original states for the PCI device */
diff --git a/src/util/virpci.c b/src/util/virpci.c
index 8094735..e82583a 100644
--- a/src/util/virpci.c
+++ b/src/util/virpci.c
@@ -55,6 +55,12 @@ 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,
+              "pciback", /* XEN */
+              "pci-stub", /* KVM */
+              "vfio-pci", /* VFIO */
+);
+
 struct _virPCIDevice {
     unsigned int  domain;
     unsigned int  bus;
@@ -74,7 +80,8 @@ struct _virPCIDevice {
     bool          has_flr;
     bool          has_pm_reset;
     bool          managed;
-    char          *stubDriver;
+
+    virPCIStubDriver stubDriver;
 
     /* used by reattach function */
     bool          unbind_from_stub;
@@ -940,7 +947,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;
@@ -991,13 +998,21 @@ virPCIDeviceReset(virPCIDevicePtr dev,
 
 
 static int
-virPCIProbeStubDriver(const char *driver)
+virPCIProbeStubDriver(virPCIStubDriver driver)
 {
+    const char *drvname = NULL;
     char *drvpath = NULL;
     bool probed = false;
 
+    if (!(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;
@@ -1008,8 +1023,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;
         }
@@ -1021,15 +1036,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;
@@ -1072,13 +1087,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)
 {
@@ -1086,7 +1094,8 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev)
     char *drvdir = NULL;
     char *path = NULL;
     char *driver = NULL;
-    const char **stubTest;
+    virPCIStubDriver stubDriver;
+    const char *stubDriverName;
     bool isStub = false;
 
     /* If the device is currently bound to one of the "well known"
@@ -1104,10 +1113,11 @@ 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)) {
+    for (stubDriver = 0; stubDriver < VIR_PCI_STUB_DRIVER_LAST; stubDriver++) {
+        stubDriverName = virPCIStubDriverTypeToString(stubDriver);
+        if (stubDriverName && STREQ(driver, stubDriverName)) {
             isStub = true;
-            VIR_DEBUG("Found stub driver %s", *stubTest);
+            VIR_DEBUG("Found stub driver %s", stubDriverName);
             break;
         }
     }
@@ -1182,9 +1192,16 @@ 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;
 
+    if (!(stubDriverName = virPCIStubDriverTypeToString(dev->stubDriver))) {
+        virReportError(VIR_ERR_INTERNAL_ERROR,
+                       "%s",
+                       _("Attempting to use unknown stub driver"));
+        return -1;
+    }
+
     if (!(stubDriverPath = virPCIDriverDir(stubDriverName))  ||
         !(driverLink = virPCIFile(dev->name, "driver")))
         goto cleanup;
@@ -1337,8 +1354,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);
@@ -1694,14 +1707,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 d9911d0..c7bdb58 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -43,6 +43,15 @@ struct _virPCIDeviceAddress {
 };
 
 typedef enum {
+    VIR_PCI_STUB_DRIVER_XEN = 0,
+    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,
     VIR_PCIE_LINK_SPEED_5,
@@ -90,10 +99,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 ce31f0f..7b2a3aa 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -2531,8 +2531,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 faebdd4..f20992b 100644
--- 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 35b4781..b5052e5 100644
--- 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;
@@ -262,15 +264,16 @@ static int
 testVirPCIDeviceDetachFail(const void *opaque)
 {
     const struct testPCIDevData *data = opaque;
+    const char *stubDriverName = NULL;
     int ret = -1;
     virPCIDevicePtr dev;
+    virPCIStubDriver stubDriver;
 
     dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function);
     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())
@@ -278,10 +281,18 @@ testVirPCIDeviceDetachFail(const void *opaque)
         virResetLastError();
         ret = 0;
     } else {
+        stubDriver = virPCIDeviceGetStubDriver(dev);
+        if (!(stubDriverName = virPCIStubDriverTypeToString(stubDriver))) {
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           "%s",
+                           "Attempting to use unknown stub driver");
+            goto cleanup;
+        }
+
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "Attaching device %s to %s should have failed",
                        virPCIDeviceGetName(dev),
-                       virPCIDeviceGetStubDriver(dev));
+                       stubDriverName);
     }
 
  cleanup:
-- 
2.5.0




More information about the libvir-list mailing list