[libvirt] [PATCH v3 0/5] Remove caching of address sets

Link to last version (1-6 are already ACK'd) https://www.redhat.com/archives/libvir-list/2016-July/msg00917.html Changes: * qemuDomainPCIAddressSetCreate is now expanded instead of creating a completely new function. It now contains qemuDomainValidateDevicePCISlotsChipsets. * A parameter was added to qemuDomainValidateDevicePCISlotsChipsets to make new address asignment optional. If it's false, then that function doesn't assign any new addresses: it just validates the current ones (and in some cases reserves addresses for the future) By the way I'd be grateful if someone took a look at my other patches: https://www.redhat.com/archives/libvir-list/2016-July/msg01205.html https://www.redhat.com/archives/libvir-list/2016-August/msg00132.html Kind regards, Tomasz Tomasz Flendrich (5): Move validating chipset PCI slots to qemuDomainPCIAddressSetCreate Make address assignment in qemuDomainPCIAddressSetCreate optional Reduce number of parameters to qemuDomainPCIAddressSetCreate qemu_hotplug: generate pci address set on demand qemu: remove pciaddrs caching src/qemu/qemu_domain.c | 1 - src/qemu/qemu_domain.h | 1 - src/qemu/qemu_domain_address.c | 222 +++++++++++++++++++++++------------------ src/qemu/qemu_domain_address.h | 6 ++ src/qemu/qemu_hotplug.c | 50 ++++++++-- 5 files changed, 175 insertions(+), 105 deletions(-) -- 1.9.1

In order to simplify the code and make it possible to recalculate the PCI address set, I incorporated qemuDomainValidateDevicePCISlotsChipsets into qemuDomainPCIAddressSetCreate. Three functions had to be moved up to be seen in qemuDomainPCIAddressSetCreate. --- src/qemu/qemu_domain_address.c | 132 ++++++++++++++++++++--------------------- 1 file changed, 65 insertions(+), 67 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 3d52d72..2e19905 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -538,63 +538,6 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, return ret; } -static virDomainPCIAddressSetPtr -qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses, - bool dryRun) -{ - virDomainPCIAddressSetPtr addrs; - size_t i; - - if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) - return NULL; - - addrs->nbuses = nbuses; - addrs->dryRun = dryRun; - - /* As a safety measure, set default model='pci-root' for first pci - * controller and 'pci-bridge' for all subsequent. After setting - * those defaults, then scan the config and set the actual model - * for all addrs[idx]->bus that already have a corresponding - * controller in the config. - * - */ - if (nbuses > 0) - virDomainPCIAddressBusSetModel(&addrs->buses[0], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); - for (i = 1; i < nbuses; i++) { - virDomainPCIAddressBusSetModel(&addrs->buses[i], - VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); - } - - for (i = 0; i < def->ncontrollers; i++) { - size_t idx = def->controllers[i]->idx; - - if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) - continue; - - if (idx >= addrs->nbuses) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Inappropriate new pci controller index %zu " - "not found in addrs"), idx); - goto error; - } - - if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], - def->controllers[i]->model) < 0) - goto error; - } - - if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) - goto error; - - return addrs; - - error: - virDomainPCIAddressSetFree(addrs); - return NULL; -} - static int qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, @@ -941,6 +884,69 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, } +static virDomainPCIAddressSetPtr +qemuDomainPCIAddressSetCreate(virDomainDefPtr def, + unsigned int nbuses, + virQEMUCapsPtr qemuCaps, + bool dryRun) +{ + virDomainPCIAddressSetPtr addrs; + size_t i; + + if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) + return NULL; + + addrs->nbuses = nbuses; + addrs->dryRun = dryRun; + + /* As a safety measure, set default model='pci-root' for first pci + * controller and 'pci-bridge' for all subsequent. After setting + * those defaults, then scan the config and set the actual model + * for all addrs[idx]->bus that already have a corresponding + * controller in the config. + * + */ + if (nbuses > 0) + virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT); + for (i = 1; i < nbuses; i++) { + virDomainPCIAddressBusSetModel(&addrs->buses[i], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE); + } + + for (i = 0; i < def->ncontrollers; i++) { + size_t idx = def->controllers[i]->idx; + + if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI) + continue; + + if (idx >= addrs->nbuses) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Inappropriate new pci controller index %zu " + "not found in addrs"), idx); + goto error; + } + + if (virDomainPCIAddressBusSetModel(&addrs->buses[idx], + def->controllers[i]->model) < 0) + goto error; + } + + if (virDomainDeviceInfoIterate(def, qemuDomainCollectPCIAddress, addrs) < 0) + goto error; + + if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, + addrs) < 0) + goto error; + + return addrs; + + error: + virDomainPCIAddressSetFree(addrs); + return NULL; +} + + static bool qemuDomainPCIBusFullyReserved(virDomainPCIAddressBusPtr bus) { @@ -1455,11 +1461,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainDeviceInfo info; /* 1st pass to figure out how many PCI bridges we need */ - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) - goto cleanup; - - if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, - addrs) < 0) + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, true))) goto cleanup; for (i = 0; i < addrs->nbuses; i++) { @@ -1506,12 +1508,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, goto cleanup; } - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false))) - goto cleanup; - if (qemuDomainSupportsPCI(def, qemuCaps)) { - if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, - addrs) < 0) + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, false))) goto cleanup; if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) -- 1.9.1

