Re: [libvirt] [PATCH 4/6] conf: new <hotplug require='yes|no'/> element for hotpluggable devices

On 08/09/2016 04:05 AM, Daniel P. Berrange wrote: It also has to know the rules about which controllers support what type of devices, and which controllers can plug into which other controllers - the full topology of PCI controllers will need to be spelled out specifically in advance, so that the management app can give the index number of the correct controller. As it stands now, some amount of this is already necessary, just because libvirt currently will only automatically add pci-bridge devices when there aren't enough PCI slots available (the code to automatically add pcie controllers as necessary is yet to be written). Still, this is all it takes to get 8 properly connected downstream-ports: <controller type='pci' model='pcie-root-port'/> <controller type='pci' model='pcie-switch-upstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> <controller type='pci' model='pcie-switch-downstream-port'/> (note that you don't need to say where they are connected, nor do you need to give an index to each one. As long as there is an available slot of the correct type for each controller as it's encountered, libvirt will hook them up and index them correctly) If the controller number was needed to specify in the device address, then not only would each controller need to be manually given an index (if you were specifying the entire domain in a single go), but each device would need to be given a unique setting - instead of just saying "I want one of these" (in the future, if there isn't "one of those" available, everything necessary to provide it will be automatically created), you have to say "I want *this* one, implying that you know everything about what "this one" provides (and you will need to manually setup "this one" and everything else required to provide it). Also, note that since each pci-root-port and pci-switch-downstream-port has only a single slot, in those cases this: <address type="pci" controller="1"/> is really just: <address type="pci" bus="1"/> and for the other cases (pci[e]-root, pci-bridge, dmi-to-pci-bridge) you will still have to keep track of how many devices you've assigned to a particular controller, to make sure that your request doesn't overflow the 31 (or 32) device limit (which brings up another problem of doing it this way - different controllers have different numbers of useful slots, so the management app will have to know about that too). And pcie/pci info on every endpoint device (I agree with your assertion that we should treat every device as potentially capable of hotplug, so we don't need that info for endpoints). The metadata would need to include: 1) upstream connection type (each type of PCI controller needs a different connection type. I've learned through all this that each controller has different rules about what it can be connected to. So far there are 9 different types, including pci-endpoint and pcie-endpoint) 2) accepted downstream connection types 3) min and max slot (currently known possibilities: 0-0, 0-31, or 0-32) 4) hotplug supported or not (yes, the management app really would need to know about this, even if they intended for all devices to be hotpluggable - they need to be able to avoid pcie-root and dmi-to-pci-bridge) Then the management app would need to keep track of which addresses are in use for each controller, so that it would be able to avoid putting too many devices on any controller. Also, the management app will now need to know for *every* device whether that device in this particular version of qemu is a PCIe device, a legacy PCI device, or one of the silly "dual mode" devices that changes according to the type of bus it's plugged into. Otherwise it won't know which controller it should plug the device into. Finally, the management app would need to add logic to grow the bus topology correctly as needed. Sometimes this is simple and sometimes not. For example, if there is an open slot available on pcie-root, you can get another hotpluggable pcie slot by simply adding a pcie-root-port. If not, then you need to find an open pcie-switch-downstream-port or pcie-root-port, then connect a pcie-switch-upstream-port to that, and connect one or more pcie-switch-downstream-ports to that. (note this implies that you should always maintain at least one unoccupied root-port, downstream-port, or an open slot on pcie-root). The whole point of my exercise has been to 1) get rid of as much legacy PCI baggage as possible, and 2) do this in a way that prevents/eliminates the need for multiple management apps to re-implement and maintain essentially the same code (especially since my trip through PCIe-land has shown me that it is fraught with lack of documentation (about the controller implementations, not the PCIe spec), misinformation (changing over time), misconceptions, and lots of annoying little details). I started out kind of liking your alternate idea, since it seemed to accomplish that in a simpler way that didn't require adding yet another knob later. But after thinking it through, I think that it provides very little extra functionality, and would require a lot of extra work for the management app, even in a simple case where you didn't care about PCI vs PCIe. In the end, my opinion is that the job of creating a proper PCIe topology and assigning devices to the correct address in that topology is a non-trivial job that should be implemented in one place. I think libvirt is in the proper place to do that, it just needs to be provided with a small amount of information from the user/management app so that libvirt won't be making any policy decisions, but just assuring that those policies are properly applied.