Re: [PATCH 1/3] libxl:refactor stdvga option support v2

[Ian Campbell <Ian.Campbell@citrix.com>]

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);
> 
>