https://bugzilla.redhat.com/show_bug.cgi?id=1046919 When a PCI device is not bound to any driver, reattach should just trigger driver probe rather than failing with Invalid device 0000:00:19.0 driver file /sys/bus/pci/devices/0000:00:19.0/driver is not a symlink While virPCIDeviceGetDriverPathAndName was documented to return success and NULL driver and path when a device is not attached to any driver but didn't do so. Thus callers could not distinguish unbound devices from failures. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virpci.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 51dbee6..8577fd4 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -236,6 +236,11 @@ virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name) if (virPCIFile(&drvlink, dev->name, "driver") < 0) goto cleanup; + if (!virFileExists(drvlink)) { + ret = 0; + goto cleanup; + } + if (virFileIsLink(drvlink) != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid device %s driver file %s is not a symlink"), @@ -1023,6 +1028,11 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) if (virPCIDeviceGetDriverPathAndName(dev, &drvdir, &driver) < 0) goto cleanup; + if (!driver) { + /* The device is not bound to any driver and we are almost done. */ + goto reprobe; + } + if (!dev->unbind_from_stub) goto remove_slot; @@ -1079,11 +1089,10 @@ reprobe: * available, then re-probing would just cause the device to be * re-bound to the stub. */ - if (virPCIDriverFile(&path, driver, "remove_id") < 0) { + if (driver && virPCIDriverFile(&path, driver, "remove_id") < 0) goto cleanup; - } - if (!virFileExists(drvdir) || virFileExists(path)) { + if (!driver || !virFileExists(drvdir) || virFileExists(path)) { if (virFileWriteStr(PCI_SYSFS "drivers_probe", dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to trigger a re-probe for PCI device '%s'"), -- 1.8.5.3

https://bugzilla.redhat.com/show_bug.cgi?id=1046919 Since commit v0.9.0-47-g4e8969e (released in 0.9.1) some failures during device detach were reported to callers of virPCIDeviceBindToStub as success. For example, even though a device seemed to be detached virsh # nodedev-detach pci_0000_07_05_0 --driver vfio Device pci_0000_07_05_0 detached one could find similar message in libvirt logs: Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device This patch fixes these paths and also avoids overwriting real errors with errors encountered during a cleanup phase. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/util/virpci.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/src/util/virpci.c b/src/util/virpci.c index 8577fd4..f6ab794 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -1123,14 +1123,15 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, { int result = -1; int reprobe = false; - char *stubDriverPath = NULL; char *driverLink = NULL; char *path = NULL; /* reused for different purposes */ - const char *newDriverName = NULL; + char *newDriverName = NULL; + virErrorPtr err = NULL; if (virPCIDriverDir(&stubDriverPath, stubDriverName) < 0 || - virPCIFile(&driverLink, dev->name, "driver") < 0) + virPCIFile(&driverLink, dev->name, "driver") < 0 || + VIR_STRDUP(newDriverName, stubDriverName) < 0) goto cleanup; if (virFileExists(driverLink)) { @@ -1138,7 +1139,6 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, /* The device is already bound to the correct driver */ VIR_DEBUG("Device %s is already bound to %s", dev->name, stubDriverName); - newDriverName = stubDriverName; result = 0; goto cleanup; } @@ -1170,6 +1170,7 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, if (virFileLinkPointsTo(driverLink, stubDriverPath)) { dev->unbind_from_stub = true; dev->remove_slot = true; + result = 0; goto remove_id; } @@ -1178,16 +1179,15 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, * PCI device happens to be IDE controller for the disk hosting * your root filesystem. */ - if (virPCIFile(&path, dev->name, "driver/unbind") < 0) { - goto cleanup; - } + if (virPCIFile(&path, dev->name, "driver/unbind") < 0) + goto remove_id; if (virFileExists(path)) { if (virFileWriteStr(path, dev->name, 0) < 0) { virReportSystemError(errno, _("Failed to unbind PCI device '%s'"), dev->name); - goto cleanup; + goto remove_id; } dev->reprobe = reprobe; } @@ -1222,7 +1222,11 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, dev->unbind_from_stub = true; } + result = 0; + remove_id: + err = virSaveLastError(); + /* If 'remove_id' exists, remove the device id from pci-stub's dynamic * ID table so that 'drivers_probe' works below. */ @@ -1233,6 +1237,7 @@ remove_id: "cannot be probed again.", dev->id, stubDriverName); } dev->reprobe = false; + result = -1; goto cleanup; } @@ -1247,24 +1252,26 @@ remove_id: "cannot be probed again.", dev->id, stubDriverName); } dev->reprobe = false; + result = -1; goto cleanup; } - newDriverName = stubDriverName; - result = 0; - cleanup: VIR_FREE(stubDriverPath); VIR_FREE(driverLink); VIR_FREE(path); - if (newDriverName && - STRNEQ_NULLABLE(dev->stubDriver, newDriverName)) { + if (result < 0) { + VIR_FREE(newDriverName); + virPCIDeviceUnbindFromStub(dev); + } else { VIR_FREE(dev->stubDriver); - result = VIR_STRDUP(dev->stubDriver, newDriverName); + dev->stubDriver = newDriverName; } - if (result < 0) - virPCIDeviceUnbindFromStub(dev); + + if (err) + virSetError(err); + virFreeError(err); return result; } -- 1.8.5.3

