[PATCH] qemu: Fix domxml-to-native command failure

Zhenzhong Duan posted 1 patch 1 year, 1 month ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20230413060732.2310576-1-zhenzhong.duan@intel.com
There is a newer version of this series
src/qemu/qemu_domain.c | 11 +++++++++++
1 file changed, 11 insertions(+)
[PATCH] qemu: Fix domxml-to-native command failure
Posted by Zhenzhong Duan 1 year, 1 month ago
virsh command domxml-to-native failed with below error but start
command suceed 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.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
---
 src/qemu/qemu_domain.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

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;
 }
-- 
2.25.1
Re: [PATCH] qemu: Fix domxml-to-native command failure
Posted by Michal Prívozník 1 year, 1 month ago
On 4/13/23 08:07, Zhenzhong Duan wrote:
> virsh command domxml-to-native failed with below error but start
> command suceed 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.
> 
> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
> ---
>  src/qemu/qemu_domain.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> 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;
> +        }
> +    }
>  

The same happens in qemuHostdevPreparePCIDevicesCheckSupport(). Should
we remove the code from there and leave just checks there? I mean, if
choosing the backend is moved here, as you suggests, then
qemuHostdevPreparePCIDevicesCheckSupport() shouldn't ever see a hostdev
with VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT. Except when the host
doesn't support VFIO. So maybe just drop the code that sets backend an
replace it with a virReportError();

Michal
RE: [PATCH] qemu: Fix domxml-to-native command failure
Posted by Duan, Zhenzhong 1 year, 1 month ago

>-----Original Message-----
>From: Michal Prívozník <mprivozn@redhat.com>
>Sent: Thursday, April 13, 2023 4:21 PM
>To: Duan, Zhenzhong <zhenzhong.duan@intel.com>; libvir-list@redhat.com
>Subject: Re: [PATCH] qemu: Fix domxml-to-native command failure
>
>On 4/13/23 08:07, Zhenzhong Duan wrote:
>> virsh command domxml-to-native failed with below error but start
>> command suceed 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.
>>
>> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
>> ---
>>  src/qemu/qemu_domain.c | 11 +++++++++++
>>  1 file changed, 11 insertions(+)
>>
>> 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;
>> +        }
>> +    }
>>
>
>The same happens in qemuHostdevPreparePCIDevicesCheckSupport(). Should
>we remove the code from there and leave just checks there? I mean, if
>choosing the backend is moved here, as you suggests, then
>qemuHostdevPreparePCIDevicesCheckSupport() shouldn't ever see a hostdev
>with VIR_DOMAIN_HOSTDEV_PCI_BACKEND_DEFAULT. Except when the host
>doesn't support VFIO. So maybe just drop the code that sets backend an
>replace it with a virReportError();

Thanks for your suggestion, sounds better, will do in v2.

Regards
Zhenzhong