[libvirt] [PATCH 17/26] qemu: Deal with PHB naming convention

[Laine Stump]

On 06/02/2017 12:07 PM, Andrea Bolognani wrote:
> Usually, a controller with alias 'x' will create a bus with the
> same name; however, the bus created by a PHBs with alias 'x' will
> be named 'x.0' instead, so we need to account for that.

This doesn't make sense to me. My recollection (without going through
the code again) is that for PCI controllers, the alias for pci-root is
"pci.0" as long as the qemu binary supports multiple PCI controllers
(otherwise it is just "pci"), and the alias is pcie.0 for pcie-root,
then all other PCI controllers have the alias (i.e. value of "contAlias"
in this function) of pci.x (where "x" is the index of the controller).

What you're saying here doesn't match that. Unless you're saying that
the PHBs will have aliases named something like "pci.1.0" (which isn't
what I see in the test cases in 19/26).

I guess I need time to digest why this is necessary. Based on my memory,
it should have worked properly already...

> 
> As an exception to the exception, the implicit PHB that's added
> automatically to every pSeries guest creates the 'pci.0' bus.
> ---
>  src/qemu/qemu_command.c | 20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
> 
> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 082ffa9..1bc3093 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -301,6 +301,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>      int ret = -1;
>      char *devStr = NULL;
>      const char *contAlias = NULL;
> +    virDomainControllerModelPCI contModel;
> +    virDomainControllerPCIModelName contModelName;
> +    int contIndex;
>  
>      if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
>          size_t i;
> @@ -313,6 +316,9 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>              if (cont->type == VIR_DOMAIN_CONTROLLER_TYPE_PCI &&
>                  cont->idx == info->addr.pci.bus) {
>                  contAlias = cont->info.alias;
> +                contModel = cont->model;
> +                contModelName = cont->opts.pciopts.modelName;
> +                contIndex = cont->opts.pciopts.idx;
>                  if (!contAlias) {
>                      virReportError(VIR_ERR_INTERNAL_ERROR,
>                                     _("Device alias was not set for PCI "
> @@ -348,7 +354,19 @@ qemuBuildDeviceAddressStr(virBufferPtr buf,
>              }
>          }
>  
> -        virBufferAsprintf(buf, ",bus=%s", contAlias);
> +        if (contModel == VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT &&
> +            contModelName == VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE &&
> +            contIndex > 0) {
> +            /* The PCI bus created by a spapr-pci-host-bridge device with
> +             * alias 'x' will be called 'x.0' rather than 'x'; however,
> +             * this does not apply to the implicit PHB in a pSeries guest,
> +             * which always has the hardcoded name 'pci.0' */
> +            virBufferAsprintf(buf, ",bus=%s.0", contAlias);
> +        } else {
> +            /* For all other controllers, the bus name matches the alias
> +             * of the corresponding controller */
> +            virBufferAsprintf(buf, ",bus=%s", contAlias);
> +        }
>  
>          if (info->addr.pci.multi == VIR_TRISTATE_SWITCH_ON)
>              virBufferAddLit(buf, ",multifunction=on");
>