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

Re: [Xen-devel] [PATCH 2/3] tools:libxl: Add qxl vga interface support v2



On Mon, Oct 22, 2012 at 10:36 PM, George Dunlap
<george.dunlap@xxxxxxxxxxxxx> wrote:
> On 18/10/12 11:19, ZhouPeng wrote:
>>
>> v4
>>
>> test and fix conflict with the latest Xen
>> -----
>>
>>
>> tools:libxl: Add qxl vga interface support for upstream-qemu-xen.
>>
>> Usage:
>>    qxl=1|0
>>
>> Signed-off-by: Zhou Peng <ailvpeng25@xxxxxxxxx>
>
>
> Thanks, Zhou Peng.  Just a couple of comments below:
>
>
>>
>> diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
>> --- a/docs/man/xl.cfg.pod.5     Mon Oct 15 16:51:44 2012 +0100
>> +++ b/docs/man/xl.cfg.pod.5     Thu Oct 18 17:53:25 2012 +0800
>> @@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin
>>
>>   Sets the amount of RAM which the emulated video card will contain,
>>   which in turn limits the resolutions and bit depths which will be
>> -available. This option is only available when using the B<stdvga>
>> -option (see below).
>> +available. This option is only available when using the B<stdvga> and
>> +B<qxl> option (see below).
>>   The default amount of video ram for stdvga is 8MB which is sufficient
>>   for e.g. 1600x1200 at 32bpp.
>> +For B<qxl> option, the default is 128MB. If B<videoram> is set greater
>> +than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
>> +error will be triggered.
>
>
> Is this a fixed value in qemu, or is this something that can be changed via
> the command-line?
Can be changed.

But Ian Campbell and me have talked on sth related with this
closely before. And we both agree to just support the default is
enough in most time. You can get it from the url below quickly, pls
read from bottom up, which can save time.

http://lists.xen.org/archives/html/xen-devel/2012-07/msg00098.html

Thanks,
-Zhou Peng
>
>
>> +
>> +        if (b_info->device_model_version ==
>> LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN
>> +            && b_info->u.hvm.vga.kind == LIBXL_VGA_INTERFACE_TYPE_QXL) {
>> +            /*
>> +             * QXL needs 128 Mib video ram by default.
>> +             */
>> +            if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
>> +                b_info->video_memkb = 128 * 1024;
>> +            }
>> +            if (b_info->video_memkb < 128 * 1024) {
>> +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
>> +                    "128 Mib videoram is necessary for qxl default");
>> +                return ERROR_INVAL;
>> +            }
>> +            if (b_info->video_memkb > 128 * 1024) {
>> +                b_info->video_memkb = 128 * 1024;
>> +                LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
>> +                            "trim videoram to qxl default: 128 Mib");
>> +            }
>> +        }
>
>
> It would be better to have a single #define for the required QXL video mem
> size, rather than duplicating "128*1024" several times.
>
> Other than that, looks pretty good to me.

Fixed in v5
> Thanks,
>  -George
>

Patch v5 is appended and attached below .
------------------

tools:libxl: Add qxl vga interface support for upstream-qemu-xen.

Usage:
  qxl=1|0

Signed-off-by: Zhou Peng <ailvpeng25@xxxxxxxxx>

diff -r c1c549c4fe9e docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5     Mon Oct 15 16:51:44 2012 +0100
+++ b/docs/man/xl.cfg.pod.5     Wed Oct 24 15:39:43 2012 +0800
@@ -930,10 +930,13 @@ in the B<VFB_SPEC_STRING> for configurin

 Sets the amount of RAM which the emulated video card will contain,
 which in turn limits the resolutions and bit depths which will be
-available. This option is only available when using the B<stdvga>
-option (see below).
+available. This option is only available when using the B<stdvga> and
+B<qxl> option (see below).
 The default amount of video ram for stdvga is 8MB which is sufficient
 for e.g. 1600x1200 at 32bpp.