From: Jincheng Miao <jmiao@redhat.com> https://bugzilla.redhat.com/show_bug.cgi?id=1046919 If none (KVM, VFIO) of the supported PCI passthrough methods is known to work on a host, it's better to fail right away with a nice error message rather than letting attachment fail with a more cryptic message such as Failed to bind PCI device '0000:07:05.0' to vfio-pci: No such device Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/qemu/qemu_driver.c | 32 +++++++++++++++++++++++--------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index b964b3c..55ce6ec 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -10789,6 +10789,8 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, int ret = -1; virNodeDeviceDefPtr def = NULL; char *xml = NULL; + bool legacy = qemuHostdevHostSupportsPassthroughLegacy(); + bool vfio = qemuHostdevHostSupportsPassthroughVFIO(); virCheckFlags(0, -1); @@ -10811,22 +10813,34 @@ qemuNodeDeviceDetachFlags(virNodeDevicePtr dev, goto cleanup; if (!driverName) { - /* prefer vfio */ - if (qemuHostdevHostSupportsPassthroughVFIO()) + if (vfio) { driverName = "vfio"; - else if (qemuHostdevHostSupportsPassthroughLegacy()) + } else if (legacy) { driverName = "kvm"; + } else { + virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s", + _("neither VFIO nor KVM device assignment is " + "currently supported on this system")); + goto cleanup; + } } - if (!driverName) { - virReportError(VIR_ERR_INVALID_ARG, "%s", - _("neither VFIO nor kvm device assignment is " - "currently supported on this system")); - goto cleanup; - } else if (STREQ(driverName, "vfio")) { + if (STREQ(driverName, "vfio")) { + if (!vfio) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("VFIO device assignment is currently not " + "supported on this system")); + goto cleanup; + } if (virPCIDeviceSetStubDriver(pci, "vfio-pci") < 0) goto cleanup; } else if (STREQ(driverName, "kvm")) { + if (!legacy) { + virReportError(VIR_ERR_ARGUMENT_UNSUPPORTED, "%s", + _("KVM device assignment is currently not " + "supported on this system")); + goto cleanup; + } if (virPCIDeviceSetStubDriver(pci, "pci-stub") < 0) goto cleanup; } else { -- 1.8.5.3

