qemu_domain: move video validation out of qemu_command

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 <phrdina@redhat.com>

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,
+            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("video type %s is not supported with QEMU"),
+                           _("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,
+            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("video type %s is not supported with QEMU"),
+                           _("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");
 

src/qemu/qemu_domain.c

@@ -2394,6 +2394,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,
@@ -2453,6 +2524,9 @@ qemuDomainDefValidate(const virDomainDef *def,
         }
     }
 
+    if (qemuDomainDefValidateVideo(def) < 0)
+        goto cleanup;
+
     ret = 0;
 
  cleanup:

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,