+ if (nbuses > 0 && !addrs->buses[0].model) { + if (virDomainPCIAddressBusSetModel(&addrs->buses[0], + VIR_DOMAIN_CONTROLLER_MODEL_PCI_ROOT) < 0)
+ } Shouldn't we use either PCI_ROOT or PCIE_ROOT based on the machine type here? And set hasPCIeRoot *after* doing so? Sorry for the questions, I guess this is the point in the
+ goto error; patch where I got a bit lost :( I'm afraid you missed this question :) Sorry about the omission. I've tried to limit the code that decides whether or not there should be a pci-root or a
On Thu, 2016-10-13 at 13:43 -0400, Laine Stump wrote: pcie-root to the one
place when default controllers are being added (I forget the name of the function right now),
I guess you mean qemuDomainDefAddDefaultDevices()? That's the function where pci{,e}-root is added, if not already present in the configuration.
and after that only decide based on whether a pci-root or pcie-root really is there in the config. My subconscious reasoning for this is that the extra calisthenics around supporting aarch64/virt with PCI(e) vs with mmio has made me wonder if there might be machinetypes in the future that could support one type of root bus or another (or none) according to what is in the config, rather than having a root bus just builtin to the machine. I don't know if that would ever happen, but if it did, this code would work without change - only the function adding the default controllers would need to be changed. (Note that I used the same logic when deciding how to right qemuDomainMachineHasPCI[e]Root())(still not sure about that name, but it can always be changed later to remove the "Machine" if someone doesn't like it)
That makes sense. My point is that the code above, if I'm reading it correctly, sets the model of bus 0 to PCI_ROOT if it's not already set. But 1) qemuDomainDefAddDefaultDevices() mentioned above should already have added pci{,e}-root to @def 2) if that's not the case, we should use either PCI_ROOT or PCIE_ROOT based on what's appropriate for the machine type Looking at the code in qemuDomainDefAddDefaultDevices() it seems like we would never find ourselves in the situation where pci{,e}-root is needed but not present in @def by the time qemuDomainPCIAddressSetCreate() is called, so I think that chunk of code should just be removed.
A DMI-to-PCI bridge with index='1' has been added automatically here... ... but it's not being used by any of the devices. So why would it be added in the first place? That is a *very* good question! Can't wait to know the answer! ;) Oh, now that I've looked in context of the patch ordering, I undestand: it's because patch 16/18 hasn't been applied yet. I'd forgotten the ordering...
Right you are :) -- Andrea Bolognani / Red Hat / Virtualization