All definition validation that doesn't depend on qemu capabilities
and was allowed previously as valid definition should be placed into
qemuDomainDefValidate.

The check whether video type is supported or not was based on an enum
that translates type into model.  Use switch to ensure that if new
video type is added, it will be properly handled.

Signed-off-by: Pavel Hrdina <phrd...@redhat.com>
---

ACKed

Changes in v2:
    - Move also check whether video type is supported or not

 src/qemu/qemu_command.c        | 66 +++----------------------------------
 src/qemu/qemu_domain.c         | 74 ++++++++++++++++++++++++++++++++++++++++++
 src/qemu/qemu_domain_address.c |  6 ----
 3 files changed, 78 insertions(+), 68 deletions(-)

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 43de08e..b9869f3 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -4261,8 +4261,8 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
     if (video->primary) {
         model = qemuDeviceVideoTypeToString(video->type);
         if (!model || STREQ(model, "")) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("video type %s is not supported with QEMU"),
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("invalid model for video type '%s'"),
                            virDomainVideoTypeToString(video->type));
             goto error;
         }
@@ -4271,12 +4271,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
             model = "virtio-gpu-pci";
         }
     } else {
-        if (video->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           "%s", _("non-primary video device must be type of 
'qxl'"));
-            goto error;
-        }
-
         if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_QXL)) {
             virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
                            "%s", _("only one video card is currently 
supported"));
@@ -4288,12 +4282,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
 
     virBufferAsprintf(&buf, "%s,id=%s", model, video->info.alias);
 
-    if (video->accel && video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) {
-        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                       _("qemu does not support the accel2d setting"));
-        goto error;
-    }
-
     if (video->accel && video->accel->accel3d == VIR_TRISTATE_SWITCH_ON) {
         if (video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO ||
             !virQEMUCapsGet(qemuCaps, QEMU_CAPS_VIRTIO_GPU_VIRGL)) {
@@ -4308,19 +4296,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
     }
 
     if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
-        if (video->vram > (UINT_MAX / 1024)) {
-            virReportError(VIR_ERR_OVERFLOW,
-                           _("value for 'vram' must be less than '%u'"),
-                           UINT_MAX / 1024);
-            goto error;
-        }
-        if (video->ram > (UINT_MAX / 1024)) {
-            virReportError(VIR_ERR_OVERFLOW,
-                           _("value for 'ram' must be less than '%u'"),
-                           UINT_MAX / 1024);
-            goto error;
-        }
-
         if (video->ram) {
             /* QEMU accepts bytes for ram_size. */
             virBufferAsprintf(&buf, ",ram_size=%u", video->ram * 1024);
@@ -4351,13 +4326,6 @@ qemuBuildDeviceVideoStr(const virDomainDef *def,
          (video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA &&
           virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) {
 
-        if (video->vram < 1024) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           "%s", _("value for 'vram' must be at least 1 MiB "
-                                   "(1024 KiB)"));
-            goto error;
-        }
-
         virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vram / 1024);
     }
 
@@ -4419,8 +4387,8 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
 
         const char *vgastr = qemuVideoTypeToString(primaryVideoType);
         if (!vgastr || STREQ(vgastr, "")) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                           _("video type %s is not supported with QEMU"),
+            virReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("invalid model for video type '%s'"),
                            virDomainVideoTypeToString(primaryVideoType));
             return -1;
         }
@@ -4447,19 +4415,6 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
             unsigned int vram64 = def->videos[0]->vram64;
             unsigned int vgamem = def->videos[0]->vgamem;
 
-            if (vram > (UINT_MAX / 1024)) {
-                virReportError(VIR_ERR_OVERFLOW,
-                       _("value for 'vram' must be less than '%u'"),
-                               UINT_MAX / 1024);
-                return -1;
-            }
-            if (ram > (UINT_MAX / 1024)) {
-                virReportError(VIR_ERR_OVERFLOW,
-                   _("value for 'ram' must be less than '%u'"),
-                               UINT_MAX / 1024);
-                return -1;
-            }
-
             if (ram) {
                 virCommandAddArg(cmd, "-global");
                 virCommandAddArgFormat(cmd, "%s.ram_size=%u",
@@ -4491,13 +4446,6 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
               virQEMUCapsGet(qemuCaps, QEMU_CAPS_VMWARE_SVGA_VGAMEM)))) {
             unsigned int vram = def->videos[0]->vram;
 
-            if (vram < 1024) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               "%s", _("value for 'vgamem' must be at "
-                                       "least 1 MiB (1024 KiB)"));
-                return -1;
-            }
-
             virCommandAddArg(cmd, "-global");
             virCommandAddArgFormat(cmd, "%s.vgamem_mb=%u",
                                    dev, vram / 1024);
