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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list