qemu: Introduce inactive PCI device list

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 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 introducing an inactive
PCI device list (just like qemu_driver->activePciHostdevs), and
the whole logic is:

  * Add the device to inactive list during nodedev-dettach
  * Remove the device from inactive list during nodedev-reattach
  * Remove the device from inactive list during attach-device
    (for non-managed device)
  * Add the device to inactive list after detach-device, only
    if the device is not managed

With the above, we have a 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;
This commit is contained in:
Osier Yang 2012-01-18 04:02:05 +08:00 committed by Eric Blake
parent a06710758c
commit 6be610bfaa
7 changed files with 77 additions and 24 deletions

View File

@ -1,7 +1,7 @@
/* /*
* qemu_conf.h: QEMU configuration management * 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 * Copyright (C) 2006 Daniel P. Berrange
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
@ -128,6 +128,9 @@ struct qemud_driver {
pciDeviceList *activePciHostdevs; pciDeviceList *activePciHostdevs;
usbDeviceList *activeUsbHostdevs; usbDeviceList *activeUsbHostdevs;
/* The devices which is are not in use by the host or any guest. */
pciDeviceList *inactivePciHostdevs;
virBitmapPtr reservedVNCPorts; virBitmapPtr reservedVNCPorts;
virSysinfoDefPtr hostsysinfo; virSysinfoDefPtr hostsysinfo;

View File

