From: ZhouPeng <zpengxen@gmail.com>
To: Ian Campbell <Ian.Campbell@citrix.com>
Cc: "Xen-Devel (E-mail)" <xen-devel@lists.xensource.com>,
Ian Jackson <Ian.Jackson@eu.citrix.com>,
Stefano Stabellini <Stefano.Stabellini@eu.citrix.com>
Subject: Re: [PATCH 1/3] libxl:refactor stdvga option support v2
Date: Wed, 27 Jun 2012 20:16:35 +0800 [thread overview]
Message-ID: <CAAh7U5NqDOjffUgSxyuYeR6hH=SAYobB=1e91MZc6MEnoSB2xw@mail.gmail.com> (raw)
In-Reply-To: <1340217533.4998.3.camel@dagon.hellion.org.uk>
[-- 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
next prev parent reply other threads:[~2012-06-27 12:16 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAAh7U5NqDOjffUgSxyuYeR6hH=SAYobB=1e91MZc6MEnoSB2xw@mail.gmail.com' \
--to=zpengxen@gmail.com \
--cc=Ian.Campbell@citrix.com \
--cc=Ian.Jackson@eu.citrix.com \
--cc=Stefano.Stabellini@eu.citrix.com \
--cc=xen-devel@lists.xensource.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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.