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@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list