version 4 Thanks. -- tools:libxl: refactor stdvga opinon support. Be ready to add and describe new vga interface Signed-off-by: Zhou Peng 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 wrote: > On Thu, 2012-06-28 at 02:42 +0100, ZhouPeng wrote: >> On Wed, Jun 27, 2012 at 11:28 PM, Ian Campbell 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