From e482693b24de7b57531d9b3ac75dc1b3bb05a4cd Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Wed, 1 May 2013 14:44:10 -0400 Subject: [PATCH] pci: autolearn name of stub driver, remove from arglist virPCIDeviceReattach and virPCIDeviceUnbindFromStub (called by virPCIDeviceReattach) had previously required the name of the stub driver as input. This is unnecessary, because the name of the driver the device is currently bound to can be found by looking at the link: /sys/bus/pci/dddd:bb:ss.ff/driver Instead of requiring that the name of the expected stub driver name and only unbinding if that one name is matched, we no longer take a driver name in the arglist for either of these functions. virPCIDeviceUnbindFromStub just compares the name of the currently bound driver to a list of "well known" stubs (right now contains "pci-stub" and "vfio-pci" for qemu, and "pciback" for xen), and only performs the unbind if it's one of those devices. This allows virsh nodedevice-reattach to work properly across a libvirtd restart, and fixes a couple of cases where we were erroneously still hard-coding "pci-stub" as the drive name. For some unknown reason, virPCIDeviceReattach had been calling modprobe on the stub driver prior to unbinding the device. This was problematic because we no longer know the name of the stub driver in that function. However, it is pointless to probe for the stub driver at that time anyway - because the device is bound to the stub driver, we are guaranteed that it is already loaded, and so that call to modprobe has been removed. --- src/qemu/qemu_driver.c | 2 +- src/qemu/qemu_hostdev.c | 4 +-- src/util/virpci.c | 68 +++++++++++++++++++++++++++++------------ src/util/virpci.h | 3 +- src/xen/xen_driver.c | 2 +- 5 files changed, 53 insertions(+), 26 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 296efe386d..94928508f0 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -9832,7 +9832,7 @@ qemuNodeDeviceReAttach(virNodeDevicePtr dev) virPCIDeviceReattachInit(pci); if (virPCIDeviceReattach(pci, driver->activePciHostdevs, - driver->inactivePciHostdevs, "pci-stub") < 0) + driver->inactivePciHostdevs) < 0) goto out; ret = 0; diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 4d1f39d2ed..e4af03666e 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -609,7 +609,7 @@ reattachdevs: /* NB: This doesn't actually re-bind to original driver, just * unbinds from the stub driver */ - virPCIDeviceReattach(dev, driver->activePciHostdevs, NULL, NULL); + virPCIDeviceReattach(dev, driver->activePciHostdevs, NULL); } cleanup: @@ -869,7 +869,7 @@ void qemuReattachPciDevice(virPCIDevicePtr dev, virQEMUDriverPtr driver) } if (virPCIDeviceReattach(dev, driver->activePciHostdevs, - driver->inactivePciHostdevs, "pci-stub") < 0) { + driver->inactivePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), err ? err->message : _("unknown error")); diff --git a/src/util/virpci.c b/src/util/virpci.c index 97bba74a38..ba60292f15 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -909,25 +909,62 @@ recheck: return -1; } +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, const char *driver) +virPCIDeviceUnbindFromStub(virPCIDevicePtr dev) { int result = -1; char *drvdir = NULL; char *path = NULL; + char *driver = NULL; + const char **stubTest; + bool isStub = false; - if (virPCIDriverDir(&drvdir, driver) < 0) + /* If the device is currently bound to one of the "well known" + * stub drivers, then unbind it, otherwise ignore it. + */ + if (virPCIFile(&path, dev->name, "driver") < 0) goto cleanup; + /* path = "/sys/bus/pci/dddd:bb:ss.ff/driver" */ + if (virFileIsLink(path) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid device %s driver file %s is not a symlink"), + dev->name, path); + goto cleanup; + } + if (virFileResolveLink(path, &drvdir) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Unable to resolve device %s driver symlink %s"), + dev->name, path); + goto cleanup; + } + /* drvdir = "/sys/bus/pci/drivers/${drivername}" */ + if (!(driver = strdup(last_component(drvdir)))) { + virReportOOMError(); + goto cleanup; + } if (!dev->unbind_from_stub) goto remove_slot; - /* If the device is bound to stub, unbind it. - */ - if (virPCIFile(&path, dev->name, "driver") < 0) - goto cleanup; + /* 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) + goto remove_slot; - if (virFileExists(drvdir) && virFileLinkPointsTo(path, drvdir)) { + if (virFileExists(drvdir)) { if (virPCIDriverFile(&path, driver, "unbind") < 0) { goto cleanup; } @@ -992,6 +1029,7 @@ cleanup: VIR_FREE(drvdir); VIR_FREE(path); + VIR_FREE(driver); return result; } @@ -1141,7 +1179,7 @@ cleanup: VIR_FREE(path); if (result < 0) { - virPCIDeviceUnbindFromStub(dev, driver); + virPCIDeviceUnbindFromStub(dev); } return result; @@ -1183,25 +1221,15 @@ virPCIDeviceDetach(virPCIDevicePtr dev, int virPCIDeviceReattach(virPCIDevicePtr dev, virPCIDeviceListPtr activeDevs, - virPCIDeviceListPtr inactiveDevs, - const char *driver) + virPCIDeviceListPtr inactiveDevs) { - if (!driver && dev->stubDriver) - driver = dev->stubDriver; - - if (virPCIProbeStubDriver(driver) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Failed to load PCI stub module %s"), driver); - return -1; - } - if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Not reattaching active device %s"), dev->name); return -1; } - if (virPCIDeviceUnbindFromStub(dev, driver) < 0) + if (virPCIDeviceUnbindFromStub(dev) < 0) return -1; /* Steal the dev from list inactiveDevs */ diff --git a/src/util/virpci.h b/src/util/virpci.h index 3911b72359..7bcadb42f6 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -54,8 +54,7 @@ int virPCIDeviceDetach(virPCIDevicePtr dev, const char *driver); int virPCIDeviceReattach(virPCIDevicePtr dev, virPCIDeviceListPtr activeDevs, - virPCIDeviceListPtr inactiveDevs, - const char *driver); + virPCIDeviceListPtr inactiveDevs); int virPCIDeviceReset(virPCIDevicePtr dev, virPCIDeviceListPtr activeDevs, virPCIDeviceListPtr inactiveDevs); diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 8971d4cc72..0642edb59b 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -2254,7 +2254,7 @@ xenUnifiedNodeDeviceReAttach(virNodeDevicePtr dev) goto out; } - if (virPCIDeviceReattach(pci, NULL, NULL, "pciback") < 0) + if (virPCIDeviceReattach(pci, NULL, NULL) < 0) goto out; ret = 0; -- 2.47.2