[libvirt] [PATCH] RFC: support "vram" and "heads" for qxl vga

[Daniel P. Berrange]

On Tue, Feb 15, 2011 at 09:47:28PM +0800, Osier Yang wrote:
> qemu command line to specify "vram":
>     -global qxl.vram_size=uint
> 
> qemu command line to specify "heads", (no need of '-device' for
> the first 'head'):
>     -device qxl,id=qxl-N (N is natural number)
> 
> This patch is just about the command line building, still left work
> on the command line parsing (hacking on "qemuBuildCommandLine"), as
> I'm even not sure if it's the right way to build the command line.
> 
> Any advise/idea is appreciated.
> ---
>  src/qemu/qemu_capabilities.c |    5 ++++-
>  src/qemu/qemu_capabilities.h |    3 ++-
>  src/qemu/qemu_command.c      |   14 ++++++++++++++
>  3 files changed, 20 insertions(+), 2 deletions(-)
> 
> diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> index cc5552c..78bcb4c 100644
> --- a/src/qemu/qemu_capabilities.c
> +++ b/src/qemu/qemu_capabilities.c
> @@ -942,9 +942,12 @@ qemuCapsComputeCmdFlags(const char *help,
>       * two features. The benefits of JSON mode now outweigh
>       * the downside.
>       */
> -     if (version >= 13000)
> +    if (version >= 13000)
>          flags |= QEMUD_CMD_FLAG_MONITOR_JSON;
> 
> +    if (strstr(help, "-global"))
> +        flags |= QEMUD_CMD_FLAG_GLOBAL;
> +

There's no need for this. Just use QEMUD_CMD_FLAG_DEVICE since
-global & -device arrived together.

> diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
> index 1687203..92af6cb 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3964,6 +3964,20 @@ qemuBuildCommandLine(virConnectPtr conn,
>                  }
> 
>                  virCommandAddArgList(cmd, "-vga", vgastr, NULL);
> +
> +                if (def->videos[0]->type == VIR_DOMAIN_VIDEO_TYPE_QXL) {
> +                    if (def->videos[0]->vram &&
> +                        (qemuCmdFlags & QEMUD_CMD_FLAG_GLOBAL)) {
> +                        virCommandAddArgFormat(cmd, "-global qxl.vram_size=%u",
> +                                               def->videos[0]->vram);
> +                    }

The '-global' parameter sets global defaults for all devices of a
particular type. 

The video handling is a little wierd, because the first card we'll
have todo via '-vga' and a '-set' and the 2nd, 3rd, etc cards will
be normal using just '-device' (see qemuBuildVideoDevStr).

So here you'll want to use -set instead of -global. And then also
modify qemuBuildVideoDevStr

> +
> +                    if (def->videos[0]->heads > 0) {
> +                        for (i = 0; i < def->videos[0]->heads - 1; i++) {
> +                            virCommandAddArgFormat(cmd, "-device %s,id=qxl-%d", vgastr, i+1);
> +                        }

This isn't right. The 'heads' field indicates one graphics card, with
multiple outputs. What you're doing here is creating multiple graphics
cards each with one output. This is something you do by adding mulitple
<video> elements in the XML instead.

It doesn't look like QXL has multiple outputs on one card, so this setting
is not relevant.


Also be sure to create a test case for this in qemuxml2argvtest

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|