On Mon, 2016-11-21 at 00:01 -0500, Laine Stump wrote:
Although nearly all host devices that are assigned to guests using vfio ("<hostdev>" devices in libvirt) are physically PCI Express
s/vfio/VFIO/ Same for the subject. [...]
@@ -558,8 +558,88 @@ qemuDomainDeviceCalculatePCIConnectFlags(virDomainDeviceDefPtr dev, return 0; } - case VIR_DOMAIN_DEVICE_HOSTDEV: - return pciFlags; + case VIR_DOMAIN_DEVICE_HOSTDEV: { + virDomainHostdevDefPtr hostdev = dev->data.hostdev; + + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) {
How about inverting this condition and returning 0 immediately unless if the wrong mode or type is used? You could get rid of one indentation level that way, and the function already has a gazillion return statements anyway. [...]
+ if (virDeviceInfoPCIAddressPresent(hostdev->info)) { + /* A guest-side address has already been assigned, so + * we can avoid reading the PCI config, and just use + * pcieFlags, since the pciConnectFlags checking is + * more relaxed when an address is already assigned + * than it is when we're looking for a new address (so + * validation will pass regardless of whether we set + * the flags to PCI or PCIE).
s/PCIE/PCIe/ [...]
+ /* If we are running with privileges, we can examine the + * PCI config contents with virPCIDeviceIsPCIExpress() for + * a definitive answer. + */ + isExpress = virPCIDeviceIsPCIExpress(pciDev); + virPCIDeviceFree(pciDev); + + if (isExpress) + return pcieFlags; + + return pciFlags; + }
Empty line here, please.
+ return 0; + } case VIR_DOMAIN_DEVICE_MEMBALLOON: switch ((virDomainMemballoonModel) dev->data.memballoon->model) {
ACK (with the nits fixed) regardless of whether you decide to go with my suggestion of reducing the indentation level. -- Andrea Bolognani / Red Hat / Virtualization