* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-02-22 10:37 ` [PATCH v3 RESEND] " Fabio Fantoni
@ 2014-02-27 14:39 ` Fabio Fantoni
2014-03-10 11:36 ` Ian Jackson
` (2 subsequent siblings)
3 siblings, 0 replies; 13+ messages in thread
From: Fabio Fantoni @ 2014-02-27 14:39 UTC (permalink / raw)
To: xen-devel; +Cc: anthony.perard, Ian.Jackson, Ian.Campbell, Stefano.Stabellini
Il 22/02/2014 11:37, Fabio Fantoni ha scritto:
> Usage:
> vga="none"
>
> Make possible to not have an emulated vga on hvm domUs.
>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>
> ---
>
> Changes in v3:
> - set video_memkb to 0 if vga is none.
> - remove a check on one condition no more needed.
>
> Changes in v2:
> - libxl_dm.c:
> if vga is none, on qemu traditional:
> - add -vga none parameter.
> - do not add -videoram parameter.
>
> ---
> docs/man/xl.cfg.pod.5 | 2 +-
> tools/libxl/libxl_create.c | 6 ++++++
> tools/libxl/libxl_dm.c | 5 +++++
> tools/libxl/libxl_types.idl | 1 +
> tools/libxl/xl_cmdimpl.c | 2 ++
> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index e15a49f..2f36143 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1082,7 +1082,7 @@ This option is deprecated, use vga="stdvga" instead.
>
> =item B<vga="STRING">
>
> -Selects the emulated video card (stdvga|cirrus).
> +Selects the emulated video card (none|stdvga|cirrus).
> The default is cirrus.
>
> =item B<vnc=BOOLEAN>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index a604cd8..9110394 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -226,6 +226,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> switch (b_info->device_model_version) {
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 8 * 1024;
> @@ -246,6 +249,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> default:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 16 * 1024;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index f6f7bbd..761bb61 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -217,6 +217,9 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + flexarray_append_pair(dm_args, "-vga", "none");
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> @@ -515,6 +518,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> GCSPRINTF("vga.vram_size_mb=%d",
> libxl__sizekb_to_mb(b_info->video_memkb)));
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 649ce50..b5a8387 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -153,6 +153,7 @@ libxl_shutdown_reason = Enumeration("shutdown_reason", [
> libxl_vga_interface_type = Enumeration("vga_interface_type", [
> (1, "CIRRUS"),
> (2, "STD"),
> + (3, "NONE"),
> ], init_val = 1)
>
> libxl_vendor_device = Enumeration("vendor_device", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 4fc46eb..4d720b4 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1667,6 +1667,8 @@ skip_vfb:
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
> } else if (!strcmp(buf, "cirrus")) {
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> + } else if (!strcmp(buf, "none")) {
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
> } else {
> fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
> exit(1);
Ping
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-02-22 10:37 ` [PATCH v3 RESEND] " Fabio Fantoni
2014-02-27 14:39 ` Fabio Fantoni
@ 2014-03-10 11:36 ` Ian Jackson
2014-03-10 12:00 ` Fabio Fantoni
2014-03-11 15:09 ` Paul Durrant
2014-03-13 12:21 ` Anthony PERARD
3 siblings, 1 reply; 13+ messages in thread
From: Ian Jackson @ 2014-03-10 11:36 UTC (permalink / raw)
To: Fabio Fantoni; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini
Fabio Fantoni writes ("[PATCH v3 RESEND] libxl: Add none to vga parameter"):
> Usage:
> vga="none"
>
> Make possible to not have an emulated vga on hvm domUs.
...
> @@ -515,6 +518,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> GCSPRINTF("vga.vram_size_mb=%d",
> libxl__sizekb_to_mb(b_info->video_memkb)));
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + break;
Stefano, does this do the right thing on upstream qemu ?
Thanks,
Ian.
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-03-10 11:36 ` Ian Jackson
@ 2014-03-10 12:00 ` Fabio Fantoni
2014-03-10 12:06 ` Ian Jackson
0 siblings, 1 reply; 13+ messages in thread
From: Fabio Fantoni @ 2014-03-10 12:00 UTC (permalink / raw)
To: Ian Jackson; +Cc: anthony.perard, xen-devel, Ian.Campbell, Stefano.Stabellini
Il 10/03/2014 12:36, Ian Jackson ha scritto:
> Fabio Fantoni writes ("[PATCH v3 RESEND] libxl: Add none to vga parameter"):
>> Usage:
>> vga="none"
>>
>> Make possible to not have an emulated vga on hvm domUs.
> ...
>> @@ -515,6 +518,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
>> GCSPRINTF("vga.vram_size_mb=%d",
>> libxl__sizekb_to_mb(b_info->video_memkb)));
>> break;
>> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
>> + break;
> Stefano, does this do the right thing on upstream qemu ?
>
> Thanks,
> Ian.
Is correct since this change:
http://xenbits.xen.org/gitweb/?p=xen.git;a=commit;h=6ef823fdfa701b3659e4161520f43b5835338fb5
v3 of patch also include all improvements recommended by Anthony Perard.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-02-22 10:37 ` [PATCH v3 RESEND] " Fabio Fantoni
2014-02-27 14:39 ` Fabio Fantoni
2014-03-10 11:36 ` Ian Jackson
@ 2014-03-11 15:09 ` Paul Durrant
2014-03-13 10:57 ` Fabio Fantoni
2014-03-13 12:21 ` Anthony PERARD
3 siblings, 1 reply; 13+ messages in thread
From: Paul Durrant @ 2014-03-11 15:09 UTC (permalink / raw)
To: Fabio Fantoni, xen-devel@lists.xensource.com
Cc: Anthony Perard, Ian Jackson, Stefano Stabellini, Ian Campbell
> -----Original Message-----
> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> bounces@lists.xen.org] On Behalf Of Fabio Fantoni
> Sent: 22 February 2014 10:37
> To: xen-devel@lists.xensource.com
> Cc: Anthony Perard; Fabio Fantoni; Ian Jackson; Ian Campbell; Stefano
> Stabellini
> Subject: [Xen-devel] [PATCH v3 RESEND] libxl: Add none to vga parameter
>
> Usage:
> vga="none"
>
> Make possible to not have an emulated vga on hvm domUs.
>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>
> ---
>
> Changes in v3:
> - set video_memkb to 0 if vga is none.
> - remove a check on one condition no more needed.
>
> Changes in v2:
> - libxl_dm.c:
> if vga is none, on qemu traditional:
> - add -vga none parameter.
> - do not add -videoram parameter.
>
> ---
> docs/man/xl.cfg.pod.5 | 2 +-
> tools/libxl/libxl_create.c | 6 ++++++
> tools/libxl/libxl_dm.c | 5 +++++
> tools/libxl/libxl_types.idl | 1 +
> tools/libxl/xl_cmdimpl.c | 2 ++
> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index e15a49f..2f36143 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1082,7 +1082,7 @@ This option is deprecated, use vga="stdvga"
> instead.
>
> =item B<vga="STRING">
>
> -Selects the emulated video card (stdvga|cirrus).
> +Selects the emulated video card (none|stdvga|cirrus).
> The default is cirrus.
>
> =item B<vnc=BOOLEAN>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index a604cd8..9110394 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -226,6 +226,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
> *gc,
> switch (b_info->device_model_version) {
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
I've just been testing this patch in conjunction with my secondary emulator series and this zeroing of the video memory is problematic. I've implemented a secondary console emulator and so still need a vram allocation, so can we still allow a specified videoram value even if the default is zero?
Paul
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 8 * 1024;
> @@ -246,6 +249,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
> *gc,
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> default:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 16 * 1024;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index f6f7bbd..761bb61 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -217,6 +217,9 @@ static char **
> libxl__build_device_model_args_old(libxl__gc *gc,
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + flexarray_append_pair(dm_args, "-vga", "none");
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> @@ -515,6 +518,8 @@ static char **
> libxl__build_device_model_args_new(libxl__gc *gc,
> GCSPRINTF("vga.vram_size_mb=%d",
> libxl__sizekb_to_mb(b_info->video_memkb)));
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 649ce50..b5a8387 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -153,6 +153,7 @@ libxl_shutdown_reason =
> Enumeration("shutdown_reason", [
> libxl_vga_interface_type = Enumeration("vga_interface_type", [
> (1, "CIRRUS"),
> (2, "STD"),
> + (3, "NONE"),
> ], init_val = 1)
>
> libxl_vendor_device = Enumeration("vendor_device", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 4fc46eb..4d720b4 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1667,6 +1667,8 @@ skip_vfb:
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
> } else if (!strcmp(buf, "cirrus")) {
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> + } else if (!strcmp(buf, "none")) {
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
> } else {
> fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
> exit(1);
> --
> 1.7.9.5
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-03-11 15:09 ` Paul Durrant
@ 2014-03-13 10:57 ` Fabio Fantoni
2014-03-13 12:55 ` Anthony PERARD
0 siblings, 1 reply; 13+ messages in thread
From: Fabio Fantoni @ 2014-03-13 10:57 UTC (permalink / raw)
To: Paul Durrant, xen-devel@lists.xensource.com
Cc: Anthony Perard, Ian Jackson, Stefano Stabellini, Ian Campbell
Il 11/03/2014 16:09, Paul Durrant ha scritto:
>> -----Original Message-----
>> From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
>> bounces@lists.xen.org] On Behalf Of Fabio Fantoni
>> Sent: 22 February 2014 10:37
>> To: xen-devel@lists.xensource.com
>> Cc: Anthony Perard; Fabio Fantoni; Ian Jackson; Ian Campbell; Stefano
>> Stabellini
>> Subject: [Xen-devel] [PATCH v3 RESEND] libxl: Add none to vga parameter
>>
>> Usage:
>> vga="none"
>>
>> Make possible to not have an emulated vga on hvm domUs.
>>
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>>
>> ---
>>
>> Changes in v3:
>> - set video_memkb to 0 if vga is none.
>> - remove a check on one condition no more needed.
>>
>> Changes in v2:
>> - libxl_dm.c:
>> if vga is none, on qemu traditional:
>> - add -vga none parameter.
>> - do not add -videoram parameter.
>>
>> ---
>> docs/man/xl.cfg.pod.5 | 2 +-
>> tools/libxl/libxl_create.c | 6 ++++++
>> tools/libxl/libxl_dm.c | 5 +++++
>> tools/libxl/libxl_types.idl | 1 +
>> tools/libxl/xl_cmdimpl.c | 2 ++
>> 5 files changed, 15 insertions(+), 1 deletion(-)
>>
>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>> index e15a49f..2f36143 100644
>> --- a/docs/man/xl.cfg.pod.5
>> +++ b/docs/man/xl.cfg.pod.5
>> @@ -1082,7 +1082,7 @@ This option is deprecated, use vga="stdvga"
>> instead.
>>
>> =item B<vga="STRING">
>>
>> -Selects the emulated video card (stdvga|cirrus).
>> +Selects the emulated video card (none|stdvga|cirrus).
>> The default is cirrus.
>>
>> =item B<vnc=BOOLEAN>
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index a604cd8..9110394 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -226,6 +226,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
>> *gc,
>> switch (b_info->device_model_version) {
>> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
>> switch (b_info->u.hvm.vga.kind) {
>> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
>> + b_info->video_memkb = 0;
> I've just been testing this patch in conjunction with my secondary emulator series and this zeroing of the video memory is problematic. I've implemented a secondary console emulator and so still need a vram allocation, so can we still allow a specified videoram value even if the default is zero?
>
> Paul
If I remember correctly video_memkb is set with xl parameter "videoram"
before setdefault of libxl_create.c, therefore the video_memkb will be 0
anyway.
I added zeroing of video_memkb after advice of anthony perard:
http://lists.xen.org/archives/html/xen-devel/2013-11/msg03692.html
The patch v2 didn't set videoram to zero and it works. If zeroing of
videoram is not needed for something else we could just revert the
change made in v3, otherwise we need to make "videoram" xl parameter
work despite the zeroing.
Can Anthony perard or someone else tell me if it is necessary to set to
zero the videoram if an emulated video card is not present?
Thanks for any reply.
>
>> + break;
>> case LIBXL_VGA_INTERFACE_TYPE_STD:
>> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>> b_info->video_memkb = 8 * 1024;
>> @@ -246,6 +249,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
>> *gc,
>> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
>> default:
>> switch (b_info->u.hvm.vga.kind) {
>> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
>> + b_info->video_memkb = 0;
>> + break;
>> case LIBXL_VGA_INTERFACE_TYPE_STD:
>> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
>> b_info->video_memkb = 16 * 1024;
>> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
>> index f6f7bbd..761bb61 100644
>> --- a/tools/libxl/libxl_dm.c
>> +++ b/tools/libxl/libxl_dm.c
>> @@ -217,6 +217,9 @@ static char **
>> libxl__build_device_model_args_old(libxl__gc *gc,
>> break;
>> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
>> break;
>> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
>> + flexarray_append_pair(dm_args, "-vga", "none");
>> + break;
>> }
>>
>> if (b_info->u.hvm.boot) {
>> @@ -515,6 +518,8 @@ static char **
>> libxl__build_device_model_args_new(libxl__gc *gc,
>> GCSPRINTF("vga.vram_size_mb=%d",
>> libxl__sizekb_to_mb(b_info->video_memkb)));
>> break;
>> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
>> + break;
>> }
>>
>> if (b_info->u.hvm.boot) {
>> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
>> index 649ce50..b5a8387 100644
>> --- a/tools/libxl/libxl_types.idl
>> +++ b/tools/libxl/libxl_types.idl
>> @@ -153,6 +153,7 @@ libxl_shutdown_reason =
>> Enumeration("shutdown_reason", [
>> libxl_vga_interface_type = Enumeration("vga_interface_type", [
>> (1, "CIRRUS"),
>> (2, "STD"),
>> + (3, "NONE"),
>> ], init_val = 1)
>>
>> libxl_vendor_device = Enumeration("vendor_device", [
>> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
>> index 4fc46eb..4d720b4 100644
>> --- a/tools/libxl/xl_cmdimpl.c
>> +++ b/tools/libxl/xl_cmdimpl.c
>> @@ -1667,6 +1667,8 @@ skip_vfb:
>> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
>> } else if (!strcmp(buf, "cirrus")) {
>> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>> + } else if (!strcmp(buf, "none")) {
>> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
>> } else {
>> fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
>> exit(1);
>> --
>> 1.7.9.5
>>
>>
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xen.org
>> http://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-03-13 10:57 ` Fabio Fantoni
@ 2014-03-13 12:55 ` Anthony PERARD
0 siblings, 0 replies; 13+ messages in thread
From: Anthony PERARD @ 2014-03-13 12:55 UTC (permalink / raw)
To: Fabio Fantoni
Cc: Ian Jackson, Paul Durrant, Stefano Stabellini,
xen-devel@lists.xensource.com, Ian Campbell
On Thu, Mar 13, 2014 at 11:57:18AM +0100, Fabio Fantoni wrote:
> Il 11/03/2014 16:09, Paul Durrant ha scritto:
> >>-----Original Message-----
> >>From: xen-devel-bounces@lists.xen.org [mailto:xen-devel-
> >>bounces@lists.xen.org] On Behalf Of Fabio Fantoni
> >>Sent: 22 February 2014 10:37
> >>To: xen-devel@lists.xensource.com
> >>Cc: Anthony Perard; Fabio Fantoni; Ian Jackson; Ian Campbell; Stefano
> >>Stabellini
> >>Subject: [Xen-devel] [PATCH v3 RESEND] libxl: Add none to vga parameter
> >>
> >>Usage:
> >> vga="none"
> >>
> >>Make possible to not have an emulated vga on hvm domUs.
> >>
> >>Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> >>
> >>---
> >>
> >>Changes in v3:
> >>- set video_memkb to 0 if vga is none.
> >>- remove a check on one condition no more needed.
> >>
> >>Changes in v2:
> >>- libxl_dm.c:
> >> if vga is none, on qemu traditional:
> >> - add -vga none parameter.
> >> - do not add -videoram parameter.
> >>
> >>---
> >> docs/man/xl.cfg.pod.5 | 2 +-
> >> tools/libxl/libxl_create.c | 6 ++++++
> >> tools/libxl/libxl_dm.c | 5 +++++
> >> tools/libxl/libxl_types.idl | 1 +
> >> tools/libxl/xl_cmdimpl.c | 2 ++
> >> 5 files changed, 15 insertions(+), 1 deletion(-)
> >>
> >>diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> >>index e15a49f..2f36143 100644
> >>--- a/docs/man/xl.cfg.pod.5
> >>+++ b/docs/man/xl.cfg.pod.5
> >>@@ -1082,7 +1082,7 @@ This option is deprecated, use vga="stdvga"
> >>instead.
> >>
> >> =item B<vga="STRING">
> >>
> >>-Selects the emulated video card (stdvga|cirrus).
> >>+Selects the emulated video card (none|stdvga|cirrus).
> >> The default is cirrus.
> >>
> >> =item B<vnc=BOOLEAN>
> >>diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >>index a604cd8..9110394 100644
> >>--- a/tools/libxl/libxl_create.c
> >>+++ b/tools/libxl/libxl_create.c
> >>@@ -226,6 +226,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
> >>*gc,
> >> switch (b_info->device_model_version) {
> >> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> >> switch (b_info->u.hvm.vga.kind) {
> >>+ case LIBXL_VGA_INTERFACE_TYPE_NONE:
> >>+ b_info->video_memkb = 0;
> >I've just been testing this patch in conjunction with my secondary emulator series and this zeroing of the video memory is problematic. I've implemented a secondary console emulator and so still need a vram allocation, so can we still allow a specified videoram value even if the default is zero?
> >
> > Paul
>
> If I remember correctly video_memkb is set with xl parameter "videoram"
> before setdefault of libxl_create.c, therefore the video_memkb will be 0
> anyway.
> I added zeroing of video_memkb after advice of anthony perard:
> http://lists.xen.org/archives/html/xen-devel/2013-11/msg03692.html
> The patch v2 didn't set videoram to zero and it works. If zeroing of
> videoram is not needed for something else we could just revert the change
> made in v3, otherwise we need to make "videoram" xl parameter work despite
> the zeroing.
> Can Anthony perard or someone else tell me if it is necessary to set to zero
> the videoram if an emulated video card is not present?
Fabio,
I think it should default to 0, yes.
But, like the other cases, set 0 only if
(b_info->video_memkb == LIBXL_MEMKB_DEFAULT), that should be better. And
in this care, "videoram" config option will be taking into account by
xl.
Paul,
"vga=none" will mean no graphic card, so no vram allocation. If
your secondary console emulator does not allocate memory for it self,
then nothing will. I'm not sure that setting "videoram=X" will help, but
it should be taking into account by xl.
Hope that helps.
>
> >
> >>+ break;
> >> case LIBXL_VGA_INTERFACE_TYPE_STD:
> >> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> >> b_info->video_memkb = 8 * 1024;
> >>@@ -246,6 +249,9 @@ int libxl__domain_build_info_setdefault(libxl__gc
> >>*gc,
> >> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> >> default:
> >> switch (b_info->u.hvm.vga.kind) {
> >>+ case LIBXL_VGA_INTERFACE_TYPE_NONE:
> >>+ b_info->video_memkb = 0;
> >>+ break;
> >> case LIBXL_VGA_INTERFACE_TYPE_STD:
> >> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> >> b_info->video_memkb = 16 * 1024;
> >>diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> >>index f6f7bbd..761bb61 100644
--
Anthony PERARD
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter
2014-02-22 10:37 ` [PATCH v3 RESEND] " Fabio Fantoni
` (2 preceding siblings ...)
2014-03-11 15:09 ` Paul Durrant
@ 2014-03-13 12:21 ` Anthony PERARD
2014-03-13 18:11 ` [PATCH v3 RESEND] libxl: Add none to vga parameter [and 2 more messages] Ian Jackson
3 siblings, 1 reply; 13+ messages in thread
From: Anthony PERARD @ 2014-03-13 12:21 UTC (permalink / raw)
To: Fabio Fantoni; +Cc: xen-devel, Ian.Jackson, Ian.Campbell, Stefano.Stabellini
On Sat, Feb 22, 2014 at 11:37:11AM +0100, Fabio Fantoni wrote:
> Usage:
> vga="none"
>
> Make possible to not have an emulated vga on hvm domUs.
>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>
Thanks,
> ---
>
> Changes in v3:
> - set video_memkb to 0 if vga is none.
> - remove a check on one condition no more needed.
>
> Changes in v2:
> - libxl_dm.c:
> if vga is none, on qemu traditional:
> - add -vga none parameter.
> - do not add -videoram parameter.
>
> ---
> docs/man/xl.cfg.pod.5 | 2 +-
> tools/libxl/libxl_create.c | 6 ++++++
> tools/libxl/libxl_dm.c | 5 +++++
> tools/libxl/libxl_types.idl | 1 +
> tools/libxl/xl_cmdimpl.c | 2 ++
> 5 files changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index e15a49f..2f36143 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1082,7 +1082,7 @@ This option is deprecated, use vga="stdvga" instead.
>
> =item B<vga="STRING">
>
> -Selects the emulated video card (stdvga|cirrus).
> +Selects the emulated video card (none|stdvga|cirrus).
> The default is cirrus.
>
> =item B<vnc=BOOLEAN>
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index a604cd8..9110394 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -226,6 +226,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> switch (b_info->device_model_version) {
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 8 * 1024;
> @@ -246,6 +249,9 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
> default:
> switch (b_info->u.hvm.vga.kind) {
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + b_info->video_memkb = 0;
> + break;
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> if (b_info->video_memkb == LIBXL_MEMKB_DEFAULT)
> b_info->video_memkb = 16 * 1024;
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index f6f7bbd..761bb61 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -217,6 +217,9 @@ static char ** libxl__build_device_model_args_old(libxl__gc *gc,
> break;
> case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + flexarray_append_pair(dm_args, "-vga", "none");
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> @@ -515,6 +518,8 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> GCSPRINTF("vga.vram_size_mb=%d",
> libxl__sizekb_to_mb(b_info->video_memkb)));
> break;
> + case LIBXL_VGA_INTERFACE_TYPE_NONE:
> + break;
> }
>
> if (b_info->u.hvm.boot) {
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 649ce50..b5a8387 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -153,6 +153,7 @@ libxl_shutdown_reason = Enumeration("shutdown_reason", [
> libxl_vga_interface_type = Enumeration("vga_interface_type", [
> (1, "CIRRUS"),
> (2, "STD"),
> + (3, "NONE"),
> ], init_val = 1)
>
> libxl_vendor_device = Enumeration("vendor_device", [
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 4fc46eb..4d720b4 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1667,6 +1667,8 @@ skip_vfb:
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD;
> } else if (!strcmp(buf, "cirrus")) {
> b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
> + } else if (!strcmp(buf, "none")) {
> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_NONE;
> } else {
> fprintf(stderr, "Unknown vga \"%s\" specified\n", buf);
> exit(1);
--
Anthony PERARD
^ permalink raw reply [flat|nested] 13+ messages in thread* Re: [PATCH v3 RESEND] libxl: Add none to vga parameter [and 2 more messages]
2014-03-13 12:21 ` Anthony PERARD
@ 2014-03-13 18:11 ` Ian Jackson
0 siblings, 0 replies; 13+ messages in thread
From: Ian Jackson @ 2014-03-13 18:11 UTC (permalink / raw)
To: Paul Durrant, Fabio Fantoni, Anthony PERARD
Cc: xen-devel, Stefano Stabellini, Ian.Campbell, Stefano.Stabellini
Fabio Fantoni writes ("[PATCH v4] libxl: Add none to vga parameter"):
> Usage:
> vga="none"
>
> Make possible to not have an emulated vga on hvm domUs.
>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
Thanks everyone, I have applied this patch.
Fabio, for future reference, it is helpful if you incorporate the
acks/reviewed-by tags etc. you have acquired, when you repost. That
way there isn't a risk of me (for example) overlooking Anthony's
review, and waiting for a qemu-related ack.
Thanks,
Ian.
^ permalink raw reply [flat|nested] 13+ messages in thread