For example: ... 5) testVirPCIDeviceIsAssignable(0005:90:01.0) ... OK 6) testVirPCIDeviceIsAssignable(0001:01:00.0) ... OK Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/virpcitest.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 82a173a..e96d7c0 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -237,8 +237,15 @@ mymain(void) # define DO_TEST_PCI(fnc, domain, bus, slot, function) \ do { \ struct testPCIDevData data = { domain, bus, slot, function }; \ - if (virtTestRun(#fnc, fnc, &data) < 0) \ + char *label = NULL; \ + if (virAsprintf(&label, "%s(%04x:%02x:%02x.%x)", \ + #fnc, domain, bus, slot, function) < 0) { \ ret = -1; \ + break; \ + } \ + if (virtTestRun(label, fnc, &data) < 0) \ + ret = -1; \ + VIR_FREE(label); \ } while (0) DO_TEST(testVirPCIDeviceNew); -- 1.8.5.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- src/libvirt_private.syms | 2 ++ src/util/virpci.c | 71 ++++++++++++++++++++++++++++-------------------- src/util/virpci.h | 5 ++++ 3 files changed, 49 insertions(+), 29 deletions(-) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 91d1304..68140e6 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1581,6 +1581,7 @@ virPCIDeviceCopy; virPCIDeviceDetach; virPCIDeviceFileIterate; virPCIDeviceFree; +virPCIDeviceGetDriverPathAndName; virPCIDeviceGetIOMMUGroupDev; virPCIDeviceGetIOMMUGroupList; virPCIDeviceGetManaged; @@ -1612,6 +1613,7 @@ virPCIDeviceSetReprobe; virPCIDeviceSetStubDriver; virPCIDeviceSetUnbindFromStub; virPCIDeviceSetUsedBy; +virPCIDeviceUnbind; virPCIDeviceWaitForCleanup; virPCIGetNetName; virPCIGetPhysicalFunction; diff --git a/src/util/virpci.c b/src/util/virpci.c index f6ab794..e2d222e 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -225,7 +225,7 @@ virPCIFile(char **buffer, const char *device, const char *file) * * Return 0 for success, -1 for error. */ -static int +int virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, char **path, char **name) { int ret = -1; @@ -1005,6 +1005,44 @@ recheck: return -1; } +int +virPCIDeviceUnbind(virPCIDevicePtr dev, bool reprobe) +{ + char *path = NULL; + char *drvpath = NULL; + char *driver = NULL; + int ret = -1; + + if (virPCIDeviceGetDriverPathAndName(dev, &drvpath, &driver) < 0) + goto cleanup; + + if (!driver) { + /* The device is not bound to any driver */ + ret = 0; + goto cleanup; + } + + if (virPCIFile(&path, dev->name, "driver/unbind") < 0) + goto cleanup; + + if (virFileExists(path)) { + if (virFileWriteStr(path, dev->name, 0) < 0) { + virReportSystemError(errno, + _("Failed to unbind PCI device '%s' from %s"), + dev->name, driver); + goto cleanup; + } + dev->reprobe = reprobe; + } + + ret = 0; +cleanup: + VIR_FREE(path); + VIR_FREE(drvpath); + VIR_FREE(driver); + return ret; +} + static const char *virPCIKnownStubs[] = { "pciback", /* used by xen */ "pci-stub", /* used by kvm legacy passthrough */ @@ -1047,18 +1085,8 @@ virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) if (!isStub) goto remove_slot; - if (virFileExists(drvdir)) { - if (virPCIDriverFile(&path, driver, "unbind") < 0) { - goto cleanup; - } - - if (virFileWriteStr(path, dev->name, 0) < 0) { - virReportSystemError(errno, - _("Failed to unbind PCI device '%s' from %s"), - dev->name, driver); - goto cleanup; - } - } + if (virPCIDeviceUnbind(dev, dev->reprobe) < 0) + goto cleanup; dev->unbind_from_stub = false; remove_slot: @@ -1174,24 +1202,9 @@ virPCIDeviceBindToStub(virPCIDevicePtr dev, goto remove_id; } - /* If the device is already bound to a driver, unbind it. - * Note, this will have rather unpleasant side effects if this - * PCI device happens to be IDE controller for the disk hosting - * your root filesystem. - */ - if (virPCIFile(&path, dev->name, "driver/unbind") < 0) + if (virPCIDeviceUnbind(dev, reprobe) < 0) goto remove_id; - if (virFileExists(path)) { - if (virFileWriteStr(path, dev->name, 0) < 0) { - virReportSystemError(errno, - _("Failed to unbind PCI device '%s'"), - dev->name); - goto remove_id; - } - dev->reprobe = reprobe; - } - /* If the device isn't already bound to pci-stub, try binding it now. */ if (!virFileLinkPointsTo(driverLink, stubDriverPath)) { diff --git a/src/util/virpci.h b/src/util/virpci.h index 08bf4c3..42c3c95 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -167,4 +167,9 @@ int virPCIDeviceAddressParse(char *address, virPCIDeviceAddressPtr bdf); int virPCIGetVirtualFunctionInfo(const char *vf_sysfs_device_path, char **pfname, int *vf_index); +int virPCIDeviceUnbind(virPCIDevicePtr dev, bool reprobe); +int virPCIDeviceGetDriverPathAndName(virPCIDevicePtr dev, + char **path, + char **name); + #endif /* __VIR_PCI_H__ */ -- 1.8.5.3

