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>

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 43de08e25f929f4c713a245ae0015d3140b6281e..b9869f32d9c5e454cd82f48ae0284072891ca69a 100644 (file)
--- 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 83ac3896c391cb653874fd393fba4be80235591b..c9378586b32f3d6c96d2587ba92dd79ad014e768 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/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:

diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
index bb16738350ddd27b1ab0f5acc51c893640982888..dc67d514d22420dc401a5a2f19968c1d5339efac 100644 (file)
--- 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,