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

Laine Stump laine at laine.org
Tue Jun 13 14:18:26 UTC 2017

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
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.

More information about the libvir-list mailing list