From 9e37f57f430055492f9bf5d0abd283eb070486d4 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Fri, 28 Jun 2013 22:35:21 -0400 Subject: [PATCH] pci: make virPCIDeviceReset more autonomous I recently patches the callers to virPCIDeviceReset() to not call it if the current driver for a device was vfio-pci (since that driver will always reset the device itself when appropriate. At the time, Dan Berrange suggested that I could instead modify virPCIDeviceReset to check the currently bound driver for the device, and decide for itself whether or not to go ahead with the reset. This patch removes the previously added checks, and replaces them with a check down in virPCIDeviceReset(), as suggested. The functional difference here is that previously we were deciding based on either the hostdev configuration or the value of stubDriverName in the virPCIDevice object, but now we are actually comparing to the "driver" link in the device's sysfs entry directly. In practice, both should be the same. --- src/qemu/qemu_hostdev.c | 6 ++---- src/qemu/qemu_hotplug.c | 5 ++--- src/util/virpci.c | 27 ++++++++++++++++++++++++--- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index f24d4661e1..21fe47f43a 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -544,8 +544,7 @@ int qemuPrepareHostdevPCIDevices(virQEMUDriverPtr driver, * can safely reset them */ for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); - if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci")) - continue; + if (virPCIDeviceReset(dev, driver->activePciHostdevs, driver->inactivePciHostdevs) < 0) goto reattachdevs; @@ -1119,8 +1118,7 @@ void qemuDomainReAttachHostdevDevices(virQEMUDriverPtr driver, for (i = 0; i < virPCIDeviceListCount(pcidevs); i++) { virPCIDevicePtr dev = virPCIDeviceListGet(pcidevs, i); - if (STREQ_NULLABLE(virPCIDeviceGetStubDriver(dev), "vfio-pci")) - continue; + if (virPCIDeviceReset(dev, driver->activePciHostdevs, driver->inactivePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index e05b4b37c6..1925fe4d1a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2492,9 +2492,8 @@ qemuDomainDetachHostPciDevice(virQEMUDriverPtr driver, if (pci) { activePci = virPCIDeviceListSteal(driver->activePciHostdevs, pci); if (activePci && - (subsys->u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO || - virPCIDeviceReset(activePci, driver->activePciHostdevs, - driver->inactivePciHostdevs) == 0)) { + virPCIDeviceReset(activePci, driver->activePciHostdevs, + driver->inactivePciHostdevs) == 0) { qemuReattachPciDevice(activePci, driver); ret = 0; } else { diff --git a/src/util/virpci.c b/src/util/virpci.c index 3f8625e2bf..c95c0e6326 100644 --- a/src/util/virpci.c +++ b/src/util/virpci.c @@ -866,8 +866,10 @@ virPCIDeviceReset(virPCIDevicePtr dev, virPCIDeviceList *activeDevs, virPCIDeviceList *inactiveDevs) { + char *drvPath = NULL; + char *drvName = NULL; int ret = -1; - int fd; + int fd = -1; if (activeDevs && virPCIDeviceListFind(activeDevs, dev)) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -875,8 +877,24 @@ virPCIDeviceReset(virPCIDevicePtr dev, return -1; } + /* If the device is currently bound to vfio-pci, ignore all + * requests to reset it, since the vfio-pci driver will always + * reset it whenever appropriate, so doing it ourselves would just + * be redundant. + */ + if (virPCIDeviceGetDriverPathAndName(dev, &drvPath, &drvName) < 0) + goto cleanup; + + if (STREQ_NULLABLE(drvName, "vfio-pci")) { + VIR_DEBUG("Device %s is bound to vfio-pci - skip reset", + dev->name); + ret = 0; + goto cleanup; + } + VIR_DEBUG("Resetting device %s", dev->name); + if ((fd = virPCIDeviceConfigOpen(dev, true)) < 0) - return -1; + goto cleanup; if (virPCIDeviceInit(dev, fd) < 0) goto cleanup; @@ -905,10 +923,13 @@ virPCIDeviceReset(virPCIDevicePtr dev, virReportError(VIR_ERR_INTERNAL_ERROR, _("Unable to reset PCI device %s: %s"), dev->name, - err ? err->message : _("no FLR, PM reset or bus reset available")); + err ? err->message : + _("no FLR, PM reset or bus reset available")); } cleanup: + VIR_FREE(drvPath); + VIR_FREE(drvName); virPCIDeviceConfigClose(dev, fd); return ret; } -- GitLab