On 04/23/2013 06:47 AM, Ján Tomko wrote:
Add a "dry run" address allocation to figure out how many bridges will be needed for all the devices without explicit addresses.
Auto-add just enough bridges to put all the devices on, or up to the bridge with the largest specified index. ---
v4: Moved the check for duplicate controller indexes to a separate patch Simplified nbuses and max_idx computation Only does two-pass allocation of PCI addresses if the machine has a PCI bus Does not contain traces of rebasing or spurious whitespace changes Tests auto-adding PCI bridges in xml->argv and xml->xml tests.
@@ -1233,9 +1236,45 @@ static bool qemuPCIAddressValidate(qemuDomainPCIAddressSetPtr addrs ATTRIBUTE_UN QEMU_PCI_ADDRESS_SLOT_LAST); return false; } + if (addr->slot == 0) { + if (addr->bus) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot 0 is unusable on PCI bridges")); + } else { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("Slot 0 on bus 0 is reserved for the host bridge"));
Indentation is off.
@@ -1326,15 +1368,53 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainObjPrivatePtr priv = NULL;
if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE)) { + int max_idx = -1;
So let's trace what happens if I have XML with no <controller> but I do use 33 PCI devices and have a capable qemu: max_idx starts at -1,
int nbuses = 0; int i; + int rv;
for (i = 0; i < def->ncontrollers; i++) { - if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) - nbuses++; + if (def->controllers[i]->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI) { + if (def->controllers[i]->idx > max_idx) + max_idx = def->controllers[i]->idx; + } + }
If no controllers were specified, it is still at -1,
+ + nbuses = max_idx + 1;
so nbuses is now 0,
+ + if (nbuses > 0 && + virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
therefore we skip this if,
+ virDomainDeviceInfo info; + /* 1st pass to figure out how many PCI bridges we need */ + if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true))) + goto cleanup; + if (qemuAssignDevicePCISlots(def, qemuCaps, addrs) < 0) + goto cleanup; + /* Reserve 1 extra slot for a (potential) bridge */ + if (qemuDomainPCIAddressSetNextAddr(addrs, &info) < 0) + goto cleanup; + + for (i = 1; i < addrs->nbuses; i++) { + if ((rv = virDomainDefMaybeAddController( + def, VIR_DOMAIN_CONTROLLER_TYPE_PCI, + i, VIR_DOMAIN_CONTROLLER_MODEL_PCI_BRIDGE)) < 0) + goto cleanup; + /* If we added a new bridge, we will need one more address */ + if (rv > 0 && qemuDomainPCIAddressSetNextAddr(addrs, &info) < 0) + goto cleanup; + } + nbuses = addrs->nbuses; + qemuDomainPCIAddressSetFree(addrs); + addrs = NULL; + + } else if (max_idx > 0) {
we don't error out,
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("PCI bridges are not supported " + "by this QEMU binary")); + goto cleanup; }
but we also didn't auto-instantiate any bridges, even if the capability is supported. Is that a problem? ACK if you can answer my question and fix the minor issues. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org