[libvirt] [PATCH 0/6] auto-assign addresses when <address type='pci'/> is specified
Cole Robinson
crobinso at redhat.com
Thu May 19 17:15:28 UTC 2016
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
>
More information about the libvir-list
mailing list