refactor the code of stdvga option 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 Mon May 28 16:10:02 2012 +0800
@@ -189,7 +189,6 @@ int libxl__domain_build_info_setdefault(
if (!b_info->u.hvm.boot) return ERROR_NOMEM;
}
- libxl_defbool_setdefault(&b_info->u.hvm.stdvga, false);
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 Mon May 28 16:10:02 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_DEFAULT:
+ break;
}
if (b_info->u.hvm.boot) {
@@ -418,8 +423,12 @@ 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_DEFAULT:
+ 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 Mon May 28 16:10:02 2012 +0800
@@ -125,9 +125,20 @@ libxl_shutdown_reason = Enumeration("shu
(4, "watchdog"),
])
+libxl_vga_interface_type = Enumeration("vga_interface_type", [
+ (0, "DEFAULT"),
+ (1, "STD"),
+ ])
+
#
# Complex libxl types
#
+
+libxl_vga_interface_info = Struct("vga_interface_info", [
+ ("type", libxl_vga_interface_type),
+ ("vramkb", MemKB),
+ ])
+
libxl_vnc_info = Struct("vnc_info", [
("enable", libxl_defbool),
# "address:port" that should be listened on
@@ -286,7 +297,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 Mon May 28 16:10:02 2012 +0800
@@ -1256,7 +1256,12 @@ 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);
+ libxl_defbool vga;
+ b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_DEFAULT;
+ if (!xlu_cfg_get_defbool(config, "stdvga", &vga, 0))
+ if (libxl_defbool_val(vga))
+ 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 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 Mon May 28 16:10:02 2012 +0800
@@ -110,8 +110,10 @@ 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));
+ libxl_defbool stdvga;
+ libxl_defbool_set(&stdvga,
+ b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_STD);
+ printf("\t\t\t(stdvga %s)\n",
libxl_defbool_to_string(stdvga));
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
Ian Campbell
2012-May-29 09:17 UTC
Re: [PATCH]libxl:refactor the code of stdvga option support
On Mon, 2012-05-28 at 09:52 +0100, ZhouPeng wrote:> refactor the code of stdvga option support. > > Be ready to add and describe new vga interfaceAre you proposing this for 4.2, which is currently in feature freeze? I''d be inclined to accept this particular for 4.2 due to the API change, but I''m curious what the "new vga interface" is going to be.> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> > > 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 Mon May 28 16:10:02 2012 +0800 > @@ -125,9 +125,20 @@ libxl_shutdown_reason = Enumeration("shu > (4, "watchdog"), > ]) > > +libxl_vga_interface_type = Enumeration("vga_interface_type", [ > + (0, "DEFAULT"),"DEFAULT" here just means "whatever qemu gives you if you don''t say otherwise"? What actually is that? I think we''d be better off having it as an explicit option too and using setdefault to convert DEFAULT->that.> + (1, "STD"), > + ]) > + > # > # Complex libxl types > # > + > +libxl_vga_interface_info = Struct("vga_interface_info", [ > + ("type", libxl_vga_interface_type), > + ("vramkb", MemKB),I can''t see "vramkb" being used anywhere. Did you mean to include it in the followup "new vga interface" patch?> + ]) > + > libxl_vnc_info = Struct("vnc_info", [ > ("enable", libxl_defbool), > # "address:port" that should be listened on > @@ -286,7 +297,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),Looks like this bit is whitespace damaged. http://wiki.xen.org/wiki/Submitting_Xen_Patches has some hints on using the mercurial patchbomb tool to avoid this and also a link to http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt;hb=HEAD which gives advice on using various mailclients to manually send patches without mangling them.> 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 Mon May 28 16:10:02 2012 +0800 > @@ -1256,7 +1256,12 @@ 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); > + libxl_defbool vga; > + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_DEFAULT; > + if (!xlu_cfg_get_defbool(config, "stdvga", &vga, 0)) > + if (libxl_defbool_val(vga))I don''t think you need to launder this through a defbool, since it is no longer one at the libxl interface. Use xlu_cfg_get_long into &l and then if (l) b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD;> + 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 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 Mon May 28 16:10:02 2012 +0800 > @@ -110,8 +110,10 @@ 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)); > + libxl_defbool stdvga; > + libxl_defbool_set(&stdvga, > + b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_STD); > + printf("\t\t\t(stdvga %s)\n", libxl_defbool_to_string(stdvga));Likewise I think this is just 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); >
ZhouPeng
2012-May-30 10:45 UTC
Re: [PATCH]libxl:refactor the code of stdvga option support
On Tue, May 29, 2012 at 5:17 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Mon, 2012-05-28 at 09:52 +0100, ZhouPeng wrote: >> refactor the code of stdvga option support. >> >> Be ready to add and describe new vga interface > > Are you proposing this for 4.2, which is currently in feature freeze? > > I''d be inclined to accept this particular for 4.2 due to the API change,for the upstream-xen and upstream-xen-qemu> but I''m curious what the "new vga interface" is going to be.QXL> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> >> >> 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 Mon May 28 16:10:02 2012 +0800 >> @@ -125,9 +125,20 @@ libxl_shutdown_reason = Enumeration("shu >> (4, "watchdog"), >> ]) >> >> +libxl_vga_interface_type = Enumeration("vga_interface_type", [ >> + (0, "DEFAULT"), > > "DEFAULT" here just means "whatever qemu gives you if you don''t say > otherwise"?Yes, It keep the same with the current libxl behavior (trans nothing for vga in the qemu cmd).> What actually is that?Cirrus will be selected by qemu in face.> I think we''d be better off having it > as an explicit option too and using setdefault to convert DEFAULT->that.Ok.> >> + (1, "STD"), >> + ]) >> + >> # >> # Complex libxl types >> # >> + >> +libxl_vga_interface_info = Struct("vga_interface_info", [ >> + ("type", libxl_vga_interface_type), >> + ("vramkb", MemKB), > > I can''t see "vramkb" being used anywhere. Did you mean to include it in > the followup "new vga interface" patch?Yes. By qxl or other vga in the future.> >> + ]) >> + >> libxl_vnc_info = Struct("vnc_info", [ >> ("enable", libxl_defbool), >> # "address:port" that should be listened on >> @@ -286,7 +297,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), > > Looks like this bit is whitespace damaged. > > http://wiki.xen.org/wiki/Submitting_Xen_Patches has some hints on using > the mercurial patchbomb tool to avoid this and also a link to > http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=blob;f=Documentation/email-clients.txt;hb=HEAD > which gives advice on using various mailclients to manually send patches > without mangling them. >> 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 Mon May 28 16:10:02 2012 +0800 >> @@ -1256,7 +1256,12 @@ 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); >> + libxl_defbool vga; >> + b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_DEFAULT; >> + if (!xlu_cfg_get_defbool(config, "stdvga", &vga, 0)) >> + if (libxl_defbool_val(vga)) > > I don''t think you need to launder this through a defbool, since it is no > longer one at the libxl interface. Use xlu_cfg_get_long into &l and > then > if (l)Ok.> b_info->u.hvm.vga.type = LIBXL_VGA_INTERFACE_TYPE_STD; > >> + 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 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 Mon May 28 16:10:02 2012 +0800 >> @@ -110,8 +110,10 @@ 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)); >> + libxl_defbool stdvga; >> + libxl_defbool_set(&stdvga, >> + b_info->u.hvm.vga.type == LIBXL_VGA_INTERFACE_TYPE_STD); >> + printf("\t\t\t(stdvga %s)\n", libxl_defbool_to_string(stdvga)); > > Likewise I think this is just > 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); >>Ok.> >-- Zhou Peng
Ian Campbell
2012-May-30 11:47 UTC
Re: [PATCH]libxl:refactor the code of stdvga option support
On Wed, 2012-05-30 at 11:45 +0100, ZhouPeng wrote:> >> Signed-off-by: Zhou Peng <ailvpeng25@gmail.com> > >> > >> 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 Mon May 28 16:10:02 2012 +0800 > >> @@ -125,9 +125,20 @@ libxl_shutdown_reason = Enumeration("shu > >> (4, "watchdog"), > >> ]) > >> > >> +libxl_vga_interface_type = Enumeration("vga_interface_type", [ > >> + (0, "DEFAULT"), > > > > "DEFAULT" here just means "whatever qemu gives you if you don''t say > > otherwise"? > Yes, > It keep the same with the current libxl behavior (trans nothing for > vga in the qemu cmd). > > What actually is that? > Cirrus will be selected by qemu in face.Lets call this option "CIRRUS" then instead of "DEFAULT" then. Thanks, Ian.