+For B<qxl> option, the default is 128MB. If B<videoram> is set greater
+than 128MB, it will be trimmed to 128MB; if set less than 128MB, an
+error will be triggered.

 When using the emulated Cirrus graphics card (B<stdvga=0>)
 the amount of video ram is fixed at 4MB which is sufficient
@@ -951,6 +954,14 @@ a Cirrus Logic GD5446 VGA card. If your
 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>
+
+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.
+QXL can also work with the VNC protocol, but it will be like a standard
+VGA without acceleration.

 =item B<vnc=BOOLEAN>

diff -r c1c549c4fe9e tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c        Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_create.c        Wed Oct 24 15:39:43 2012 +0800
@@ -232,8 +232,31 @@ int libxl__domain_build_info_setdefault(
     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) {
+            /*
+             * QXL needs 128 Mib video ram by default.
+             */
+#define QXL_VIDEO_RAM_DEFAULT (128 * 1024)
+            if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT) {
+                b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+            }
+            if (b_info->video_memkb < QXL_VIDEO_RAM_DEFAULT) {
+                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
+                    "128 Mib videoram is necessary for qxl default");
+                return ERROR_INVAL;
+            }
+            if (b_info->video_memkb > QXL_VIDEO_RAM_DEFAULT) {
+                b_info->video_memkb = QXL_VIDEO_RAM_DEFAULT;
+                LIBXL__LOG(CTX, LIBXL__LOG_WARNING,
+                            "trim videoram to qxl default: 128 Mib");
+            }
+#undef QXL_VIDEO_RAM_DEFAULT
+        }
         if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
             b_info->video_memkb = 8 * 1024;
+
         if (b_info->u.hvm.timer_mode == LIBXL_TIMER_MODE_DEFAULT)
             b_info->u.hvm.timer_mode =
                 LIBXL_TIMER_MODE_NO_DELAY_FOR_MISSED_TICKS;
diff -r c1c549c4fe9e tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c    Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_dm.c    Wed Oct 24 15:39:43 2012 +0800
@@ -181,6 +181,8 @@ static char ** libxl__build_device_model
             break;
         case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
             break;
+        case LIBXL_VGA_INTERFACE_TYPE_QXL:
+             break;
         }

         if (b_info->u.hvm.boot) {
@@ -430,6 +432,9 @@ static char ** libxl__build_device_model
             break;
         case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
             flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+            break;
+        case LIBXL_VGA_INTERFACE_TYPE_QXL:
+            flexarray_vappend(dm_args, "-vga", "qxl", NULL);
             break;
         }

diff -r c1c549c4fe9e tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl       Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/libxl_types.idl       Wed Oct 24 15:39:43 2012 +0800
@@ -130,6 +130,7 @@ libxl_vga_interface_type = Enumeration("
 libxl_vga_interface_type = Enumeration("vga_interface_type", [
     (1, "CIRRUS"),
     (2, "STD"),
+    (3, "QXL"),
     ], init_val = 0)

 #
diff -r c1c549c4fe9e tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c  Mon Oct 15 16:51:44 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c  Wed Oct 24 15:39:43 2012 +0800
@@ -1402,6 +1402,14 @@ skip_vfb:
             b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD :
                                          LIBXL_VGA_INTERFACE_TYPE_CIRRUS;

+        if (!xlu_cfg_get_long(config, "qxl", &l, 0)) {
+            if (l) {
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_QXL;
+            } else if (!b_info->u.hvm.vga.kind) {
+                b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
+            }
+        }
+
         xlu_cfg_get_defbool(config, "vnc", &b_info->u.hvm.vnc.enable, 0);
         xlu_cfg_replace_string (config, "vnclisten",
                                 &b_info->u.hvm.vnc.listen, 0);


--
Zhou Peng

Attachment: spice.tools.libxl.qxl.support+docs.v5.diff
Description: Binary data

_______________________________________________
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®.