On Wed, Oct 05, 2016 at 01:26:31PM -0400, Laine Stump wrote: Note that changes to libvirt conf files are not usable by libguestfs. The setting would need to go into the XML, and please also make it possible to determine if $random version of libvirt supports the setting, either by a version check or something in capabilities. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com Fedora Windows cross-compiler. Compile Windows programs, test, and build Windows installers. Over 100 libraries supported. http://fedoraproject.org/wiki/MinGW

On 10/06/2016 06:58 AM, Andrea Bolognani wrote: We need to have *something* new in the config related to this, otherwise it will be impossible to add new "available for hotplug" ports at the same time as new unaddressed endpoint devices are added (unless an extra port is added for each endpoint device as well, and of course that is assuming that the user knows about things like auto-added memory balloon devices, and which devices are going to be PCIe and which will be legacy PCI, etc). If the "available" ports aren't somehow marked, the endpoint devices will end up being placed on them. I looked in an example domain config Rich pastebin-ed in IRC yesterday. These are the PCI devices he has: * <controller type='scsi' model='virtio-scsi'> - currently put on a pci-bridge, but will end up on a root-port after my patches * <controller type='usb' model='blah'> x 4 - a set of USB2 controllers. This will turn into a single USB3 controller on a root-port after my patches. Alternately, since it seems you don't use it, you could eliminate it with: <controller type='usb' model='none'/> * <controller type='sata'> - this is fixed on 00:1f.2, and there's nothing we can do to get rid of it or even change its address * <controller type='pci' model='dmi-to-pci-bridge'> - the need for this will be eliminated by my patches * <controller type='pci' model='pci-bridge'> - also eliminated * <controller type='virtio-serial'> - currently on the pci-bridge, but will move to a root-port after my patches * <memballoon model='virtio'> - Rich, do you actually use this device? Seems doubtful. If not, then add <memballoon model='none'/> to your config to prevent it. After getting rid of the controllers that will be eliminated by my patches, and the "default" devices that you can get rid of with manual intervention (model='none'), you'll be left with the following devices on pcie-root-ports which *could* be manually moved to pcie-root (thus eliminating one pcie-root-port from the domain): 1) virtio-scsi controller 2) virtio-serial controller and nothing else. Manually address those two to be on bus 0 (pcie-root), and (with my patches) you've reduced your PCI device+controller count from the current 10 down to 3 (including the sata controller).

On 10/06/2016 11:20 AM, Richard W.M. Jones wrote: For 440fx it makes no difference. For Q35 it can eliminate the "extra PCI controllers" - current upstream libvirt will add a dmi-to-pci-bridge, then a pci-bridge plugged into that, and add your PCI devices to that; after my in-progress patches, libvirt will add a pcie-root-port for each virtio device, and plug them into those. If you manually address the devices to "some unused slot on bus 0", then they will be directly plugged into pcie-root, and no extra controllers will be needed. As far as recommendations, I guess you could manually assign addresses for those two devices that would otherwise be open in both 440fx and Q35. Generally 00:1 (chipset devices) and 00:2 (video) are in use on a 440fx domain, and 00:1 (video) and 00:1F (chipset devices) on a Q35. If you don't disable USB, then USB controllers will also be added at 00:3 (?) on 440fx) and 00:1D on Q35; but you don't use USB so you don't need to worry about this. So you could just manually assign the virtio-scsi and virtio-serial devices to have these two addresses: <address type='pci' bus='0' slot='3'/> <address type='pci' bus='0' slot='4'/> (domain and function default to '0' if not specified - in older libvirt they were required by the RNG, but otherwise optional, in new libvirt they're completely optional). That should work for both machinetypes and not cause any of the stupid outdated "you won't be able to add a video device in the future!" warnings.

On 10/06/2016 11:31 AM, Daniel P. Berrange wrote: That works for the initial definition of the domain, but as soon as you've saved it once, there will be controllers explicitly in the config, and since we don't have any way of differentiating between auto-added controllers and those specifically requested by the user, we have to assume they were explicitly added, so such a check is then meaningless because you will *always* have PCI controllers. Say you create a domain definition with no controllers, you would get enough for the devices in the initial config, plus "N" more empty root ports. Let's say you then add 4 more devices (either hotplug or coldplug, doesn't matter). Those devices are placed on the existing unused pcie-root-ports. But now all your ports are full, and since you have PCI controllers in the config, libvirt is going to say "Ah, this user knows what they want to do, so I'm not going to add any extras! I'm so smart!". This would be especially maddening in the case of "coldplug", where libvirt could have easily added a new controller to accomodate the new device, but didn't. Unless we don't care what happens after the initial definition (and then adding of "N" new devices), trying to behave properly purely based on whether or not there are any PCI controllers present in the config isn't going to work. (BTW, isn't there something wrt aarch64 about "no pci controllers in config means use mmio for devices", or something like that? (Or maybe we were just *thinking* about that and didn't actually do it, I don't remember). Using the lack of PCI controllers in the config to imply "automatically add necessary + extra controllers" would directly conflict with that.)

