[PATCH v2 0/9] qemu: Move <hostdev> preparation into qemuDomainPrepareHostdev()
v2 of: https://listman.redhat.com/archives/libvir-list/2023-April/239378.html diff to v1: - More patches - Dropped qemuHostdevPreparePCIDevicesCheckSupport() completely - Dropped virQEMUCaps passing (patch 9/9) - Dropped even more checks effectively dead code Michal Prívozník (9): qemuDomainAttachHostDevice: Prepare device early and for all types qemu_domain: Move internals of qemuDomainPrepareHostdev() into a separate function qemu: Move <hostdev/> PCI backend setting into qemuDomainPrepareHostdev() qemuxml2argvtest: Drop needless PCI backend setting qemu: Deny all but VFIO PCI backends in hostdev prepare phase qemu_hotplug: Drop PCI backend check in qemuDomainAttachHostPCIDevice() qemu: Move <hostdev> SCSI path generation into qemuDomainPrepareHostdev() qemu: Remove empty functions qemu: Stop virQEMUCaps propagation into qemuHostdevPreparePCIDevices() src/qemu/qemu_domain.c | 167 ++++++++++++++++++++++++++++----------- src/qemu/qemu_driver.c | 56 ------------- src/qemu/qemu_hostdev.c | 67 +--------------- src/qemu/qemu_hostdev.h | 2 - src/qemu/qemu_hotplug.c | 43 ++-------- src/qemu/qemu_process.c | 60 +------------- src/qemu/qemu_process.h | 3 - tests/qemuxml2argvmock.c | 10 +++ tests/qemuxml2argvtest.c | 28 ------- 9 files changed, 142 insertions(+), 294 deletions(-) -- 2.39.2
When attaching a hostdev of a SCSI subsys, qemuDomainPrepareHostdev() is called. This makes sense because the function prepares just SCSI hostdevs ignoring others. But this will soon change. Thefore, move the function call out of qemuDomainAttachHostSCSIDevice() and into qemuDomainAttachHostDevice(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 53a0874556..64c62ea114 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2485,9 +2485,6 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriver *driver, qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1); - if (qemuDomainPrepareHostdev(hostdev, priv) < 0) - goto cleanup; - if (qemuProcessPrepareHostHostdev(hostdev) < 0) goto cleanup; @@ -2769,6 +2766,9 @@ qemuDomainAttachHostDevice(virQEMUDriver *driver, return -1; } + if (qemuDomainPrepareHostdev(hostdev, vm->privateData) < 0) + return -1; + switch (hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: if (qemuDomainAttachHostPCIDevice(driver, vm, -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:40PM +0200, Michal Privoznik wrote:
When attaching a hostdev of a SCSI subsys, qemuDomainPrepareHostdev() is called. This makes sense because the function prepares just SCSI hostdevs ignoring others. But this will soon change. Thefore, move the function call out of qemuDomainAttachHostSCSIDevice() and into qemuDomainAttachHostDevice().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
So far, qemuDomainPrepareHostdev() is a NOP for anything but a SCSI hostdev. This will change soon. Therefore, move the SCSI hostdev preparation into a separate function (qemuDomainPrepareHostdevSCSI()) and make qemuDomainPrepareHostdev() call function corresponding to the hostdev type (or nothing if the type doesn't need any preparation). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 110 ++++++++++++++++++++++++----------------- 1 file changed, 65 insertions(+), 45 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 41db98880c..f462476d2a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11239,55 +11239,75 @@ qemuDomainPrepareDiskSource(virDomainDiskDef *disk, } +static int +qemuDomainPrepareHostdevSCSI(virDomainHostdevDef *hostdev, + qemuDomainObjPrivate *priv) +{ + virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; + virStorageSource *src = NULL; + + switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: + virObjectUnref(scsisrc->u.host.src); + scsisrc->u.host.src = virStorageSourceNew(); + src = scsisrc->u.host.src; + + src->type = VIR_STORAGE_TYPE_BLOCK; + + break; + + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: + src = scsisrc->u.iscsi.src; + break; + + case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST: + default: + virReportEnumRangeError(virDomainHostdevSCSIProtocolType, scsisrc->protocol); + return -1; + } + + if (src) { + const char *backendalias = hostdev->info->alias; + + src->readonly = hostdev->readonly; + src->id = qemuDomainStorageIDNew(priv); + src->nodestorage = g_strdup_printf("libvirt-%d-backend", src->id); + backendalias = src->nodestorage; + + if (src->auth) { + virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI; + qemuDomainStorageSourcePrivate *srcPriv = qemuDomainStorageSourcePrivateFetch(src); + + if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, + backendalias, + NULL, 0, + usageType, + src->auth->username, + &src->auth->seclookupdef))) + return -1; + } + } + + return 0; +} + + int qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, qemuDomainObjPrivate *priv) { - if (virHostdevIsSCSIDevice(hostdev)) { - virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - virStorageSource *src = NULL; - - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: - virObjectUnref(scsisrc->u.host.src); - scsisrc->u.host.src = virStorageSourceNew(); - src = scsisrc->u.host.src; - - src->type = VIR_STORAGE_TYPE_BLOCK; - - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: - src = scsisrc->u.iscsi.src; - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST: - default: - virReportEnumRangeError(virDomainHostdevSCSIProtocolType, scsisrc->protocol); - return -1; - } - - if (src) { - const char *backendalias = hostdev->info->alias; - - src->readonly = hostdev->readonly; - src->id = qemuDomainStorageIDNew(priv); - src->nodestorage = g_strdup_printf("libvirt-%d-backend", src->id); - backendalias = src->nodestorage; - - if (src->auth) { - virSecretUsageType usageType = VIR_SECRET_USAGE_TYPE_ISCSI; - qemuDomainStorageSourcePrivate *srcPriv = qemuDomainStorageSourcePrivateFetch(src); - - if (!(srcPriv->secinfo = qemuDomainSecretInfoSetupFromSecret(priv, - backendalias, - NULL, 0, - usageType, - src->auth->username, - &src->auth->seclookupdef))) - return -1; - } - } + if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) + return 0; + + switch ((virDomainHostdevSubsysType)hostdev->source.subsys.type) { + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: + return qemuDomainPrepareHostdevSCSI(hostdev, priv); + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: + break; } return 0; -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:41PM +0200, Michal Privoznik wrote:
So far, qemuDomainPrepareHostdev() is a NOP for anything but a SCSI hostdev. This will change soon. Therefore, move the SCSI hostdev preparation into a separate function (qemuDomainPrepareHostdevSCSI()) and make qemuDomainPrepareHostdev() call function corresponding to the hostdev type (or nothing if the type doesn't need any preparation).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
virsh command domxml-to-native failed with below error but start command succeed for same domain xml. "internal error: invalid PCI passthrough type 'default'" If a <hostdev> PCI backend is not set in the XML, the supported one is then chosen in qemuHostdevPreparePCIDevicesCheckSupport(). But this function is not called anywhere from qemuConnectDomainXMLToNative(). But qemuDomainPrepareHostdev() is. And it is also called from domain startup/hotplug code. Therefore, move the backend setting to the common path and drop qemuHostdevPreparePCIDevicesCheckSupport(). Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 50 +++++++++++++++++++++++++++++++- src/qemu/qemu_hostdev.c | 64 ++--------------------------------------- src/qemu/qemu_hotplug.c | 2 +- 3 files changed, 53 insertions(+), 63 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f462476d2a..58cd3dd710 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11292,6 +11292,53 @@ qemuDomainPrepareHostdevSCSI(virDomainHostdevDef *hostdev, } +static int +qemuDomainPrepareHostdevPCI(virDomainHostdevDef *hostdev, + virQEMUCaps *qemuCaps) +{ + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; + + /* assign defaults for hostdev passthrough */ + switch (*backend) { + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: + if (supportsPassthroughVFIO) { + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("VFIO PCI device assignment is not supported by this version of QEMU")); + return -1; + } + } else { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support passthrough of host PCI devices")); + return -1; + } + break; + + case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: + if (!supportsPassthroughVFIO) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("host doesn't support VFIO PCI passthrough")); + return false; + } + 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: + break; + } + + return true; +} + + int qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, qemuDomainObjPrivate *priv) @@ -11302,8 +11349,9 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, switch ((virDomainHostdevSubsysType)hostdev->source.subsys.type) { case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI: return qemuDomainPrepareHostdevSCSI(hostdev, priv); - case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI: + return qemuDomainPrepareHostdevPCI(hostdev, priv->qemuCaps); + case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_USB: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_SCSI_HOST: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_MDEV: case VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_LAST: diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 45cd1066f0..49347019ea 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -153,60 +153,6 @@ qemuHostdevHostSupportsPassthroughVFIO(void) } -static bool -qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, - size_t nhostdevs, - virQEMUCaps *qemuCaps) -{ - bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); - size_t i; - - /* assign defaults for hostdev passthrough */ - for (i = 0; i < nhostdevs; i++) { - virDomainHostdevDef *hostdev = hostdevs[i]; - virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; - - if (hostdev->mode != VIR_DOMAIN_HOSTDEV_MODE_SUBSYS) - continue; - if (hostdev->source.subsys.type != VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) - continue; - - switch (*backend) { - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: - if (supportsPassthroughVFIO && - virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; - } else { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("host doesn't support passthrough of " - "host PCI devices")); - return false; - } - - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: - if (!supportsPassthroughVFIO) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("host doesn't support VFIO PCI passthrough")); - return false; - } - 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: - break; - } - } - - return true; -} - int qemuHostdevPrepareOneNVMeDisk(virQEMUDriver *driver, const char *name, @@ -235,15 +181,11 @@ qemuHostdevPreparePCIDevices(virQEMUDriver *driver, const unsigned char *uuid, virDomainHostdevDef **hostdevs, int nhostdevs, - virQEMUCaps *qemuCaps, + virQEMUCaps *qemuCaps G_GNUC_UNUSED, unsigned int flags) { - virHostdevManager *hostdev_mgr = driver->hostdevMgr; - - if (!qemuHostdevPreparePCIDevicesCheckSupport(hostdevs, nhostdevs, qemuCaps)) - return -1; - - return virHostdevPreparePCIDevices(hostdev_mgr, QEMU_DRIVER_NAME, + return virHostdevPreparePCIDevices(driver->hostdevMgr, + QEMU_DRIVER_NAME, name, uuid, hostdevs, nhostdevs, flags); } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 64c62ea114..fab8935346 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1478,7 +1478,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, &hostdev, 1, priv->qemuCaps, flags) < 0) return -1; - /* this could have been changed by qemuHostdevPreparePCIDevices */ + /* this could have been changed by qemuDomainPrepareHostdevPCI() */ backend = hostdev->source.subsys.u.pci.backend; switch (backend) { -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:42PM +0200, Michal Privoznik wrote:
virsh command domxml-to-native failed with below error but start command succeed for same domain xml.
"internal error: invalid PCI passthrough type 'default'"
If a <hostdev> PCI backend is not set in the XML, the supported one is then chosen in qemuHostdevPreparePCIDevicesCheckSupport(). But this function is not called anywhere from qemuConnectDomainXMLToNative(). But qemuDomainPrepareHostdev() is. And it is also called from domain startup/hotplug code. Therefore, move the backend setting to the common path and drop qemuHostdevPreparePCIDevicesCheckSupport().
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
The qemuxml2argvtest does a bit of 'fixups' to parsed virDomainDef just before generating the cmd line. For instance, it sets PCI backend for hostdevs (to VFIO). The reason for this is that we want to make the test host independent and thus letting the code chose backend at runtime might render different results on different machines. But this is not necessary, as virpcimock (that the test uses) already creates a fake, but stable environment (where /dev/vfio/vfio and IOMMU groups exist), thus qemuHostdevHostSupportsPassthroughVFIO() returns true, regardless of the actual host support. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvtest.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 23e0c4054c..b65db3bbd9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -407,12 +407,6 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, for (i = 0; i < vm->def->nhostdevs; i++) { virDomainHostdevDef *hostdev = vm->def->hostdevs[i]; - if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { - hostdev->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; - } - if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:43PM +0200, Michal Privoznik wrote:
The qemuxml2argvtest does a bit of 'fixups' to parsed virDomainDef just before generating the cmd line. For instance, it sets PCI backend for hostdevs (to VFIO). The reason for this is that we want to make the test host independent and thus letting the code chose backend at runtime might render different results on different machines. But this is not necessary, as virpcimock (that the test uses) already creates a fake, but stable environment (where /dev/vfio/vfio and IOMMU groups exist), thus qemuHostdevHostSupportsPassthroughVFIO() returns true, regardless of the actual host support.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
--- tests/qemuxml2argvtest.c | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 23e0c4054c..b65db3bbd9 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -407,12 +407,6 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, for (i = 0; i < vm->def->nhostdevs; i++) { virDomainHostdevDef *hostdev = vm->def->hostdevs[i];
- if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && - hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI && - hostdev->source.subsys.u.pci.backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT) { - hostdev->source.subsys.u.pci.backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; - } - if (virHostdevIsSCSIDevice(hostdev)) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi;
-- 2.39.2
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@redhat.com> --- 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)); break; } -- 2.39.2
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@redhat.com> --- 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.
break; }
-- 2.39.2
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@redhat.com> --- 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. Michal
On Tue, Apr 25, 2023 at 10:56:43AM +0200, Michal Prívozník wrote:
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@redhat.com> --- 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.
Yeah, that looks fine, thanks Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
There is no way the qemuDomainAttachHostPCIDevice() function can be called over a hostdev with other PCI backend than VFIO. And even if it were, then the check is written so poorly that it lets some types through (e.g. KVM) only to let qemuBuildPCIHostdevDevProps() called afterwards fail properly. Drop this check and rely on qemuDomainPrepareHostdevPCI() (and worst case scenario even qemuBuildPCIHostdevDevProps()) to report the proper error. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fab8935346..566aa1f7df 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1466,7 +1466,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, bool teardownlabel = false; bool teardowndevice = false; bool teardownmemlock = false; - int backend; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0; @@ -1478,32 +1477,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, &hostdev, 1, priv->qemuCaps, flags) < 0) return -1; - /* this could have been changed by qemuDomainPrepareHostdevPCI() */ - backend = hostdev->source.subsys.u.pci.backend; - - switch (backend) { - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of qemu")); - goto error; - } - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU does not support device assignment mode '%1$s'"), - virDomainHostdevSubsysPCIBackendTypeToString(backend)); - goto error; - break; - } - if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0) goto error; teardownmemlock = true; @@ -1517,8 +1490,7 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, if (qemuSecuritySetHostdevLabel(driver, vm, hostdev) < 0) goto error; - if (backend != VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO) - teardownlabel = true; + teardownlabel = true; qemuAssignDeviceHostdevAlias(vm->def, &info->alias, -1); -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:45PM +0200, Michal Privoznik wrote:
There is no way the qemuDomainAttachHostPCIDevice() function can be called over a hostdev with other PCI backend than VFIO. And
"with PCI backend other than VFIO"
even if it were, then the check is written so poorly that it lets some types through (e.g. KVM) only to let qemuBuildPCIHostdevDevProps() called afterwards fail properly.
Drop this check and rely on qemuDomainPrepareHostdevPCI() (and worst case scenario even qemuBuildPCIHostdevDevProps()) to report the proper error.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hotplug.c | 30 +----------------------------- 1 file changed, 1 insertion(+), 29 deletions(-)
diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index fab8935346..566aa1f7df 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1466,7 +1466,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, bool teardownlabel = false; bool teardowndevice = false; bool teardownmemlock = false; - int backend; g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver); unsigned int flags = 0;
@@ -1478,32 +1477,6 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, &hostdev, 1, priv->qemuCaps, flags) < 0) return -1;
- /* this could have been changed by qemuDomainPrepareHostdevPCI() */ - backend = hostdev->source.subsys.u.pci.backend; - - switch (backend) { - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: - if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("VFIO PCI device assignment is not " - "supported by this version of qemu")); - goto error; - } - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_KVM: - break; - - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_XEN: - case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_TYPE_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("QEMU does not support device assignment mode '%1$s'"), - virDomainHostdevSubsysPCIBackendTypeToString(backend));
This is a better error message for example for the previous patch except the fact that for _TYPE_LAST this reports "... assignment mode '<null>'", but that's probably a non-issue (famous last words?). Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
When preparing a SCSI <hostdev/> with passthrough of a host SCSI adapter (i.e. no protocol), a virStorageSource structure is initialized and stored inside virDomainHostdevDef. But the source structure is filled in many places, with almost the same code. Firstly, qemuProcessPrepareHostHostdev() and qemuConnectDomainXMLToNativePrepareHostHostdev() are the same. Secondly, qemuDomainPrepareHostdev() allocates the src structure, only to let qemuProcessPrepareHostHostdev() fill src->path later. Well, src->path can be filled at the same place where the src structure is allocated (qemuDomainPrepareHostdev()) which renders the other two functions needless. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 10 ++++++++++ src/qemu/qemu_driver.c | 32 +------------------------------- src/qemu/qemu_process.c | 32 +------------------------------- tests/qemuxml2argvmock.c | 10 ++++++++++ tests/qemuxml2argvtest.c | 22 ---------------------- 5 files changed, 22 insertions(+), 84 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 72f36c807b..c5520505bf 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11244,7 +11244,9 @@ qemuDomainPrepareHostdevSCSI(virDomainHostdevDef *hostdev, qemuDomainObjPrivate *priv) { virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; + virDomainHostdevSubsysSCSIHost *scsihostsrc = &scsisrc->u.host; virStorageSource *src = NULL; + g_autofree char *devstr = NULL; switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: @@ -11252,7 +11254,15 @@ qemuDomainPrepareHostdevSCSI(virDomainHostdevDef *hostdev, scsisrc->u.host.src = virStorageSourceNew(); src = scsisrc->u.host.src; + if (!(devstr = virSCSIDeviceGetSgName(NULL, + scsihostsrc->adapter, + scsihostsrc->bus, + scsihostsrc->target, + scsihostsrc->unit))) + return -1; + src->type = VIR_STORAGE_TYPE_BLOCK; + src->path = g_strdup_printf("/dev/%s", devstr); break; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 523a83682c..d6f9d0796a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6166,38 +6166,8 @@ static char static int -qemuConnectDomainXMLToNativePrepareHostHostdev(virDomainHostdevDef *hostdev) +qemuConnectDomainXMLToNativePrepareHostHostdev(virDomainHostdevDef *hostdev G_GNUC_UNUSED) { - if (virHostdevIsSCSIDevice(hostdev)) { - virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: { - virDomainHostdevSubsysSCSIHost *scsihostsrc = &scsisrc->u.host; - virStorageSource *src = scsisrc->u.host.src; - g_autofree char *devstr = NULL; - - if (!(devstr = virSCSIDeviceGetSgName(NULL, - scsihostsrc->adapter, - scsihostsrc->bus, - scsihostsrc->target, - scsihostsrc->unit))) - return -1; - - src->path = g_strdup_printf("/dev/%s", devstr); - break; - } - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST: - default: - virReportEnumRangeError(virDomainHostdevSCSIProtocolType, scsisrc->protocol); - return -1; - } - } - return 0; } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 8baa882875..0bc19479d4 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6510,38 +6510,8 @@ qemuProcessPrepareDomainHostdevs(virDomainObj *vm, int -qemuProcessPrepareHostHostdev(virDomainHostdevDef *hostdev) +qemuProcessPrepareHostHostdev(virDomainHostdevDef *hostdev G_GNUC_UNUSED) { - if (virHostdevIsSCSIDevice(hostdev)) { - virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: { - virDomainHostdevSubsysSCSIHost *scsihostsrc = &scsisrc->u.host; - virStorageSource *src = scsisrc->u.host.src; - g_autofree char *devstr = NULL; - - if (!(devstr = virSCSIDeviceGetSgName(NULL, - scsihostsrc->adapter, - scsihostsrc->bus, - scsihostsrc->target, - scsihostsrc->unit))) - return -1; - - src->path = g_strdup_printf("/dev/%s", devstr); - break; - } - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST: - default: - virReportEnumRangeError(virDomainHostdevSCSIProtocolType, scsisrc->protocol); - return -1; - } - } - return 0; } diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index f566ec539a..400dd5c020 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -82,6 +82,16 @@ virSCSIVHostOpenVhostSCSI(int *vhostfd) return 0; } +char * +virSCSIDeviceGetSgName(const char *sysfs_prefix G_GNUC_UNUSED, + const char *adapter G_GNUC_UNUSED, + unsigned int bus G_GNUC_UNUSED, + unsigned int target G_GNUC_UNUSED, + unsigned long long unit G_GNUC_UNUSED) +{ + return g_strdup_printf("sg0"); +} + int virNetDevTapCreate(char **ifname, const char *tunpath G_GNUC_UNUSED, diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index b65db3bbd9..dad9758be4 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -404,28 +404,6 @@ testCompareXMLToArgvCreateArgs(virQEMUDriver *drv, disk->src->hostcdrom = true; } - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevDef *hostdev = vm->def->hostdevs[i]; - - if (virHostdevIsSCSIDevice(hostdev)) { - virDomainHostdevSubsysSCSI *scsisrc = &hostdev->source.subsys.u.scsi; - - switch ((virDomainHostdevSCSIProtocolType) scsisrc->protocol) { - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_NONE: - scsisrc->u.host.src->path = g_strdup("/dev/sg0"); - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_ISCSI: - break; - - case VIR_DOMAIN_HOSTDEV_SCSI_PROTOCOL_TYPE_LAST: - default: - virReportEnumRangeError(virDomainHostdevSCSIProtocolType, scsisrc->protocol); - return NULL; - } - } - } - if (vm->def->vsock) { virDomainVsockDef *vsock = vm->def->vsock; qemuDomainVsockPrivate *vsockPriv = -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:46PM +0200, Michal Privoznik wrote:
When preparing a SCSI <hostdev/> with passthrough of a host SCSI adapter (i.e. no protocol), a virStorageSource structure is initialized and stored inside virDomainHostdevDef. But the source structure is filled in many places, with almost the same code.
Firstly, qemuProcessPrepareHostHostdev() and qemuConnectDomainXMLToNativePrepareHostHostdev() are the same.
Secondly, qemuDomainPrepareHostdev() allocates the src structure, only to let qemuProcessPrepareHostHostdev() fill src->path later.
Well, src->path can be filled at the same place where the src structure is allocated (qemuDomainPrepareHostdev()) which renders the other two functions needless.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
After previous cleanup, there are some functions that do nothing: qemuConnectDomainXMLToNativePrepareHostHostdev() qemuConnectDomainXMLToNativePrepareHost() qemuProcessPrepareHostHostdev() qemuProcessPrepareHostHostdevs() Remove them. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_driver.c | 26 -------------------------- src/qemu/qemu_hotplug.c | 3 --- src/qemu/qemu_process.c | 27 --------------------------- src/qemu/qemu_process.h | 3 --- 4 files changed, 59 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d6f9d0796a..fdd5fb0826 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -6165,29 +6165,6 @@ static char } -static int -qemuConnectDomainXMLToNativePrepareHostHostdev(virDomainHostdevDef *hostdev G_GNUC_UNUSED) -{ - return 0; -} - - -static int -qemuConnectDomainXMLToNativePrepareHost(virDomainObj *vm) -{ - size_t i; - - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevDef *hostdev = vm->def->hostdevs[i]; - - if (qemuConnectDomainXMLToNativePrepareHostHostdev(hostdev) < 0) - return -1; - } - - return 0; -} - - static char *qemuConnectDomainXMLToNative(virConnectPtr conn, const char *format, const char *xmlData, @@ -6244,9 +6221,6 @@ static char *qemuConnectDomainXMLToNative(virConnectPtr conn, VIR_QEMU_PROCESS_START_COLD) < 0) return NULL; - if (qemuConnectDomainXMLToNativePrepareHost(vm) < 0) - return NULL; - if (!(cmd = qemuProcessCreatePretendCmdBuild(vm, NULL))) return NULL; diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 566aa1f7df..bf9d16e82b 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -2457,9 +2457,6 @@ qemuDomainAttachHostSCSIDevice(virQEMUDriver *driver, qemuAssignDeviceHostdevAlias(vm->def, &hostdev->info->alias, -1); - if (qemuProcessPrepareHostHostdev(hostdev) < 0) - goto cleanup; - if (!(data = qemuBuildHostdevSCSIAttachPrepare(hostdev, &backendalias, priv->qemuCaps))) goto cleanup; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0bc19479d4..75a1d88cd7 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6509,29 +6509,6 @@ qemuProcessPrepareDomainHostdevs(virDomainObj *vm, } -int -qemuProcessPrepareHostHostdev(virDomainHostdevDef *hostdev G_GNUC_UNUSED) -{ - return 0; -} - - -static int -qemuProcessPrepareHostHostdevs(virDomainObj *vm) -{ - size_t i; - - for (i = 0; i < vm->def->nhostdevs; i++) { - virDomainHostdevDef *hostdev = vm->def->hostdevs[i]; - - if (qemuProcessPrepareHostHostdev(hostdev) < 0) - return -1; - } - - return 0; -} - - /** * qemuProcessRebootAllowed: * @def: domain definition @@ -7233,10 +7210,6 @@ qemuProcessPrepareHost(virQEMUDriver *driver, if (qemuProcessPrepareHostStorage(driver, vm, flags) < 0) return -1; - VIR_DEBUG("Preparing hostdevs (host-side)"); - if (qemuProcessPrepareHostHostdevs(vm) < 0) - return -1; - VIR_DEBUG("Preparing external devices"); if (qemuExtDevicesPrepareHost(driver, vm) < 0) return -1; diff --git a/src/qemu/qemu_process.h b/src/qemu/qemu_process.h index b171f0464c..5d1b24a038 100644 --- a/src/qemu/qemu_process.h +++ b/src/qemu/qemu_process.h @@ -111,9 +111,6 @@ int qemuProcessPrepareDomain(virQEMUDriver *driver, int qemuProcessOpenVhostVsock(virDomainVsockDef *vsock); -int qemuProcessPrepareHostHostdev(virDomainHostdevDef *hostdev); - - int qemuProcessPrepareHostBackendChardevHotplug(virDomainObj *vm, virDomainDeviceDef *dev) G_NO_INLINE; -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:47PM +0200, Michal Privoznik wrote:
After previous cleanup, there are some functions that do nothing:
qemuConnectDomainXMLToNativePrepareHostHostdev() qemuConnectDomainXMLToNativePrepareHost() qemuProcessPrepareHostHostdev() qemuProcessPrepareHostHostdevs()
Remove them.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
After previous cleanups, qemuHostdevPreparePCIDevices() no longer needs virQEMUCaps. Drop its passing from callers. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_hostdev.c | 5 +---- src/qemu/qemu_hostdev.h | 2 -- src/qemu/qemu_hotplug.c | 4 ++-- src/qemu/qemu_process.c | 3 +-- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 49347019ea..f329442f8c 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -181,7 +181,6 @@ qemuHostdevPreparePCIDevices(virQEMUDriver *driver, const unsigned char *uuid, virDomainHostdevDef **hostdevs, int nhostdevs, - virQEMUCaps *qemuCaps G_GNUC_UNUSED, unsigned int flags) { return virHostdevPreparePCIDevices(driver->hostdevMgr, @@ -261,7 +260,6 @@ qemuHostdevPrepareMediatedDevices(virQEMUDriver *driver, int qemuHostdevPrepareDomainDevices(virQEMUDriver *driver, virDomainDef *def, - virQEMUCaps *qemuCaps, unsigned int flags) { if (!def->nhostdevs && !def->ndisks) @@ -271,8 +269,7 @@ qemuHostdevPrepareDomainDevices(virQEMUDriver *driver, return -1; if (qemuHostdevPreparePCIDevices(driver, def->name, def->uuid, - def->hostdevs, def->nhostdevs, - qemuCaps, flags) < 0) + def->hostdevs, def->nhostdevs, flags) < 0) return -1; if (qemuHostdevPrepareUSBDevices(driver, def->name, diff --git a/src/qemu/qemu_hostdev.h b/src/qemu/qemu_hostdev.h index 2d484db878..3e9adc57a9 100644 --- a/src/qemu/qemu_hostdev.h +++ b/src/qemu/qemu_hostdev.h @@ -52,7 +52,6 @@ int qemuHostdevPreparePCIDevices(virQEMUDriver *driver, const unsigned char *uuid, virDomainHostdevDef **hostdevs, int nhostdevs, - virQEMUCaps *qemuCaps, unsigned int flags); int qemuHostdevPrepareUSBDevices(virQEMUDriver *driver, const char *name, @@ -73,7 +72,6 @@ int qemuHostdevPrepareMediatedDevices(virQEMUDriver *driver, int nhostdevs); int qemuHostdevPrepareDomainDevices(virQEMUDriver *driver, virDomainDef *def, - virQEMUCaps *qemuCaps, unsigned int flags); void qemuHostdevReAttachOneNVMeDisk(virQEMUDriver *driver, diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index bf9d16e82b..b8b0a107aa 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1473,8 +1473,8 @@ qemuDomainAttachHostPCIDevice(virQEMUDriver *driver, if (!cfg->relaxedACS) flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; - if (qemuHostdevPreparePCIDevices(driver, vm->def->name, vm->def->uuid, - &hostdev, 1, priv->qemuCaps, flags) < 0) + if (qemuHostdevPreparePCIDevices(driver, vm->def->name, + vm->def->uuid, &hostdev, 1, flags) < 0) return -1; if (qemuDomainAdjustMaxMemLockHostdev(vm, hostdev) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 75a1d88cd7..8d7fd28e37 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -7164,8 +7164,7 @@ qemuProcessPrepareHost(virQEMUDriver *driver, hostdev_flags |= VIR_HOSTDEV_STRICT_ACS_CHECK; if (flags & VIR_QEMU_PROCESS_START_NEW) hostdev_flags |= VIR_HOSTDEV_COLD_BOOT; - if (qemuHostdevPrepareDomainDevices(driver, vm->def, priv->qemuCaps, - hostdev_flags) < 0) + if (qemuHostdevPrepareDomainDevices(driver, vm->def, hostdev_flags) < 0) return -1; VIR_DEBUG("Preparing chr device backends"); -- 2.39.2
On Mon, Apr 24, 2023 at 12:41:48PM +0200, Michal Privoznik wrote:
After previous cleanups, qemuHostdevPreparePCIDevices() no longer needs virQEMUCaps. Drop its passing from callers.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
participants (3)
-
Martin Kletzander -
Michal Privoznik -
Michal Prívozník