* [PATCH 1/3] libxl:refactor stdvga option support v2
@ 2012-06-05 11:19 ZhouPeng
2012-06-06 10:50 ` Ian Jackson
2012-06-06 11:47 ` Ian Campbell
0 siblings, 2 replies; 13+ messages in thread
From: ZhouPeng @ 2012-06-05 11:19 UTC (permalink / raw)
To: Ian Campbell; +Cc: Xen-Devel (E-mail), ian.jackson, Stefano Stabellini
[-- Attachment #1: Type: text/plain, Size: 5859 bytes --]
changeset: 25453:7bd08f83a2ce
user: Zhou Peng <ailvpeng25@gmail.com>
date: Tue Jun 05 17:39:37 2012 +0800
files: tools/libxl/libxl.h tools/libxl/libxl_create.c
tools/libxl/libxl_dm.c tools/libxl/libxl_types.idl
tools/libxl/xl_cmdimpl.c tools/libxl/xl_sxp.c
description:
tools:libxl: refactor stdvga opinon support.
Be ready to add and describe new vga interface
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl.h Tue Jun 05 17:39:37 2012 +0800
@@ -342,6 +342,7 @@ typedef struct libxl__ctx libxl_ctx;
#define LIBXL_TIMER_MODE_DEFAULT -1
#define LIBXL_MEMKB_DEFAULT ~0ULL
+#define LIBXL_VGA_INTERFACE_TYPE_DEFAULT -1
#include "_libxl_types.h"
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
@@ -193,7 +193,8 @@ int libxl__domain_build_info_setdefault(
if (!b_info->u.hvm.boot) return ERROR_NOMEM;
}
- libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false);
+ if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
@@ -175,8 +175,13 @@ static char ** libxl__build_device_model
libxl__sizekb_to_mb(b_info->video_memkb)),
NULL);
}
- if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
+
+ switch (b_info->u.hvm.vga.type) {
+ case LIBXL_VGA_INTERFACE_TYPE_STD:
flexarray_append(dm_args, "-std-vga");
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -418,8 +423,13 @@ static char ** libxl__build_device_model
flexarray_append(dm_args, spiceoptions);
}
- if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
- flexarray_vappend(dm_args, "-vga", "std", NULL);
+ switch (b_info->u.hvm.vga.type) {
+ case LIBXL_VGA_INTERFACE_TYPE_STD:
+ flexarray_vappend(dm_args, "-vga", "std", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
}
if (b_info->u.hvm.boot) {
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
@@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu
(4, "watchdog"),
])
+libxl_vga_interface_type = Enumeration("vga_interface_type", [
+ (0, "CIRRUS"),
+ (1, "STD"),
+ ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
+
#
# Complex libxl types
#
+
+libxl_vga_interface_info = Struct("vga_interface_info", [
+ ("type", libxl_vga_interface_type),
+ ])
+
libxl_vnc_info = Struct("vnc_info", [
("enable", libxl_defbool),
# "address:port" that should be listened on
@@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain
("nested_hvm", libxl_defbool),
("incr_generationid",libxl_defbool),
("nographic", libxl_defbool),
- ("stdvga", libxl_defbool),
+ ("vga",
libxl_vga_interface_info),
("vnc", libxl_vnc_info),
# keyboard layout, default is
en-us keyboard
("keymap", string),
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
@@ -1256,7 +1256,10 @@ skip_vfb:
#undef parse_extra_args
if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0);
+ if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
+ if (l)
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
+
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);
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800
@@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d
libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
printf("\t\t\t(no_incr_generationid %s)\n",
libxl_defbool_to_string(b_info->u.hvm.incr_generationid));
- printf("\t\t\t(stdvga %s)\n",
- libxl_defbool_to_string(b_info->u.hvm.stdvga));
+ printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type ==
+ LIBXL_VGA_INTERFACE_TYPE_STD ?
+ "True" : "False");
printf("\t\t\t(vnc %s)\n",
libxl_defbool_to_string(b_info->u.hvm.vnc.enable));
printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen);
--
Zhou Peng
[-- Attachment #2: spice.tools.libxl.stdvga.refactor.v2.diff --]
[-- Type: application/octet-stream, Size: 5866 bytes --]
changeset: 25453:7bd08f83a2ce
user: Zhou Peng <ailvpeng25@gmail.com>
date: Tue Jun 05 17:39:37 2012 +0800
files: tools/libxl/libxl.h tools/libxl/libxl_create.c tools/libxl/libxl_dm.c tools/libxl/libxl_types.idl tools/libxl/xl_cmdimpl.c tools/libxl/xl_sxp.c
description:
tools:libxl: refactor stdvga opinon support.
Be ready to add and describe new vga interface
Signed-off-by: Zhou Peng <ailvpeng25@gmail.com>
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl.h
--- a/tools/libxl/libxl.h Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl.h Tue Jun 05 17:39:37 2012 +0800
@@ -342,6 +342,7 @@ typedef struct libxl__ctx libxl_ctx;
#define LIBXL_TIMER_MODE_DEFAULT -1
#define LIBXL_MEMKB_DEFAULT ~0ULL
+#define LIBXL_VGA_INTERFACE_TYPE_DEFAULT -1
#include "_libxl_types.h"
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_create.c
--- a/tools/libxl/libxl_create.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_create.c Tue Jun 05 17:39:37 2012 +0800
@@ -193,7 +193,8 @@ int libxl__domain_build_info_setdefault(
if (!b_info->u.hvm.boot) return ERROR_NOMEM;
}
- libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false);
+ if (b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_DEFAULT)
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true);
if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) {
libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true);
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_dm.c
--- a/tools/libxl/libxl_dm.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_dm.c Tue Jun 05 17:39:37 2012 +0800
@@ -175,8 +175,13 @@ static char ** libxl__build_device_model
libxl__sizekb_to_mb(b_info->video_memkb)),
NULL);
}
- if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
+
+ switch (b_info->u.hvm.vga.type) {
+ case LIBXL_VGA_INTERFACE_TYPE_STD:
flexarray_append(dm_args, "-std-vga");
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -418,8 +423,13 @@ static char ** libxl__build_device_model
flexarray_append(dm_args, spiceoptions);
}
- if (libxl_defbool_val(b_info->u.hvm.stdvga)) {
- flexarray_vappend(dm_args, "-vga", "std", NULL);
+ switch (b_info->u.hvm.vga.type) {
+ case LIBXL_VGA_INTERFACE_TYPE_STD:
+ flexarray_vappend(dm_args, "-vga", "std", NULL);
+ break;
+ case LIBXL_VGA_INTERFACE_TYPE_CIRRUS:
+ flexarray_vappend(dm_args, "-vga", "cirrus", NULL);
+ break;
}
if (b_info->u.hvm.boot) {
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/libxl_types.idl Tue Jun 05 17:39:37 2012 +0800
@@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu
(4, "watchdog"),
])
+libxl_vga_interface_type = Enumeration("vga_interface_type", [
+ (0, "CIRRUS"),
+ (1, "STD"),
+ ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT")
+
#
# Complex libxl types
#
+
+libxl_vga_interface_info = Struct("vga_interface_info", [
+ ("type", libxl_vga_interface_type),
+ ])
+
libxl_vnc_info = Struct("vnc_info", [
("enable", libxl_defbool),
# "address:port" that should be listened on
@@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain
("nested_hvm", libxl_defbool),
("incr_generationid",libxl_defbool),
("nographic", libxl_defbool),
- ("stdvga", libxl_defbool),
+ ("vga", libxl_vga_interface_info),
("vnc", libxl_vnc_info),
# keyboard layout, default is en-us keyboard
("keymap", string),
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800
@@ -1256,7 +1256,10 @@ skip_vfb:
#undef parse_extra_args
if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
- xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0);
+ if (!xlu_cfg_get_long(config, "stdvga", &l, 0))
+ if (l)
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;
+
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);
diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c
--- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100
+++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800
@@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d
libxl_defbool_to_string(b_info->u.hvm.nested_hvm));
printf("\t\t\t(no_incr_generationid %s)\n",
libxl_defbool_to_string(b_info->u.hvm.incr_generationid));
- printf("\t\t\t(stdvga %s)\n",
- libxl_defbool_to_string(b_info->u.hvm.stdvga));
+ printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type ==
+ LIBXL_VGA_INTERFACE_TYPE_STD ?
+ "True" : "False");
printf("\t\t\t(vnc %s)\n",
libxl_defbool_to_string(b_info->u.hvm.vnc.enable));
printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen);
[-- Attachment #3: Type: text/plain, Size: 126 bytes --]
_______________________________________________
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 1/3] libxl:refactor stdvga option support v2 2012-06-05 11:19 [PATCH 1/3] libxl:refactor stdvga option support v2 ZhouPeng @ 2012-06-06 10:50 ` Ian Jackson 2012-06-06 11:10 ` ZhouPeng 2012-06-06 11:47 ` Ian Campbell 1 sibling, 1 reply; 13+ messages in thread From: Ian Jackson @ 2012-06-06 10:50 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Campbell, Stefano Stabellini ZhouPeng writes ("[Xen-devel] [PATCH 1/3] libxl:refactor stdvga option support v2"): > description: > tools:libxl: refactor stdvga opinon support. > > Be ready to add and describe new vga interface Thanks. If Ian C is happy with the idl changes then I am happy with this interface change for 4.2. Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> However I think patches 2 and 3 of your series come too late for the 4.2 freeze. Thanks for showing them to us, though. We should reconsider them after 4.2. Ian. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-06 10:50 ` Ian Jackson @ 2012-06-06 11:10 ` ZhouPeng 0 siblings, 0 replies; 13+ messages in thread From: ZhouPeng @ 2012-06-06 11:10 UTC (permalink / raw) To: Ian Jackson; +Cc: Xen-Devel (E-mail), Ian Campbell, Stefano Stabellini On Wed, Jun 6, 2012 at 6:50 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote: > ZhouPeng writes ("[Xen-devel] [PATCH 1/3] libxl:refactor stdvga option support v2"): >> description: >> tools:libxl: refactor stdvga opinon support. >> >> Be ready to add and describe new vga interface > > Thanks. > > If Ian C is happy with the idl changes then I am happy with this > interface change for 4.2. > > Acked-by: Ian Jackson <ian.jackson@eu.citrix.com> > > However I think patches 2 and 3 of your series come too late for the > 4.2 freeze. Thanks for showing them to us, though. We should > reconsider them after 4.2. It's ok to wait until the 4.3 dev cycle. Thanks. > Ian. -- Zhou Peng ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-05 11:19 [PATCH 1/3] libxl:refactor stdvga option support v2 ZhouPeng 2012-06-06 10:50 ` Ian Jackson @ 2012-06-06 11:47 ` Ian Campbell 2012-06-07 2:34 ` ZhouPeng 1 sibling, 1 reply; 13+ messages in thread From: Ian Campbell @ 2012-06-06 11:47 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini On Tue, 2012-06-05 at 12:19 +0100, ZhouPeng wrote: > # Complex libxl types > # > + > +libxl_vga_interface_info = Struct("vga_interface_info", [ > + ("type", libxl_vga_interface_type), > + ]) Unfortunately "type" is a reserved word in ocaml (and possibly other languages, which causes the bindings to fail to build: make[4]: Entering directory `/local/scratch/ianc/devel/committer.git/tools/ocaml/libs/xl' MLDEP File "xenlight.ml", line 116, characters 2-6: Error: Syntax error MLI xenlight.cmi File "xenlight.mli", line 116, characters 2-6: Error: Syntax error: 'end' expected File "xenlight.mli", line 113, characters 28-31: Error: This 'sig' might be unmatched Ideally we'd make the bindings generator do appropriate substitutions on keywords but the usual workaround is to s/type/kind for the field name. BTW, I wasn't going to bounce the patch over this but could LIBXL_VGA_INTERFACE_TYPE_DEFAULT not be part of the IDL definition of the type? I'm not sure why we don't do the same for LIBXL_TIMER_MODE_DEFAULT already. Ian. > + > libxl_vnc_info = Struct("vnc_info", [ > ("enable", libxl_defbool), > # "address:port" that should be listened on > @@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain > ("nested_hvm", libxl_defbool), > ("incr_generationid",libxl_defbool), > ("nographic", libxl_defbool), > - ("stdvga", libxl_defbool), > + ("vga", > libxl_vga_interface_info), > ("vnc", libxl_vnc_info), > # keyboard layout, default is > en-us keyboard > ("keymap", string), > diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800 > @@ -1256,7 +1256,10 @@ skip_vfb: > #undef parse_extra_args > > if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > + if (l) > + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD; > + > 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); > diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c > --- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100 > +++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800 > @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d > libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); > printf("\t\t\t(no_incr_generationid %s)\n", > libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); > - printf("\t\t\t(stdvga %s)\n", > - libxl_defbool_to_string(b_info->u.hvm.stdvga)); > + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type == > + LIBXL_VGA_INTERFACE_TYPE_STD ? > + "True" : "False"); > printf("\t\t\t(vnc %s)\n", > libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); > printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-06 11:47 ` Ian Campbell @ 2012-06-07 2:34 ` ZhouPeng 2012-06-20 18:38 ` Ian Campbell 0 siblings, 1 reply; 13+ messages in thread From: ZhouPeng @ 2012-06-07 2:34 UTC (permalink / raw) To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini On Wed, Jun 6, 2012 at 7:47 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Tue, 2012-06-05 at 12:19 +0100, ZhouPeng wrote: >> # Complex libxl types >> # >> + >> +libxl_vga_interface_info = Struct("vga_interface_info", [ >> + ("type", libxl_vga_interface_type), >> + ]) > > Unfortunately "type" is a reserved word in ocaml (and possibly other > languages, which causes the bindings to fail to build: Thanks for your review. I didn't build against ocaml. I 'make install' in tools/libxl directly. > make[4]: Entering directory `/local/scratch/ianc/devel/committer.git/tools/ocaml/libs/xl' > MLDEP > File "xenlight.ml", line 116, characters 2-6: > Error: Syntax error > MLI xenlight.cmi > File "xenlight.mli", line 116, characters 2-6: > Error: Syntax error: 'end' expected > File "xenlight.mli", line 113, characters 28-31: > Error: This 'sig' might be unmatched > > Ideally we'd make the bindings generator do appropriate substitutions on > keywords but the usual workaround is to s/type/kind for the field name. > > BTW, I wasn't going to bounce the patch over this but could Sorry, I'm not farmiliar with the vocabulary 'bounce', do you mean s/type/kind is necessary or not? I think you mean necessary, right? > LIBXL_VGA_INTERFACE_TYPE_DEFAULT not be part of the IDL definition of > the type? do you mean this way below? libxl_vga_interface_type = Enumeration("vga_interface_type", [ (-1, "DEFAULT"), (0, "CIRRUS"), (1, "STD"), ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT") In my understanding, LIBXL_VGA_INTERFACE_TYPE_DEFAULT is not really a meaningful vga type (even not present default vga to use, but just tag the variable has never be initialized, so in meaningless state). It's only used to check if libxl_vga_interface_type var is initialized (whether stdvga setted by vm.cfg), to avoid random value initialized. It's equal with LIBXL_MEMKB_DEFAULT in this context. It's the same with LIBXL_TIMER_MODE_DEFAULT > I'm not sure why we don't do the same for > LIBXL_TIMER_MODE_DEFAULT already. > > Ian. > > >> + >> libxl_vnc_info = Struct("vnc_info", [ >> ("enable", libxl_defbool), >> # "address:port" that should be listened on >> @@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain >> ("nested_hvm", libxl_defbool), >> ("incr_generationid",libxl_defbool), >> ("nographic", libxl_defbool), >> - ("stdvga", libxl_defbool), >> + ("vga", >> libxl_vga_interface_info), >> ("vnc", libxl_vnc_info), >> # keyboard layout, default is >> en-us keyboard >> ("keymap", string), >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c >> --- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100 >> +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800 >> @@ -1256,7 +1256,10 @@ skip_vfb: >> #undef parse_extra_args >> >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) >> + if (l) >> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD; >> + >> 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); >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c >> --- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100 >> +++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800 >> @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d >> libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); >> printf("\t\t\t(no_incr_generationid %s)\n", >> libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); >> - printf("\t\t\t(stdvga %s)\n", >> - libxl_defbool_to_string(b_info->u.hvm.stdvga)); >> + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type == >> + LIBXL_VGA_INTERFACE_TYPE_STD ? >> + "True" : "False"); >> printf("\t\t\t(vnc %s)\n", >> libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); >> printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); >> >> > > -- Zhou Peng ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-07 2:34 ` ZhouPeng @ 2012-06-20 18:38 ` Ian Campbell 2012-06-27 12:16 ` ZhouPeng 0 siblings, 1 reply; 13+ messages in thread From: Ian Campbell @ 2012-06-20 18:38 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini Sorry for the late response, I seem to have lost this reply on my desktop somewhere... On Thu, 2012-06-07 at 03:34 +0100, ZhouPeng wrote: > On Wed, Jun 6, 2012 at 7:47 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Tue, 2012-06-05 at 12:19 +0100, ZhouPeng wrote: > >> # Complex libxl types > >> # > >> + > >> +libxl_vga_interface_info = Struct("vga_interface_info", [ > >> + ("type", libxl_vga_interface_type), > >> + ]) > > > > Unfortunately "type" is a reserved word in ocaml (and possibly other > > languages, which causes the bindings to fail to build: > Thanks for your review. > > I didn't build against ocaml. > I 'make install' in tools/libxl directly. > > make[4]: Entering directory `/local/scratch/ianc/devel/committer.git/tools/ocaml/libs/xl' > > MLDEP > > File "xenlight.ml", line 116, characters 2-6: > > Error: Syntax error > > MLI xenlight.cmi > > File "xenlight.mli", line 116, characters 2-6: > > Error: Syntax error: 'end' expected > > File "xenlight.mli", line 113, characters 28-31: > > Error: This 'sig' might be unmatched > > > > Ideally we'd make the bindings generator do appropriate substitutions on > > keywords but the usual workaround is to s/type/kind for the field name. > > > > BTW, I wasn't going to bounce the patch over this but could > > Sorry, I'm not farmiliar with the vocabulary 'bounce', > do you mean s/type/kind is necessary or not? Yes, it is necessary. By "bounce" I meant ask you to resubmit. IOW I wasn't going to ask you to resubmit over the LIBXL_VGA_INTERFACE_TYPE_DEFAULT part, but I figured since you needed to fix the ocaml stuff I would mention it. > I think you mean necessary, right? > > LIBXL_VGA_INTERFACE_TYPE_DEFAULT not be part of the IDL definition of > > the type? > do you mean this way below? > libxl_vga_interface_type = Enumeration("vga_interface_type", [ > (-1, "DEFAULT"), > (0, "CIRRUS"), > (1, "STD"), > ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT") > > In my understanding, LIBXL_VGA_INTERFACE_TYPE_DEFAULT is not really > a meaningful vga type (even not present default vga to use, but just tag the > variable has never be initialized, so in meaningless state). > It's only used to check if libxl_vga_interface_type var is > initialized (whether stdvga setted by vm.cfg), to > avoid random value initialized. Actually, looking at the definition of libxl_vga_interface_type you should avoid using 0 as a real thing and then this becomes libxl_vga_interface_type = Enumeration("vga_interface_type", [ (1, "CIRRUS"), (2, "STD"), ]) Since the "default default" is 0 you can test for default just by using !b_info.... here. You'll notice that most other enums in the IDL have this property except where the specific values come from elsewhere (like timer mode). Several enums also include "UNKNOWN" as an explicit entry, which is much the same as including "DEFAULT" IMHO. > It's equal with LIBXL_MEMKB_DEFAULT in this context. This is semantically a bit different to the enum case. > It's the same with LIBXL_TIMER_MODE_DEFAULT TIMER_MODE is not a good example to follow because the specific values come from the hypervisor and libxl simple matches them. > > I'm not sure why we don't do the same for > > LIBXL_TIMER_MODE_DEFAULT already. > > > > Ian. > > > > > >> + > >> libxl_vnc_info = Struct("vnc_info", [ > >> ("enable", libxl_defbool), > >> # "address:port" that should be listened on > >> @@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain > >> ("nested_hvm", libxl_defbool), > >> ("incr_generationid",libxl_defbool), > >> ("nographic", libxl_defbool), > >> - ("stdvga", libxl_defbool), > >> + ("vga", > >> libxl_vga_interface_info), > >> ("vnc", libxl_vnc_info), > >> # keyboard layout, default is > >> en-us keyboard > >> ("keymap", string), > >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c > >> --- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100 > >> +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800 > >> @@ -1256,7 +1256,10 @@ skip_vfb: > >> #undef parse_extra_args > >> > >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > >> + if (l) > >> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD; > >> + > >> 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); > >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c > >> --- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100 > >> +++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800 > >> @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d > >> libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); > >> printf("\t\t\t(no_incr_generationid %s)\n", > >> libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); > >> - printf("\t\t\t(stdvga %s)\n", > >> - libxl_defbool_to_string(b_info->u.hvm.stdvga)); > >> + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type == > >> + LIBXL_VGA_INTERFACE_TYPE_STD ? > >> + "True" : "False"); > >> printf("\t\t\t(vnc %s)\n", > >> libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); > >> printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); > >> > >> > > > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-20 18:38 ` Ian Campbell @ 2012-06-27 12:16 ` ZhouPeng 2012-06-27 15:28 ` Ian Campbell 0 siblings, 1 reply; 13+ messages in thread From: ZhouPeng @ 2012-06-27 12:16 UTC (permalink / raw) To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 11696 bytes --] New version patch. ----- tools:libxl: refactor stdvga opinon support. Be ready to add and describe new vga interface Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> diff -r 592d15bd4d5e tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_create.c Wed Jun 27 20:06:39 2012 +0800 @@ -189,7 +189,8 @@ int libxl__domain_build_info_setdefault( if (!b_info->u.hvm.boot) return ERROR_NOMEM; } - libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false); + if (!b_info->u.hvm.vga.kind) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); diff -r 592d15bd4d5e tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_dm.c Wed Jun 27 20:06:39 2012 +0800 @@ -175,8 +175,13 @@ static char ** libxl__build_device_model libxl__sizekb_to_mb(b_info->video_memkb)), NULL); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { + + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: flexarray_append(dm_args, "-std-vga"); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + break; } if (b_info->u.hvm.boot) { @@ -418,8 +423,13 @@ static char ** libxl__build_device_model flexarray_append(dm_args, spiceoptions); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { - flexarray_vappend(dm_args, "-vga", "std", NULL); + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: + flexarray_vappend(dm_args, "-vga", "std", NULL); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + flexarray_vappend(dm_args, "-vga", "cirrus", NULL); + break; } if (b_info->u.hvm.boot) { diff -r 592d15bd4d5e tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_types.idl Wed Jun 27 20:06:39 2012 +0800 @@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu (4, "watchdog"), ]) +libxl_vga_interface_type = Enumeration("vga_interface_type", [ + (1, "CIRRUS"), + (2, "STD"), + ], init_val = 0) + # # Complex libxl types # + +libxl_vga_interface_info = Struct("vga_interface_info", [ + ("kind", libxl_vga_interface_type), + ]) + libxl_vnc_info = Struct("vnc_info", [ ("enable", libxl_defbool), # "address:port" that should be listened on @@ -286,7 +296,7 @@ libxl_domain_build_info = Struct("domain ("nested_hvm", libxl_defbool), ("incr_generationid",libxl_defbool), ("nographic", libxl_defbool), - ("stdvga", libxl_defbool), + ("vga", libxl_vga_interface_info), ("vnc", libxl_vnc_info), # keyboard layout, default is en-us keyboard ("keymap", string), diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 @@ -1256,7 +1256,10 @@ skip_vfb: #undef parse_extra_args if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) + if (l) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; + 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); diff -r 592d15bd4d5e tools/libxl/xl_sxp.c --- a/tools/libxl/xl_sxp.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_sxp.c Wed Jun 27 20:06:39 2012 +0800 @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); printf("\t\t\t(no_incr_generationid %s)\n", libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); - printf("\t\t\t(stdvga %s)\n", - libxl_defbool_to_string(b_info->u.hvm.stdvga)); + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind == + LIBXL_VGA_INTERFACE_TYPE_STD ? + "True" : "False"); printf("\t\t\t(vnc %s)\n", libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); On Thu, Jun 21, 2012 at 2:38 AM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > Sorry for the late response, I seem to have lost this reply on my desktop somewhere... > > On Thu, 2012-06-07 at 03:34 +0100, ZhouPeng wrote: >> On Wed, Jun 6, 2012 at 7:47 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> > On Tue, 2012-06-05 at 12:19 +0100, ZhouPeng wrote: >> >> # Complex libxl types >> >> # >> >> + >> >> +libxl_vga_interface_info = Struct("vga_interface_info", [ >> >> + ("type", libxl_vga_interface_type), >> >> + ]) >> > >> > Unfortunately "type" is a reserved word in ocaml (and possibly other >> > languages, which causes the bindings to fail to build: >> Thanks for your review. >> >> I didn't build against ocaml. >> I 'make install' in tools/libxl directly. >> > make[4]: Entering directory `/local/scratch/ianc/devel/committer.git/tools/ocaml/libs/xl' >> > MLDEP >> > File "xenlight.ml", line 116, characters 2-6: >> > Error: Syntax error >> > MLI xenlight.cmi >> > File "xenlight.mli", line 116, characters 2-6: >> > Error: Syntax error: 'end' expected >> > File "xenlight.mli", line 113, characters 28-31: >> > Error: This 'sig' might be unmatched >> > >> > Ideally we'd make the bindings generator do appropriate substitutions on >> > keywords but the usual workaround is to s/type/kind for the field name. >> > >> > BTW, I wasn't going to bounce the patch over this but could >> >> Sorry, I'm not farmiliar with the vocabulary 'bounce', >> do you mean s/type/kind is necessary or not? > > Yes, it is necessary. > > By "bounce" I meant ask you to resubmit. IOW I wasn't going to ask you > to resubmit over the LIBXL_VGA_INTERFACE_TYPE_DEFAULT part, but I > figured since you needed to fix the ocaml stuff I would mention it. > >> I think you mean necessary, right? >> > LIBXL_VGA_INTERFACE_TYPE_DEFAULT not be part of the IDL definition of >> > the type? >> do you mean this way below? >> libxl_vga_interface_type = Enumeration("vga_interface_type", [ >> (-1, "DEFAULT"), >> (0, "CIRRUS"), >> (1, "STD"), >> ], init_val = "LIBXL_VGA_INTERFACE_TYPE_DEFAULT") >> >> In my understanding, LIBXL_VGA_INTERFACE_TYPE_DEFAULT is not really >> a meaningful vga type (even not present default vga to use, but just tag the >> variable has never be initialized, so in meaningless state). >> It's only used to check if libxl_vga_interface_type var is >> initialized (whether stdvga setted by vm.cfg), to >> avoid random value initialized. > > Actually, looking at the definition of libxl_vga_interface_type you > should avoid using 0 as a real thing and then this becomes > libxl_vga_interface_type = Enumeration("vga_interface_type", [ > (1, "CIRRUS"), > (2, "STD"), > ]) > > Since the "default default" is 0 you can test for default just by > using !b_info.... here. > > You'll notice that most other enums in the IDL have this property except > where the specific values come from elsewhere (like timer mode). > > Several enums also include "UNKNOWN" as an explicit entry, which is much > the same as including "DEFAULT" IMHO. > >> It's equal with LIBXL_MEMKB_DEFAULT in this context. > > This is semantically a bit different to the enum case. > >> It's the same with LIBXL_TIMER_MODE_DEFAULT > > TIMER_MODE is not a good example to follow because the specific values > come from the hypervisor and libxl simple matches them. > >> > I'm not sure why we don't do the same for >> > LIBXL_TIMER_MODE_DEFAULT already. >> > >> > Ian. >> > >> > >> >> + >> >> libxl_vnc_info = Struct("vnc_info", [ >> >> ("enable", libxl_defbool), >> >> # "address:port" that should be listened on >> >> @@ -281,7 +291,7 @@ libxl_domain_build_info = Struct("domain >> >> ("nested_hvm", libxl_defbool), >> >> ("incr_generationid",libxl_defbool), >> >> ("nographic", libxl_defbool), >> >> - ("stdvga", libxl_defbool), >> >> + ("vga", >> >> libxl_vga_interface_info), >> >> ("vnc", libxl_vnc_info), >> >> # keyboard layout, default is >> >> en-us keyboard >> >> ("keymap", string), >> >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_cmdimpl.c >> >> --- a/tools/libxl/xl_cmdimpl.c Sat Jun 02 08:39:45 2012 +0100 >> >> +++ b/tools/libxl/xl_cmdimpl.c Tue Jun 05 17:39:37 2012 +0800 >> >> @@ -1256,7 +1256,10 @@ skip_vfb: >> >> #undef parse_extra_args >> >> >> >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { >> >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); >> >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) >> >> + if (l) >> >> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD; >> >> + >> >> 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); >> >> diff -r 6bea63e6c780 -r 7bd08f83a2ce tools/libxl/xl_sxp.c >> >> --- a/tools/libxl/xl_sxp.c Sat Jun 02 08:39:45 2012 +0100 >> >> +++ b/tools/libxl/xl_sxp.c Tue Jun 05 17:39:37 2012 +0800 >> >> @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d >> >> libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); >> >> printf("\t\t\t(no_incr_generationid %s)\n", >> >> libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); >> >> - printf("\t\t\t(stdvga %s)\n", >> >> - libxl_defbool_to_string(b_info->u.hvm.stdvga)); >> >> + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.type == >> >> + LIBXL_VGA_INTERFACE_TYPE_STD ? >> >> + "True" : "False"); >> >> printf("\t\t\t(vnc %s)\n", >> >> libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); >> >> printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); >> >> >> >> >> > >> > >> >> >> > > > > > > > > > -- Zhou Peng [-- Attachment #2: spice.tools.libxl.stdvga.refactor.v3.diff --] [-- Type: application/octet-stream, Size: 5046 bytes --] tools:libxl: refactor stdvga opinon support. Be ready to add and describe new vga interface Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> diff -r 592d15bd4d5e tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_create.c Wed Jun 27 20:06:39 2012 +0800 @@ -189,7 +189,8 @@ int libxl__domain_build_info_setdefault( if (!b_info->u.hvm.boot) return ERROR_NOMEM; } - libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false); + if (!b_info->u.hvm.vga.kind) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); diff -r 592d15bd4d5e tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_dm.c Wed Jun 27 20:06:39 2012 +0800 @@ -175,8 +175,13 @@ static char ** libxl__build_device_model libxl__sizekb_to_mb(b_info->video_memkb)), NULL); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { + + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: flexarray_append(dm_args, "-std-vga"); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + break; } if (b_info->u.hvm.boot) { @@ -418,8 +423,13 @@ static char ** libxl__build_device_model flexarray_append(dm_args, spiceoptions); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { - flexarray_vappend(dm_args, "-vga", "std", NULL); + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: + flexarray_vappend(dm_args, "-vga", "std", NULL); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + flexarray_vappend(dm_args, "-vga", "cirrus", NULL); + break; } if (b_info->u.hvm.boot) { diff -r 592d15bd4d5e tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_types.idl Wed Jun 27 20:06:39 2012 +0800 @@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu (4, "watchdog"), ]) +libxl_vga_interface_type = Enumeration("vga_interface_type", [ + (1, "CIRRUS"), + (2, "STD"), + ], init_val = 0) + # # Complex libxl types # + +libxl_vga_interface_info = Struct("vga_interface_info", [ + ("kind", libxl_vga_interface_type), + ]) + libxl_vnc_info = Struct("vnc_info", [ ("enable", libxl_defbool), # "address:port" that should be listened on @@ -286,7 +296,7 @@ libxl_domain_build_info = Struct("domain ("nested_hvm", libxl_defbool), ("incr_generationid",libxl_defbool), ("nographic", libxl_defbool), - ("stdvga", libxl_defbool), + ("vga", libxl_vga_interface_info), ("vnc", libxl_vnc_info), # keyboard layout, default is en-us keyboard ("keymap", string), diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 @@ -1256,7 +1256,10 @@ skip_vfb: #undef parse_extra_args if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) + if (l) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; + 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); diff -r 592d15bd4d5e tools/libxl/xl_sxp.c --- a/tools/libxl/xl_sxp.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_sxp.c Wed Jun 27 20:06:39 2012 +0800 @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); printf("\t\t\t(no_incr_generationid %s)\n", libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); - printf("\t\t\t(stdvga %s)\n", - libxl_defbool_to_string(b_info->u.hvm.stdvga)); + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind == + LIBXL_VGA_INTERFACE_TYPE_STD ? + "True" : "False"); printf("\t\t\t(vnc %s)\n", libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ 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 1/3] libxl:refactor stdvga option support v2 2012-06-27 12:16 ` ZhouPeng @ 2012-06-27 15:28 ` Ian Campbell 2012-06-28 1:42 ` ZhouPeng 0 siblings, 1 reply; 13+ messages in thread From: Ian Campbell @ 2012-06-27 15:28 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini > diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 > @@ -1256,7 +1256,10 @@ skip_vfb: > #undef parse_extra_args > > if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > + if (l) > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; I think this needs to be: if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) b_info->u.hvm.vga.kind = l ? \ LIBXL_VGA_INTERFACE_TYPE_STD : \ LIBXL_VGA_INTERFACE_TYPE_CIRRUS; so that both "stdvga=0" and "stdvga=1" result in what the user asked for without relying on the libxl default remaining CIRRUS. Ian. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-27 15:28 ` Ian Campbell @ 2012-06-28 1:42 ` ZhouPeng 2012-06-28 5:33 ` Ian Campbell 0 siblings, 1 reply; 13+ messages in thread From: ZhouPeng @ 2012-06-28 1:42 UTC (permalink / raw) To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c >> --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 >> +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 >> @@ -1256,7 +1256,10 @@ skip_vfb: >> #undef parse_extra_args >> >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) >> + if (l) >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; > > I think this needs to be: > if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > b_info->u.hvm.vga.kind = l ? \ > LIBXL_VGA_INTERFACE_TYPE_STD : \ > LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > > so that both "stdvga=0" and "stdvga=1" result in what the user asked for > without relying on the libxl default remaining CIRRUS. IMHO, this make simple to be complex. I think the check and set-default later in libxl_create.c as a whole is enough. + if (!b_info->u.hvm.vga.kind) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; 'as a whole' here, I means if more vga type added in the future, we don't need to set the default one by one, redundantly. > Ian. > -- Zhou Peng ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-28 1:42 ` ZhouPeng @ 2012-06-28 5:33 ` Ian Campbell 2012-06-28 7:56 ` ZhouPeng 0 siblings, 1 reply; 13+ messages in thread From: Ian Campbell @ 2012-06-28 5:33 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini On Thu, 2012-06-28 at 02:42 +0100, ZhouPeng wrote: > On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c > >> --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 > >> +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 > >> @@ -1256,7 +1256,10 @@ skip_vfb: > >> #undef parse_extra_args > >> > >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > >> + if (l) > >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; > > > > I think this needs to be: > > if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > > b_info->u.hvm.vga.kind = l ? \ > > LIBXL_VGA_INTERFACE_TYPE_STD : \ > > LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > > > > so that both "stdvga=0" and "stdvga=1" result in what the user asked for > > without relying on the libxl default remaining CIRRUS. > > IMHO, this make simple to be complex. > > I think the check and set-default later in libxl_create.c as a whole is enough. This is in libxl, as I said above xl should not rely on the default remaining CIRRUS. If the user says "stdvga=0" then xl needs to explicitly ask for CIRRUS, despite the fact that it currently happens to be the default. If we make the libxl default BLARGLE in the future then stdvga=0 still needs to mean CIRRUS. > + if (!b_info->u.hvm.vga.kind) > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > > 'as a whole' here, I means if more vga type added in the future, we > don't need to set the default one by one, redundantly. ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-28 5:33 ` Ian Campbell @ 2012-06-28 7:56 ` ZhouPeng 2012-06-28 8:40 ` Ian Campbell 0 siblings, 1 reply; 13+ messages in thread From: ZhouPeng @ 2012-06-28 7:56 UTC (permalink / raw) To: Ian Campbell; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini [-- Attachment #1: Type: text/plain, Size: 7240 bytes --] version 4 Thanks. -- tools:libxl: refactor stdvga opinon support. Be ready to add and describe new vga interface Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> diff -r 592d15bd4d5e tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_create.c Thu Jun 28 15:50:10 2012 +0800 @@ -189,7 +189,8 @@ int libxl__domain_build_info_setdefault( if (!b_info->u.hvm.boot) return ERROR_NOMEM; } - libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false); + if (!b_info->u.hvm.vga.kind) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); diff -r 592d15bd4d5e tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_dm.c Thu Jun 28 15:50:10 2012 +0800 @@ -175,8 +175,13 @@ static char ** libxl__build_device_model libxl__sizekb_to_mb(b_info->video_memkb)), NULL); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { + + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: flexarray_append(dm_args, "-std-vga"); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + break; } if (b_info->u.hvm.boot) { @@ -418,8 +423,13 @@ static char ** libxl__build_device_model flexarray_append(dm_args, spiceoptions); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { - flexarray_vappend(dm_args, "-vga", "std", NULL); + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: + flexarray_vappend(dm_args, "-vga", "std", NULL); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + flexarray_vappend(dm_args, "-vga", "cirrus", NULL); + break; } if (b_info->u.hvm.boot) { diff -r 592d15bd4d5e tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_types.idl Thu Jun 28 15:50:10 2012 +0800 @@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu (4, "watchdog"), ]) +libxl_vga_interface_type = Enumeration("vga_interface_type", [ + (1, "CIRRUS"), + (2, "STD"), + ], init_val = 0) + # # Complex libxl types # + +libxl_vga_interface_info = Struct("vga_interface_info", [ + ("kind", libxl_vga_interface_type), + ]) + libxl_vnc_info = Struct("vnc_info", [ ("enable", libxl_defbool), # "address:port" that should be listened on @@ -286,7 +296,7 @@ libxl_domain_build_info = Struct("domain ("nested_hvm", libxl_defbool), ("incr_generationid",libxl_defbool), ("nographic", libxl_defbool), - ("stdvga", libxl_defbool), + ("vga", libxl_vga_interface_info), ("vnc", libxl_vnc_info), # keyboard layout, default is en-us keyboard ("keymap", string), diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Thu Jun 28 15:50:10 2012 +0800 @@ -1256,7 +1256,10 @@ skip_vfb: #undef parse_extra_args if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) + b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD : + 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); diff -r 592d15bd4d5e tools/libxl/xl_sxp.c --- a/tools/libxl/xl_sxp.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_sxp.c Thu Jun 28 15:50:10 2012 +0800 @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); printf("\t\t\t(no_incr_generationid %s)\n", libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); - printf("\t\t\t(stdvga %s)\n", - libxl_defbool_to_string(b_info->u.hvm.stdvga)); + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind == + LIBXL_VGA_INTERFACE_TYPE_STD ? + "True" : "False"); printf("\t\t\t(vnc %s)\n", libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); On Thu, Jun 28, 2012 at 1:33 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > On Thu, 2012-06-28 at 02:42 +0100, ZhouPeng wrote: >> On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: >> >> diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c >> >> --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 >> >> +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 >> >> @@ -1256,7 +1256,10 @@ skip_vfb: >> >> #undef parse_extra_args >> >> >> >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { >> >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); >> >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) >> >> + if (l) >> >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; >> > >> > I think this needs to be: >> > if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) >> > b_info->u.hvm.vga.kind = l ? \ >> > LIBXL_VGA_INTERFACE_TYPE_STD : \ >> > LIBXL_VGA_INTERFACE_TYPE_CIRRUS; >> > >> > so that both "stdvga=0" and "stdvga=1" result in what the user asked for >> > without relying on the libxl default remaining CIRRUS. >> >> IMHO, this make simple to be complex. >> >> I think the check and set-default later in libxl_create.c as a whole is enough. > > This is in libxl, as I said above xl should not rely on the default > remaining CIRRUS. > > If the user says "stdvga=0" then xl needs to explicitly ask for CIRRUS, > despite the fact that it currently happens to be the default. > > If we make the libxl default BLARGLE in the future then stdvga=0 still > needs to mean CIRRUS. > >> + if (!b_info->u.hvm.vga.kind) >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; >> >> 'as a whole' here, I means if more vga type added in the future, we >> don't need to set the default one by one, redundantly. > -- Zhou Peng [-- Attachment #2: spice.tools.libxl.stdvga.refactor.v4.diff --] [-- Type: application/octet-stream, Size: 5102 bytes --] tools:libxl: refactor stdvga opinon support. Be ready to add and describe new vga interface Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> diff -r 592d15bd4d5e tools/libxl/libxl_create.c --- a/tools/libxl/libxl_create.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_create.c Thu Jun 28 15:50:10 2012 +0800 @@ -189,7 +189,8 @@ int libxl__domain_build_info_setdefault( if (!b_info->u.hvm.boot) return ERROR_NOMEM; } - libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false); + if (!b_info->u.hvm.vga.kind) + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); diff -r 592d15bd4d5e tools/libxl/libxl_dm.c --- a/tools/libxl/libxl_dm.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_dm.c Thu Jun 28 15:50:10 2012 +0800 @@ -175,8 +175,13 @@ static char ** libxl__build_device_model libxl__sizekb_to_mb(b_info->video_memkb)), NULL); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { + + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: flexarray_append(dm_args, "-std-vga"); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + break; } if (b_info->u.hvm.boot) { @@ -418,8 +423,13 @@ static char ** libxl__build_device_model flexarray_append(dm_args, spiceoptions); } - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { - flexarray_vappend(dm_args, "-vga", "std", NULL); + switch (b_info->u.hvm.vga.kind) { + case LIBXL_VGA_INTERFACE_TYPE_STD: + flexarray_vappend(dm_args, "-vga", "std", NULL); + break; + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: + flexarray_vappend(dm_args, "-vga", "cirrus", NULL); + break; } if (b_info->u.hvm.boot) { diff -r 592d15bd4d5e tools/libxl/libxl_types.idl --- a/tools/libxl/libxl_types.idl Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/libxl_types.idl Thu Jun 28 15:50:10 2012 +0800 @@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu (4, "watchdog"), ]) +libxl_vga_interface_type = Enumeration("vga_interface_type", [ + (1, "CIRRUS"), + (2, "STD"), + ], init_val = 0) + # # Complex libxl types # + +libxl_vga_interface_info = Struct("vga_interface_info", [ + ("kind", libxl_vga_interface_type), + ]) + libxl_vnc_info = Struct("vnc_info", [ ("enable", libxl_defbool), # "address:port" that should be listened on @@ -286,7 +296,7 @@ libxl_domain_build_info = Struct("domain ("nested_hvm", libxl_defbool), ("incr_generationid",libxl_defbool), ("nographic", libxl_defbool), - ("stdvga", libxl_defbool), + ("vga", libxl_vga_interface_info), ("vnc", libxl_vnc_info), # keyboard layout, default is en-us keyboard ("keymap", string), diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_cmdimpl.c Thu Jun 28 15:50:10 2012 +0800 @@ -1256,7 +1256,10 @@ skip_vfb: #undef parse_extra_args if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) + b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD : + 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); diff -r 592d15bd4d5e tools/libxl/xl_sxp.c --- a/tools/libxl/xl_sxp.c Fri May 18 16:19:21 2012 +0100 +++ b/tools/libxl/xl_sxp.c Thu Jun 28 15:50:10 2012 +0800 @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); printf("\t\t\t(no_incr_generationid %s)\n", libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); - printf("\t\t\t(stdvga %s)\n", - libxl_defbool_to_string(b_info->u.hvm.stdvga)); + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind == + LIBXL_VGA_INTERFACE_TYPE_STD ? + "True" : "False"); printf("\t\t\t(vnc %s)\n", libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); [-- Attachment #3: Type: text/plain, Size: 126 bytes --] _______________________________________________ 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 1/3] libxl:refactor stdvga option support v2 2012-06-28 7:56 ` ZhouPeng @ 2012-06-28 8:40 ` Ian Campbell 2012-06-29 16:32 ` Ian Jackson 0 siblings, 1 reply; 13+ messages in thread From: Ian Campbell @ 2012-06-28 8:40 UTC (permalink / raw) To: ZhouPeng; +Cc: Xen-Devel (E-mail), Ian Jackson, Stefano Stabellini On Thu, 2012-06-28 at 08:56 +0100, ZhouPeng wrote: > version 4 > > Thanks. > -- > > tools:libxl: refactor stdvga opinon support. > > Be ready to add and describe new vga interface > > Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> Acked-by: Ian Campbell <ian.campbell@citrix.com> > > diff -r 592d15bd4d5e tools/libxl/libxl_create.c > --- a/tools/libxl/libxl_create.c Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/libxl_create.c Thu Jun 28 15:50:10 2012 +0800 > @@ -189,7 +189,8 @@ int libxl__domain_build_info_setdefault( > if (!b_info->u.hvm.boot) return ERROR_NOMEM; > } > > - libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false); > + if (!b_info->u.hvm.vga.kind) > + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > libxl_defbool_setdefault(&b_info->u.hvm.vnc.enable, true); > if (libxl_defbool_val(b_info->u.hvm.vnc.enable)) { > libxl_defbool_setdefault(&b_info->u.hvm.vnc.findunused, true); > diff -r 592d15bd4d5e tools/libxl/libxl_dm.c > --- a/tools/libxl/libxl_dm.c Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/libxl_dm.c Thu Jun 28 15:50:10 2012 +0800 > @@ -175,8 +175,13 @@ static char ** libxl__build_device_model > libxl__sizekb_to_mb(b_info->video_memkb)), > NULL); > } > - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { > + > + switch (b_info->u.hvm.vga.kind) { > + case LIBXL_VGA_INTERFACE_TYPE_STD: > flexarray_append(dm_args, "-std-vga"); > + break; > + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: > + break; > } > > if (b_info->u.hvm.boot) { > @@ -418,8 +423,13 @@ static char ** libxl__build_device_model > flexarray_append(dm_args, spiceoptions); > } > > - if (libxl_defbool_val(b_info->u.hvm.stdvga)) { > - flexarray_vappend(dm_args, "-vga", "std", NULL); > + switch (b_info->u.hvm.vga.kind) { > + case LIBXL_VGA_INTERFACE_TYPE_STD: > + flexarray_vappend(dm_args, "-vga", "std", NULL); > + break; > + case LIBXL_VGA_INTERFACE_TYPE_CIRRUS: > + flexarray_vappend(dm_args, "-vga", "cirrus", NULL); > + break; > } > > if (b_info->u.hvm.boot) { > diff -r 592d15bd4d5e tools/libxl/libxl_types.idl > --- a/tools/libxl/libxl_types.idl Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/libxl_types.idl Thu Jun 28 15:50:10 2012 +0800 > @@ -125,9 +125,19 @@ libxl_shutdown_reason = Enumeration("shu > (4, "watchdog"), > ]) > > +libxl_vga_interface_type = Enumeration("vga_interface_type", [ > + (1, "CIRRUS"), > + (2, "STD"), > + ], init_val = 0) > + > # > # Complex libxl types > # > + > +libxl_vga_interface_info = Struct("vga_interface_info", [ > + ("kind", libxl_vga_interface_type), > + ]) > + > libxl_vnc_info = Struct("vnc_info", [ > ("enable", libxl_defbool), > # "address:port" that should be listened on > @@ -286,7 +296,7 @@ libxl_domain_build_info = Struct("domain > ("nested_hvm", libxl_defbool), > ("incr_generationid",libxl_defbool), > ("nographic", libxl_defbool), > - ("stdvga", libxl_defbool), > + ("vga", > libxl_vga_interface_info), > ("vnc", libxl_vnc_info), > # keyboard layout, default is > en-us keyboard > ("keymap", string), > diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Thu Jun 28 15:50:10 2012 +0800 > @@ -1256,7 +1256,10 @@ skip_vfb: > #undef parse_extra_args > > if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > + b_info->u.hvm.vga.kind = l ? LIBXL_VGA_INTERFACE_TYPE_STD : > + 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); > diff -r 592d15bd4d5e tools/libxl/xl_sxp.c > --- a/tools/libxl/xl_sxp.c Fri May 18 16:19:21 2012 +0100 > +++ b/tools/libxl/xl_sxp.c Thu Jun 28 15:50:10 2012 +0800 > @@ -110,8 +110,9 @@ void printf_info_sexp(int domid, libxl_d > libxl_defbool_to_string(b_info->u.hvm.nested_hvm)); > printf("\t\t\t(no_incr_generationid %s)\n", > libxl_defbool_to_string(b_info->u.hvm.incr_generationid)); > - printf("\t\t\t(stdvga %s)\n", > - libxl_defbool_to_string(b_info->u.hvm.stdvga)); > + printf("\t\t\t(stdvga %s)\n", b_info->u.hvm.vga.kind == > + LIBXL_VGA_INTERFACE_TYPE_STD ? > + "True" : "False"); > printf("\t\t\t(vnc %s)\n", > libxl_defbool_to_string(b_info->u.hvm.vnc.enable)); > printf("\t\t\t(vnclisten %s)\n", b_info->u.hvm.vnc.listen); > > On Thu, Jun 28, 2012 at 1:33 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > > On Thu, 2012-06-28 at 02:42 +0100, ZhouPeng wrote: > >> On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote: > >> >> diff -r 592d15bd4d5e tools/libxl/xl_cmdimpl.c > >> >> --- a/tools/libxl/xl_cmdimpl.c Fri May 18 16:19:21 2012 +0100 > >> >> +++ b/tools/libxl/xl_cmdimpl.c Wed Jun 27 20:06:39 2012 +0800 > >> >> @@ -1256,7 +1256,10 @@ skip_vfb: > >> >> #undef parse_extra_args > >> >> > >> >> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) { > >> >> - xlu_cfg_get_defbool(config, "stdvga", &b_info->u.hvm.stdvga, 0); > >> >> + if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > >> >> + if (l) > >> >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_STD; > >> > > >> > I think this needs to be: > >> > if (!xlu_cfg_get_long(config, "stdvga", &l, 0)) > >> > b_info->u.hvm.vga.kind = l ? \ > >> > LIBXL_VGA_INTERFACE_TYPE_STD : \ > >> > LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > >> > > >> > so that both "stdvga=0" and "stdvga=1" result in what the user asked for > >> > without relying on the libxl default remaining CIRRUS. > >> > >> IMHO, this make simple to be complex. > >> > >> I think the check and set-default later in libxl_create.c as a whole is enough. > > > > This is in libxl, as I said above xl should not rely on the default > > remaining CIRRUS. > > > > If the user says "stdvga=0" then xl needs to explicitly ask for CIRRUS, > > despite the fact that it currently happens to be the default. > > > > If we make the libxl default BLARGLE in the future then stdvga=0 still > > needs to mean CIRRUS. > > > >> + if (!b_info->u.hvm.vga.kind) > >> + b_info->u.hvm.vga.kind = LIBXL_VGA_INTERFACE_TYPE_CIRRUS; > >> > >> 'as a whole' here, I means if more vga type added in the future, we > >> don't need to set the default one by one, redundantly. > > > > > ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/3] libxl:refactor stdvga option support v2 2012-06-28 8:40 ` Ian Campbell @ 2012-06-29 16:32 ` Ian Jackson 0 siblings, 0 replies; 13+ messages in thread From: Ian Jackson @ 2012-06-29 16:32 UTC (permalink / raw) To: Ian Campbell; +Cc: ZhouPeng, Xen-Devel (E-mail), Stefano Stabellini Ian Campbell writes ("Re: [Xen-devel] [PATCH 1/3] libxl:refactor stdvga option support v2"): > On Thu, 2012-06-28 at 08:56 +0100, ZhouPeng wrote: > > version 4 > > > > Thanks. > > -- > > > > tools:libxl: refactor stdvga opinon support. > > > > Be ready to add and describe new vga interface > > > > Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> > > Acked-by: Ian Campbell <ian.campbell@citrix.com> Thanks, Committed-by: Ian Jackson <ian.jackson@eu.citrix.com> ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2012-06-29 16:32 UTC | newest] Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2012-06-05 11:19 [PATCH 1/3] libxl:refactor stdvga option support v2 ZhouPeng 2012-06-06 10:50 ` Ian Jackson 2012-06-06 11:10 ` ZhouPeng 2012-06-06 11:47 ` Ian Campbell 2012-06-07 2:34 ` ZhouPeng 2012-06-20 18:38 ` Ian Campbell 2012-06-27 12:16 ` ZhouPeng 2012-06-27 15:28 ` Ian Campbell 2012-06-28 1:42 ` ZhouPeng 2012-06-28 5:33 ` Ian Campbell 2012-06-28 7:56 ` ZhouPeng 2012-06-28 8:40 ` Ian Campbell 2012-06-29 16:32 ` Ian Jackson
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.