With a parameter, we can decide whether we want qemuDomainValidateDevicePCISlotsChipsets to assign new addresses, or simply verify the existing ones and reserve slots for potential future devices. The PCI address set will be recalculated instead of being cached, and during recalculation we want to forbid assignment of new addresses. --- src/qemu/qemu_domain_address.c | 67 ++++++++++++++++++++++++++++++++++-------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 2e19905..f916df2 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -542,7 +542,8 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, static int qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs, + bool assign) { int ret = -1; size_t i; @@ -567,6 +568,11 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->controllers[i]->info.addr.pci.domain = 0; def->controllers[i]->info.addr.pci.bus = 0; @@ -587,6 +593,11 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->controllers[i]->info.addr.pci.domain = 0; def->controllers[i]->info.addr.pci.bus = 0; @@ -615,6 +626,11 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, */ virDomainVideoDefPtr primaryVideo = def->videos[0]; if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; @@ -676,7 +692,8 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, static int qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs, + bool assign) { int ret = -1; size_t i; @@ -703,6 +720,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } def->controllers[i]->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; def->controllers[i]->info.addr.pci.domain = 0; def->controllers[i]->info.addr.pci.bus = 0; @@ -726,18 +748,23 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, * get assigned to the same slot as the UHCI1 when * addresses are later assigned to all devices.) */ - bool assign = false; + bool assignUSB = false; memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1D; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { - assign = true; + assignUSB = true; } else { tmp_addr.slot = 0x1A; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) - assign = true; + assignUSB = true; } - if (assign) { + if (assignUSB) { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, false, true) < 0) goto cleanup; @@ -763,6 +790,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, true, false) < 0) goto cleanup; @@ -806,6 +838,11 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, */ virDomainVideoDefPtr primaryVideo = def->videos[0]; if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { + if (!assign) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("All addresses should have been already assigned by now.")); + goto cleanup; + } memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; @@ -868,15 +905,16 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, static int qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, - virDomainPCIAddressSetPtr addrs) + virDomainPCIAddressSetPtr addrs, + bool assign) { if (qemuDomainMachineIsI440FX(def) && - qemuDomainValidateDevicePCISlotsPIIX3(def, qemuCaps, addrs) < 0) { + qemuDomainValidateDevicePCISlotsPIIX3(def, qemuCaps, addrs, assign) < 0) { return -1; } if (qemuDomainMachineIsQ35(def) && - qemuDomainValidateDevicePCISlotsQ35(def, qemuCaps, addrs) < 0) { + qemuDomainValidateDevicePCISlotsQ35(def, qemuCaps, addrs, assign) < 0) { return -1; } @@ -888,7 +926,8 @@ static virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned int nbuses, virQEMUCapsPtr qemuCaps, - bool dryRun) + bool dryRun, + bool assign) { virDomainPCIAddressSetPtr addrs; size_t i; @@ -936,7 +975,7 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, goto error; if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, - addrs) < 0) + addrs, assign) < 0) goto error; return addrs; @@ -1461,7 +1500,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainDeviceInfo info; /* 1st pass to figure out how many PCI bridges we need */ - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, true))) + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, + true, true))) goto cleanup; for (i = 0; i < addrs->nbuses; i++) { @@ -1509,7 +1549,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } if (qemuDomainSupportsPCI(def, qemuCaps)) { - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, false))) + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, + false, true))) goto cleanup; if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) -- 1.9.1

