1. Patchew
  2. Libvirt
  3. View series

[PATCH v2] 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/20230414053458.17966-1-zhenzhong.duan@intel.com
src/qemu/qemu_domain.c  | 11 +++++++++++
src/qemu/qemu_hostdev.c |  8 ++++----
2 files changed, 15 insertions(+), 4 deletions(-)
[PATCH v2] 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 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
Re: [PATCH v2] qemu: Fix domxml-to-native command failure
Posted by Michal Prívozník 1 year, 1 month ago 
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
RE: [PATCH v2] 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: 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

Patchew 2.1-devel-0870f84

© 2016 - 2024 Red Hat, Inc.