Re: [libvirt] [PATCH 2/2] HACK: qemu: aarch64: Use virtio-pci if user specifies PCI controller

On 03/09/2016 09:54 AM, Daniel P. Berrange wrote: That's just a more limited case of the same type of feature creep though - you're specifying a preference for what type of address you want, just in a different place. (The odd thing is that we're adding it only to remedy what is apparently a temporary problem, and even then it's a problem that wouldn't exist if we just said simply this: "If virtio-mmio is specified, then use virtio-mmio; If no address is specified, use pci". This puts the burden on people insisting on the old / soon-to-be-deprecated (?) virtio-mmio address type to add a little something to the XML (and a very simple little something at that!). For a short time, this could cause problems for people who create new domains using qemu binaries that don't have a pci bus on aarch64/virt yet, but that will be easily solvable (by adding "<address type='virtio-mmio'/>", which is nearly as simple as typing "<address type='pci'/>"). A downside of setting your preference with <address type="pci"/> vs. having the preference in a separate element is that you can no longer cause a "re-addressing" of all the devices by simply removing all the <address> elements (and it's really that that got me thinking about adding hints/preferences in the <target> element). I don't know how important that is; I suppose when the producer of the XML is some other software it's a non-issue, when the producer is a human using virsh edit it would make life much simpler once in awhile (and the suggestion in the previous paragraph would eliminate that problem anyway). So is there a specific reason why we need to keep virtio-mmio as the default, and require explicitly saying "address type='pci'" in order to get a PCI address? First, I agree with your vigilance against unnecessary feature creep. Both because keeping things simple makes it easier to maintain and use, and also because it's very difficult (or even impossible) to change something once it's gone in, in the case that you have second thoughts. But, expanding beyond just the aarch64/virt mmio vs. pci problem (which I had already done in this thread, and which is what got us into this "feature creep" discussion in the first place :-)... I remember when we were first adding support for Q35 that we said it should be as easy to create a domain with Q35 machinetype as it is for 440fx, i.e. you should be able to do it without manually specifying any PCI addresses (that's why we have the strange default bus hierarchy, with a dmi-to-pci-bridge and a pci-bridge, and all devices going onto the pci-bridge). But of course 440fx is very simplistic - all slots are standard PCI and all are hotpluggable. On Q35 there could be room for choices though, in particular PCI vs PCIe and hotpluggable vs. not. (and yeah, also what NUMA node the bus is on; I agree that one is "getting out there"). So since there are choices, libvirt has by definition begun implementing policy when it auto-assigns *any* PCI address (current policy is "require all auto-assigned device addresses to be hotpluggable standard PCI). And if libvirt doesn't provide an easy way to choose, it will have implemented a defacto mandatory policy (unless you force full specification of PCI addresses, which goes against the "make it easy" goal). And the current policy is looking (for Q35 and aarch64/virt at least) less and less like the correct one - in the years since it was put in, 1) we've learned that qemu doesn't care, and will not ever care, that a PCI device has been plugged into a PCIe slot, 2) we now have support for several PCIe controllers we didn't previously have, 3) people have started complaining that their devices are in PCI slots rather than PCIe, 4) even though it was stated at the time I wrote the code to auto-add a pci-bridge to every Q35 domain that pci-bridge's failure to support hotplug was a temporary bug, I just learned yesterday that it apparently still doesn't work, and 5) some platforms (e.g. our favorite aarch64/virt) are emulating a hardware platform that has *never* had standard PCI slots, only PCIe. Beyond that, there is no place that provides a simple encoding of which type of controller provides which type of slot, and what is allowed to plug into what. If you require the management application/human to manually specify all the PCI addresses as soon as they have a need for one of these basic characteristics, then not only has it become cumbersome to define a domain (because the management app has to maintain a data structure to keep track of which PCI addresses are in use and which aren't), but it means that the management application also needs to know all sorts of rules about which PCI controllers are actually pcie vs. pci, and which accept hotplug devices vs. which don't, as well as things like the min/max slot number for each controller, and which ones can plug into where, e.g. a pcie-root-port can only plug into pcie-root or pcie-expander-bus, and a pcie-switch-downstream-port can only plug into a pcie-switch-upstream-port, etc. Requiring a management app to get all of that right just so that they can pick between a hotpluggable and non-hotpluggable slot seems like an overly large burden (and prone to error). In the end, if libvirt makes it simple for the management app to specify what kind of slot it wants, rather than requiring it to specify the exact slot, then libvirt isn't making any policy decisions, it's just making it easier for the management app to implement its own policy decisions, without requiring the management app to know all the details about which controller implements what kind of connection etc, and that does seem to fit within libvirt's purpose.