On 06/12/2017 10:14 PM, Laine Stump wrote: > On 06/12/2017 10:08 PM, Laine Stump wrote: >> On 06/02/2017 12:07 PM, Andrea Bolognani wrote: >>> pSeries guests will soon be allowed to have multiple >>> PHBs (pci-root controllers), which of course means that >>> all but one of them will have a non-zero index; hence, >>> we'll need to relax the current check. >>> >>> However, right now the check is performed in the conf >>> module, which is generic rather than tied to the QEMU >>> driver, and where we don't have information such as the >>> guest machine type available. >>> >>> To make this change of behavior possible down the line, >>> we need to move the check from the XML parser to the >>> driver. Unfortunately, this means duplicating code :(\ >> >> >> Maybe instead we should just eliminate this check (since the pSeries >> case shows that it's an invalid assumption). And since the index is >> really just something used internally by libvirt, we really don't even >> need to verify that one of the pci controllers has index=0. All we >> *really* care about is that there is at least one "root" PCI bus, and >> that no device includes a bus# in its PCI address that isn't represented >> by the index in one of the PCI controllers. >> >> (But for the purposes of this patch, I think we can just remove the >> validation in conf and not worry about adding it elsewhere).\\ > > After looking at the next patch, I may have changed my mind. If we > remove the validation here, then we would still have to add in > validation when the commandline is generated. But I suppose it's better > to give an error at domain definition time rather than domain runtime, > so this makes sense. I don't think all of those drivers actually use PCI > controllers though, do they? (Look for which ones actually call the > functions to assign PCI addresses). > Refreshing my memory by looking at the code - I think only the qemu driver and bhyve driver assign PCI addresses (they're certainly the only ones that use VIR_DOMAIN_CONTROLLER_TYPE_PCI or VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT, or call virDomainPCIAddressReserveNextAddr()). So I'm fairly certain you only need to add the check in those two drivers. (Additionally, bhyve only supports pci-root, not pcie-root - see src/bhyve/bhyve_command.c:509) So, ACK if you remove the extra checks in all the other drivers aside from qemu and bhyve. -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
