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 :-) > > 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 Thanks, Cole > > Cole Robinson (1): > tests: qemu: test <address type='pci'/> with aarch64 > > Laine Stump (5): > conf: move virDomainDeviceInfo definition from domain_conf.h to > device_conf.h > conf: new functions to check if PCI address is wanted/present > conf: allow type='pci' addresses with no address attributes specified > bhyve: auto-assign addresses when <address type='pci'/> is specified > qemu: auto-assign addresses when <address type='pci'/> is specified > > docs/schemas/basictypes.rng | 8 +- > src/bhyve/bhyve_device.c | 10 +- > src/conf/device_conf.c | 6 +- > src/conf/device_conf.h | 132 ++++++++++++++++++++- > src/conf/domain_addr.c | 2 +- > src/conf/domain_conf.c | 13 +- > src/conf/domain_conf.h | 129 -------------------- > src/qemu/qemu_domain_address.c | 64 +++++----- > ...l2argv-aarch64-virtio-pci-manual-addresses.args | 4 +- > ...ml2argv-aarch64-virtio-pci-manual-addresses.xml | 5 + > .../qemuxml2argv-pci-autofill-addr.args | 25 ++++ > .../qemuxml2argv-pci-autofill-addr.xml | 35 ++++++ > tests/qemuxml2argvtest.c | 1 + > ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 5 + > .../qemuxml2xmlout-pci-autofill-addr.xml | 41 +++++++ > tests/qemuxml2xmltest.c | 1 + > 16 files changed, 298 insertions(+), 183 deletions(-) > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.args > create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-pci-autofill-addr.xml > create mode 100644 tests/qemuxml2xmloutdata/qemuxml2xmlout-pci-autofill-addr.xml > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list