[libvirt] [PATCH 2/4] qemu: prefer the PCI bus alias from status XML

[John Ferlan]

On 11/29/2017 09:58 AM, Ján Tomko wrote:
> For some corner cases, virQEMUCapsHasPCIMultiBus depends on the QEMU
> version, which is by desing not stored in the status XML and therefore

design

> it cannot be fixed for all existing running domains.
> 
> Prefer the controller alias read from the status XML when formatting
> PCI addresses and only fall back to using virQEMUCapsHasPCIMultiBus
> if the alias is a user alias.
> 
> This fixes hotplug after daemon restart for domains not using user
> aliases.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=1518148


should this mention a partial revert of 937f3195 ?

Or maybe there should be a patch to revert that patch and another to do
what's being done here with respect to only taking this "path" when a UA
alias is being used.

John

(Although I see Pavel R-B'd things - I figured I'd drop a couple of
thoughts too).


> ---
>  src/qemu/qemu_command.c | 42 ++++++++++++++++++++++--------------------
>  1 file changed, 22 insertions(+), 20 deletions(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index c78f1b83b..677cfc995 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -318,29 +318,31 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>  
>              if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
>                  cont->idx == info->addr.pci.bus) {
> +                contAlias = cont->info.alias;
>                  contIsPHB = virDomainControllerIsPSeriesPHB(cont);
>                  contTargetIndex = cont->opts.pciopts.targetIndex;
>  
> -                /* When domain has builtin pci-root controller we don't put it
> -                 * onto cmd line. Therefore we can't set its alias. In that
> -                 * case, use the default one. */
> -                if (!qemuDomainIsPSeries(domainDef) &&
> -                    cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
> -                    if (virQEMUCapsHasPCIMultiBus(qemuCaps, domainDef))
> -                        contAlias = "pci.0";
> -                    else
> -                        contAlias = "pci";
> -                } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
> -                    contAlias = "pcie.0";
> -                } else {
> -                    contAlias = cont->info.alias;
> -                    if (!contAlias) {
> -                        virReportError(VIR_ERR_INTERNAL_ERROR,
> -                                       _("Device alias was not set for PCI "
> -                                         "controller with index %u required "
> -                                         "for device at address %s"),
> -                                       info->addr.pci.bus, devStr);
> -                        goto cleanup;
> +                if (!contAlias) {
> +                    virReportError(VIR_ERR_INTERNAL_ERROR,
> +                                   _("Device alias was not set for PCI "
> +                                     "controller with index %u required "
> +                                     "for device at address %s"),
> +                                   info->addr.pci.bus, devStr);
> +                    goto cleanup;
> +                }
> +
> +                if (STRPREFIX(contAlias, VIR_DOMAIN_USER_ALIAS_PREFIX)) {
> +                    /* When domain has builtin pci-root controller we don't put it
> +                     * onto cmd line. Therefore we can't set its alias. In that
> +                     * case, use the default one. */
> +                    if (!qemuDomainIsPSeries(domainDef) &&
> +                        cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
> +                        if (virQEMUCapsHasPCIMultiBus(qemuCaps, domainDef))
> +                            contAlias = "pci.0";
> +                        else
> +                            contAlias = "pci";
> +                    } else if (cont->model == VIR_DOMAIN_CONTROLLER_MODEL_PCIE_ROOT) {
> +                        contAlias = "pcie.0";
>                      }
>                  }
>                  break;
>