Re: [libvirt] [PATCH v2 4/4] qemu: assign vfio devices to PCIe addresses when appropriate
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