From abb7a4bd6bb416c55c8bf664c2cb9a5cec411503 Mon Sep 17 00:00:00 2001 From: Laine Stump Date: Wed, 7 Sep 2016 12:29:30 -0400 Subject: [PATCH] qemu: set/use proper pciConnectFlags during hotplug Before now, all the qemu hotplug functions assumed that all devices to be hotplugged were legacy PCI endpoint devices (VIR_PCI_CONNECT_TYPE_PCI_DEVICE). This worked out "okay", because all devices *are* legacy PCI endpoint devices on x86/440fx machinetypes, and hotplug didn't work properly on machinetypes using PCIe anyway (hotplugging onto a legacy PCI slot doesn't work, and until commit b87703cf any attempt to manually specify a PCIe address for a hotplugged device would be erroneously rejected). This patch makes all qemu hotplug operations honor the pciConnectFlags set by the single all-knowing function qemuDomainDeviceCalculatePCIConnectFlags(). This is done in 3 steps, but in a single commit since we would have to touch the other points at each step anyway: 1) add a flags argument to the hypervisor-agnostic virDomainPCIAddressEnsureAddr() (previously it hardcoded ..._PCI_DEVICE) 2) add a new qemu-specific function qemuDomainEnsurePCIAddress() which gets the correct pciConnectFlags for the device from qemuDomainDeviceConnectFlags(), then calls virDomainPCIAddressEnsureAddr(). 3) in qemu_hotplug.c replace all calls to virDomainPCIAddressEnsureAddr() with calls to qemuDomainEnsurePCIAddress() So in effect, we're putting a "shim" on top of all calls to virDomainPCIAddressEnsureAddr() that sets the right pciConnectFlags. --- src/conf/domain_addr.c | 10 ++-------- src/conf/domain_addr.h | 3 ++- src/qemu/qemu_domain_address.c | 30 ++++++++++++++++++++++++++++++ src/qemu/qemu_domain_address.h | 4 ++++ src/qemu/qemu_hotplug.c | 23 ++++++++++++++++------- 5 files changed, 54 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 2af9c646c2..0346471ea4 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -487,17 +487,11 @@ virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) { int ret = -1; char *addrStr = NULL; - /* Flags should be set according to the particular device, - * but only the caller knows the type of device. Currently this - * function is only used for hot-plug, though, and hot-plug is - * only supported for standard PCI devices, so we can safely use - * the setting below */ - virDomainPCIConnectFlags flags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE); if (!(addrStr = virDomainPCIAddressAsString(&dev->addr.pci))) goto cleanup; diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index ff80fce6fe..bcec2c7e48 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -144,7 +144,8 @@ int virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs, ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, - virDomainDeviceInfoPtr dev) + virDomainDeviceInfoPtr dev, + virDomainPCIConnectFlags flags) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index d16251303c..4cdff7a57d 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -2230,6 +2230,36 @@ qemuDomainAssignAddresses(virDomainDefPtr def, return 0; } +/** + * qemuDomainEnsurePCIAddress: + * + * @obj: the virDomainObjPtr for the domain. This will include + * qemuCaps and address cache (if there is one) + * + * @dev: the device that we need to ensure has a PCI address + * + * if @dev should have a PCI address but doesn't, assign an address on + * a compatible PCI bus, and set it in @dev->...info. If there is an + * address already, validate that it is on a compatible bus, based on + * @dev->...info.pciConnectFlags. + * + * returns 0 on success -1 on failure. + */ +int +qemuDomainEnsurePCIAddress(virDomainObjPtr obj, + virDomainDeviceDefPtr dev) +{ + qemuDomainObjPrivatePtr priv = obj->privateData; + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + + if (!info) + return 0; + + qemuDomainFillDevicePCIConnectFlags(obj->def, dev, priv->qemuCaps); + + return virDomainPCIAddressEnsureAddr(priv->pciaddrs, info, + info->pciConnectFlags); +} void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index ecb92b5400..e532bd8522 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -37,6 +37,10 @@ int qemuDomainAssignAddresses(virDomainDefPtr def, bool newDomain) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int qemuDomainEnsurePCIAddress(virDomainObjPtr obj, + virDomainDeviceDefPtr dev) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, virDomainDeviceInfoPtr info, const char *devstr); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 0dcbee62c7..6b3a068200 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -304,6 +304,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, int ret = -1; int rv; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_DISK, { .disk = disk } }; virErrorPtr orig_err; char *devstr = NULL; char *drivestr = NULL; @@ -344,7 +345,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, goto error; } else if (!disk->info.type || disk->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &disk->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto error; } releaseaddr = true; @@ -462,6 +463,8 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, const char* type = virDomainControllerTypeToString(controller->type); char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_CONTROLLER, + { .controller = controller } }; virDomainCCWAddressSetPtr ccwaddrs = NULL; bool releaseaddr = false; @@ -501,7 +504,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &controller->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto cleanup; } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -935,6 +938,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainNetDefPtr net) { qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_NET, { .net = net } }; virErrorPtr originalError = NULL; char **tapfdName = NULL; int *tapfd = NULL; @@ -1133,7 +1137,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("virtio-s390 net device cannot be hotplugged.")); goto cleanup; - } else if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &net->info) < 0) { + } else if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) { goto cleanup; } @@ -1365,6 +1369,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, virDomainHostdevDefPtr hostdev) { qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_HOSTDEV, + { .hostdev = hostdev } }; int ret; char *devstr = NULL; int configfd = -1; @@ -1435,7 +1441,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto error; - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto error; releaseaddr = true; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO && @@ -1771,6 +1777,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm, { virDomainDefPtr def = vm->def; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_CHR, { .chr = chr } }; if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CONSOLE && chr->targetType == VIR_DOMAIN_CHR_CONSOLE_TARGET_TYPE_VIRTIO) { @@ -1780,7 +1787,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainObjPtr vm, } else if (chr->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_SERIAL && chr->targetType == VIR_DOMAIN_CHR_SERIAL_TARGET_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &chr->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) return -1; return 1; @@ -1936,6 +1943,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, { virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_RNG, { .rng = rng } }; virErrorPtr orig_err; char *devstr = NULL; char *charAlias = NULL; @@ -1978,7 +1986,7 @@ qemuDomainAttachRNGDevice(virConnectPtr conn, if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &rng->info) < 0) + if (qemuDomainEnsurePCIAddress(vm, &dev) < 0) goto cleanup; } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -2496,6 +2504,7 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, virErrorPtr orig_err = NULL; virJSONValuePtr props = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; + virDomainDeviceDef dev = { VIR_DOMAIN_DEVICE_SHMEM, { .shmem = shmem } }; switch ((virDomainShmemModel)shmem->model) { case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_PLAIN: @@ -2522,7 +2531,7 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, if ((shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE || shmem->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) && - (virDomainPCIAddressEnsureAddr(priv->pciaddrs, &shmem->info) < 0)) + (qemuDomainEnsurePCIAddress(vm, &dev) < 0)) return -1; if (!(shmstr = qemuBuildShmemDevStr(vm->def, shmem, priv->qemuCaps)))