[libvirt] [PATCH v4 0/5] Remove caching of address sets
Link to the previous version: https://www.redhat.com/archives/libvir-list/2016-August/msg00721.html Changes in v4: * instead of failing, qemuDomainValidateDevicePCISlotsChipsets now simply displays a warning when a new address is being assigned during pci address set recalculation Tomasz Flendrich (5): Move validating chipset PCI slots to qemuDomainPCIAddressSetCreate Warn when assigning new pci addresses during recalculation 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 | 215 +++++++++++++++++++++++------------------ src/qemu/qemu_domain_address.h | 6 ++ src/qemu/qemu_hotplug.c | 50 ++++++++-- 5 files changed, 168 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 show whether we want qemuDomainValidateDevicePCISlotsChipsets to assign new addresses or simply verify the existing ones and reserve slots for potential future devices. If the parameter is false and some device is missing an address, a warning will be displayed. The PCI address set will be recalculated instead of being cached, and during recalculation all the addresses should have already been assigned. --- src/qemu/qemu_domain_address.c | 60 +++++++++++++++++++++++++++++++++--------- 1 file changed, 47 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 2e19905..70b6978 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,10 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } 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 +592,10 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } 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 +624,10 @@ qemuDomainValidateDevicePCISlotsPIIX3(virDomainDefPtr def, */ virDomainVideoDefPtr primaryVideo = def->videos[0]; if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 2; @@ -676,7 +689,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 +717,10 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, goto cleanup; } } else { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } 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 +744,22 @@ 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) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, false, true) < 0) goto cleanup; @@ -763,6 +785,10 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 0x1E; if (!virDomainPCIAddressSlotInUse(addrs, &tmp_addr)) { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } if (virDomainPCIAddressReserveAddr(addrs, &tmp_addr, flags, true, false) < 0) goto cleanup; @@ -806,6 +832,10 @@ qemuDomainValidateDevicePCISlotsQ35(virDomainDefPtr def, */ virDomainVideoDefPtr primaryVideo = def->videos[0]; if (virDeviceInfoPCIAddressWanted(&primaryVideo->info)) { + if (!assign) { + VIR_WARN("During PCI address set recalculation " + "no new addresses should be assigned."); + } memset(&tmp_addr, 0, sizeof(tmp_addr)); tmp_addr.slot = 1; @@ -868,15 +898,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 +919,8 @@ static virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, unsigned int nbuses, virQEMUCapsPtr qemuCaps, - bool dryRun) + bool dryRun, + bool assign) { virDomainPCIAddressSetPtr addrs; size_t i; @@ -936,7 +968,7 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, goto error; if (qemuDomainValidateDevicePCISlotsChipsets(def, qemuCaps, - addrs) < 0) + addrs, assign) < 0) goto error; return addrs; @@ -1461,7 +1493,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 +1542,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 70b6978..27ad54a 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -917,14 +917,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; @@ -1493,7 +1503,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; @@ -1530,7 +1540,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, virDomainPCIAddressReserveNextSlot(addrs, &info, flags) < 0) goto cleanup; } - nbuses = addrs->nbuses; virDomainPCIAddressSetFree(addrs); addrs = NULL; @@ -1542,7 +1551,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 27ad54a..575d0f1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -915,7 +915,7 @@ qemuDomainValidateDevicePCISlotsChipsets(virDomainDefPtr def, } -static virDomainPCIAddressSetPtr +virDomainPCIAddressSetPtr qemuDomainPCIAddressSetCreate(virDomainDefPtr def, virQEMUCapsPtr qemuCaps, bool dryRun, @@ -1834,12 +1834,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 575d0f1..0aa940b 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1475,12 +1475,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; @@ -1634,14 +1632,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: @@ -1814,7 +1804,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
On Fri, Aug 19, 2016 at 06:03:22AM +0200, Tomasz Flendrich wrote:
Link to the previous version: https://www.redhat.com/archives/libvir-list/2016-August/msg00721.html
Changes in v4: * instead of failing, qemuDomainValidateDevicePCISlotsChipsets now simply displays a warning when a new address is being assigned during pci address set recalculation
Even though this works now, I'd be in favour of handling this (and adding some tests actually) after the persistent device attachment series [1], see replies in that thread. Martin [1] https://www.redhat.com/archives/libvir-list/2016-July/msg01205.html
participants (2)
-
Martin Kletzander -
Tomasz Flendrich