[libvirt] [PATCH v6 11/17] [RFC] qemu: if pci-bridge is in PCIe config w/o dmi-to-pci-bridge, add it

[Laine Stump]

On 11/09/2016 11:26 AM, Andrea Bolognani wrote:
> On Mon, 2016-11-07 at 14:50 -0500, Laine Stump wrote:
>> I'm undecided if it is worthwhile to add this...
>>   
>> Up until now it has been legal to have something like this in the xml:
>>   
>>     <controller type='pci' model='pcie-root' index='0'/>
>>     <controller type='pci' model='pci-bridge' index='2'/>
>>   
>> (for example, see the existing test case
>> "usb-controller-default-q35").  This is handled in
>> qemuDomainPCIAddressSetCreate() when it's adding in controllers to
>> fill holes in the indexes.)
>>   
>> Since we have always added the following to every config:
>>   
>>     <controller type='pci' model='dmi-to-pci-bridge' index='1'/>
>>   
>> there has always been a proper place for the unaddressed pci-bridge to
>> plug in.
>>   
>> A upcoming commit will eliminate the forced adding of a
>> dmi-to-pci-bridge to *every* Q35 domain config, though. This would
>> mean the above-mentioned test case (and one other) would begin to
>> fail. There are two possible solutions to this problem:
>>   
>> 1) change the test cases, and made the above construct an error (note
>> that in the future it will work just fine to not list *any* pci
>> controller, and they will all be auto-added as needed).
>>   
>> or
>>   
>> 2) This patch, which specifically looks for the case where we have a
>> pci-bridge (but no dmi-to-pci-bridge) in a PCIe domain config and
>> auto-adds the necessary dmi-to-pci-bridge. This can only work in the
>> case that the pci-bridge has been given an index such that there is an
>> unused index with a lower value for the dmi-to-pci-bridge to occupy.
>>   
>> This seems like a kind of odd corner case that nobody should need to
>> do in the future anyway. On the other hand, I already wrote the code
>> and it works, so I might as well send it and see what opinions (if
>> any) it generates.
> I say let's forget about this.

Forgotten. I may need to replace this patch with one that changes the 
test case. Alternately I could change the test case in the same patch 
that breaks it. Yeah, I'll probably do the latter.

>
> I'm fairly certain nobody is using this ludicrous pattern
> outside of our test suite, and even if they did their guests
> configurations would already have had the dmi-to-pci-bridge
> added long ago, so no risks of breakage there.
>
> So, unless somebody has very compelling arguments for
> maintaining such a "feature", consider this a
>
> NACK
>
> -- 
> Andrea Bolognani / Red Hat / Virtualization
>