diff --git a/src/pci.c b/src/pci.c index 74f7ef071fe9fee1c7bd8c52859b52e16024f6e4..6a2e860007e46e8891046f173ebfb34aac4b23eb 100644 --- a/src/pci.c +++ b/src/pci.c @@ -225,7 +225,11 @@ pciWrite32(pciDevice *dev, unsigned pos, uint32_t val) pciWrite(dev, pos, &buf[0], sizeof(buf)); } -typedef int (*pciIterPredicate)(pciDevice *, pciDevice *); +typedef int (*pciIterPredicate)(virConnectPtr, + virDomainObjPtr, + pciDevice *, + pciResetCheckFunc, + pciDevice *); /* Iterate over available PCI devices calling @predicate * to compare each one to @dev. @@ -234,8 +238,10 @@ typedef int (*pciIterPredicate)(pciDevice *, pciDevice *); */ static int pciIterDevices(virConnectPtr conn, + virDomainObjPtr vm, pciIterPredicate predicate, pciDevice *dev, + pciResetCheckFunc check, pciDevice **matched) { DIR *dir; @@ -254,7 +260,7 @@ pciIterDevices(virConnectPtr conn, while ((entry = readdir(dir))) { unsigned domain, bus, slot, function; - pciDevice *try; + pciDevice *check_dev; /* Ignore '.' and '..' */ if (entry->d_name[0] == '.') @@ -266,18 +272,18 @@ pciIterDevices(virConnectPtr conn, continue; } - try = pciGetDevice(conn, domain, bus, slot, function); - if (!try) { + check_dev = pciGetDevice(conn, domain, bus, slot, function); + if (!check_dev) { 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(conn, vm, dev, check, check_dev)) { + VIR_DEBUG("%s %s: iter matched on %s", dev->id, dev->name, check_dev->name); + *matched = check_dev; break; } - pciFreeDevice(conn, try); + pciFreeDevice(conn, check_dev); } closedir(dir); return ret; @@ -379,63 +385,73 @@ pciDetectPowerManagementReset(pciDevice *dev) return 0; } -/* Any devices other than the one supplied on the same domain/bus ? */ +/* Check any devices other than the one supplied on the same domain/bus */ static int -pciSharesBus(pciDevice *a, pciDevice *b) +pciCheckSharesBus(virConnectPtr conn, + virDomainObjPtr vm, + pciDevice *dev, + pciResetCheckFunc check, + pciDevice *check_dev) { - return - a->domain == b->domain && - a->bus == b->bus && - (a->slot != b->slot || - a->function != b->function); + if (check_dev->domain != dev->domain || check_dev->bus != dev->bus) + return 0; + if (check_dev->slot == dev->slot && check_dev->function == dev->function) + return 0; + if (check(conn, vm, check_dev)) + return 0; + return 1; } -static int -pciBusContainsOtherDevices(virConnectPtr conn, pciDevice *dev) +static pciDevice * +pciBusCheckOtherDevices(virConnectPtr conn, + virDomainObjPtr vm, + pciDevice *dev, + pciResetCheckFunc check) { - pciDevice *matched = NULL; - if (pciIterDevices(conn, pciSharesBus, dev, &matched) < 0) - return 1; - if (!matched) - return 0; - pciFreeDevice(conn, matched); - return 1; + pciDevice *conflict = NULL; + pciIterDevices(conn, vm, pciCheckSharesBus, dev, check, &conflict); + return conflict; } -/* Is @a the parent of @b ? */ +/* Is @check_dev the parent of @dev ? */ static int -pciIsParent(pciDevice *a, pciDevice *b) +pciIsParent(virConnectPtr conn ATTRIBUTE_UNUSED, + virDomainObjPtr vm ATTRIBUTE_UNUSED, + pciDevice *dev, + pciResetCheckFunc check ATTRIBUTE_UNUSED, + pciDevice *check_dev) { uint16_t device_class; uint8_t header_type, secondary, subordinate; - if (a->domain != b->domain) + if (check_dev->domain != dev->domain) return 0; /* Is it a bridge? */ - device_class = pciRead16(a, PCI_CLASS_DEVICE); + device_class = pciRead16(check_dev, 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_dev, 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_dev, PCI_SECONDARY_BUS); + subordinate = pciRead8(check_dev, 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_dev->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, NULL, pciIsParent, dev, NULL, &parent); return parent; } @@ -443,9 +459,12 @@ pciGetParentDevice(virConnectPtr conn, pciDevice *dev) * devices behind a bus. */ static int -pciTrySecondaryBusReset(virConnectPtr conn, pciDevice *dev) +pciTrySecondaryBusReset(virConnectPtr conn, + virDomainObjPtr vm, + pciDevice *dev, + pciResetCheckFunc check) { - pciDevice *parent; + pciDevice *parent, *conflict; uint8_t config_space[PCI_CONF_LEN]; uint16_t ctl; int ret = -1; @@ -455,10 +474,11 @@ 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 = pciBusCheckOtherDevices(conn, vm, dev, check))) { pciReportError(conn, VIR_ERR_NO_SUPPORT, - _("Other devices on bus with %s, not doing bus reset"), - dev->name); + _("Unable to reset %s using bus reset as this would cause %s to be reset"), + dev->name, conflict->name); + pciFreeDevice(conn, conflict); return -1; } @@ -572,13 +592,24 @@ pciInitDevice(virConnectPtr conn, pciDevice *dev) } int -pciResetDevice(virConnectPtr conn, pciDevice *dev) +pciResetDevice(virConnectPtr conn, + virDomainObjPtr vm, + pciDevice *dev, + pciResetCheckFunc check) { int ret = -1; if (!dev->initted && pciInitDevice(conn, dev) < 0) return -1; + /* Check that the device isn't owned by a running VM */ + if (!check(conn, vm, dev)) { + pciReportError(conn, VIR_ERR_NO_SUPPORT, + _("Unable to reset PCI device %s: device is in use"), + dev->name); + return -1; + } + /* KVM will perform FLR when starting and stopping * a guest, so there is no need for us to do it here. */ @@ -594,7 +625,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, vm, dev, check); if (ret < 0) { virErrorPtr err = virGetLastError(); diff --git a/src/pci.h b/src/pci.h index 47882efb4d21f8d48a2e160ef69fbd2867229a5f..15da057f167471a7659fb4ce9d48a4fe59516452 100644 --- a/src/pci.h +++ b/src/pci.h @@ -24,6 +24,7 @@ #include #include "internal.h" +#include "domain_conf.h" typedef struct _pciDevice pciDevice; @@ -38,7 +39,17 @@ int pciDettachDevice (virConnectPtr conn, pciDevice *dev); int pciReAttachDevice (virConnectPtr conn, pciDevice *dev); -int pciResetDevice (virConnectPtr conn, - pciDevice *dev); + +/* pciResetDevice() may cause other devices to be reset; + * The check function is called for each other device to + * be reset and by returning zero may cause the reset to + * fail if it is not safe to reset the supplied device. + */ +typedef int (*pciResetCheckFunc)(virConnectPtr, virDomainObjPtr, pciDevice *); + +int pciResetDevice(virConnectPtr conn, + virDomainObjPtr vm, + pciDevice *dev, + pciResetCheckFunc check); #endif /* __VIR_PCI_H__ */ diff --git a/src/qemu_driver.c b/src/qemu_driver.c index 6d1ec0663065071f98993268dd0d4c2f51f6914f..bfa06a55d0408717433ecb94e7f3716a17918306 100644 --- a/src/qemu_driver.c +++ b/src/qemu_driver.c @@ -1329,8 +1329,10 @@ static int qemudNextFreeVNCPort(struct qemud_driver *driver ATTRIBUTE_UNUSED) { return -1; } -static int qemuPrepareHostDevices(virConnectPtr conn, - virDomainDefPtr def) { +static int +qemuPrepareHostDevices(virConnectPtr conn, virDomainObjPtr vm) +{ + virDomainDefPtr def = vm->def; int i; /* We have to use 2 loops here. *All* devices must @@ -1388,7 +1390,7 @@ static int qemuPrepareHostDevices(virConnectPtr conn, if (!dev) goto error; - if (pciResetDevice(conn, dev) < 0) { + if (pciResetDevice(conn, vm, dev, NULL) < 0) { pciFreeDevice(conn, dev); goto error; } @@ -1403,8 +1405,9 @@ error: } static void -qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr def) +qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainObjPtr vm) { + virDomainDefPtr def = vm->def; int i; /* Again 2 loops; reset all the devices before re-attach */ @@ -1431,7 +1434,7 @@ qemuDomainReAttachHostDevices(virConnectPtr conn, virDomainDefPtr def) continue; } - if (pciResetDevice(conn, dev) < 0) { + if (pciResetDevice(conn, vm, dev, NULL) < 0) { virErrorPtr err = virGetLastError(); VIR_ERROR(_("Failed to reset PCI device: %s\n"), err ? err->message : ""); @@ -2001,7 +2004,7 @@ static int qemudStartVMDaemon(virConnectPtr conn, if (qemuSetupCgroup(conn, driver, vm) < 0) goto cleanup; - if (qemuPrepareHostDevices(conn, vm->def) < 0) + if (qemuPrepareHostDevices(conn, vm) < 0) goto cleanup; if (VIR_ALLOC(vm->monitor_chr) < 0) { @@ -2183,7 +2186,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, vm); retry: if ((ret = qemuRemoveCgroup(conn, driver, vm)) < 0) { @@ -5247,7 +5250,7 @@ static int qemudDomainAttachHostPciDevice(virConnectPtr conn, return -1; if (pciDettachDevice(conn, pci) < 0 || - pciResetDevice(conn, pci) < 0) { + pciResetDevice(conn, vm, pci, NULL) < 0) { pciFreeDevice(conn, pci); return -1; } @@ -7049,7 +7052,7 @@ qemudNodeDeviceReset (virNodeDevicePtr dev) if (!pci) return -1; - if (pciResetDevice(dev->conn, pci) < 0) + if (pciResetDevice(dev->conn, NULL, pci, NULL) < 0) goto out; ret = 0; diff --git a/src/xen_unified.c b/src/xen_unified.c index f2ffc2512d80eaf9828c273605cf33cf42eb06ba..dd8f10b161cf808d15b20f7c5dcef6294c3e2e58 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, NULL, pci, NULL) < 0) goto out; ret = 0;