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@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"));
@@ -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."));
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index ba4a92ec0a..8655db609e 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -2852,10 +2852,13 @@ mymain(void)
             QEMU_CAPS_NEC_USB_XHCI);
 
     /* VM XML has invalid arch/ostype/virttype combo, but the SKIP flag
-     * will avoid the error. Still, we expect qemu driver to complain about
-     * missing machine error, and not crash */
+     * will avoid the error during parse. This will cause us to fill in
+     * the missing machine type using the i386 binary, despite it being
+     * the wrong binary for the arch. We expect to get a failure about
+     * bad arch later when creating the pretend command.
+     */
     DO_TEST_FULL("missing-machine",
-                 ARG_FLAGS, FLAG_EXPECT_PARSE_ERROR | FLAG_EXPECT_FAILURE,
+                 ARG_FLAGS, FLAG_EXPECT_FAILURE,
                  ARG_PARSEFLAGS, VIR_DOMAIN_DEF_PARSE_SKIP_VALIDATE,
                  ARG_QEMU_CAPS, NONE);
 
-- 
2.23.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

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@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

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list