Re: [PATCH v2 5/9] qemu: Deny all but VFIO PCI backends in hostdev prepare phase

On 4/24/23 13:11, Martin Kletzander wrote: > On Mon, Apr 24, 2023 at 12:41:44PM +0200, Michal Privoznik wrote: >> We used to support KVM and VFIO style of PCI assignment. The >> former was dropped in v5.7.0-rc1~103 and thus we only support >> VFIO. All other backends lead to an error (see >> qemuBuildPCIHostdevDevProps(), or qemuBuildPCIHostdevDevStr() as >> it used to be called in the era of aforementioned commit). >> >> Might as well report the error in prepare phase and save hassle >> of proceeding with device preparation (e.g. in case of hotplug >> overriding the device's driver, setting seclabels, etc.). >> >> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com&gt; >> --- >> src/qemu/qemu_domain.c | 7 +++---- >> 1 file changed, 3 insertions(+), 4 deletions(-) >> >> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c >> index 58cd3dd710..72f36c807b 100644 >> --- a/src/qemu/qemu_domain.c >> +++ b/src/qemu/qemu_domain.c >> @@ -11326,12 +11326,11 @@ >> qemuDomainPrepareHostdevPCI(virDomainHostdevDef *hostdev, >>         break; >> >>     case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: >> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", >> -                       _("host doesn't support legacy PCI >> passthrough")); >> -        return false; >> - >>     case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: >>     case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: >> +        virReportError(VIR_ERR_INTERNAL_ERROR, >> +                       _("invalid PCI passthrough type '%1$s'"), >> +                       >> virDomainHostdevSubsysPCIBackendTypeToString(*backend)); > > This is a bit misleading as it is not "invalid", it's just unsupported, > and on top of that you're changing it to internal error and I think it > should still be CONFIG_UNSUPPORTED, at least for KVM and XEN.  _LAST > (and possibly default:) is still an internal error of course. Fair enough. How about this then? case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("host doesn't support legacy PCI passthrough")); return false; case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("QEMU does not support device assignment mode '%1$s'"), virDomainHostdevSubsysPCIBackendTypeToString(*backend)); return false; default: case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: virReportEnumRangeError(virDomainHostdevSubsysPCIBackendType, *backend); break; } I have this as a fixup patch on the top of this one (well, this is how it looks after the patch), locally.