Re: [libvirt] [PATCH v2 6/6] tests: qemu: More aarch64 virtio and pci tests

On 02/09/2016 10:59 AM, Cole Robinson wrote: > Clarify the point of some of the test cases by renaming them. Add more > xml2xml tests. > --- > ...> qemuxml2argv-aarch64-virtio-pci-default.args} | 0 > ...=> qemuxml2argv-aarch64-virtio-pci-default.xml} | 0 > ...2argv-aarch64-virtio-pci-manual-addresses.args} | 0 > ...l2argv-aarch64-virtio-pci-manual-addresses.xml} | 0 > tests/qemuxml2argvtest.c | 11 +++- > .../qemuxml2xmlout-aarch64-aavmf-virtio-mmio.xml | 6 +- > .../qemuxml2xmlout-aarch64-virtio-pci-default.xml | 69 > ++++++++++++++++++++++ > ...2xmlout-aarch64-virtio-pci-manual-addresses.xml | 53 +++++++++++++++++ > tests/qemuxml2xmltest.c | 17 +++++- > 9 files changed, 152 insertions(+), 4 deletions(-) > rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-mmio-default-pci.args > => qemuxml2argv-aarch64-virtio-pci-default.args} (100%) > rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-mmio-default-pci.xml > => qemuxml2argv-aarch64-virtio-pci-default.xml} (100%) > rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-virtio-pci.args => > qemuxml2argv-aarch64-virtio-pci-manual-addresses.args} (100%) > rename tests/qemuxml2argvdata/{qemuxml2argv-aarch64-virtio-pci.xml => > qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml} (100%) > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-default.xml > create mode 100644 > tests/qemuxml2xmloutdata/qemuxml2xmlout-aarch64-virtio-pci-manual-addresses.xml > > diff --git > a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.args > b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args > similarity index 100% > rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.args > rename to tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.args > diff --git > a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.xml > b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml > similarity index 100% > rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-mmio-default-pci.xml > rename to tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-default.xml > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.args > b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args > similarity index 100% > rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.args > rename to > tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.args > diff --git a/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.xml > b/tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml > similarity index 100% > rename from tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci.xml > rename to > tests/qemuxml2argvdata/qemuxml2argv-aarch64-virtio-pci-manual-addresses.xml I'll take your word that the new names are more proper :-) > diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c > index 92043d2..3c7693b 100644 > --- a/tests/qemuxml2argvtest.c > +++ b/tests/qemuxml2argvtest.c > @@ -1650,13 +1650,20 @@ mymain(void) > QEMU_CAPS_NODEFCONFIG, QEMU_CAPS_DTB, > QEMU_CAPS_DEVICE_VIRTIO_MMIO, > QEMU_CAPS_DEVICE_VIRTIO_RNG, QEMU_CAPS_OBJECT_RNG_RANDOM); > - DO_TEST("aarch64-mmio-default-pci", > + > + /* Demonstrates the virtio-pci default... namely that there isn't any! > + q35 style PCI controllers will be added if the binary supports it, This reminds me that the code that was originally intended only for Q35 will automatically add a dmi-to-pci-bridge and a pci-bridge in addition to the pcie-root. While these other controllers are apparently supported on arm too, an email exchange awhile back with [somebody, I forget who] pointed out this isn't really necessary, so I wonder if we really want it that way (again, this is code in qemuDomainDefAddDefaultDevices()) - maybe arm only wants to add pcie-root but not the others (of course we would also need to make the slot assignment code allow auto-assigning to pcie-root, or maybe put in a pcie-switch-upstream-port and a collection of pcie-switch-downstream-ports as necessary (similar to how we automatically add pci-bridges as necessary today). But that is of course *not* the subject of this patch (although doing what I describe will add churn to the files being touched here), so I mention it only in passing.

I like the added comments describing what is being tested! That should be done more often! (either in the .c file, or maybe we could add some metadata to the xml test files describing what's being tested. That might prevent someone from unknowingly removing some important bit of a test case some day). ACK