This file is used by PCI detach and reattach APIs to probe for a driver that handles a specific device. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/virpcimock.c | 40 +++++++++++++++++++++++++++++----------- 1 file changed, 29 insertions(+), 11 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index 49759b0..bf56143 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -91,6 +91,10 @@ char *fakesysfsdir; * Unbind driver from the device. * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). * + * /sys/bus/pci/drivers_probe + * Probe for a driver that handles the specified device. + * Data in format "DDDD:BB:DD.F" (Domain:Bus:Device.Function). + * * As a little hack, we are not mocking write to these files, but close() * instead. The advantage is we don't need any self growing array to hold the * partial writes and construct them back. We can let all the writes finish, @@ -568,27 +572,39 @@ cleanup: } static int +pci_driver_handle_drivers_probe(const char *path) +{ + struct pciDevice *dev; + + if (!(dev = pci_device_find_by_content(path))) { + errno = ENODEV; + return -1; + } + + if (dev->driver) + return 0; + + return pci_device_autobind(dev); +} + +static int pci_driver_handle_change(int fd ATTRIBUTE_UNUSED, const char *path) { int ret; const char *file = last_component(path); - if (STREQ(file, "bind")) { - /* handle write to bind */ + if (STREQ(file, "bind")) ret = pci_driver_handle_bind(path); - } else if (STREQ(file, "unbind")) { - /* handle write to unbind */ + else if (STREQ(file, "unbind")) ret = pci_driver_handle_unbind(path); - } else if (STREQ(file, "new_id")) { - /* handle write to new_id */ + else if (STREQ(file, "new_id")) ret = pci_driver_handle_new_id(path); - } else if (STREQ(file, "remove_id")) { - /* handle write to remove_id */ + else if (STREQ(file, "remove_id")) ret = pci_driver_handle_remove_id(path); - } else { - /* yet not handled write */ + else if (STREQ(file, "drivers_probe")) + ret = pci_driver_handle_drivers_probe(path); + else ABORT("Not handled write to: %s", path); - } return ret; } @@ -766,6 +782,8 @@ init_env(void) if (!(fakesysfsdir = getenv("LIBVIRT_FAKE_SYSFS_DIR"))) ABORT("Missing LIBVIRT_FAKE_SYSFS_DIR env variable\n"); + make_file(fakesysfsdir, "drivers_probe", NULL, -1); + # define MAKE_PCI_DRIVER(name, ...) \ pci_driver_new(name, __VA_ARGS__, -1, -1) -- 1.8.5.3