We can as well generate the number of buses inside that function. Fewer parameters will be useful when recalculating the pci address set on demand. --- src/qemu/qemu_domain_address.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index f916df2..ade4b64 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -924,14 +924,24 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, static virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, - unsigned int nbuses, virQEMUCapsPtr qemuCaps, bool dryRun, bool assign) { virDomainPCIAddressSetPtr addrs; + int max_idx = -1; + int nbuses = 0; size_t i; + for (i = 0; i < def->ncontrollers; i++) { + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if ((int) def->controllers[i]->idx > max_idx) + max_idx = def->controllers[i]->idx; + } + } + + nbuses = max_idx + 1; + if ((addrs = virDomainPCIAddressSetAlloc(nbuses)) == NULL) return NULL; @@ -1500,7 +1510,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainDeviceInfo info; /* 1st pass to figure out how many PCI bridges we need */ - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, + if (!(addrs = qemuDomainPCIAddressSetCreate(def, qemuCaps, true, true))) goto cleanup; @@ -1537,7 +1547,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) goto cleanup; } - nbuses = addrs->nbuses; virDomainPCIAddressSetFree(addrs); addrs = NULL; @@ -1549,7 +1558,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } if (qemuDomainSupportsPCI(def, qemuCaps)) { - if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, qemuCaps, + if (!(addrs = qemuDomainPCIAddressSetCreate(def, qemuCaps, false, true))) goto cleanup; -- 1.9.1

Dropping the caching of pci address set. Instead of using the cached address set, functions in qemu_hotplug.c now recalculate it on demand. --- src/qemu/qemu_domain_address.c | 8 +------ src/qemu/qemu_domain_address.h | 6 +++++ src/qemu/qemu_hotplug.c | 50 +++++++++++++++++++++++++++++++++++------- 3 files changed, 49 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index ade4b64..2870afc 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -922,7 +922,7 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, } -static virDomainPCIAddressSetPtr +virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, bool dryRun, @@ -1841,12 +1841,6 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, if (!devstr) devstr = info->alias; - if (virDeviceInfoPCIAddressPresent(info) && - virDomainPCIAddressReleaseSlot(priv->pciaddrs, - &info->addr.pci) < 0) - VIR_WARN("Unable to release PCI address on %s", - NULLSTR(devstr)); - if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_USB && priv->usbaddrs && virDomainUSBAddressRelease(priv->usbaddrs, info) < 0) diff --git a/src/qemu/qemu_domain_address.h b/src/qemu/qemu_domain_address.h index 11d6e92..dee352b 100644 --- a/src/qemu/qemu_domain_address.h +++ b/src/qemu/qemu_domain_address.h @@ -45,6 +45,12 @@ virDomainCCWAddressSetPtr qemuDomainCCWAddrSetCreateFromDomain(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); +virDomainPCIAddressSetPtr +qemuDomainPCIAddressSetCreate(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps, + bool dryRun, + bool assign); + # define __QEMU_DOMAIN_ADDRESS_H__ #endif /* __QEMU_DOMAIN_ADDRESS_H__ */ diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 00e4a75..51f06bc 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -314,6 +314,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, bool secobjAdded = false; bool encobjAdded = false; virDomainCCWAddressSetPtr ccwaddrs = NULL; + virDomainPCIAddressSetPtr pciaddrs = NULL; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); const char *src = virDomainDiskGetSource(disk); virJSONValuePtr secobjProps = NULL; @@ -345,7 +346,10 @@ 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 (!(pciaddrs = qemuDomainPCIAddressSetCreate(vm->def, priv->qemuCaps, + false, false))) + goto error; + if (virDomainPCIAddressEnsureAddr(pciaddrs, &disk->info) < 0) goto error; } releaseaddr = true; @@ -420,6 +424,7 @@ qemuDomainAttachVirtioDiskDevice(virConnectPtr conn, virJSONValueFree(encobjProps); qemuDomainSecretDiskDestroy(disk); virDomainCCWAddressSetFree(ccwaddrs); + virDomainPCIAddressSetFree(pciaddrs); VIR_FREE(devstr); VIR_FREE(drivestr); VIR_FREE(drivealias); @@ -464,6 +469,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, char *devstr = NULL; qemuDomainObjPrivatePtr priv = vm->privateData; virDomainCCWAddressSetPtr ccwaddrs = NULL; + virDomainPCIAddressSetPtr pciaddrs = NULL; bool releaseaddr = false; if (controller->type != VIR_DOMAIN_CONTROLLER_TYPE_SCSI) { @@ -502,7 +508,10 @@ 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 (!(pciaddrs = qemuDomainPCIAddressSetCreate(vm->def, priv->qemuCaps, + false, false))) + goto cleanup; + if (virDomainPCIAddressEnsureAddr(pciaddrs, &controller->info) < 0) goto cleanup; } else if (controller->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) @@ -541,6 +550,7 @@ int qemuDomainAttachControllerDevice(virQEMUDriverPtr driver, VIR_FREE(devstr); virDomainCCWAddressSetFree(ccwaddrs); + virDomainPCIAddressSetFree(pciaddrs); return ret; } @@ -932,6 +942,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virNetDevBandwidthPtr actualBandwidth; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); virDomainCCWAddressSetPtr ccwaddrs = NULL; + virDomainPCIAddressSetPtr pciaddrs = NULL; size_t i; /* preallocate new slot for device */ @@ -1078,8 +1089,12 @@ 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) { - goto cleanup; + } else { + if (!(pciaddrs = qemuDomainPCIAddressSetCreate(vm->def, priv->qemuCaps, + false, false))) + goto cleanup; + if (virDomainPCIAddressEnsureAddr(pciaddrs, &net->info) < 0) + goto cleanup; } releaseaddr = true; @@ -1246,6 +1261,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, VIR_FREE(vhostfdName); virObjectUnref(cfg); virDomainCCWAddressSetFree(ccwaddrs); + virDomainPCIAddressSetFree(pciaddrs); return ret; @@ -1297,6 +1313,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, bool teardownlabel = false; int backend; virQEMUDriverConfigPtr cfg = virQEMUDriverGetConfig(driver); + virDomainPCIAddressSetPtr pciaddrs = NULL; unsigned int flags = 0; if (VIR_REALLOC_N(vm->def->hostdevs, vm->def->nhostdevs + 1) < 0) @@ -1358,8 +1375,14 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1) < 0) goto error; - if (virDomainPCIAddressEnsureAddr(priv->pciaddrs, hostdev->info) < 0) + + if (!(pciaddrs = qemuDomainPCIAddressSetCreate(vm->def, priv->qemuCaps, + false, false))) + goto error; + + if (virDomainPCIAddressEnsureAddr(pciaddrs, hostdev->info) < 0) goto error; + releaseaddr = true; if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO && virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_PCI_CONFIGFD)) { @@ -1419,6 +1442,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, cleanup: virObjectUnref(cfg); + virDomainPCIAddressSetFree(pciaddrs); return -1; } @@ -1591,6 +1615,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, { int ret = -1; virDomainVirtioSerialAddrSetPtr vioaddrs = NULL; + virDomainPCIAddressSetPtr pciaddrs = NULL; if (!(vioaddrs = virDomainVirtioSerialAddrSetCreateFromDomain(def))) goto cleanup; @@ -1604,7 +1629,10 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, } 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 (!(pciaddrs = qemuDomainPCIAddressSetCreate(def, priv->qemuCaps, + false, false))) + goto cleanup; + if (virDomainPCIAddressEnsureAddr(pciaddrs, &chr->info) < 0) goto cleanup; ret = 1; @@ -1636,6 +1664,7 @@ qemuDomainAttachChrDeviceAssignAddr(virDomainDefPtr def, cleanup: virDomainVirtioSerialAddrSetFree(vioaddrs); + virDomainPCIAddressSetFree(pciaddrs); return ret; } @@ -1727,6 +1756,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, bool objAdded = false; virJSONValuePtr props = NULL; virDomainCCWAddressSetPtr ccwaddrs = NULL; + virDomainPCIAddressSetPtr pciaddrs = NULL; const char *type; int ret = -1; int rv; @@ -1754,8 +1784,11 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, 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) - return -1; + if (!(pciaddrs = qemuDomainPCIAddressSetCreate(vm->def, priv->qemuCaps, + false, false))) + goto cleanup; + if (virDomainPCIAddressEnsureAddr(pciaddrs, &rng->info) < 0) + goto cleanup; } else if (rng->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW) { if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) goto cleanup; @@ -1813,6 +1846,7 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, VIR_FREE(objAlias); VIR_FREE(devstr); virDomainCCWAddressSetFree(ccwaddrs); + virDomainPCIAddressSetFree(pciaddrs); return ret; exit_monitor: -- 1.9.1

