Re: [Xen-devel] [PATCH v3] libxl: add basic spice support for pv domUs

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Fri, 2014-04-11 at 10:45 +0200, Fabio Fantoni wrote:
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 3376b5c..dc54c9b 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>      if (!b_info->event_channels)
>          b_info->event_channels = 1023;
>  
> +    libxl_defbool_setdefault(&b_info->spice.enable, false);
> +    if (libxl_defbool_val(b_info->spice.enable)) {
> +        libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
> +        libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
> +        libxl_defbool_setdefault(&b_info->spice.vdagent, false);
> +        libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
> +    }
> +
>      switch (b_info->type) {
>      case LIBXL_DOMAIN_TYPE_HVM:
>          if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
>  
>          if (!b_info->u.hvm.usbversion &&
> -            (b_info->u.hvm.spice.usbredirection > 0) )
> +            (b_info->spice.usbredirection > 0) )
>              b_info->u.hvm.usbversion = 2;
>  
> -        if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) 
> &&
> +        if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
>              ( libxl_defbool_val(b_info->u.hvm.usb)
>              || b_info->u.hvm.usbdevice_list
>              || b_info->u.hvm.usbdevice) ){
> @@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>              libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
>          }
>  
> -        libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
> -        if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
> -            libxl_defbool_setdefault(&b_info->u.hvm.spice.disable_ticketing,
> -                                     false);
> -            libxl_defbool_setdefault(&b_info->u.hvm.spice.agent_mouse, true);
> -            libxl_defbool_setdefault(&b_info->u.hvm.spice.vdagent, false);
> -            libxl_defbool_setdefault(&b_info->u.hvm.spice.clipboard_sharing,
> -                                     false);
> -        }

libxl appear to now ignore b_info->u.hvm.spice completely. Unfortunately
this will break backwards compatibility.

It should be possible to build a application written against old libxl
against this new libxl and have everything continue to work.
Unfortunately this means you need to jump through a few hoops in the
library code. You should define some sane precedence between
b_info->spice and b_info->u.hvm.spice and copy things from u.hvm.spice
to spice as necessary. I think it makes sense to say that ->spice takes
precedence over ->u.hvm.spice. IOW if ->spice.enable == false and
->u.hvm.spice.enable is true then copy ->u.hvm.spice into ->spice. You
should think through the consequences of this though in case I am
confused.

You also need to announce the presence of this new field via a
LIBXL_HAVE #define in libxl.h (there are plenty of existing examples).
You should document the precedence which you decide on as part of the
associated comment.

>          xlu_cfg_get_defbool(config, "spicevdagent",
> -                            &b_info->u.hvm.spice.vdagent, 0);
> +                            &b_info->spice.vdagent, 0);
>          xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
> -                            &b_info->u.hvm.spice.clipboard_sharing, 0);
> +                            &b_info->spice.clipboard_sharing, 0);
>          if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
> -            b_info->u.hvm.spice.usbredirection = l;
> +            b_info->spice.usbredirection = l;

So it appears that some members of b_info->spice are not relevant to PV
guests?



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel