[PATCH] virQEMUDriverGetDomainCapabilities: Validate machine type

Michal Privoznik posted 1 patch 1 year, 4 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/3bcb849655732d83651876f4e1942f623fe5e7de.1671702494.git.mprivozn@redhat.com
src/qemu/qemu_conf.c | 7 +++++++
1 file changed, 7 insertions(+)
[PATCH] virQEMUDriverGetDomainCapabilities: Validate machine type
Posted by Michal Privoznik 1 year, 4 months ago
When calling virConnectGetDomainCapabilities() (exposed as virsh
domcapabilities) users have option to specify whatever sub-set of
{ emulatorbin, arch, machine, virttype } they want. Then we have
a logic (hidden in virQEMUCapsCacheLookupDefault()) that picks
qemuCaps that satisfy values passed by user. And whatever was not
specified is then set to the default value as specified by picked
qemuCaps. For instance: if no machine type was provided but
emulatorbin was, then the machine type is set to the default one
as defined by the emulatorbin.

Or, when just virttype was set then the remaining three values
are set to their respective defaults. Except, we have a crasher
in this case:

  # virsh domcapabilities --virttype hvf
  error: Disconnected from qemu:///system due to end of file
  error: failed to get emulator capabilities
  error: End of file while reading data: Input/output error

This is because for 'hvf' virttype (at least my) QEMU does not
have any machine type. Therefore, @machine is set to NULL and the
rest of the code does not expect that.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_conf.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index ae5bbcd138..cbd339f594 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -1454,6 +1454,13 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriver *driver,
     g_autoptr(virDomainCaps) domCaps = NULL;
     const char *path = virQEMUCapsGetBinary(qemuCaps);
 
+    if (!virQEMUCapsIsMachineSupported(qemuCaps, virttype, machine)) {
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("the machine '%s' is not supported by emulator '%s'"),
+                       NULLSTR(machine), path);
+        return NULL;
+    }
+
     if (!(domCaps = virDomainCapsNew(path, machine, arch, virttype)))
         return NULL;
 
-- 
2.38.2
Re: [PATCH] virQEMUDriverGetDomainCapabilities: Validate machine type
Posted by Michal Prívozník 1 year, 4 months ago
On 12/22/22 10:48, Michal Privoznik wrote:
> When calling virConnectGetDomainCapabilities() (exposed as virsh
> domcapabilities) users have option to specify whatever sub-set of
> { emulatorbin, arch, machine, virttype } they want. Then we have
> a logic (hidden in virQEMUCapsCacheLookupDefault()) that picks
> qemuCaps that satisfy values passed by user. And whatever was not
> specified is then set to the default value as specified by picked
> qemuCaps. For instance: if no machine type was provided but
> emulatorbin was, then the machine type is set to the default one
> as defined by the emulatorbin.
> 
> Or, when just virttype was set then the remaining three values
> are set to their respective defaults. Except, we have a crasher
> in this case:
> 
>   # virsh domcapabilities --virttype hvf
>   error: Disconnected from qemu:///system due to end of file
>   error: failed to get emulator capabilities
>   error: End of file while reading data: Input/output error
> 
> This is because for 'hvf' virttype (at least my) QEMU does not
> have any machine type. Therefore, @machine is set to NULL and the
> rest of the code does not expect that.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/qemu/qemu_conf.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
> index ae5bbcd138..cbd339f594 100644
> --- a/src/qemu/qemu_conf.c
> +++ b/src/qemu/qemu_conf.c
> @@ -1454,6 +1454,13 @@ virQEMUDriverGetDomainCapabilities(virQEMUDriver *driver,
>      g_autoptr(virDomainCaps) domCaps = NULL;
>      const char *path = virQEMUCapsGetBinary(qemuCaps);
>  
> +    if (!virQEMUCapsIsMachineSupported(qemuCaps, virttype, machine)) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("the machine '%s' is not supported by emulator '%s'"),
> +                       NULLSTR(machine), path);
> +        return NULL;
> +    }
> +
>      if (!(domCaps = virDomainCapsNew(path, machine, arch, virttype)))
>          return NULL;
>  

Self-NAK. We can do even better and validate the @arch and @virttype
too. V2 coming shortly.

Michal