[libvirt] [PATCH v2 2/3] qemu: check os type / virt type / arch in validate callback

[Michal Privoznik]

On 12/11/19 4:58 PM, Daniel P. Berrangé wrote:
> Don't check os type / virt type / arch in the post-parse callback
> because we can't assume qemuCaps is non-NULL at this point. It
> also conceptually belongs to the validation callback.
> 
> Signed-off-by: Daniel P. Berrangé <berrange at redhat.com>
> ---
>   src/qemu/qemu_domain.c   | 42 ++++++++++++++++++++--------------------
>   tests/qemuxml2argvtest.c |  9 ++++++---
>   2 files changed, 27 insertions(+), 24 deletions(-)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 27926c7670..3e1bd52d9f 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -4865,27 +4865,6 @@ qemuDomainDefPostParse(virDomainDefPtr def,
>           return 1;
>       }
>   
> -    if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Emulator '%s' does not support os type '%s'"),
> -                       def->emulator, virDomainOSTypeToString(def->os.type));
> -        return -1;
> -    }
> -
> -    if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Emulator '%s' does not support arch '%s'"),
> -                       def->emulator, virArchToString(def->os.arch));
> -        return -1;
> -    }
> -
> -    if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
> -        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> -                       _("Emulator '%s' does not support virt type '%s'"),
> -                       def->emulator, virDomainVirtTypeToString(def->virtType));
> -        return -1;
> -    }
> -
>       if (def->os.bootloader || def->os.bootloaderArgs) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                          _("bootloader is not supported by QEMU"));

One can argue that this check belongs to Validate() too.

> @@ -5142,6 +5121,27 @@ qemuDomainDefValidate(const virDomainDef *def,
>                                               def->emulator)))
>           goto cleanup;
>   
> +    if (def->os.type != VIR_DOMAIN_OSTYPE_HVM) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Emulator '%s' does not support os type '%s'"),
> +                       def->emulator, virDomainOSTypeToString(def->os.type));
> +        goto cleanup;
> +    }
> +
> +    if (!virQEMUCapsIsArchSupported(qemuCaps, def->os.arch)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Emulator '%s' does not support arch '%s'"),
> +                       def->emulator, virArchToString(def->os.arch));
> +        goto cleanup;
> +    }
> +
> +    if (!virQEMUCapsIsVirtTypeSupported(qemuCaps, def->virtType)) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Emulator '%s' does not support virt type '%s'"),
> +                       def->emulator, virDomainVirtTypeToString(def->virtType));
> +        goto cleanup;
> +    }
> +
>       if (def->mem.min_guarantee) {
>           virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
>                          _("Parameter 'min_guarantee' not supported by QEMU."));

Michal