11:59 a.m.
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.
---
Changes in v2:
* add VIR_PCI_STUB_DRIVER_NONE so we can detect when no driver has
been configured for a specific device
* simplify code in testVirPCIDeviceDetachFail() by not reading the
driver back from the device, since we set it a few lines above
testVirPCIDeviceDetachFail
src/libvirt_private.syms | 2 ++
src/libxl/libxl_driver.c | 3 +-
src/qemu/qemu_driver.c | 6 ++--
src/util/virhostdev.c | 45 +++++++++----------------
src/util/virpci.c | 86 +++++++++++++++++++++++++++---------------------
src/util/virpci.h | 17 +++++++---
src/xen/xen_driver.c | 3 +-
tests/virhostdevtest.c | 5 +--
tests/virpcitest.c | 23 ++++++-------
9 files changed, 99 insertions(+), 91 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 0b5ddc1..79074cc 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2005,6 +2005,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 4f82c01..9491f0f 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -5073,8 +5073,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 783a7cd..26a63f1 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -12882,8 +12882,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",
@@ -12891,8 +12890,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 4065535..f9072a4 100644
--- 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 21eacf5..aec7b69 100644
--- 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 (!(drvname = virPCIStubDriverTypeToString(driver)) ||
+ driver == VIR_PCI_STUB_DRIVER_NONE) {
+ 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 e628ab8..1606731 100644
--- a/src/util/virpci.h
+++ b/src/util/virpci.h
@@ -43,6 +43,16 @@ struct _virPCIDeviceAddress {
};
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,
VIR_PCIE_LINK_SPEED_5,
@@ -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 285a553..8d9ac07 100644
--- a/src/xen/xen_driver.c
+++ b/src/xen/xen_driver.c
@@ -2532,8 +2532,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 2a74976..1b30681 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 1ade3e5..fb0c970 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;
@@ -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:
--
2.5.0