Re: [libvirt] [PATCH 03/11] conf: Remove dubious code from virDomainPCIAddressSetGrow()

On 04/03/2018 07:12 PM, John Ferlan wrote: I would hope that such a reference wouldn't need to be explained :-) Sure you can - you just need properly sized large bare wires. This is saying that if the device you need a slot for is a pci-bridge (or has exactly the same connection requirements as a pci-bridge) and if the root bus is pcie-root then you need to add a dmi-to-pci-bridge (you wouldn't be in this function in the first place if you already *had* a dmi-to-pci-bridge, so there's no need to check if you have one). This would happen if someone manually added a pci-bridge device to a pure pcie topology. Normally I would say that this should stay, as I believe we *should* try to support partially-specified topologies as much as possible (since you've dealt with libvirt-qe bugzilla reports, you too know of some of the odd scenarios they come up with - e.g. removing one controller from a working config.). However, currently when an un-addressed pci-bridge is encountered (which is the only time we should get to this code), the search for a slot that accepts a pci-bridge will find an empty slot on the *that same pci-bridge* and we end up logging an error indicating that (I forget the exact error) - I could *swear* that at some point in the past it wasn't dead code, and that it actually helped to resolve a bug filed by libvirt-qe, but experimentation shows that certainly isn't the case now. In the meantime, if I remove that code (and don't apply any of your patches) then a pure pcie domain *can* be successfully edited to add a single pci-bridge (as long as you specify an index, *and* there is an unused index of a smaller value), but what ends up in the "proper/validated" config is a pci-bridge that is plugged directly into a pcie-root-port (which is also wrong, but silently so). So I guess I reserve my judgement until I see what happens with your entire series applied, which I'll do tomorrow morning.