[libvirt] [PATCH 0/6] auto-assign addresses when <address type='pci'/> is specified

[Ján Tomko]

On Thu, May 19, 2016 at 01:15:28PM -0400, Cole Robinson wrote:
> On 05/18/2016 03:23 PM, Laine Stump wrote:
> > This is an alternative to Cole's series that permits <address
> > type='pci'/> to force assignment of a PCI address, which is
> > particularly useful on platforms that could connect the same device in
> > different ways (e.g. aarch64/virt).
> > 
> > Here is Cole's last iteration of the series:
> > 
> >   https://www.redhat.com/archives/libvir-list/2016-May/msg01088.html
> > 
> > I had expressed a dislike of the "auto_allocate" flag that his series
> > temporarily adds to the address (while simultaneously changing the
> > address type to NONE) and suggested just changing all the necessary
> > places to check for a valid PCI address instead of just checking the
> > address type. He replied that this wasn't as simple as it looked, so I
> > decided to try it; turns out he's right. But I still like this method
> > better because it's not playing tricks with the address type, or
> > adding a temporary private attribute to what should be pure config
> > data.
> > 
> > Your opinion may vary though :-)
> > 

I like this series more than counting XML elements and temporarily
changing the types.

> > Note that patch 5/6 incorporates the same test case that Cole used in
> > his penultimate patch, and I've added his patch to check the case of
> > aarch64 at the end as well.
> > 
> 
> ACK series, but it's missing formatdomain.html.in changes. You can grab the
> check from my patch #2
> 
> I'm fine with this approach but I'll just list the downsides
> 
> - Less generalizable, for adding additional address types in the future, but
> that's hypothetical at this point
> - We don't raise an explicit error for drivers that don't support this type of
> address allocation, like libxl. If it matters we could add a domain XML parse
> feature to throw an explicit error though
> - checking info->type == DEVICE_ADDRESS_TYPE_PCI is now a suspect pattern and
> needs to be considered carefully in other parts of the code
> 
> upsides:
> - less magic
> - I think it will make allocating address at hotplug time simpler as well
> 

Yes, qemu_hotplug.c has a few of those places using == DEVICE_ADDRESS_TYPE_PCI
untouched by this series.

Jan