The cached pci address set is not required anymore, because the set is now being recalculated from the domain definition on demand, so the cache can be deleted. --- src/qemu/qemu_domain.c | 1 - src/qemu/qemu_domain.h | 1 - src/qemu/qemu_domain_address.c | 14 ++------------ 3 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 6c0f261..24e594b 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -1328,7 +1328,6 @@ qemuDomainObjPrivateFree(void *data) virObjectUnref(priv->qemuCaps); virCgroupFree(&priv->cgroup); - virDomainPCIAddressSetFree(priv->pciaddrs); virDomainUSBAddressSetFree(priv->usbaddrs); virDomainChrSourceDefFree(priv->monConfig); qemuDomainObjFreeJob(priv); diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 0613093..af96d4a 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -184,7 +184,6 @@ struct _qemuDomainObjPrivate { bool beingDestroyed; char *pidfile; - virDomainPCIAddressSetPtr pciaddrs; virDomainUSBAddressSetPtr usbaddrs; virQEMUCapsPtr qemuCaps; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 2870afc..ade1c21 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1482,12 +1482,10 @@ qemuDomainAddressFindNewBusNr(virDomainDefPtr def) static int qemuDomainAssignPCIAddresses(virDomainDefPtr def, - virQEMUCapsPtr qemuCaps, - virDomainObjPtr obj) + virQEMUCapsPtr qemuCaps) { int ret = -1; virDomainPCIAddressSetPtr addrs = NULL; - qemuDomainObjPrivatePtr priv = NULL; int max_idx = -1; int nbuses = 0; size_t i; @@ -1641,14 +1639,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, } } - if (obj && obj->privateData) { - priv = obj->privateData; - /* if this is the live domain object, we persist the PCI addresses */ - virDomainPCIAddressSetFree(priv->pciaddrs); - priv->pciaddrs = addrs; - addrs = NULL; - } - ret = 0; cleanup: @@ -1821,7 +1811,7 @@ qemuDomainAssignAddresses(virDomainDefPtr def, qemuDomainAssignARMVirtioMMIOAddresses(def, qemuCaps); - if (qemuDomainAssignPCIAddresses(def, qemuCaps, obj) < 0) + if (qemuDomainAssignPCIAddresses(def, qemuCaps) < 0) return -1; if (newDomain && qemuDomainAssignUSBAddresses(def, obj) < 0) -- 1.9.1
participants (1)
-
Tomasz Flendrich