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