On 09/30/2016 12:02 PM, Pavel Hrdina wrote:
Signed-off-by: Pavel Hrdina <phrdina@redhat.com> --- src/qemu/qemu_command.c | 69 ++++++++++++++++++++++++++++--------------------- 1 file changed, 40 insertions(+), 29 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index aef8c79..15bb381 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -4451,43 +4451,54 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, virQEMUCapsPtr qemuCaps) { size_t i; - int primaryVideoType; + char *str = NULL;
- if (!def->videos) - return 0; + for (i = 0; i < def->nvideos; i++) { + virDomainVideoDefPtr video = def->videos[i];
- primaryVideoType = def->videos[0]->type; + switch (video->type) { + case VIR_DOMAIN_VIDEO_TYPE_VGA: + case VIR_DOMAIN_VIDEO_TYPE_CIRRUS: + case VIR_DOMAIN_VIDEO_TYPE_VMVGA: + case VIR_DOMAIN_VIDEO_TYPE_VIRTIO: + case VIR_DOMAIN_VIDEO_TYPE_QXL: + if (video->primary) {
So thinking back to my patch 5 comment - this essentially assumes def->videos[0] and video->primary are equivalent, right?
+ if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY)) {
- if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY)) { - for (i = 0; i < def->nvideos; i++) { - char *str; - virCommandAddArg(cmd, "-device"); - if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], - qemuCaps))) - return -1; + virCommandAddArg(cmd, "-device");
- virCommandAddArg(cmd, str); - VIR_FREE(str); - } - } else { - if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { - /* nothing - vga has no effect on Xen pvfb */ - } else { - if (qemuBuildVgaVideoCommand(cmd, def, qemuCaps) < 0) - return -1; - } + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i],
This would be "video", not the def->videos[i]
+ qemuCaps))) + return -1; + + virCommandAddArg(cmd, str); + VIR_FREE(str); + } else { + if (qemuBuildVgaVideoCommand(cmd, def, qemuCaps) < 0)
And from the previous patch passing video here still works
+ return -1; + } + } else { + virCommandAddArg(cmd, "-device");
- for (i = 1; i < def->nvideos; i++) { - char *str; + if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i],
Likewise "video" not the def->videos[i]
+ qemuCaps))) + return -1;
- virCommandAddArg(cmd, "-device"); + virCommandAddArg(cmd, str); + VIR_FREE(str); + } + break;
- if (!(str = qemuBuildDeviceVideoStr(def, def->videos[i], - qemuCaps))) - return -1; + case VIR_DOMAIN_VIDEO_TYPE_VBOX: + case VIR_DOMAIN_VIDEO_TYPE_XEN:
Doesn't putting this here break the previous else check of: - if (primaryVideoType == VIR_DOMAIN_VIDEO_TYPE_XEN) { - /* nothing - vga has no effect on Xen pvfb */ IOW: Shouldn't the XEN type code check code "skip" def->videos[0] (or video->primary)? ACK with the proper usage of video not def->videos[i] and making sure the _XEN case is handled properly John
+ case VIR_DOMAIN_VIDEO_TYPE_PARALLELS: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("video type '%s' is not supported with QEMU"), + virDomainVideoTypeToString(video->type)); + return -1;
- virCommandAddArg(cmd, str); - VIR_FREE(str); + case VIR_DOMAIN_VIDEO_TYPE_LAST: + break; } }