[libvirt] [PATCH 5/6] qemu: Allow multiple bridges when pci-bridges is not available

[Laine Stump]

On 02/21/2017 02:57 PM, Andrea Bolognani wrote:
> qemuDomainAssignPCIAddresses() hardcoded the assumption
> that the only way to support devices on a non-zero bus is
> to add one or more pci-bridges; however, since we now
> support a large selection of PCI controllers that can be
> used instead, the assumption is no longer true.
>
> Moreover, this check was always redundant, because the
> only sensible time to check for the availability of
> pci-bridge is when building the QEMU command line, and
> such a check is of course already in place.
>
> In fact, there were *two* such checks, but since one of
> the two was relying on the incorrect assumption explained
> above, and it was redundant anyway, it has been dropped.
> ---
>   src/qemu/qemu_command.c        |  7 -------
>   src/qemu/qemu_domain_address.c | 10 +---------
>   2 files changed, 1 insertion(+), 16 deletions(-)
>
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index f0b938f..0ff1e7d 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -342,13 +342,6 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>               }
>           }
>   
> -        if (info->addr.pci.bus != 0 &&
> -            !virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
> -            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                           _("Multiple PCI buses are not supported "
> -                             "with this QEMU binary"));
> -            goto cleanup;
> -        }
>           virBufferAsprintf(buf, ",bus=%s", contAlias);
>   
>           if (info->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index 5b75044..f8995c9 100644
> --- a/src/qemu/qemu_domain_address.c
> +++ b/src/qemu/qemu_domain_address.c
> @@ -1983,9 +1983,7 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>       if (qemuDomainFillAllPCIConnectFlags(def, qemuCaps, driver) < 0)
>           goto cleanup;
>   
> -    if (nbuses > 0 &&
> -        virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_PCI_BRIDGE)) {
> -
> +    if (nbuses > 0) {

I go past this check every couple months and each time I think "that is 
wrong. I should fix it.", but then forget by the time I'm done with 
whatever else I was doing.

But beyond that, it seems like we should be able to remove the "if 
(nbuses > 0)" too - if nbuses is 0, that means there is no root bus to 
put any additional buses on, and *that* means that if there is any 
device requiring a PCI address, it's going to cause a failure while 
trying to assign addresses whether it's the "trial pass" (checking if we 
need to add more controllers) or the "real pass" (that actually assigns 
the addresses that will be used) - we could just as easily fail during 
the trial pass.

That's immaterial to your change though. ACK on what you've got here 
(and if you want to see what happens when you remove the rest of the 
conditional, that would be good too).

>           /* 1st pass to figure out how many PCI bridges we need */
>           if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, true)))
>               goto cleanup;
> @@ -2109,12 +2107,6 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def,
>           nbuses = addrs->nbuses;
>           virDomainPCIAddressSetFree(addrs);
>           addrs = NULL;
> -
> -    } else if (max_idx > 0) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> -                       _("PCI bridges are not supported "
> -                         "by this QEMU binary"));
> -        goto cleanup;
>       }
>   
>       if (!(addrs = qemuDomainPCIAddressSetCreate(def, nbuses, false)))