Re: [libvirt] [PATCH v3 01/11] conf: fix fromConfig argument to virDomainPCIAddressReserveAddr()
On Mon, 2016-12-19 at 10:25 -0500, Laine Stump wrote:
Although setting virDomainPCIAddressReserveAddr()'s
fromConfig=true is
correct when a PCI addres is coming from a domain's config, the *true*
purpose of the fromConfig argument is to lower restrictions on what
kind of device can plug into what kind of controller - if fromConfig
is true, then a PCIE_DEVICE can plug into a slot that is marked as
only compatible with PCI_DEVICE (and vice versa), and the HOTPLUG flag
is ignored.
For a long time there have been several calls to
virDomainPCIAddressReserveAddr() that have fromConfig incorrectly set
to false - it's correct that the addresses aren't coming from user
config, but they are coming from hardcoded exceptions in libvirt that
should, if anything, pay *even less* attention to following the
pciConnectFlags (under the assumption that the libvirt programmer knew
what they were doing).
It seems to me that many issues with incorrect usage of the
fromConfig parameter can be traced back to the fact that it's
being assigned two related but not overlapping meanings:
* the address being assigned or validated comes from the
user configuration and error reporting should be worded
accordingly
* use more relaxed rules when assigning or validating the
address, eg. allow legacy PCI devices to be plugged into
PCIe slots and whatnot
The former basically implies the latter, because we assume
the user[1] knows what they are doing; however, the spots
you're touching with this patch only call for the latter
behavior.
[...]
@@ -591,7 +591,7 @@
virDomainPCIAddressReserveSlot(virDomainPCIAddressSetPtr addrs,
virPCIDeviceAddressPtr addr,
virDomainPCIConnectFlags flags)
{
- return virDomainPCIAddressReserveAddr(addrs, addr, flags, false);
+ return virDomainPCIAddressReserveAddr(addrs, addr, flags, true);
}
All uses of virDomainPCIAddressReserveSlot() in the QEMU
driver are for devices we're adding ourselves with explicit
PCI addresses, and those in the bhyve driver are for devices
that come from the guest configuration, so this change
should be safe.
That said, it leads to a weird situation in which
ReserveSlot() implies fromConfig=true and ReserveNextSlot()
implies fromConfig=false, which is very confusing.
I know you're going to rename and change both functions
further down the series so it's not a big issue for now.
ACK
[1] Just like libvirt programmers ;)
--
Andrea Bolognani / Red Hat / Virtualization