9:10 a.m.
As we start/shutdown guests, or hotplug/hot-unplug devices, we can add
or delete devices as appropriate from a list of active devices.
Then, in pciReset(), we can use this to determine whether its safe to
reset a device as a side effect of resetting another device.
* src/qemu_conf.h: add activePciHostdevs to qemud_driver
* src/qemu_driver.c: maintain the activePciHostdevs list, and pass it
to pciResetDevice()
* src/pci.[ch]: pass the activeDevs list to pciResetDevice() and use
it to determine whether a Secondary Bus Reset is safe
---
src/pci.c | 102 ++++++++++++++++++++++++++------------------
src/pci.h | 3 +-
src/qemu_conf.h | 3 +
src/qemu_driver.c | 123 ++++++++++++++++++++++++++++++++++++++++++----------
src/xen_unified.c | 2 +-
5 files changed, 165 insertions(+), 68 deletions(-)
diff --git a/src/pci.c b/src/pci.c
index a37eaf7..96e5d6d 100644
--- a/src/pci.c
+++ b/src/pci.c
@@ -226,7 +226,7 @@ pciWrite32(pciDevice *dev, unsigned pos, uint32_t val)
pciWrite(dev, pos, &buf[0], sizeof(buf));
}
-typedef int (*pciIterPredicate)(pciDevice *, pciDevice *);
+typedef int (*pciIterPredicate)(pciDevice *, pciDevice *, void *);
/* Iterate over available PCI devices calling @predicate
* to compare each one to @dev.
@@ -237,7 +237,8 @@ static int
pciIterDevices(virConnectPtr conn,
pciIterPredicate predicate,
pciDevice *dev,
- pciDevice **matched)
+ pciDevice **matched,
+ void *data)
{
DIR *dir;
struct dirent *entry;
@@ -255,7 +256,7 @@ pciIterDevices(virConnectPtr conn,
while ((entry = readdir(dir))) {
unsigned domain, bus, slot, function;
- pciDevice *try;
+ pciDevice *check;
/* Ignore '.' and '..' */
if (entry->d_name[0] == '.')
@@ -267,18 +268,18 @@ pciIterDevices(virConnectPtr conn,
continue;
}
- try = pciGetDevice(conn, domain, bus, slot, function);
- if (!try) {
+ check = pciGetDevice(conn, domain, bus, slot, function);
+ if (!check) {
ret = -1;
break;
}
- if (predicate(try, dev)) {
- VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name,
try->name);
- *matched = try;
+ if (predicate(dev, check, data)) {
+ VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name,
check->name);
+ *matched = check;
break;
}
- pciFreeDevice(conn, try);
+ pciFreeDevice(conn, check);
}
closedir(dir);
return ret;
@@ -380,63 +381,70 @@ pciDetectPowerManagementReset(pciDevice *dev)
return 0;
}
-/* Any devices other than the one supplied on the same domain/bus ? */
+/* Any active devices other than the one supplied on the same domain/bus ? */
static int
-pciSharesBus(pciDevice *a, pciDevice *b)
+pciSharesBusWithActive(pciDevice *dev, pciDevice *check, void *data)
{
- return
- a->domain == b->domain &&
- a->bus == b->bus &&
- (a->slot != b->slot ||
- a->function != b->function);
-}
+ pciDeviceList *activeDevs = data;
-static int
-pciBusContainsOtherDevices(virConnectPtr conn, pciDevice *dev)
-{
- pciDevice *matched = NULL;
- if (pciIterDevices(conn, pciSharesBus, dev, &matched) < 0)
- return 1;
- if (!matched)
+ if (dev->domain != check->domain ||
+ dev->bus != check->bus ||
+ (check->slot == check->slot &&
+ check->function == check->function))
+ return 0;
+
+ if (activeDevs && !pciDeviceListFind(activeDevs, check))
return 0;
- pciFreeDevice(conn, matched);
+
return 1;
}
-/* Is @a the parent of @b ? */
+static pciDevice *
+pciBusContainsActiveDevices(virConnectPtr conn,
+ pciDevice *dev,
+ pciDeviceList *activeDevs)
+{
+ pciDevice *active = NULL;
+ if (pciIterDevices(conn, pciSharesBusWithActive,
+ dev, &active, activeDevs) < 0)
+ return NULL;
+ return active;
+}
+
+/* Is @check the parent of @dev ? */
static int
-pciIsParent(pciDevice *a, pciDevice *b)
+pciIsParent(pciDevice *dev, pciDevice *check, void *data ATTRIBUTE_UNUSED)
{
uint16_t device_class;
uint8_t header_type, secondary, subordinate;
- if (a->domain != b->domain)
+ if (dev->domain != check->domain)
return 0;
/* Is it a bridge? */
- device_class = pciRead16(a, PCI_CLASS_DEVICE);
+ device_class = pciRead16(check, PCI_CLASS_DEVICE);
if (device_class != PCI_CLASS_BRIDGE_PCI)
return 0;
/* Is it a plane? */
- header_type = pciRead8(a, PCI_HEADER_TYPE);
+ header_type = pciRead8(check, PCI_HEADER_TYPE);
if ((header_type & PCI_HEADER_TYPE_MASK) != PCI_HEADER_TYPE_BRIDGE)
return 0;
- secondary = pciRead8(a, PCI_SECONDARY_BUS);
- subordinate = pciRead8(a, PCI_SUBORDINATE_BUS);
+ secondary = pciRead8(check, PCI_SECONDARY_BUS);
+ subordinate = pciRead8(check, PCI_SUBORDINATE_BUS);
- VIR_DEBUG("%s %s: found parent device %s\n", b->id, b->name,
a->name);
+ VIR_DEBUG("%s %s: found parent device %s\n", dev->id, dev->name,
check->name);
/* No, it's superman! */
- return (b->bus >= secondary && b->bus <= subordinate);
+ return (dev->bus >= secondary && dev->bus <= subordinate);
}
static pciDevice *
pciGetParentDevice(virConnectPtr conn, pciDevice *dev)
{
pciDevice *parent = NULL;
- pciIterDevices(conn, pciIsParent, dev, &parent);
+ pciIterDevices(conn, pciIsParent, dev, &parent, NULL);
return parent;
}
@@ -444,9 +452,11 @@ pciGetParentDevice(virConnectPtr conn, pciDevice *dev)
* devices behind a bus.
*/
static int
-pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev)
+pciTrySecondaryBusReset(virConnectPtr conn,
+ pciDevice *dev,
+ pciDeviceList *activeDevs)
{
- pciDevice *parent;
+ pciDevice *parent, *conflict;
uint8_t config_space[PCI_CONF_LEN];
uint16_t ctl;
int ret = -1;
@@ -456,10 +466,10 @@ pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev)
* In future, we could allow it so long as those devices
* are not in use by the host or other guests.
*/
- if (pciBusContainsOtherDevices(conn, dev)) {
+ if ((conflict = pciBusContainsActiveDevices(conn, dev, activeDevs))) {
pciReportError(conn, VIR_ERR_NO_SUPPORT,
- _("Other devices on bus with %s, not doing bus reset"),
- dev->name);
+ _("Active %s devices on bus with %s, not doing bus
reset"),
+ conflict->name, dev->name);
return -1;
}
@@ -573,10 +583,18 @@ pciInitDevice(virConnectPtr conn, pciDevice *dev)
}
int
-pciResetDevice(virConnectPtr conn, pciDevice *dev)
+pciResetDevice(virConnectPtr conn,
+ pciDevice *dev,
+ pciDeviceList *activeDevs)
{
int ret = -1;
+ if (activeDevs && pciDeviceListFind(activeDevs, dev)) {
+ pciReportError(conn, VIR_ERR_INTERNAL_ERROR,
+ _("Not resetting active device %s"), dev->name);
+ return -1;
+ }
+
if (!dev->initted && pciInitDevice(conn, dev) < 0)
return -1;
@@ -595,7 +613,7 @@ pciResetDevice(virConnectPtr conn, pciDevice *dev)
/* Bus reset is not an option with the root bus */
if (ret < 0 && dev->bus != 0)
- ret = pciTrySecondaryBusReset(conn, dev);
+ ret = pciTrySecondaryBusReset(conn, dev, activeDevs);
if (ret < 0) {
virErrorPtr err = virGetLastError();
diff --git a/src/pci.h b/src/pci.h
index 75fbd51..685b0af 100644
--- a/src/pci.h
+++ b/src/pci.h
@@ -44,7 +44,8 @@ int pciDettachDevice (virConnectPtr conn,
int pciReAttachDevice (virConnectPtr conn,
pciDevice *dev);
int pciResetDevice (virConnectPtr conn,
- pciDevice *dev);
+ pciDevice *dev,
+ pciDeviceList *activeDevs);
void pciDeviceSetManaged(pciDevice *dev,
unsigned managed);
unsigned pciDeviceGetManaged(pciDevice *dev);
diff --git a/src/qemu_conf.h b/src/qemu_conf.h
index 8f9e1c1..a126dac 100644
--- a/src/qemu_conf.h
+++ b/src/qemu_conf.h
@@ -35,6 +35,7 @@
#include "threads.h"
#include "security.h"
#include "cgroup.h"
+#include "pci.h"
#define qemudDebug(fmt, ...) do {} while(0)
@@ -111,6 +112,8 @@ struct qemud_driver {
virSecurityDriverPtr securityDriver;
char *saveImageFormat;
+
+ pciDeviceList *activePciHostdevs;
};
diff --git a/src/qemu_driver.c b/src/qemu_driver.c
index f2b807b..70d3d55 100644
--- a/src/qemu_driver.c
+++ b/src/qemu_driver.c
@@ -128,6 +128,9 @@ static int qemudDomainSetMemoryBalloon(virConnectPtr conn,
static int qemudDetectVcpuPIDs(virConnectPtr conn,
virDomainObjPtr vm);
+static int qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
+ virDomainDefPtr def);
+
static struct qemud_driver *qemu_driver = NULL;
static int qemuCgroupControllerActive(struct qemud_driver *driver,
@@ -320,6 +323,10 @@ qemuReconnectDomain(struct qemud_driver *driver,
goto error;
}
+ if (qemuUpdateActivePciHostdevs(driver, obj->def) < 0) {
+ goto error;
+ }
+
if (obj->def->seclabel.type == VIR_DOMAIN_SECLABEL_DYNAMIC &&
driver->securityDriver &&
driver->securityDriver->domainReserveSecurityLabel &&
@@ -524,6 +531,9 @@ qemudStartup(int privileged) {
if ((qemu_driver->caps = qemudCapsInit(NULL)) == NULL)
goto out_of_memory;
+ if ((qemu_driver->activePciHostdevs = pciDeviceListNew(NULL)) == NULL)
+ goto error;
+
if (qemudLoadDriverConfig(qemu_driver, driverConf) < 0) {
goto error;
}
@@ -648,6 +658,7 @@ qemudShutdown(void) {
return -1;
qemuDriverLock(qemu_driver);
+ pciDeviceListFree(NULL, qemu_driver->activePciHostdevs);
virCapabilitiesFree(qemu_driver->caps);
virDomainObjListFree(&qemu_driver->domains);
@@ -1371,7 +1382,38 @@ qemuGetPciHostDeviceList(virConnectPtr conn,
}
static int
-qemuPrepareHostDevices(virConnectPtr conn, virDomainDefPtr def)
+qemuUpdateActivePciHostdevs(struct qemud_driver *driver,
+ virDomainDefPtr def)
+{
+ pciDeviceList *pcidevs;
+ int i, ret;
+
+ if (!def->nhostdevs)
+ return 0;
+
+ if (!(pcidevs = qemuGetPciHostDeviceList(NULL, def)))
+ return -1;
+
+ ret = 0;
+
+ for (i = 0; i < pcidevs->count; i++) {
+ if (pciDeviceListAdd(NULL,
+ driver->activePciHostdevs,
+ pcidevs->devs[i]) < 0) {
+ ret = -1;
+ break;
+ }
+ pcidevs->devs[i] = NULL;
+ }
+
+ pciDeviceListFree(NULL, pcidevs);
+ return ret;
+}
+
+static int
+qemuPrepareHostDevices(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainDefPtr def)
{
pciDeviceList *pcidevs;
int i;
@@ -1382,10 +1424,11 @@ qemuPrepareHostDevices(virConnectPtr conn, virDomainDefPtr def)
if (!(pcidevs = qemuGetPciHostDeviceList(conn, def)))
return -1;
- /* We have to use 2 loops here. *All* devices must
+ /* We have to use 3 loops here. *All* devices must
* be detached before we reset any of them, because
* in some cases you have to reset the whole PCI,
- * which impacts all devices on it
+ * which impacts all devices on it. Also, all devices
+ * must be reset before being marked as active.
*/
/* XXX validate that non-managed device isn't in use, eg
@@ -1401,9 +1444,19 @@ qemuPrepareHostDevices(virConnectPtr conn, virDomainDefPtr def)
/* Now that all the PCI hostdevs have be dettached, we can safely
* reset them */
for (i = 0; i < pcidevs->count; i++)
- if (pciResetDevice(conn, pcidevs->devs[i]) < 0)
+ if (pciResetDevice(conn, pcidevs->devs[i],
+ driver->activePciHostdevs) < 0)
goto error;
+ /* Now mark all the devices as active */
+ for (i = 0; i < pcidevs->count; i++) {
+ if (pciDeviceListAdd(conn,
+ driver->activePciHostdevs,
+ pcidevs->devs[i]) < 0)
+ goto error;
+ pcidevs->devs[i] = NULL;
+ }
+
pciDeviceListFree(conn, pcidevs);
return 0;
@@ -1413,7 +1466,9 @@ error:
}
static void
-qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr def)
+qemuDomainReAttachHostDevices(virConnectPtr conn,
+ struct qemud_driver *driver,
+ virDomainDefPtr def)
{
pciDeviceList *pcidevs;
int i;
@@ -1429,10 +1484,15 @@ qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr
def)
return;
}
- /* Again 2 loops; reset all the devices before re-attach */
+ /* Again 3 loops; mark all devices as inactive before reset
+ * them and reset all the devices before re-attach */
for (i = 0; i < pcidevs->count; i++)
- if (pciResetDevice(conn, pcidevs->devs[i]) < 0) {
+ pciDeviceListDel(conn, driver->activePciHostdevs, pcidevs->devs[i]);
+
+ for (i = 0; i < pcidevs->count; i++)
+ if (pciResetDevice(conn, pcidevs->devs[i],
+ driver->activePciHostdevs) < 0) {
virErrorPtr err = virGetLastError();
VIR_ERROR(_("Failed to reset PCI device: %s\n"),
err ? err->message : "");
@@ -1976,7 +2036,7 @@ static int qemudStartVMDaemon(virConnectPtr conn,
if (qemuSetupCgroup(conn, driver, vm) < 0)
goto cleanup;
- if (qemuPrepareHostDevices(conn, vm->def) < 0)
+ if (qemuPrepareHostDevices(conn, driver, vm->def) < 0)
goto cleanup;
if (VIR_ALLOC(vm->monitor_chr) < 0) {
@@ -2158,7 +2218,7 @@ static void qemudShutdownVMDaemon(virConnectPtr conn,
VIR_WARN("Failed to restore all device ownership for %s",
vm->def->name);
- qemuDomainReAttachHostDevices(conn, vm->def);
+ qemuDomainReAttachHostDevices(conn, driver, vm->def);
retry:
if ((ret = qemuRemoveCgroup(conn, driver, vm)) < 0) {
@@ -5279,6 +5339,7 @@ cleanup:
}
static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -5301,42 +5362,42 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
return -1;
if ((hostdev->managed && pciDettachDevice(conn, pci) < 0) ||
- pciResetDevice(conn, pci) < 0) {
+ pciResetDevice(conn, pci, driver->activePciHostdevs) < 0) {
+ pciFreeDevice(conn, pci);
+ return -1;
+ }
+
+ if (pciDeviceListAdd(conn, driver->activePciHostdevs, pci) < 0) {
pciFreeDevice(conn, pci);
return -1;
}
- pciFreeDevice(conn, pci);
+ cmd = reply = NULL;
if (virAsprintf(&cmd, "pci_add pci_addr=auto host
host=%.2x:%.2x.%.1x",
hostdev->source.subsys.u.pci.bus,
hostdev->source.subsys.u.pci.slot,
hostdev->source.subsys.u.pci.function) < 0) {
virReportOOMError(conn);
- return -1;
+ goto error;
}
if (qemudMonitorCommand(vm, cmd, &reply) < 0) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
"%s", _("cannot attach host pci device"));
- VIR_FREE(cmd);
- return -1;
+ goto error;
}
if (strstr(reply, "invalid type: host")) {
qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT, "%s",
_("PCI device assignment is not supported by this version
of qemu"));
- VIR_FREE(cmd);
- VIR_FREE(reply);
- return -1;
+ goto error;
}
if (qemudParsePciAddReply(vm, reply, &domain, &bus, &slot) < 0) {
qemudReportError(conn, dom, NULL, VIR_ERR_OPERATION_FAILED,
_("parsing pci_add reply failed: %s"), reply);
- VIR_FREE(cmd);
- VIR_FREE(reply);
- return -1;
+ goto error;
}
hostdev->source.subsys.u.pci.guest_addr.domain = domain;
@@ -5349,6 +5410,14 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn,
VIR_FREE(cmd);
return 0;
+
+error:
+ pciDeviceListDel(conn, driver->activePciHostdevs, pci);
+
+ VIR_FREE(reply);
+ VIR_FREE(cmd);
+
+ return -1;
}
static int qemudDomainAttachHostUsbDevice(virConnectPtr conn,
@@ -5422,7 +5491,7 @@ static int qemudDomainAttachHostDevice(virConnectPtr conn,
switch (hostdev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
- return qemudDomainAttachHostPciDevice(conn, vm, dev);
+ return qemudDomainAttachHostPciDevice(conn, driver, vm, dev);
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB:
return qemudDomainAttachHostUsbDevice(conn, vm, dev);
default:
@@ -5749,6 +5818,7 @@ cleanup:
}
static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
+ struct qemud_driver *driver,
virDomainObjPtr vm,
virDomainDeviceDefPtr dev)
{
@@ -5832,7 +5902,8 @@ static int qemudDomainDetachHostPciDevice(virConnectPtr conn,
if (!pci)
ret = -1;
else {
- if (pciResetDevice(conn, pci) < 0)
+ pciDeviceListDel(conn, driver->activePciHostdevs, pci);
+ if (pciResetDevice(conn, pci, driver->activePciHostdevs) < 0)
ret = -1;
if (detach->managed && pciReAttachDevice(conn, pci) < 0)
ret = -1;
@@ -5868,7 +5939,7 @@ static int qemudDomainDetachHostDevice(virConnectPtr conn,
switch (hostdev->source.subsys.type) {
case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI:
- ret = qemudDomainDetachHostPciDevice(conn, vm, dev);
+ ret = qemudDomainDetachHostPciDevice(conn, driver, vm, dev);
break;
default:
qemudReportError(conn, dom, NULL, VIR_ERR_NO_SUPPORT,
@@ -7092,6 +7163,7 @@ out:
static int
qemudNodeDeviceReset (virNodeDevicePtr dev)
{
+ struct qemud_driver *driver = dev->conn->privateData;
pciDevice *pci;
unsigned domain, bus, slot, function;
int ret = -1;
@@ -7103,11 +7175,14 @@ qemudNodeDeviceReset (virNodeDevicePtr dev)
if (!pci)
return -1;
- if (pciResetDevice(dev->conn, pci) < 0)
+ qemuDriverLock(driver);
+
+ if (pciResetDevice(dev->conn, pci, driver->activePciHostdevs) < 0)
goto out;
ret = 0;
out:
+ qemuDriverUnlock(driver);
pciFreeDevice(dev->conn, pci);
return ret;
}
diff --git a/src/xen_unified.c b/src/xen_unified.c
index f2ffc25..dfa9ca5 100644
--- a/src/xen_unified.c
+++ b/src/xen_unified.c
@@ -1641,7 +1641,7 @@ xenUnifiedNodeDeviceReset (virNodeDevicePtr dev)
if (!pci)
return -1;
- if (pciResetDevice(dev->conn, pci) < 0)
+ if (pciResetDevice(dev->conn, pci, NULL) < 0)
goto out;
ret = 0;
--
1.6.2.5