Re: [libvirt] [PATCH 4/5] qemu: Check if PCI passhtrough is available in the host when starting

On 09/20/13 13:57, Daniel P. Berrange wrote: > On Fri, Sep 20, 2013 at 11:06:59AM +0200, Peter Krempa wrote: >> Check the presence of the selected PCI passthrough option when starting >> a VM. >> --- >> src/qemu/qemu_process.c | 41 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 41 insertions(+) >> >> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c >> index dd16f6c..e36ab99 100644 >> --- a/src/qemu/qemu_process.c >> +++ b/src/qemu/qemu_process.c >> @@ -3481,6 +3481,8 @@ int qemuProcessStart(virConnectPtr conn, >> unsigned int stop_flags; >> virQEMUDriverConfigPtr cfg; >> virCapsPtr caps = NULL; >> + int supportsPassthroughKVM = -1; >> + int supportsPassthroughVFIO = -1; >> >> /* Okay, these are just internal flags, >> * but doesn't hurt to check */ >> @@ -3713,6 +3715,45 @@ int qemuProcessStart(virConnectPtr conn, >> goto cleanup; >> } >> >> + /* assign defaults for hostdev passthrough */ >> + for (i = 0; i < vm->def->nhostdevs; i++) { >> + virDomainHostdevDefPtr hostdev = vm->def->hostdevs[i]; >> + >> + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && >> + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { >> + int *backend = &hostdev->source.subsys.u.pci.backend; >> + >> + /* cache host state of passthrough support */ >> + if (supportsPassthroughKVM == -1 || supportsPassthroughVFIO == -1) { >> + supportsPassthroughKVM = qemuHostdevHostSupportsPassthroughLegacy(); >> + supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); >> + } >> + >> + switch ((virDomainHostdevSubsysPciBackendType) *backend) { >> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: >> + if (!supportsPassthroughVFIO) { >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("host doesn't support VFIO PCI passthrough")); >> + goto cleanup; >> + } >> + break; >> + >> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: >> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: >> + if (!supportsPassthroughKVM) { >> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> + _("host doesn't support legacy PCI passthrough")); >> + goto cleanup; >> + } >> + >> + break; >> + >> + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: >> + break; >> + } >> + } >> + } > It seems like the body of this loop ought to be shared with the hotplug > code, in a helper function, so we don't have so much extra code in this > start method Yeah, the basic idea of the code is the same between those two places. The issue with the hotplug code path is that it's using only one device, whereas here we iterate through a list. I will change this to a common func where the hotplug code will pass a fake array of just the single device.