qemuDomainDefValidateVideo() is just a loop performing various checks
on each video device. Rather than maintaining this outlyer function
called from qemuDomainDefValidateVideo(), just fold the validations
into qemuDomainDeviceDefValidateVideo(), which is called once for each
video device (my guess is that ...DeviceDefValidateVideo() didn't
exist yet when ...DomainDefValidateVideo() was added, but I haven't
verified this).
---
I randomly noticed this when looking up something else in the validation code...
src/qemu/qemu_domain.c | 151 +++++++++++++++++++++----------------------------
1 file changed, 66 insertions(+), 85 deletions(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 74b82450b..2ca45fde2 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3216,79 +3216,6 @@ 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:
- case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
- 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 &&
- video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
- 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;
-}
-
-
#define QEMU_MAX_VCPUS_WITHOUT_EIM 255
@@ -3388,9 +3315,6 @@ qemuDomainDefValidate(const virDomainDef *def,
}
}
- if (qemuDomainDefValidateVideo(def) < 0)
- goto cleanup;
-
ret = 0;
cleanup:
@@ -3851,18 +3775,75 @@ qemuDomainDeviceDefValidateHostdev(const virDomainHostdevDef *hostdev,
static int
qemuDomainDeviceDefValidateVideo(const virDomainVideoDef *video)
{
- if (video->type == VIR_DOMAIN_VIDEO_TYPE_QXL &&
- video->vgamem) {
- if (video->vgamem < 1024) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("value for 'vgamem' must be at least 1 MiB "
- "(1024 KiB)"));
+ switch (video->type) {
+ case VIR_DOMAIN_VIDEO_TYPE_XEN:
+ case VIR_DOMAIN_VIDEO_TYPE_VBOX:
+ case VIR_DOMAIN_VIDEO_TYPE_PARALLELS:
+ case VIR_DOMAIN_VIDEO_TYPE_DEFAULT:
+ 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 &&
+ video->type != VIR_DOMAIN_VIDEO_TYPE_VIRTIO) {
+ 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->vgamem) {
+ if (video->vgamem < 1024) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("value for 'vgamem' must be at least 1 MiB "
+ "(1024 KiB)"));
+ return -1;
+ }
- if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) {
- virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
- _("value for 'vgamem' must be power of two"));
+ if (video->vgamem != VIR_ROUND_UP_POWER_OF_TWO(video->vgamem)) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("value for 'vgamem' must be power of two"));
+ 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;
}
}
--
2.13.6
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
On 12/18/2017 10:35 AM, Laine Stump wrote: > > qemuDomainDefValidateVideo() is just a loop performing various checks > on each video device. Rather than maintaining this outlyer function *outlying > called from qemuDomainDefValidateVideo(), just fold the validations > into qemuDomainDeviceDefValidateVideo(), which is called once for each > video device (my guess is that ...DeviceDefValidateVideo() didn't > exist yet when ...DomainDefValidateVideo() was added, but I haven't Everything between "(my guess is..." and "...verified this)." could have been below the --- Anyway, the simple answer is qemuDomainDefValidateVideo was introduced in 2.4 as commit id 133fb140 and qemuDomainDeviceDefValidateVideo was introduced in 3.10 as commit id ab948b629. Thankfully gitk makes this determination really easy! To take it one step further, I think you'll recognize the committer of the original qemuDomainDeviceDefValidate implementation in v2.0 as commit id d987f63a. > verified this). > --- > > I randomly noticed this when looking up something else in the validation code... > > > src/qemu/qemu_domain.c | 151 +++++++++++++++++++++---------------------------- > 1 file changed, 66 insertions(+), 85 deletions(-) > Reviewed-by: John Ferlan <jferlan@redhat.com> John -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
© 2016 - 2025 Red Hat, Inc.