Especially for devices that are not bound to any driver. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/virpcimock.c | 3 + tests/virpcitest.c | 157 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 159 insertions(+), 1 deletion(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index bf56143..f8ea9c7 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -809,6 +809,9 @@ init_env(void) MAKE_PCI_DEVICE("0005:90:01.0", 0x1033, 0x0035); MAKE_PCI_DEVICE("0005:90:01.1", 0x1033, 0x0035); MAKE_PCI_DEVICE("0005:90:01.2", 0x1033, 0x00e0); + MAKE_PCI_DEVICE("0000:0a:01.0", 0x8086, 0x0047); + MAKE_PCI_DEVICE("0000:0a:02.0", 0x8286, 0x0048); + MAKE_PCI_DEVICE("0000:0a:03.0", 0x8386, 0x0048); } diff --git a/tests/virpcitest.c b/tests/virpcitest.c index e96d7c0..848014d 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -34,6 +34,30 @@ # define VIR_FROM_THIS VIR_FROM_NONE static int +testVirPCIDeviceCheckDriver(virPCIDevicePtr dev, const char *expected) +{ + char *path = NULL; + char *driver = NULL; + int ret = -1; + + if (virPCIDeviceGetDriverPathAndName(dev, &path, &driver) < 0) + goto cleanup; + + if (STRNEQ_NULLABLE(driver, expected)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + "PCI device %s driver mismatch: %s, expecting %s", + virPCIDeviceGetName(dev), driver, expected); + goto cleanup; + } + + ret = 0; +cleanup: + VIR_FREE(path); + VIR_FREE(driver); + return ret; +} + +static int testVirPCIDeviceNew(const void *opaque ATTRIBUTE_UNUSED) { int ret = -1; @@ -89,6 +113,9 @@ testVirPCIDeviceDetach(const void *oaque ATTRIBUTE_UNUSED) if (virPCIDeviceDetach(dev[i], activeDevs, inactiveDevs) < 0) goto cleanup; + if (testVirPCIDeviceCheckDriver(dev[i], "pci-stub") < 0) + goto cleanup; + CHECK_LIST_COUNT(activeDevs, 0); CHECK_LIST_COUNT(inactiveDevs, i + 1); } @@ -188,6 +215,7 @@ struct testPCIDevData { unsigned int bus; unsigned int slot; unsigned int function; + const char *driver; }; static int @@ -208,6 +236,88 @@ cleanup: return ret; } +static int +testVirPCIDeviceDetachSingle(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); + if (!dev) + goto cleanup; + + if (virPCIDeviceSetStubDriver(dev, "pci-stub") < 0 || + virPCIDeviceDetach(dev, NULL, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + virPCIDeviceFree(dev); + return ret; +} + +static int +testVirPCIDeviceReattachSingle(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); + if (!dev) + goto cleanup; + + virPCIDeviceReattachInit(dev); + if (virPCIDeviceReattach(dev, NULL, NULL) < 0) + goto cleanup; + + ret = 0; +cleanup: + virPCIDeviceFree(dev); + return ret; +} + +static int +testVirPCIDeviceCheckDriverTest(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); + if (!dev) + goto cleanup; + + if (testVirPCIDeviceCheckDriver(dev, data->driver) < 0) + goto cleanup; + + ret = 0; +cleanup: + virPCIDeviceFree(dev); + return ret; +} + +static int +testVirPCIDeviceUnbind(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); + if (!dev) + goto cleanup; + + if (virPCIDeviceUnbind(dev, false) < 0) + goto cleanup; + + ret = 0; +cleanup: + virPCIDeviceFree(dev); + return ret; +} + # define FAKESYSFSDIRTEMPLATE abs_builddir "/fakesysfsdir-XXXXXX" static int @@ -236,7 +346,9 @@ mymain(void) # define DO_TEST_PCI(fnc, domain, bus, slot, function) \ do { \ - struct testPCIDevData data = { domain, bus, slot, function }; \ + struct testPCIDevData data = { \ + domain, bus, slot, function, NULL \ + }; \ char *label = NULL; \ if (virAsprintf(&label, "%s(%04x:%02x:%02x.%x)", \ #fnc, domain, bus, slot, function) < 0) { \ @@ -248,6 +360,28 @@ mymain(void) VIR_FREE(label); \ } while (0) +# define DO_TEST_PCI_DRIVER(domain, bus, slot, function, driver) \ + do { \ + struct testPCIDevData data = { \ + domain, bus, slot, function, driver \ + }; \ + char *label = NULL; \ + if (virAsprintf(&label, "PCI driver %04x:%02x:%02x.%x is %s", \ + domain, bus, slot, function, \ + NULLSTR(driver)) < 0) { \ + ret = -1; \ + break; \ + } \ + if (virtTestRun(label, testVirPCIDeviceCheckDriverTest, \ + &data) < 0) \ + ret = -1; \ + VIR_FREE(label); \ + } while (0) + + /* Changes made to individual devices are persistent and the + * tests often rely on the state set by previous tests. + */ + DO_TEST(testVirPCIDeviceNew); DO_TEST(testVirPCIDeviceDetach); DO_TEST(testVirPCIDeviceReset); @@ -255,6 +389,27 @@ mymain(void) DO_TEST_PCI(testVirPCIDeviceIsAssignable, 5, 0x90, 1, 0); DO_TEST_PCI(testVirPCIDeviceIsAssignable, 1, 1, 0, 0); + /* Reattach a device already bound to non-stub a driver */ + DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, "i915"); + DO_TEST_PCI(testVirPCIDeviceReattachSingle, 0, 0x0a, 1, 0); + DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, "i915"); + + /* Reattach an unbound device */ + DO_TEST_PCI(testVirPCIDeviceUnbind, 0, 0x0a, 1, 0); + DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, NULL); + DO_TEST_PCI(testVirPCIDeviceReattachSingle, 0, 0x0a, 1, 0); + DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, "i915"); + + /* Detach an unbound device */ + DO_TEST_PCI_DRIVER(0, 0x0a, 2, 0, NULL); + DO_TEST_PCI(testVirPCIDeviceDetachSingle, 0, 0x0a, 2, 0); + DO_TEST_PCI_DRIVER(0, 0x0a, 2, 0, "pci-stub"); + + /* Reattach an unknown unbound device */ + DO_TEST_PCI_DRIVER(0, 0x0a, 3, 0, NULL); + DO_TEST_PCI(testVirPCIDeviceReattachSingle, 0, 0x0a, 3, 0); + DO_TEST_PCI_DRIVER(0, 0x0a, 3, 0, NULL); + if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL) virFileDeleteTree(fakesysfsdir); -- 1.8.5.3