On 10/06/2016 12:10 PM, Daniel P. Berrange wrote: Even if it was adding offline, and there wasn't any place to put a new device? (i.e., the operation would fail without adding a controller, and libvirt was able to add it). Or am I taking your statement beyond its intent (I'm good at that :-) Do you mean use of the term "coldplug" at all, or talking about what happens when you add a device to the persistent config of the domain but not to the currently running guest itself? Right. That works great for adding devices "offline" (since you don't like the term "coldplug" :-). It's just in the case of hotplug that it's problematic, because you can't add new PCI controllers to a running system (big digression here - skip if you like...) (well, it *could* be possible to hotplug an upstream port plus some number of downstream ports, but qemu doesn't support it because it attaches devices one by one, and guest has to be notified of the entire contraption at once when the upstream port is attached - so you would have to send the attach for all the downstream ports first, then the upstream, but you *can't* do it in that order because then qemu doesn't yet know about the id (alias) you're going to give to the upstream port at the time you're attaching the downstreams. Anyway, even if and when qemu does support hotplugging upstream+downstream ports, you can't add more downstream ports to an existing upstream afterwards, so you would end up with some horrid scheme where you had to always make sure you had at least one open downstream or root-port open every time a device was added.) Or it's because there have already been "N" new devices added since the domain was defined, and they're now trying to *hotplug* device "N+1". I'm fine with that behavior, I just want to make sure everyone understands this restriction beforehand. So here's a rewording of your description (with a couple additional conditions) to see if I understand everything correctly: 1) during initial domain definition: A) If there are *no pci controllers at all* (not even a pci-root or pcie-root) *and there are any unaddressed devices that need a PCI slot* then auto-add enough controllers for the requested devices, *and* make sure there are enough empty slots for "N" (do we stick with 4? or make it 3?) devices to be added later without needing more controllers. (So, if the domain has no PCI devices, we don't add anything extra, and also if it only has PCI devices that already have addresses, then we also don't add anything extra). B) if there is at least one pci controller specified in the XML, and there are any unused slots in the pci controllers in the provided XML, then use them for the unaddressed devices. If there are more devices that need an address at this time, also add controllers for them, but no *extra* controllers. (Note to Rich: libguestfs could avoid the extra controllers either by adding a pci-root/pcie-root to the XML, or by manually addressing the devices. The latter would actually be better, since it would avoid the need for any pcie-root-ports). 2) When adding a device to the persistent config (i.e. offline): if there is an empty slot on a controller, use it. If not, add a controller for that device *but no extra controllers* 3) when adding a device to the guest machine (i.e. hotplug / online), if there is an empty slot on a controller, use it. If not, then fail. The differences I see from what (I think) you suggested are: * if there aren't any unaddressed pci devices (even if there are no controllers in the config), then we also don't add any extra controllers (although we will of course add the pci-root or pcie-root, to acknowledge it is there). * if another controller is needed for adding a device offline, it's okay to add it.

On 10/07/2016 11:16 AM, Andrea Bolognani wrote: A better way to put it is that we guarantee there will be "N" (3 or 4 or whatever) slots available when the domain is originally defined. Once any change has been made, all bets are off. Yeah, if they know enough to be adding a root-port, then they know enough to add extras. I thought about that; I'm not sure. In the case of libguestfs, even if Rich added a pcie-root, I guess he would still be manually addressing his pci devices, so that would be clue enough that he knew what he was doing and didn't want any extra.

On 10/10/2016 05:27 AM, Andrea Bolognani wrote: I implemented this over the weekend, and it turns out that it's much less disruptive to ignore whether the pcie-root controller was added by the user or by libvirt. This is because pcie-root is added much earlier (when we are adding the implicit/default devices for the domain), so by the time we get to the function that assigns PCI addresses (which is where we auto-add all the additional PCI controllers) we no longer have any information about whether pcie-root was specified in the XML, or if it was auto-added. The way I have it implemented now is to add the extra controllers if the initial controller count was <= 1 *and* any new PCI controllers were auto-added during address assignment (which implies that an unaddressed device needed an address, so we added a controller to satisfy it). I suppose if I want to be pedantic, I need to actually keep track of whether or not there were any unaddressed devices (in the case that the only devices they requested were primary video + USB2 + SATA controller, no new PCI controllers would be added).