Re: [libvirt] [PATCH 3/4] tests: Test generic PCIe Root Ports
On Wed, 2017-03-15 at 17:49 -0400, Laine Stump wrote:
[...]
> @@ -3,7 +3,7 @@
> <uuid>11dbdcdd-4c3b-482b-8903-9bdb8c0a2774</uuid>
> <memory unit='KiB'>2097152</memory>
> <currentMemory unit='KiB'>2097152</currentMemory>
> - <vcpu placement='static' cpuset='0-1'>2</vcpu>
> + <vcpu placement='static'>2</vcpu>
You made some other changes to the input XML beyond just the differences
in root ports. Mostly they're innocuous and easy to verify, but...
[...]
> </controller>
> <controller type='pci' index='2'
model='pcie-root-port'>
> <model name='ioh3420'/>
> - <target chassis='40' port='0x1a'/>
> + <target chassis='2' port='0x11'/>
...you removed the <target chassis='40' port='0x1a'/> from the
input
file, but that was there for a reason - it was in the test to assure
that non-default values specified for chassis and port would be honored.
Please put that back in.
I'll just leave it alone and add a comment about its purpose
instead. Possibly change the name so it reflects the idea
behind the test a little bit better.
> + DO_TEST("pcie-root-port-q35",
> QEMU_CAPS_DEVICE_IOH3420,
If we were going to continue using ioh3420 for Q35, I would suggest that
you should add QEMU_CAPS_DEVICE_PCIE_ROOT_PORT here to verify that the
output still uses ioh3420.
It's there, exactly for that purpose ;)
But as I said earlier I think we should
switch Q35 to using the generic root port too, so.... you *still* should
add that CAP, and change the expected output (and add a separate
"...-q35-old" test that doesn't have the cap for the generic root port)
Yeah, I'll have just two new tests, one where the capability
for the new controller is present and one where it's not.
--
Andrea Bolognani / Red Hat / Virtualization