
On 10/10/2016 05:27 AM, Andrea Bolognani wrote:
On Fri, 2016-10-07 at 12:11 -0400, Laine Stump wrote:
So instead of guaranteeing that there will always be an empty slot available for hotplug during a single start/destroy cycle of the guest, we would be guaranteeing that there will be 3/4 empty slots available for either hotplug or coldplug throughout the entire life of the guest.
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. Okay, your alternative reading matches with my understanding of your proposal :)
Sounds like a pretty good compromise to me.
The only problem I can think of is that there might be management applications that add eg. a pcie-root in the XML when first defining a guest, and after the change such guests would get zero hotpluggable ports. Then again it's probably okay to expect such management applications to add the necessary number of pcie-root-ports themselves.
Yeah, if they know enough to be adding a root-port, then they know enough to add extras. Note that I wrote pcie-root, not pcie-root-port.
Maybe we could relax the wording on A) and ignore any pci{,e}-root? Even though there is really no reason for either a user or a management application to add them explicitly when defining a guest, I feel like they might be special enough to deserve an exception.
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. Yeah, I'm not sure about that either, I just felt like it would be good to think about it. I don't really see problems going one way or the other, and I guess your initial proposal is slightly more strict, so we might as well go for it and relax it later if a compelling use case is found.
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).