@@ -4505,12 +4453,6 @@ qemuBuildVideoCommandLine(virCommandPtr cmd,
 
         for (i = 1; i < def->nvideos; i++) {
             char *str;
-            if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
-                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
-                               _("video type %s is only valid as primary video 
card"),
-                               
virDomainVideoTypeToString(def->videos[0]->type));
-                return -1;
-            }
 
             virCommandAddArg(cmd, "-device");
 
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 83ac389..c937858 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -2395,6 +2395,77 @@ qemuDomainDefPostParse(virDomainDefPtr def,
 
 
 static int
+qemuDomainDefValidateVideo(const virDomainDef *def)
+{
+    size_t i;
+    virDomainVideoDefPtr video;
+
+    for (i = 0; i < def->nvideos; i++) {
+        video = def->videos[i];
+
+        switch (video->type) {
+        case VIR_DOMAIN_VIDEO_TYPE_XEN:
+        case VIR_DOMAIN_VIDEO_TYPE_VBOX:
+        case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("video type '%s' is not supported with QEMU"),
+                           virDomainVideoTypeToString(video->type));
+            return -1;
+        case VIR_DOMAIN_VIDEO_TYPE_VGA:
+        case VIR_DOMAIN_VIDEO_TYPE_CIRRUS:
+        case VIR_DOMAIN_VIDEO_TYPE_VMVGA:
+        case VIR_DOMAIN_VIDEO_TYPE_QXL:
+        case VIR_DOMAIN_VIDEO_TYPE_VIRTIO:
+        case VIR_DOMAIN_VIDEO_TYPE_LAST:
+            break;
+        }
+
+        if (!video->primary &&
+            video->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("video type '%s' is only valid as primary "
+                             "video device"),
+                           virDomainVideoTypeToString(video->type));
+            return -1;
+        }
+
+        if (video->accel && video->accel->accel2d == VIR_TRISTATE_SWITCH_ON) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("qemu does not support the accel2d setting"));
+            return -1;
+        }
+
+        if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
+            if (video->vram > (UINT_MAX / 1024)) {
+                virReportError(VIR_ERR_OVERFLOW,
+                               _("value for 'vram' must be less than '%u'"),
+                               UINT_MAX / 1024);
+                return -1;
+            }
+            if (video->ram > (UINT_MAX / 1024)) {
+                virReportError(VIR_ERR_OVERFLOW,
+                               _("value for 'ram' must be less than '%u'"),
+                               UINT_MAX / 1024);
+                return -1;
+            }
+        }
+
+        if (video->type == VIR_DOMAIN_VIDEO_TYPE_VGA ||
+            video->type == VIR_DOMAIN_VIDEO_TYPE_VMVGA) {
+            if (video->vram && video->vram < 1024) {
+                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                               "%s", _("value for 'vram' must be at least "
+                                       "1 MiB (1024 KiB)"));
+                return -1;
+            }
+        }
+    }
+
+    return 0;
+}
+
+
+static int
 qemuDomainDefValidate(const virDomainDef *def,
                       virCapsPtr caps,
                       void *opaque)
@@ -2453,6 +2524,9 @@ qemuDomainDefValidate(const virDomainDef *def,
         }
     }
 
+    if (qemuDomainDefValidateVideo(def) < 0)
+        goto cleanup;
+
     ret = 0;
 
  cleanup:
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index bb16738..dc67d51 100644
--- a/src/qemu/qemu_domain_address.c
+++ b/src/qemu/qemu_domain_address.c
@@ -1242,13 +1242,7 @@ qemuDomainAssignDevicePCISlots(virDomainDefPtr def,
             goto error;
     }
 
-    /* Further non-primary video cards which have to be qxl type */
     for (i = 1; i < def->nvideos; i++) {
-        if (def->videos[i]->type != VIR_DOMAIN_VIDEO_TYPE_QXL) {
-            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
-                           _("non-primary video device must be type of 
'qxl'"));
-            goto error;
-        }
         if (!virDeviceInfoPCIAddressWanted(&def->videos[i]->info))
             continue;
         if (virDomainPCIAddressReserveNextSlot(addrs, &def->videos[i]->info,
-- 
2.10.1

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

Reply via email to