Such driver can be used to make sure PCI APIs fail properly. Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/virpcimock.c | 33 +++++++++++++++++++++------------ 1 file changed, 21 insertions(+), 12 deletions(-) diff --git a/tests/virpcimock.c b/tests/virpcimock.c index f8ea9c7..b514619 100644 --- a/tests/virpcimock.c +++ b/tests/virpcimock.c @@ -107,11 +107,19 @@ char *fakesysfsdir; * */ +enum driverActions { + PCI_ACTION_BIND = 1 << 0, + PCI_ACTION_UNBIND = 1 << 1, + PCI_ACTION_NEW_ID = 1 << 2, + PCI_ACTION_REMOVE_ID = 1 << 3, +}; + struct pciDriver { char *name; int *vendor; /* List of vendor:device IDs the driver can handle */ int *device; size_t len; /* @len is used for both @vendor and @device */ + int fail; /* Actions on this driver which should fail? */ }; struct pciDevice { @@ -143,7 +151,7 @@ static void pci_device_new_from_stub(const struct pciDevice *data); static struct pciDevice *pci_device_find_by_id(const char *id); static struct pciDevice *pci_device_find_by_content(const char *path); -static void pci_driver_new(const char *name, ...); +static void pci_driver_new(const char *name, int fail, ...); static struct pciDriver *pci_driver_find_by_dev(struct pciDevice *dev); static struct pciDriver *pci_driver_find_by_path(const char *path); static int pci_driver_bind(struct pciDriver *driver, struct pciDevice *dev); @@ -415,7 +423,7 @@ pci_device_autobind(struct pciDevice *dev) * PCI Driver functions */ static void -pci_driver_new(const char *name, ...) +pci_driver_new(const char *name, int fail, ...) { struct pciDriver *driver; va_list args; @@ -427,10 +435,12 @@ pci_driver_new(const char *name, ...) virAsprintfQuiet(&driverpath, "%s/drivers/%s", fakesysfsdir, name) < 0) ABORT_OOM(); + driver->fail = fail; + if (virFileMakePath(driverpath) < 0) ABORT("Unable to create: %s", driverpath); - va_start(args, name); + va_start(args, fail); while ((vendor = va_arg(args, int)) != -1) { if ((device = va_arg(args, int)) == -1) @@ -497,8 +507,8 @@ pci_driver_bind(struct pciDriver *driver, int ret = -1; char *devpath = NULL, *driverpath = NULL; - if (dev->driver) { - /* Device already bound */ + if (dev->driver || PCI_ACTION_BIND & driver->fail) { + /* Device already bound or failing driver requested */ errno = ENODEV; return ret; } @@ -544,8 +554,8 @@ pci_driver_unbind(struct pciDriver *driver, int ret = -1; char *devpath = NULL, *driverpath = NULL; - if (dev->driver != driver) { - /* Device not bound to the @driver */ + if (dev->driver != driver || PCI_ACTION_UNBIND & driver->fail) { + /* Device not bound to the @driver or failing driver used */ errno = ENODEV; return ret; } @@ -651,8 +661,7 @@ pci_driver_handle_new_id(const char *path) int vendor, device; char buf[32]; - if (!driver) { - /* This should never happen (TM) */ + if (!driver || PCI_ACTION_NEW_ID & driver->fail) { errno = ENODEV; goto cleanup; } @@ -707,8 +716,7 @@ pci_driver_handle_remove_id(const char *path) int vendor, device; char buf[32]; - if (!driver) { - /* This should never happen (TM) */ + if (!driver || PCI_ACTION_REMOVE_ID & driver->fail) { errno = ENODEV; goto cleanup; } @@ -785,11 +793,12 @@ init_env(void) make_file(fakesysfsdir, "drivers_probe", NULL, -1); # define MAKE_PCI_DRIVER(name, ...) \ - pci_driver_new(name, __VA_ARGS__, -1, -1) + pci_driver_new(name, 0, __VA_ARGS__, -1, -1) MAKE_PCI_DRIVER("iwlwifi", 0x8086, 0x0044); MAKE_PCI_DRIVER("i915", 0x8086, 0x0046, 0x8086, 0x0047); MAKE_PCI_DRIVER("pci-stub", -1, -1); + pci_driver_new("vfio-pci", PCI_ACTION_BIND, -1, -1); # define MAKE_PCI_DEVICE(Id, Vendor, Device, ...) \ do { \ -- 1.8.5.3

Signed-off-by: Jiri Denemark <jdenemar@redhat.com> --- tests/virpcitest.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/tests/virpcitest.c b/tests/virpcitest.c index 848014d..6a2291d 100644 --- a/tests/virpcitest.c +++ b/tests/virpcitest.c @@ -258,6 +258,37 @@ cleanup: } static int +testVirPCIDeviceDetachFail(const void *opaque) +{ + const struct testPCIDevData *data = opaque; + int ret = -1; + virPCIDevicePtr dev; + + dev = virPCIDeviceNew(data->domain, data->bus, data->slot, data->function); + if (!dev) + goto cleanup; + + if (virPCIDeviceSetStubDriver(dev, "vfio-pci") < 0) + goto cleanup; + + if (virPCIDeviceDetach(dev, NULL, NULL) < 0) { + if (virTestGetVerbose() || virTestGetDebug()) + virDispatchError(NULL); + virResetLastError(); + ret = 0; + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + "Attaching device %s to %s should have failed", + virPCIDeviceGetName(dev), + virPCIDeviceGetStubDriver(dev)); + } + +cleanup: + virPCIDeviceFree(dev); + return ret; +} + +static int testVirPCIDeviceReattachSingle(const void *opaque) { const struct testPCIDevData *data = opaque; @@ -389,6 +420,8 @@ mymain(void) DO_TEST_PCI(testVirPCIDeviceIsAssignable, 5, 0x90, 1, 0); DO_TEST_PCI(testVirPCIDeviceIsAssignable, 1, 1, 0, 0); + DO_TEST_PCI(testVirPCIDeviceDetachFail, 0, 0x0a, 1, 0); + /* Reattach a device already bound to non-stub a driver */ DO_TEST_PCI_DRIVER(0, 0x0a, 1, 0, "i915"); DO_TEST_PCI(testVirPCIDeviceReattachSingle, 0, 0x0a, 1, 0); -- 1.8.5.3