From 53a1db4dfcae87ee42e8f7bbf5f746f0547da9ae Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 14 Jun 2010 17:12:35 -0400 Subject: [PATCH] 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. 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 e8c5c35651..ce0999ab46 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2865,7 +2865,7 @@ qemuPrepareHostPCIDevices(struct qemud_driver *driver, goto cleanup; if (pciDeviceGetManaged(dev) && - pciDettachDevice(dev) < 0) + pciDettachDevice(dev, driver->activePciHostdevs) < 0) goto cleanup; } @@ -2967,7 +2967,7 @@ qemuPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED, static void -qemudReattachManagedDevice(pciDevice *dev) +qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver) { int retries = 100; @@ -2977,7 +2977,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")); @@ -3024,7 +3024,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); @@ -7772,7 +7772,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; @@ -7860,7 +7860,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); @@ -8771,7 +8771,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); } @@ -11277,6 +11277,7 @@ out: static int qemudNodeDeviceDettach (virNodeDevicePtr dev) { + struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; @@ -11288,11 +11289,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; } @@ -11300,6 +11303,7 @@ out: static int qemudNodeDeviceReAttach (virNodeDevicePtr dev) { + struct qemud_driver *driver = dev->conn->privateData; pciDevice *pci; unsigned domain, bus, slot, function; int ret = -1; @@ -11311,11 +11315,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 b93f28c32e..b2e16738bb 100644 --- a/src/util/pci.c +++ b/src/util/pci.c @@ -814,7 +814,7 @@ pciBindDeviceToStub(pciDevice *dev, const char *driver) } int -pciDettachDevice(pciDevice *dev) +pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -823,6 +823,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); } @@ -875,7 +881,7 @@ pciUnBindDeviceFromStub(pciDevice *dev, const char *driver) } int -pciReAttachDevice(pciDevice *dev) +pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs) { const char *driver = pciFindStubDriver(); if (!driver) { @@ -884,6 +890,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 0ddbaa747f..9aef81f9e2 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 2fd52bad08..3dd673b64e 100644 --- a/src/xen/xen_driver.c +++ b/src/xen/xen_driver.c @@ -1846,7 +1846,7 @@ xenUnifiedNodeDeviceDettach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciDettachDevice(pci) < 0) + if (pciDettachDevice(pci, NULL) < 0) goto out; ret = 0; @@ -1869,7 +1869,7 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev) if (!pci) return -1; - if (pciReAttachDevice(pci) < 0) + if (pciReAttachDevice(pci, NULL) < 0) goto out; ret = 0;