[libvirt] [PATCH 09/26] conf: Move index number checking to drivers

[Laine Stump]

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.