On 04/25/2013 02:39 AM, Eric Blake wrote: > On 04/23/2013 06:47 AM, Ján Tomko wrote: >> @@ -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? No, if the machine has no buses, we would have no place to put the bridge in. (Unless it's a PCI express machine, but libvirt doesn't know how to use those yet) And we'll error out anyway with: error: XML error: No PCI buses available either in GetNextSlot called by AssignPCISlots for devices without an address, or in PCIAddressValidate called by CollectPCIAddress for explicitly specified PCI addresses. Jan -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list