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

[Cole Robinson]

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
>