Xen devel - Aug 2013 - [PATCH v4] libxl: Spice vdagent support for upstream qemu

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.

Changes from v3:
- Simple refresh.

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 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 08d6cc4..170bc11 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1130,6 +1130,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 (1)
 (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.
+
 =back
 
 =head3 Miscellaneous Emulated Hardware
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 0c32d0b..62b283f 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -272,6 +272,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 4035b6d..fb6fbf4 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -472,6 +472,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",
+                    "-chardev",
"spicevmc,id=vdagent,name=vdagent", "-device",
+                   
"virtserialport,chardev=vdagent,name=com.redhat.spice.0",
+                    NULL);
+            }
         }
 
         switch (b_info->u.hvm.vga.kind) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 85341a0..eb242db 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -175,6 +175,7 @@ libxl_spice_info = Struct("spice_info", [
     ("disable_ticketing", libxl_defbool),
     ("passwd",      string),
     ("agent_mouse", libxl_defbool),
+    ("vdagent",     libxl_defbool),
     ])
 
 libxl_sdl_info = Struct("sdl_info", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 884f050..9892783 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1495,6 +1495,8 @@ skip_vfb:
                                 &b_info->u.hvm.spice.passwd, 0);
         xlu_cfg_get_defbool(config, "spiceagent_mouse",
                             &b_info->u.hvm.spice.agent_mouse, 0);
+        xlu_cfg_get_defbool(config, "spicevdagent",
+                            &b_info->u.hvm.spice.vdagent, 0);
         xlu_cfg_get_defbool(config, "nographic",
&b_info->u.hvm.nographic, 0);
         xlu_cfg_get_defbool(config, "gfx_passthru", 
                             &b_info->u.hvm.gfx_passthru, 0);
-- 
1.7.9.5

Usage: spice_clipboard_sharing=1|0 (default=1)
Disables Spice clipboard sharing (copy/paste). It requires spicevdagent
enabled.

Changes from v2:
- Changed name of xl option from spicedisable_copypaste to
  spice_clipboard_sharing
- Inverted the default based on option name.
- Improved description.

Requires this patch:
libxl: Spice vdagent support for upstream qemu v4

Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
 docs/man/xl.cfg.pod.5       |    5 +++++
 tools/libxl/libxl_create.c  |    2 ++
 tools/libxl/libxl_dm.c      |    4 ++++
 tools/libxl/libxl_types.idl |    1 +
 tools/libxl/xl_cmdimpl.c    |    2 ++
 5 files changed, 14 insertions(+)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 170bc11..ad9757c 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1139,6 +1139,11 @@ 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.
 
+=item B<spice_clipboard_sharing=BOOLEAN>
+
+Disables Spice clipboard sharing (copy/paste). It requires spicevdagent
+enabled. The default is true (1).
+
 =back
 
 =head3 Miscellaneous Emulated Hardware
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 62b283f..6a87018 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -273,6 +273,8 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
                                      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,
+                                     true);
         }
 
         libxl_defbool_setdefault(&b_info->u.hvm.nographic, false);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index fb6fbf4..43c3bec 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -351,6 +351,10 @@ static char *dm_spice_options(libxl__gc *gc,
         opt = libxl__sprintf(gc, "%s,password=%s", opt,
spice->passwd);
     opt = libxl__sprintf(gc, "%s,agent-mouse=%s", opt,
                          libxl_defbool_val(spice->agent_mouse) ?
"on" : "off");
+
+    if (!libxl_defbool_val(spice->clipboard_sharing))
+        opt = libxl__sprintf(gc, "%s,disable-copy-paste", opt);
+
     return opt;
 }
 
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index eb242db..8c79a09 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -176,6 +176,7 @@ libxl_spice_info = Struct("spice_info", [
     ("passwd",      string),
     ("agent_mouse", libxl_defbool),
     ("vdagent",     libxl_defbool),
+    ("clipboard_sharing", libxl_defbool),
     ])
 
 libxl_sdl_info = Struct("sdl_info", [
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 9892783..95d23bb 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1497,6 +1497,8 @@ skip_vfb:
                             &b_info->u.hvm.spice.agent_mouse, 0);
         xlu_cfg_get_defbool(config, "spicevdagent",
                             &b_info->u.hvm.spice.vdagent, 0);
+        xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
+                            &b_info->u.hvm.spice.clipboard_sharing, 0);
         xlu_cfg_get_defbool(config, "nographic",
&b_info->u.hvm.nographic, 0);
         xlu_cfg_get_defbool(config, "gfx_passthru", 
                             &b_info->u.hvm.gfx_passthru, 0);
