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
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
