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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list