On Mon, Aug 09, 2010 at 02:46:07PM +0200, Jiri Denemark wrote:
device_del command is not synchronous for PCI devices, it merely
asks
the guest to release the device and returns. If the host wants to use
that device before the guest actually releases it, we are in big
trouble. To avoid this, we already added a loop which waits up to 10
seconds until the device is actually released before we do anything else
with that device. But we only added this loop for managed PCI devices
before we try reattach them back to the host.
However, we need to wait even for non-managed devices. We don't reattach
them automatically, but we still want to prevent the host from using it.
This was revealed thanks to sVirt: when we relabel sysfs files
corresponding to the PCI device before the guest finished releasing the
device, qemu is no longer allowed to access those files and if it wants
(as a result of guest's request) to write anything to them, it just
exits, which kills the guest.
This is not a proper fix and needs some further work both on libvirt and
qemu side in the future.
---
I was thinking about another possibility to implement this since the
wait loop doesn't have to be connected to the reattach action. We could
move that loop into a separate function and let qemudReattach function
do just what its name suggest. But in that case, we would need to add
the call to such function before every call to qemudReattach, which
doesn't look any better to me.
Jirka
src/qemu/qemu_driver.c | 17 +++++++++--------
1 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 57b8271..e4f47d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -3209,16 +3209,17 @@ qemuPrepareChardevDevice(virDomainDefPtr def ATTRIBUTE_UNUSED,
static void
-qemudReattachManagedDevice(pciDevice *dev, struct qemud_driver *driver)
+qemudReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
{
int retries = 100;
+ while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
+ && retries) {
+ usleep(100*1000);
+ retries--;
+ }
+
if (pciDeviceGetManaged(dev)) {
- while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
- && retries) {
- usleep(100*1000);
- retries--;
- }
if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to re-attach PCI device: %s"),
@@ -3264,7 +3265,7 @@ qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
- qemudReattachManagedDevice(dev, driver);
+ qemudReattachPciDevice(dev, driver);
}
pciDeviceListFree(pcidevs);
@@ -8997,7 +8998,7 @@ static int qemudDomainDetachHostPciDevice(struct qemud_driver
*driver,
pciDeviceListDel(driver->activePciHostdevs, pci);
if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0)
ret = -1;
- qemudReattachManagedDevice(pci, driver);
+ qemudReattachPciDevice(pci, driver);
pciFreeDevice(pci);
}
ACK, this is not perfect as you pointed out but without QEmu support
we can't do any better I think.
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/