Re: [libvirt] [PATCH v3 13/26] conf: Parse and format <target index='...'/>

On Fri, 2017-06-30 at 17:10 +0530, Shivaprasad bhat wrote: >> @@ -9348,6 +9351,23 @@ virDomainControllerDefParseXML(xmlNodePtr node, >> goto error; >> } >> } >> + if (idx) { >> + if (virStrToLong_i(idx, NULL, 0, >> + &def->opts.pciopts.idx) < 0) { >> + virReportError(VIR_ERR_XML_ERROR, >> + _("Invalid target index '%s' in PCI controller"), >> + idx); >> + goto error; >> + } >> + if (def->opts.pciopts.idx < 0 || >> + def->opts.pciopts.idx > 31) { >> + virReportError(VIR_ERR_XML_ERROR, >> + _("PCI controller target index '%s' out of " >> + "range - must be 0-31"), >> + idx); >> + goto error; >> + } >> + } > > Better to enforce def->idx == 0 to have def->opts.pciopts.idx as 0 ? That would always assure def->idx == 0 as the implicit PHB. > > Qemu today allows "-device spapr-pci-host-bridge,index=X,id=pci.0" when X != 0, and we will generate it for implcit phb(which we normally dont) > when def->opts.pciopts.idx != 0 in Patch 18. Not sure what all problems/confusions could come because of this inconsistency. Agreed. The attached patch should be squashed into this commit to prevent such a configuration from being accepted. I'm also working on a test case for this specific configuration. Laine, are you okay with me squashing this in and keeping the Reviewed-by?