[Spice-devel] [PATCH v3] qemu: Use heads parameter for QXL driver

[Martin Kletzander]

On Tue, Jul 07, 2015 at 11:44:11AM +0200, Martin Kletzander wrote:
>On Mon, Jul 06, 2015 at 09:18:59AM +0100, Frediano Ziglio wrote:
>>Allows to specify maximum number of head to QXL driver.
>>
>>The patch to support the "max_outputs" in Qemu is still not merged but
>>I got agreement on the name of the argument.
>>
>
>This shouldn't be part of the commit message, we can't push this in
>until the code is in qemu anyways, so I'll remove it.
>
>>Actually can be a compatiblity problem as heads in the XML configuration
>>was set by default to '1'.
>>
>>Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>---
>>src/qemu/qemu_capabilities.c | 2 ++
>>src/qemu/qemu_capabilities.h | 1 +
>>src/qemu/qemu_command.c      | 5 +++++
>>3 files changed, 8 insertions(+)
>>
>>Changes from v2:
>>- removed capability tests (Martin Kletzander)
>>
>>diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
>>index 27686c3..68060cd 100644
>>--- a/src/qemu/qemu_capabilities.c
>>+++ b/src/qemu/qemu_capabilities.c
>>@@ -287,6 +287,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST,
>>              "aarch64-off",
>>
>>              "vhost-user-multiqueue", /* 190 */
>>+              "qxl-vga.max_outputs",
>>    );
>>
>>
>>@@ -1649,6 +1650,7 @@ static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxl[] = {
>>
>>static struct virQEMUCapsStringFlags virQEMUCapsObjectPropsQxlVga[] = {
>>    { "vgamem_mb", QEMU_CAPS_QXL_VGA_VGAMEM },
>>+    { "max_outputs", QEMU_CAPS_QXL_VGA_MAX_OUTPUTS },
>>};
>>
>>struct virQEMUCapsObjectTypeProps {
>>diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h
>>index 30aa504..02f9e81 100644
>>--- a/src/qemu/qemu_capabilities.h
>>+++ b/src/qemu/qemu_capabilities.h
>>@@ -230,6 +230,7 @@ typedef enum {
>>    QEMU_CAPS_DEVICE_PCI_SERIAL  = 188, /* -device pci-serial */
>>    QEMU_CAPS_CPU_AARCH64_OFF    = 189, /* -cpu ...,aarch64=off */
>>    QEMU_CAPS_VHOSTUSER_MULTIQUEUE = 190, /* vhost-user with -netdev queues= */
>>+    QEMU_CAPS_QXL_VGA_MAX_OUTPUTS = 191, /* qxl-vga.max_outputs */
>>
>>    QEMU_CAPS_LAST,                   /* this must always be the last item */
>>} virQEMUCapsFlags;
>>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>>index 25a7bc6..59666e7 100644
>>--- a/src/qemu/qemu_command.c
>>+++ b/src/qemu/qemu_command.c
>>@@ -5661,6 +5661,11 @@ qemuBuildDeviceVideoStr(virDomainDefPtr def,
>>            /* QEMU accepts mebibytes for vgamem_mb. */
>>            virBufferAsprintf(&buf, ",vgamem_mb=%u", video->vgamem / 1024);
>>        }
>>+
>>+        if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_QXL_VGA_MAX_OUTPUTS) &&
>>+            video->heads > 0) {
>>+            virBufferAsprintf(&buf, ",max_outputs=%u", video->heads);
>>+        }
>
>Looks good to me.  I would add a comment here why we're not erroring
>out, but rahter using the parameter only if supported, maybe also add
>some VIR_INFO output, but no need to resend it just for that.
>
>Let's just wait for QEMU to have this in.  If it's in and I missed the
>info, feel free to remind me about pushing this patch to libvirt.
>

Looking back at this patch, I figured there are no tests to make sure
this doesn't break few commits afterwards.  Take a look at the
tests/qemuxml2argvtest and add at least one there, please.  If the
patch looks like it's adding multiple things, feel free to split it
(one for the capability, other for tests and the hunk in
qemu_command.c).

Sorry for not noticing this earlier.

>Martin
>
>>    } else if (video->vram &&
>>        ((video->type == VIR_DOMAIN_VIDEO_TYPE_VGA &&
>>          virQEMUCapsGet(qemuCaps, QEMU_CAPS_VGA_VGAMEM)) ||
>>--
>>2.1.0
>>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20150710/1885094e/attachment.sig>