6:14 p.m.
On 01/17/2012 01:02 PM, Osier Yang wrote:
pciTrySecondaryBusReset checks if there is active device on the
same bus, however, qemu driver doesn't maintain an effective
list for the inactive devices, and it passes meaningless argment
s/argment/argument/
for parameter "inactiveDevs". e.g.
(qemuPrepareHostdevPCIDevices)
if (!(pcidevs = qemuGetPciHostDeviceList(hostdevs, nhostdevs)))
return -1;
...skipped...
if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0)
goto reattachdevs;
NB, the "pcidevs" used above are extracted from domain def, and
thus one won't be able to attach a device of which bus has other
device even detached from host (nodedev-detach). To see more
details of the problem:
RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=773667
This patch is to resolve the problem by introduce an inactive
s/introduce/introducing/
PCI device list (just like qemu_driver->activePciHostdevs), and
the whole logic is:
* Add the device to inactive list when do nodedev-dettach
s/when do/during/
* Remove the device from inactive list when do nodedev-reattach
* Remove the device from inactive list when do attach-device
(for not managed device)
* Add the device to inactive list after detach-device, only
if the device is not managed
With above, we have sufficient inactive PCI device list, and thus
we can use it for pciResetDevice. e.g.(qemuPrepareHostdevPCIDevices)
if (pciResetDevice(dev, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
goto reattachdevs;
v1 ~ v2
Fixed a potential crash.
---
Got a testing box from Miroslav and tested the patch, it works well.
I'm glad you were able to test it; I tried to reproduce things locally,
but didn't quite have the right hardware, so I'm reviewing this on
inspection alone. But it is a serious enough issue that I'm okay
pushing once the nits are fixed.
---
src/qemu/qemu_conf.h | 5 +++++
src/qemu/qemu_driver.c | 19 +++++++++++++++----
src/qemu/qemu_hostdev.c | 32 +++++++++++++++++++++++---------
src/util/pci.c | 28 ++++++++++++++++++++++++----
src/util/pci.h | 8 ++++++--
src/xen/xen_driver.c | 4 ++--
7 files changed, 77 insertions(+), 22 deletions(-)
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index d8d7915..3f1b668 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -128,6 +128,11 @@ struct qemud_driver {
pciDeviceList *activePciHostdevs;
usbDeviceList *activeUsbHostdevs;
+ /* The device which is not used by both host
+ * and any guest.
The devices which are not in use by the host or any guest.
@@ -778,6 +781,7 @@ qemudShutdown(void) {
qemuDriverLock(qemu_driver);
pciDeviceListFree(qemu_driver->activePciHostdevs);
+ pciDeviceListFree(qemu_driver->inactivePciHostdevs);
usbDeviceListFree(qemu_driver->activeUsbHostdevs);
We'll probably have to repeat this exercise for USB passthrough devices,
but that can be a separate patch.
@@ -445,8 +452,13 @@ void qemuReattachPciDevice(pciDevice *dev,
struct qemud_driver *driver)
{
int retries = 100;
- if (!pciDeviceGetManaged(dev))
+ /* If the device is not managed and was attached to guest
+ * successfully, it must had be inactive.
s/had be/have been/
Overall, the design looks sound. I squashed in your followup, plus
this, then pushed:
diff --git i/src/qemu/qemu_conf.h w/src/qemu/qemu_conf.h
index 3f1b668..7d79823 100644
--- i/src/qemu/qemu_conf.h
+++ w/src/qemu/qemu_conf.h
@@ -1,7 +1,7 @@
/*
* qemu_conf.h: QEMU configuration management
*
- * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -128,9 +128,7 @@ struct qemud_driver {
pciDeviceList *activePciHostdevs;
usbDeviceList *activeUsbHostdevs;
- /* The device which is not used by both host
- * and any guest.
- */
+ /* The devices which is are not in use by the host or any guest. */
pciDeviceList *inactivePciHostdevs;
virBitmapPtr reservedVNCPorts;
diff --git i/src/qemu/qemu_hostdev.c w/src/qemu/qemu_hostdev.c
index 6141cfe..b3cad8e 100644
--- i/src/qemu/qemu_hostdev.c
+++ w/src/qemu/qemu_hostdev.c
@@ -1,7 +1,7 @@
/*
* qemu_hostdev.c: QEMU hostdev management
*
- * Copyright (C) 2006-2007, 2009-2011 Red Hat, Inc.
+ * Copyright (C) 2006-2007, 2009-2012 Red Hat, Inc.
* Copyright (C) 2006 Daniel P. Berrange
*
* This library is free software; you can redistribute it and/or
@@ -453,7 +453,7 @@ void qemuReattachPciDevice(pciDevice *dev, struct
qemud_driver *driver)
int retries = 100;
/* If the device is not managed and was attached to guest
- * successfully, it must had be inactive.
+ * successfully, it must have been inactive.
*/
if (!pciDeviceGetManaged(dev)) {
pciDeviceListAdd(driver->inactivePciHostdevs, dev);
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org