[libvirt] [PATCH 3/4] tests: Test generic PCIe Root Ports

[Laine Stump]

On 03/16/2017 10:37 AM, Andrea Bolognani wrote:
> 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.

It was the original test added when we added support for pcie-root-port,
and the idea behind the test was to test automatic and manual setting of
every associated attribute (i.e. it's not a special purpose test
intended just to test manual setting of the attributes in <target>, if
that's what you're implying.


> 
>>> +    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 ;)

Yeah, I don't know how I missed that.

> 
>> 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
>