[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8] tools/libxl: Add qxl vga interface support for upstream-qemu-xen.



On Mon, 2013-01-28 at 16:03 +0000, Fabio Fantoni wrote:
> tools/libxl: Add qxl vga interface support for
>   upstream-qemu-xen.
> 
> Usage:
>    qxl=1|0
> 
> Changes from v7:
> - Fix videoram settings parameters for qemu.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxxxxx>
> Signed-off-by: Zhou Peng <zpengxen@xxxxxxxxx>

Unfortunately this patch is whitespace damaged in various places.

> ---
>   docs/man/xl.cfg.pod.5       |   11 +++++++++++
>   tools/libxl/libxl_create.c  |   12 ++++++++++++
>   tools/libxl/libxl_dm.c      |   15 +++++++++++++++
>   tools/libxl/libxl_types.idl |    1 +
>   tools/libxl/xl_cmdimpl.c    |    7 ++++++-
>   5 files changed, 45 insertions(+), 1 deletion(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 9c5cdcd..a0f0dc3 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -984,6 +984,9 @@ the amount of video ram is fixed at 4MB which is 
> sufficient
>   for 1024x768 at 32 bpp and videoram option is currently working
>   only when using the upstream qemu-xen device-model.
> 
> +For B<qxl> vga, the default is both default and minimal 128MB.
> +If B<videoram> is set less than 128MB, an error will be triggered.
> +
>   =item B<stdvga=BOOLEAN>
> 
>   Select a standard VGA card with VBE (VESA BIOS Extensions) as the
> @@ -992,6 +995,14 @@ a Cirrus Logic GD5446 VGA card. If your guest 
> supports VBE 2.0 or
>   later (e.g. Windows XP onwards) then you should enable this.
>   stdvga supports more video ram and bigger resolutions than Cirrus.
> 
> +=item B<qxl=BOOLEAN>

What happens if I give qxl=1 and stdvga=1?

Perhaps we should deprecate stdvga and add a new option:
        vga = "stdvga|cirrus|qxl"
?
> +
> +Select a QXL VGA card as the emulated graphics device.
> +In general, QXL should work with the Spice remote display protocol
> +for acceleration, and QXL driver is necessary in guest in this case.

Do we have any docs on where to obtain this driver and how to install
it? On the wiki perhaps, a link would be useful.

> +QXL can also work with the VNC protocol, but it will be like a standard
> +VGA without acceleration.
> +
>   =item B<vnc=BOOLEAN>
> 
>   Allow access to the display via the VNC protocol.  This enables the
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 55014e5..4761b5a 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -197,6 +197,18 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>       case LIBXL_DOMAIN_TYPE_HVM:
>           if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
>               b_info->shadow_memkb = 0;
> +
> +        if (b_info->device_model_version == 
> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
> +            && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
> +            if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
> +                b_info->video_memkb = (128 * 1024);
> +            }else if (b_info->video_memkb < (128 * 1024)) {
> +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                    "128 Mib videoram is the minimum for qxl default");

You can use the LOG() macros to shorten this line (and in other places
including you videoram patch too).

Should this error out on qemu == traditional and vga == QXL?

> +                return ERROR_INVAL;
> +            }
> +        }
> +
>           if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>               b_info->video_memkb = 8 * 1024;
>           else if (b_info->video_memkb < 8192){
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index 465b1fd..0813258 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -437,6 +439,19 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>                   NULL);
>               }
>               break;
> +        case LIBXL_VGA_INTERFACE_TYPE_QXL:
> +            flexarray_vappend(dm_args, "-vga", "qxl", NULL);

flexarray_append_pair() ?

> +            if (b_info->video_memkb) {
> +                flexarray_vappend(dm_args, "-global",
> +                libxl__sprintf(gc, "qxl-vga.vram_size_mb=%lu",
> +                (b_info->video_memkb/2/1024)),
> +                NULL);
> +                flexarray_vappend(dm_args, "-global",
> +                libxl__sprintf(gc, "qxl-vga.ram_size_mb=%lu",
> +                (b_info->video_memkb/2/1024)),
> +                NULL);

You could combine the second two vappends into a single one, or use
flexarray_append_pair().

The /2 is because the videomem is split between vram and ram? A comment
to that affect would be useful.

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 080bbd8..b4f7a0e 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1469,7 +1469,12 @@ skip_vfb:
>   #undef parse_extra_args
> 
>       if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> -        if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
> +        if (!xlu_cfg_get_long(config, "qxl", &l, 0))
> +            b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_QXL :
> + LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> +
> +        if (!xlu_cfg_get_long(config, "stdvga", &l, 0) &&
> +            b_info->u.hvm.vga.kind != LIBXL_VGA_INTERFACE_TYPE_QXL)
>               b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
> LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.