On 07/03/2017 09:51 AM, Andrea Bolognani wrote:
> On Fri, 2017-06-30 at 13:56 +0530, Shivaprasad G Bhat wrote:
>> - /* TODO: Detect at runtime once we start using more than just
>> - * the default PCI Host Bridge */
>> - nPCIHostBridges = 1;
>> + for (i = 0; i < def->ncontrollers; i++) {
>> + if (def->controllers[i]->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI
>> ||
>> + def->controllers[i]->model !=
>> VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) {
>> + continue;
>> + }
>> + nPCIHostBridges++;
>> + }
>
> Just to be on the safe side, we should probably make sure the
> pci-root controller is actually a PHB by looking at modelName
> as well, like:
>
> for (i = 0; i < def->ncontrollers; i++) {
> virDomainControllerDefPtr cont = def->controllers[i];
>
> if (cont->type != VIR_DOMAIN_CONTROLLER_TYPE_PCI ||
> cont->model != VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT ||
> cont->opts.pciopts->modelName !=
> VIR_DOMAIN_CONTROLLER_PCI_MODEL_NAME_SPAPR_PCI_HOST_BRIDGE) {
> continue;
> }
>
> nPCIHostBridges++;
> }
>
> Boy, that model name sure is a mouthful[1].
>
> I think we might have enough occurrences of this pattern to
> warrant the creation of a virDomainControllerIsPCIHostBridge()
> helper function, which you could then use in your patch.
>
> That said, it might be smarter to do so in a follow-up cleanup
> commit in order not to invalidate existing Reviewed-by tags.
> Laine, what would be your preference?
Either is fine with me.
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
