On 06/15/10 - 01:13:00PM, Daniel P. Berrange wrote:
> > @@ -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,7 +11191,7 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev)
> > if (!pci)
> > return -1;
> >
> > - if (pciDettachDevice(pci) < 0)
> > + if (pciDettachDevice(pci, driver->activePciHostdevs) < 0)
> > goto out;
> >
> > ret = 0;
> > @@ -11202,6 +11203,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,7 +11215,7 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev)
> > if (!pci)
> > return -1;
> >
> > - if (pciReAttachDevice(pci) < 0)
> > + if (pciReAttachDevice(pci, driver->activePciHostdevs) < 0)
> > goto out;
> >
> > ret = 0;
>
> You're accessing 'driver' here without first locking it
D'oh, of course. Thanks for that. Updated patch below.
>From 56b1d0463790b52e23975968c54d4f9f9a3c5fbd Mon Sep 17 00:00:00 2001
From: Chris Lalancette <clalance@xxxxxxxxxx>
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 <clalance@xxxxxxxxxx>
---
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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list