@ -588,6 +588,9 @@ qemudStartup(int privileged) {
if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL) if ((qemu_driver->activeUsbHostdevs = usbDeviceListNew()) == NULL)
goto error; goto error;
if ((qemu_driver->inactivePciHostdevs = pciDeviceListNew()) == NULL)
goto error;
if (privileged) { if (privileged) {
if (chown(qemu_driver->libDir, qemu_driver->user, qemu_driver->group) < 0) { if (chown(qemu_driver->libDir, qemu_driver->user, qemu_driver->group) < 0) {
virReportSystemError(errno, virReportSystemError(errno,
@ -778,6 +781,7 @@ qemudShutdown(void) {
qemuDriverLock(qemu_driver); qemuDriverLock(qemu_driver);
pciDeviceListFree(qemu_driver->activePciHostdevs); pciDeviceListFree(qemu_driver->activePciHostdevs);
pciDeviceListFree(qemu_driver->inactivePciHostdevs);
usbDeviceListFree(qemu_driver->activeUsbHostdevs); usbDeviceListFree(qemu_driver->activeUsbHostdevs);
virCapabilitiesFree(qemu_driver->caps); virCapabilitiesFree(qemu_driver->caps);
@ -9211,6 +9215,7 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev)
pciDevice *pci; pciDevice *pci;
unsigned domain, bus, slot, function; unsigned domain, bus, slot, function;
int ret = -1; int ret = -1;
bool in_inactive_list = false;
if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0) if (qemudNodeDeviceGetPciInfo(dev, &domain, &bus, &slot, &function) < 0)
return -1; return -1;
@ -9220,13 +9225,17 @@ qemudNodeDeviceDettach (virNodeDevicePtr dev)
return -1; return -1;
qemuDriverLock(driver); qemuDriverLock(driver);
if (pciDettachDevice(pci, driver->activePciHostdevs) < 0) in_inactive_list = pciDeviceListFind(driver->inactivePciHostdevs, pci);
if (pciDettachDevice(pci, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
goto out; goto out;
ret = 0; ret = 0;
out: out:
qemuDriverUnlock(driver); qemuDriverUnlock(driver);
pciFreeDevice(pci); if (in_inactive_list)
pciFreeDevice(pci);
return ret; return ret;
} }
@ -9248,7 +9257,8 @@ qemudNodeDeviceReAttach (virNodeDevicePtr dev)
pciDeviceReAttachInit(pci); pciDeviceReAttachInit(pci);
qemuDriverLock(driver); qemuDriverLock(driver);
if (pciReAttachDevice(pci, driver->activePciHostdevs) < 0) if (pciReAttachDevice(pci, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
goto out; goto out;
ret = 0; ret = 0;
@ -9275,7 +9285,8 @@ qemudNodeDeviceReset (virNodeDevicePtr dev)
qemuDriverLock(driver); qemuDriverLock(driver);
if (pciResetDevice(pci, driver->activePciHostdevs, NULL) < 0) if (pciResetDevice(pci, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
goto out; goto out;
ret = 0; ret = 0;

View File

@ -1,7 +1,7 @@
/* /*
* qemu_hostdev.c: QEMU hostdev management * 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 * Copyright (C) 2006 Daniel P. Berrange
* *
* This library is free software; you can redistribute it and/or * This library is free software; you can redistribute it and/or
@ -212,7 +212,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDevice *dev = pciDeviceListGet(pcidevs, i);
if (pciDeviceGetManaged(dev) && if (pciDeviceGetManaged(dev) &&
pciDettachDevice(dev, driver->activePciHostdevs) < 0) pciDettachDevice(dev, driver->activePciHostdevs, NULL) < 0)
goto reattachdevs; goto reattachdevs;
} }
@ -220,7 +220,8 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
* can safely reset them */ * can safely reset them */
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDevice *dev = pciDeviceListGet(pcidevs, i);
if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) if (pciResetDevice(dev, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0)
goto reattachdevs; goto reattachdevs;
} }
@ -233,7 +234,13 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
} }
} }
/* Loop 5: Now set the used_by_domain of the device in /* Loop 5: Now remove the devices from inactive list. */
for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i);
pciDeviceListDel(driver->inactivePciHostdevs, dev);
}
/* Loop 6: Now set the used_by_domain of the device in
* driver->activePciHostdevs as domain name. * driver->activePciHostdevs as domain name.
*/ */
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
@ -245,7 +252,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
pciDeviceSetUsedBy(activeDev, name); pciDeviceSetUsedBy(activeDev, name);
} }
/* Loop 6: Now set the original states for hostdev def */ /* Loop 7: Now set the original states for hostdev def */
for (i = 0; i < nhostdevs; i++) { for (i = 0; i < nhostdevs; i++) {
pciDevice *dev; pciDevice *dev;
pciDevice *pcidev; pciDevice *pcidev;
@ -277,7 +284,7 @@ int qemuPrepareHostdevPCIDevices(struct qemud_driver *driver,
pciFreeDevice(dev); pciFreeDevice(dev);
} }
/* Loop 7: Now steal all the devices from pcidevs */ /* Loop 8: Now steal all the devices from pcidevs */
while (pciDeviceListCount(pcidevs) > 0) { while (pciDeviceListCount(pcidevs) > 0) {
pciDevice *dev = pciDeviceListGet(pcidevs, 0); pciDevice *dev = pciDeviceListGet(pcidevs, 0);
pciDeviceListSteal(pcidevs, dev); pciDeviceListSteal(pcidevs, dev);
@ -298,7 +305,7 @@ inactivedevs:
reattachdevs: reattachdevs:
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDevice *dev = pciDeviceListGet(pcidevs, i);
pciReAttachDevice(dev, driver->activePciHostdevs); pciReAttachDevice(dev, driver->activePciHostdevs, NULL);
} }
cleanup: cleanup:
@ -445,8 +452,13 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
{ {
int retries = 100; int retries = 100;
if (!pciDeviceGetManaged(dev)) /* If the device is not managed and was attached to guest
* successfully, it must have been inactive.
*/
if (!pciDeviceGetManaged(dev)) {
pciDeviceListAdd(driver->inactivePciHostdevs, dev);
return; return;
}
while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device") while (pciWaitForDeviceCleanup(dev, "kvm_assigned_device")
&& retries) { && retries) {
@ -454,7 +466,8 @@ void qemuReattachPciDevice(pciDevice *dev, struct qemud_driver *driver)
retries--; retries--;
} }
if (pciReAttachDevice(dev, driver->activePciHostdevs) < 0) { if (pciReAttachDevice(dev, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0) {
virErrorPtr err = virGetLastError(); virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to re-attach PCI device: %s"), VIR_ERROR(_("Failed to re-attach PCI device: %s"),
err ? err->message : _("unknown error")); err ? err->message : _("unknown error"));
@ -503,7 +516,8 @@ void qemuDomainReAttachHostdevDevices(struct qemud_driver *driver,
for (i = 0; i < pciDeviceListCount(pcidevs); i++) { for (i = 0; i < pciDeviceListCount(pcidevs); i++) {
pciDevice *dev = pciDeviceListGet(pcidevs, i); pciDevice *dev = pciDeviceListGet(pcidevs, i);
if (pciResetDevice(dev, driver->activePciHostdevs, pcidevs) < 0) { if (pciResetDevice(dev, driver->activePciHostdevs,
driver->inactivePciHostdevs) < 0) {
virErrorPtr err = virGetLastError(); virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to reset PCI device: %s"), VIR_ERROR(_("Failed to reset PCI device: %s"),
err ? err->message : _("unknown error")); err ? err->message : _("unknown error"));

View File

@ -2029,7 +2029,8 @@ qemuDomainDetachHostPciDevice(struct qemud_driver *driver,
detach->source.subsys.u.pci.function); detach->source.subsys.u.pci.function);
if (pci) { if (pci) {
activePci = pciDeviceListSteal(driver->activePciHostdevs, pci); activePci = pciDeviceListSteal(driver->activePciHostdevs, pci);
if (pciResetDevice(activePci, driver->activePciHostdevs, NULL)) if (pciResetDevice(activePci, driver->activePciHostdevs,
driver->inactivePciHostdevs) == 0)
qemuReattachPciDevice(activePci, driver); qemuReattachPciDevice(activePci, driver);
else else
ret = -1; ret = -1;

View File

@ -1117,7 +1117,9 @@ cleanup:
} }
int int
pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs) pciDettachDevice(pciDevice *dev,
pciDeviceList *activeDevs,
pciDeviceList *inactiveDevs)
{ {
const char *driver = pciFindStubDriver(); const char *driver = pciFindStubDriver();
if (!driver) { if (!driver) {
@ -1132,11 +1134,22 @@ pciDettachDevice(pciDevice *dev, pciDeviceList *activeDevs)
return -1; return -1;
} }
return pciBindDeviceToStub(dev, driver); if (pciBindDeviceToStub(dev, driver) < 0)
return -1;
/* Add the dev into list inactiveDevs */
if (inactiveDevs && !pciDeviceListFind(inactiveDevs, dev)) {
if (pciDeviceListAdd(inactiveDevs, dev) < 0)
return -1;
}
return 0;
} }
int int
pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs) pciReAttachDevice(pciDevice *dev,
pciDeviceList *activeDevs,
pciDeviceList *inactiveDevs)
{ {
const char *driver = pciFindStubDriver(); const char *driver = pciFindStubDriver();
if (!driver) { if (!driver) {
@ -1151,7 +1164,14 @@ pciReAttachDevice(pciDevice *dev, pciDeviceList *activeDevs)
return -1; return -1;
} }
return pciUnbindDeviceFromStub(dev, driver); if (pciUnbindDeviceFromStub(dev, driver) < 0)
return -1;
/* Steal the dev from list inactiveDevs */
if (inactiveDevs)
pciDeviceListSteal(inactiveDevs, dev);
return 0;
} }
/* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on /* Certain hypervisors (like qemu/kvm) map the PCI bar(s) on

View File

@ -40,8 +40,12 @@ pciDevice *pciGetDevice (unsigned domain,
unsigned function); unsigned function);
void pciFreeDevice (pciDevice *dev); void pciFreeDevice (pciDevice *dev);
const char *pciDeviceGetName (pciDevice *dev); const char *pciDeviceGetName (pciDevice *dev);
int pciDettachDevice (pciDevice *dev, pciDeviceList *activeDevs); int pciDettachDevice (pciDevice *dev,
int pciReAttachDevice (pciDevice *dev, pciDeviceList *activeDevs); pciDeviceList *activeDevs,
pciDeviceList *inactiveDevs);
int pciReAttachDevice (pciDevice *dev,
pciDeviceList *activeDevs,
pciDeviceList *inactiveDevs);
int pciResetDevice (pciDevice *dev, int pciResetDevice (pciDevice *dev,
pciDeviceList *activeDevs, pciDeviceList *activeDevs,
pciDeviceList *inactiveDevs); pciDeviceList *inactiveDevs);

View File

@ -1985,7 +1985,7 @@ xenUnifiedNodeDeviceDettach (virNodeDevicePtr dev)
if (!pci) if (!pci)
return -1; return -1;
if (pciDettachDevice(pci, NULL) < 0) if (pciDettachDevice(pci, NULL, NULL) < 0)
goto out; goto out;
ret = 0; ret = 0;
@ -2075,7 +2075,7 @@ xenUnifiedNodeDeviceReAttach (virNodeDevicePtr dev)
goto out; goto out;
} }
if (pciReAttachDevice(pci, NULL) < 0) if (pciReAttachDevice(pci, NULL, NULL) < 0)
goto out; goto out;
ret = 0; ret = 0;