Re: [libvirt] [PATCH 7/8] qemu: Wire up address type=pci auto_allocate
On 03/23/2016 02:20 PM, Laine Stump wrote:
On 03/08/2016 11:36 AM, Cole Robinson wrote:
> We do this in 2 passes: before PCI addresses are about to be collected,
> we convert type=pci auto_allocate=true to type=none auto_allocate=true,
> since the existing code is already expecting type=none here.
>
> After all PCI allocation should be complete, we do another pass of the
> device addresses converting type=pci auto_allocate=true to
> auto_allocate=false, so we don't trigger the unallocated address
> validation check in generic domain code.
This sounds confusing. What about instead changing the existing code so that
it checks for a valid PCI address instead of checking for type=none? A simple
check for this is if domain == bus == slot == 0 (bus 0 is *always* either a
pcie-root or a pci-root, and for both of those slot 0 is reserved, so once an
address has been assigned, it will never be 0000:00:00.x .)
So rather than doing this dance with type and auto_allocate, you can just
modify the auto allocation to say:
if (info->type == ...NONE ||
(info->type == ...PCI &&
!virDevicePCIAddressIsValid(&info->addr.pci)) {
// Bob Loblaw
}
(virDevicePCIAddressIsValid() has some extra checks for values being too
large, but does the essential check for domain/bus/slot == 0 as well)
I'm refreshing these patches at the moment. I poked at this but it's a bit of
a pain... Multiple places in the address assignment code assume 'if info->type
== PCI, an address has been specified', like the code the scoops up all the
already specified PCI addresses to check for collisions. Every place that
makes that assumption now needs to be taught to check for auto_allocate and
PCIDeviceAddressIsValid which seems fragile, since other checks may slip in
that don't take those values into account.
I'm reposting the patches using the old technique, but I'm happy to change
things if we can come up with some more clear and sustainable alternative
Thanks,
Cole