[PATCH v2] qemu: Fix domxml-to-native command failure
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 host PCI device's driver attribute isn't defined in domain xml, qemu driver will choose a proper value based on host environment before starting qemu process. But this is missed in domxml-to-native command, add the same logic so domxml-to-native could pass and replace the relevant code in qemuHostdevPreparePCIDevicesCheckSupport() with an error report. Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- v2: replace setting with an error report, suggested by Michal src/qemu/qemu_domain.c | 11 +++++++++++ src/qemu/qemu_hostdev.c | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63b13b6875c9..517c90f93f0a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11274,6 +11274,17 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, } } } + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; + + if (*backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT && + supportsPassthroughVFIO && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } + } return 0; } diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 45cd1066f0a5..305a323a0769 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -175,15 +175,15 @@ qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (supportsPassthroughVFIO && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemuDomainPrepareHostdev() should have set backend " + "to 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; + return false; case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO: if (!supportsPassthroughVFIO) { -- 2.25.1
On 4/14/23 07:34, Zhenzhong Duan 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 host PCI device's driver attribute isn't defined in domain xml, qemu driver will choose a proper value based on host environment before starting qemu process. But this is missed in domxml-to-native command, add the same logic so domxml-to-native could pass and replace the relevant code in qemuHostdevPreparePCIDevicesCheckSupport() with an error report.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- v2: replace setting with an error report, suggested by Michal
src/qemu/qemu_domain.c | 11 +++++++++++ src/qemu/qemu_hostdev.c | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63b13b6875c9..517c90f93f0a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11274,6 +11274,17 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, } } } + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = &hostdev->source.subsys.u.pci.backend; + + if (*backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT && + supportsPassthroughVFIO && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } + }
return 0; } diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 45cd1066f0a5..305a323a0769 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -175,15 +175,15 @@ qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (supportsPassthroughVFIO && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemuDomainPrepareHostdev() should have set backend " + "to VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO"));
Ha! I've found one path where the backend is not set: qemuDomainAttachHostPCIDevice(). So we need a bit more code movement. But after that, this branch is effectively a dead code. In fact, if we move things a bit a lot of code will become dead. So let me respin with more patches. Michal
-----Original Message----- From: Michal Prívozník <mprivozn@redhat.com> Sent: Friday, April 14, 2023 9:06 PM To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; libvir-list@redhat.com Cc: Peng, Chao P <chao.p.peng@intel.com> Subject: Re: [PATCH v2] qemu: Fix domxml-to-native command failure
On 4/14/23 07:34, Zhenzhong Duan 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 host PCI device's driver attribute isn't defined in domain xml, qemu driver will choose a proper value based on host environment before starting qemu process. But this is missed in domxml-to-native command, add the same logic so domxml-to-native could pass and replace the relevant code in qemuHostdevPreparePCIDevicesCheckSupport() with an error report.
Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- v2: replace setting with an error report, suggested by Michal
src/qemu/qemu_domain.c | 11 +++++++++++ src/qemu/qemu_hostdev.c | 8 ++++---- 2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 63b13b6875c9..517c90f93f0a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -11274,6 +11274,17 @@ qemuDomainPrepareHostdev(virDomainHostdevDef *hostdev, } } } + if (hostdev->mode == VIR_DOMAIN_HOSTDEV_MODE_SUBSYS && + hostdev->source.subsys.type == VIR_DOMAIN_HOSTDEV_SUBSYS_TYPE_PCI) { + bool supportsPassthroughVFIO = qemuHostdevHostSupportsPassthroughVFIO(); + virDomainHostdevSubsysPCIBackendType *backend = + &hostdev->source.subsys.u.pci.backend; + + if (*backend == VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT && + supportsPassthroughVFIO && + virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { + *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + } + }
return 0; } diff --git a/src/qemu/qemu_hostdev.c b/src/qemu/qemu_hostdev.c index 45cd1066f0a5..305a323a0769 100644 --- a/src/qemu/qemu_hostdev.c +++ b/src/qemu/qemu_hostdev.c @@ -175,15 +175,15 @@ qemuHostdevPreparePCIDevicesCheckSupport(virDomainHostdevDef **hostdevs, case VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT: if (supportsPassthroughVFIO && virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VFIO_PCI)) { - *backend = VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO; + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("qemuDomainPrepareHostdev() should have set backend " + "to + VIR_DOMAIN_HOSTDEV_PCI_BACKEND_VFIO"));
Ha! I've found one path where the backend is not set: qemuDomainAttachHostPCIDevice(). So we need a bit more code movement. But after that, this branch is effectively a dead code. Oh, I see, the hot plug case.
In fact, if we move things a bit a lot of code will become dead. So let me respin with more patches.
Thanks for doing that. Regards Zhenzhong
participants (3)
-
Duan, Zhenzhong -
Michal Prívozník -
Zhenzhong Duan