>From 56b1d0463790b52e23975968c54d4f9f9a3c5fbd Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 14 Jun 2010 17:12:35 -0400 Subject: [PATCH v2 1/2] Check for active PCI devices when doing nodedevice operations. In the current libvirt PCI code, there is no checking whether a PCI device is in use by a guest when doing node device detach or reattach. This causes problems when a device is assigned to a guest, and the administrator starts issuing nodedevice commands. Make it so that we check the list of active devices when trying to detach/reattach, and only allow the operation if the device is not assigned to a guest. v2: Add locking to qemudNodeDeviceDettach and qemudNodeDeviceReAttach Signed-off-by: Chris Lalancette --- src/qemu/qemu_driver.c | 24 +++++++++++++++--------- src/util/pci.c | 16 ++++++++++++++-- src/util/pci.h | 4 ++-- src/xen/xen_driver.c | 4 ++-- 4 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index fc5585e..769ee0f 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2858,7 +2858,7 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver, goto cleanup; if (pciDeviceGetManaged(dev) && - pciDettachDevice(dev) < 0) + pciDettachDevice(dev, driver->activePciHostdevs) < 0) goto cleanup; } @@ -2938,7 +2938,7 @@ qemuPrepareHostDevices(struct qemud_driver *driver, static void -qemudReattachManagedDevice(pciDevice *dev) +qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver) { int retries = 100; @@ -2948,7 +2948,7 @@ qemudReattachManagedDevice(pciDevice *dev) usleep(100*1000); retries--; } - if (pciReAttachDevice(dev) < 0) { + if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to re-attach PCI device: %s"), err ? err->message : _("unknown error")); @@ -2995,7 +2995,7 @@ qemuDomainReAttachHostDevices(struct qemud_driver *driver, for (i = 0; i < pciDeviceListCount(pcidevs); i++) { pciDevice *dev = pciDeviceListGet(pcidevs, i); - qemudReattachManagedDevice(dev); + qemudReattachManagedDevice(dev, driver); } pciDeviceListFree(pcidevs); @@ -7727,7 +7727,7 @@ static int qemudDomainAttachHostPciDevice(struct qemud_driver *driver, return -1; if (!pciDeviceIsAssignable(pci, !driver->relaxedACS) || - (hostdev->managed && pciDettachDevice(pci) < 0) || + (hostdev->managed && pciDettachDevice(pci, driver->activePciHostdevs) < 0) || pciResetDevice(pci, driver->activePciHostdevs) < 0) { pciFreeDevice(pci); return -1; @@ -7815,7 +7815,7 @@ error: if (pciResetDevice(pci, driver->activePciHostdevs) < 0) VIR_WARN0("Unable to reset PCI device after assign failure"); else if (hostdev->managed && - pciReAttachDevice(pci) < 0) + pciReAttachDevice(pci, driver->activePciHostdevs) < 0) VIR_WARN0("Unable to re-attach PCI device after assign failure"); pciFreeDevice(pci); @@ -8726,7 +8726,7 @@ static int qemudDomainDetachHostPciDevice(struct qemud_driver *driver, pciDeviceListDel(driver->activePciHostdevs, pci); if (pciResetDevice(pci, driver->activePciHostdevs) < 0) ret = -1; - qemudReattachManagedDevice(pci); + qemudReattachManagedDevice(pci, driver); pciFreeDevice(pci); } @@ -11179,6 +11179,7 @@ out: static int qemudNodeDeviceDettach (virNodeDevicePtr dev) { + struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; @@ -11190,11 +11191,13 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciDettachDevice(pci) < 0) + qemuDriverLock(driver); + if (pciDettachDevice(pci, driver->activePciHostdevs) < 0) goto out; ret = 0; out: + qemuDriverUnlock(driver); pciFreeDevice(pci); return ret; } @@ -11202,6 +11205,7 @@ out: static int qemudNodeDeviceReAttach (virNodeDevicePtr dev) { + struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; @@ -11213,11 +11217,13 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciReAttachDevice(pci) < 0) + qemuDriverLock(driver); + if (pciReAttachDevice(pci, driver->activePciHostdevs) < 0) goto out; ret = 0; out: + qemuDriverUnlock(driver); pciFreeDevice(pci); return ret; } diff --git a/src/util/pci.c b/src/util/pci.c index 0c6d308..fe128db 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -815,7 +815,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver) } int -pciDettachDevice(pciDevice *dev) +pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -824,6 +824,12 @@ pciDettachDevice(pciDevice *dev) return -1; } + if (activeDevs && pciDeviceListFind(activeDevs, dev)) { + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Not detaching active device %s"), dev->name); + return -1; + } + return pciBindDeviceToStub(dev, driver); } @@ -876,7 +882,7 @@ pciUnBindDeviceFromStub(pciDevice *dev, const char *driver) } int -pciReAttachDevice(pciDevice *dev) +pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -885,6 +891,12 @@ pciReAttachDevice(pciDevice *dev) return -1; } + if (activeDevs && pciDeviceListFind(activeDevs, dev)) { + pciReportError(VIR_ERR_INTERNAL_ERROR, + _("Not reattaching active device %s"), dev->name); + return -1; + } + return pciUnBindDeviceFromStub(dev, driver); } diff --git a/src/util/pci.h b/src/util/pci.h index fdef18c..7fe3d1f 100644 --- a/src/util/pci.h +++ b/src/util/pci.h @@ -32,8 +32,8 @@ pciDevice *pciGetDevice (unsigned domain, unsigned slot, unsigned function); void pciFreeDevice (pciDevice *dev); -int pciDettachDevice (pciDevice *dev); -int pciReAttachDevice (pciDevice *dev); +int pciDettachDevice (pciDevice *dev, pciDeviceList *activeDevs); +int pciReAttachDevice (pciDevice *dev, pciDeviceList *activeDevs); int pciResetDevice (pciDevice *dev, pciDeviceList *activeDevs); void pciDeviceSetManaged(pciDevice *dev, diff --git a/src/xen/xen_driver.c b/src/xen/xen_driver.c index 91f0acd..5f8efa7 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1838,7 +1838,7 @@ xenUnifiedNodeDeviceDettach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciDettachDevice(pci) < 0) + if (pciDettachDevice(pci, NULL) < 0) goto out; ret = 0; @@ -1861,7 +1861,7 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciReAttachDevice(pci) < 0) + if (pciReAttachDevice(pci, NULL) < 0) goto out; ret = 0; -- 1.6.6.1