-- 
1.7.9.5

On Tue, 2013-08-27 at 17:03 +0200, Fabio Fantoni wrote:

Sorry for the delay in looking at this.
> Usage: spice_clipboard_sharing=1|0 (default=1)
> Disables Spice clipboard sharing (copy/paste). It requires spicevdagent
> enabled.
> 
> Changes from v2:
> - Changed name of xl option from spicedisable_copypaste to
>   spice_clipboard_sharing
> - Inverted the default based on option name.
> - Improved description.
> 
> Requires this patch:
> libxl: Spice vdagent support for upstream qemu v4
The normal way to post these would be as a series:
[PATCH v4 0/2] spice vdagent support (intro)
`- [PATCH v4 1/2] libxl: Spice vdagent support for upstream qemu
`- [PATCH v4 2/2] libxl: Spice disable-copy-paste support for upstream qemu

However I can''t see any reason not to fold these into a single patch.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
> ---
>  docs/man/xl.cfg.pod.5       |    5 +++++
>  tools/libxl/libxl_create.c  |    2 ++
>  tools/libxl/libxl_dm.c      |    4 ++++
>  tools/libxl/libxl_types.idl |    1 +
>  tools/libxl/xl_cmdimpl.c    |    2 ++
Please can you also patch tools/libxl/libxl.h to add a #define for
LIBXL_HAVE_SPICE_VDAGENT (there is a comment and plenty of other
examples of such things in libxl.h). If you keep the patches separate
then this belongs in the first one, I think we can get away without a
separate #define for the clipboard setting if they are committed around
the same time.
>  5 files changed, 14 insertions(+)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 170bc11..ad9757c 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1139,6 +1139,11 @@ 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.
>  
> +=item B<spice_clipboard_sharing=BOOLEAN>
> +
> +Disables Spice clipboard sharing (copy/paste). It requires spicevdagent
> +enabled. The default is true (1).
I think this should default to off, since there are security
implications to having it on.

Il 09/09/2013 15:03, Ian Campbell ha scritto:
> On Tue, 2013-08-27 at 17:03 +0200, Fabio Fantoni wrote:
>
> Sorry for the delay in looking at this.
>
>> Usage: spice_clipboard_sharing=1|0 (default=1)
>> Disables Spice clipboard sharing (copy/paste). It requires spicevdagent
>> enabled.
>>
>> Changes from v2:
>> - Changed name of xl option from spicedisable_copypaste to
>>    spice_clipboard_sharing
>> - Inverted the default based on option name.
>> - Improved description.
>>
>> Requires this patch:
>> libxl: Spice vdagent support for upstream qemu v4
> The normal way to post these would be as a series:
> [PATCH v4 0/2] spice vdagent support (intro)
> `- [PATCH v4 1/2] libxl: Spice vdagent support for upstream qemu
> `- [PATCH v4 2/2] libxl: Spice disable-copy-paste support for upstream qemu
>
> However I can''t see any reason not to fold these into a single
patch.
>
>> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>> ---
>>   docs/man/xl.cfg.pod.5       |    5 +++++
>>   tools/libxl/libxl_create.c  |    2 ++
>>   tools/libxl/libxl_dm.c      |    4 ++++
>>   tools/libxl/libxl_types.idl |    1 +
>>   tools/libxl/xl_cmdimpl.c    |    2 ++
> Please can you also patch tools/libxl/libxl.h to add a #define for
> LIBXL_HAVE_SPICE_VDAGENT (there is a comment and plenty of other
> examples of such things in libxl.h). If you keep the patches separate
> then this belongs in the first one, I think we can get away without a
> separate #define for the clipboard setting if they are committed around
> the same time.
>
>>   5 files changed, 14 insertions(+)
>>
>> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>> index 170bc11..ad9757c 100644
>> --- a/docs/man/xl.cfg.pod.5
>> +++ b/docs/man/xl.cfg.pod.5
>> @@ -1139,6 +1139,11 @@ 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.
>>   
>> +=item B<spice_clipboard_sharing=BOOLEAN>
>> +
>> +Disables Spice clipboard sharing (copy/paste). It requires
spicevdagent
>> +enabled. The default is true (1).
> I think this should default to off, since there are security
> implications to having it on.
>
>
Thanks for reply, probably toworrow I''ll post new version of patch with
all your advices.