Re: [Xen-devel] [PATCH v3] libxl: Spice vdagent support for upstream qemu

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@xxxxxxxxxx wrote:
> Usage: spicevdagent=1|0 (default=0)
> Enables spice vdagent. The Spice vdagent is an optional component for
> enhancing user experience and performing guest-oriented management
> tasks. Its features includes: client mouse mode (no need to grab mouse
> by client, no mouse lag), automatic adjustment of screen resolution,
> copy and paste (text and image) between client and domU. It also
> requires vdagent service installed on domU o.s. to work.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx>
> ---
>  docs/man/xl.cfg.pod.5       |    9 +++++++++
>  tools/libxl/libxl_create.c  |    1 +
>  tools/libxl/libxl_dm.c      |    6 ++++++
>  tools/libxl/libxl_types.idl |    1 +
>  tools/libxl/xl_cmdimpl.c    |    2 ++
>  5 files changed, 19 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index f8b4576..766862d 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1123,6 +1123,15 @@ Specify the ticket password which is used by a client 
> for connection.
>  Whether SPICE agent is used for client mouse mode. The default is true
>  (turn on)
>  
> +=item B<spicevdagent=BOOLEAN>
> +
> +Enables spice vdagent. The Spice vdagent is an optional component for
> +enhancing user experience and performing guest-oriented management
> +tasks. Its features includes: client mouse mode (no need to grab mouse
> +by client, no mouse lag), automatic adjustment of screen resolution,
> +copy and paste (text and image) between client and domU. It also
> +requires vdagent service installed on domU o.s. to work. The default is 0.
> +

For a boolean option, I think we should represent the value in
true/false not 1/0.

And as you said it is an optional component, what happens if QEMU
doesn't have that component compiled in but user specifies this option
to be true?

>  =back
>  
>  =head3 Miscellaneous Emulated Hardware
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index cb9c822..8db5460 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -288,6 +288,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>              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.nographic, false);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index d10a58f..bc605e4 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -465,6 +465,12 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>  
>              flexarray_append(dm_args, "-spice");
>              flexarray_append(dm_args, spiceoptions);
> +            if (libxl_defbool_val(b_info->u.hvm.spice.vdagent)) {
> +                flexarray_vappend(dm_args, "-device", "virtio-serial",

Dependency on VirtIO? Should this be documented as well? Does this mean
that vdagent can only work with HVM? AFAICT we don't support VirtIO in
PV.


Wei.

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