zpengxen at gmail
Jun 27, 2012, 5:16 AM
Post #7 of 13
(212 views)
Permalink
|
|
Re: [PATCH 1/3] libxl:refactor stdvga option support v2
[In reply to]
|
|
New version patch.
-----
tools:libxl: refactor stdvga opinon support.
Be ready to add and describe new vga interface
Signed-off-by: Zhou Peng <ailvpeng25 [at] gmail>
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 [at] citrix> 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 [at] citrix> 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
|
spice.tools.libxl.stdvga.refactor.v2.diff
