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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list