[PATCH v3] libxl: Spice vdagent support for upstream qemu

[PATCH v3] libxl: Spice vdagent support for upstream qemu

[fantonifabio]

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@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 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.
+
 =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",
+                    "-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 ecf1f0b..14425d1 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -172,6 +172,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 c1a969b..44a632c 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1491,6 +1491,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

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

[Wei Liu]

On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@tiscali.it 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@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 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.

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

[Fabio Fantoni]

[-- Attachment #1.1: Type: text/plain, Size: 3964 bytes --]

Il 06/05/2013 16:44, Wei Liu ha scritto:
> On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@tiscali.it 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@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 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.
>
Spice vdagent is available only if spice is available, and spice is 
available only on hvm domU for now. There is no way for libxl to check 
if the qemu build has all the required components before running it. For 
example xen build qemu-xen without all the features supported by libxl 
(not only the ones insert by me) and debian experimental qemu build 
support all libxl features. If there isn't spice enabled nothing 
happens, and if spice is enabled but the qemu build doesn't have it, 
qemu doesn't start.


[-- Attachment #1.2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4510 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Wei Liu]

On Mon, May 06, 2013 at 04:17:00PM +0100, Fabio Fantoni wrote:
> >>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.
> >
> Spice vdagent is available only if spice is available, and spice is
> available only on hvm domU for now. There is no way for libxl to
> check if the qemu build has all the required components before
> running it. For example xen build qemu-xen without all the features
> supported by libxl (not only the ones insert by me) and debian
> experimental qemu build support all libxl features. If there isn't
> spice enabled nothing happens, and if spice is enabled but the qemu
> build doesn't have it, qemu doesn't start.
> 

We cannot check QEMU features in libxl, but we can control build options
for QEMU.

Do we need to modify tools/Makefile to make QEMU include spice?
Presumably the answer is "no" because you didn't have it in your patch
but it is better to be clear.


Wei.

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

[Fabio Fantoni]

[-- Attachment #1.1: Type: text/plain, Size: 1820 bytes --]

Il 06/05/2013 18:28, Wei Liu ha scritto:
> On Mon, May 06, 2013 at 04:17:00PM +0100, Fabio Fantoni wrote:
>>>> 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.
>>>
>> Spice vdagent is available only if spice is available, and spice is
>> available only on hvm domU for now. There is no way for libxl to
>> check if the qemu build has all the required components before
>> running it. For example xen build qemu-xen without all the features
>> supported by libxl (not only the ones insert by me) and debian
>> experimental qemu build support all libxl features. If there isn't
>> spice enabled nothing happens, and if spice is enabled but the qemu
>> build doesn't have it, qemu doesn't start.
>>
> We cannot check QEMU features in libxl, but we can control build options
> for QEMU.
>
> Do we need to modify tools/Makefile to make QEMU include spice?
> Presumably the answer is "no" because you didn't have it in your patch
> but it is better to be clear.
>
>
> Wei.
>
>
I did one year ago but all version of my patch was rejected, for example 
one of these was:
http://lists.xen.org/archives/html/xen-devel/2012-04/msg00743.html


[-- Attachment #1.2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4510 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[Ian Campbell]

On Mon, 2013-05-06 at 15:44 +0100, Wei Liu wrote:
> On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@tiscali.it 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@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 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.

The xl cfg file parser doesn't handle true/false so this option is just
following all the existing boolean options in the xl configuration file.

Supporting true/false might be a nice general enhancement to make, not
sure what the implications are in terms of compatibility with xm config
files, but I think it is OK for xl to move in, so long as moving from xm
to xl continues to work.

> > 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?

The docs are in the HVM only part of the man page.

Ian.

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

[Wei Liu]

On Tue, May 07, 2013 at 10:37:19AM +0100, Ian Campbell wrote:
> On Mon, 2013-05-06 at 15:44 +0100, Wei Liu wrote:
> > On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@tiscali.it 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@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 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.
> 
> The xl cfg file parser doesn't handle true/false so this option is just
> following all the existing boolean options in the xl configuration file.
> 
> Supporting true/false might be a nice general enhancement to make, not
> sure what the implications are in terms of compatibility with xm config
> files, but I think it is OK for xl to move in, so long as moving from xm
> to xl continues to work.
> 

What I meant is in the documentation we should use true / false instead
of 1 / 0. Other options use true / false as well, like e820_host and
spiceagent_mouse.


Wei.

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

[Ian Campbell]

On Tue, 2013-05-07 at 10:58 +0100, Wei Liu wrote:
> On Tue, May 07, 2013 at 10:37:19AM +0100, Ian Campbell wrote:
> > On Mon, 2013-05-06 at 15:44 +0100, Wei Liu wrote:
> > > On Mon, May 06, 2013 at 03:15:44PM +0100, fantonifabio@tiscali.it 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@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 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.
> > 
> > The xl cfg file parser doesn't handle true/false so this option is just
> > following all the existing boolean options in the xl configuration file.
> > 
> > Supporting true/false might be a nice general enhancement to make, not
> > sure what the implications are in terms of compatibility with xm config
> > files, but I think it is OK for xl to move in, so long as moving from xm
> > to xl continues to work.
> > 
> 
> What I meant is in the documentation we should use true / false instead
> of 1 / 0. Other options use true / false as well, like e820_host and
> spiceagent_mouse.

Or right, we do seem to be rather inconsistent about that.

Given that the parser (currently) only understands 0 and 1 (or other
integers I guess) I think the most consistent usage would be to write
"true (1)" or "false (0)" everywhere.

Do you fancy putting together a patch to make this so?

Ian.

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

[Wei Liu]

On Tue, May 07, 2013 at 11:18:43AM +0100, Ian Campbell wrote:
[...]
> > 
> > What I meant is in the documentation we should use true / false instead
> > of 1 / 0. Other options use true / false as well, like e820_host and
> > spiceagent_mouse.
> 
> Or right, we do seem to be rather inconsistent about that.
> 
> Given that the parser (currently) only understands 0 and 1 (or other
> integers I guess) I think the most consistent usage would be to write
> "true (1)" or "false (0)" everywhere.
> 
> Do you fancy putting together a patch to make this so?
> 

We already have clear definition of BOOLEAN type around line 37 in the same
file. IMHO using "true" / "false" is sufficent.


Wei.

> Ian.
> 

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

[Ian Campbell]

On Tue, 2013-05-07 at 11:38 +0100, Wei Liu wrote:
> On Tue, May 07, 2013 at 11:18:43AM +0100, Ian Campbell wrote:
> [...]
> > > 
> > > What I meant is in the documentation we should use true / false instead
> > > of 1 / 0. Other options use true / false as well, like e820_host and
> > > spiceagent_mouse.
> > 
> > Or right, we do seem to be rather inconsistent about that.
> > 
> > Given that the parser (currently) only understands 0 and 1 (or other
> > integers I guess) I think the most consistent usage would be to write
> > "true (1)" or "false (0)" everywhere.
> > 
> > Do you fancy putting together a patch to make this so?
> > 
> 
> We already have clear definition of BOOLEAN type around line 37 in the same
> file. IMHO using "true" / "false" is sufficent.

People never read this stuff properly.

In any case the current docs use 0/1 and true/false in approximately
equal measure...

Ian.

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

[Wei Liu]

On Tue, May 07, 2013 at 11:41:25AM +0100, Ian Campbell wrote:
> On Tue, 2013-05-07 at 11:38 +0100, Wei Liu wrote:
> > On Tue, May 07, 2013 at 11:18:43AM +0100, Ian Campbell wrote:
> > [...]
> > > > 
> > > > What I meant is in the documentation we should use true / false instead
> > > > of 1 / 0. Other options use true / false as well, like e820_host and
> > > > spiceagent_mouse.
> > > 
> > > Or right, we do seem to be rather inconsistent about that.
> > > 
> > > Given that the parser (currently) only understands 0 and 1 (or other
> > > integers I guess) I think the most consistent usage would be to write
> > > "true (1)" or "false (0)" everywhere.
> > > 
> > > Do you fancy putting together a patch to make this so?
> > > 
> > 
> > We already have clear definition of BOOLEAN type around line 37 in the same
> > file. IMHO using "true" / "false" is sufficent.
> 
> People never read this stuff properly.
> 
> In any case the current docs use 0/1 and true/false in approximately
> equal measure...
> 

Haha, patch on the way.


Wei.

> Ian.
> 

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

[Ian Jackson]

fantonifabio@tiscali.it writes ("[PATCH v3] 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.

Thanks, that's helpful.  What are the security implications ?

In particular, does it mean that when the user has a spice client
connected to a guest, the guest can spy on the user's clipboard all
the time ?

Thanks,
Ian.

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

[Fabio Fantoni]

[-- Attachment #1.1: Type: text/plain, Size: 905 bytes --]

Il 07/05/2013 19:06, Ian Jackson ha scritto:
> fantonifabio@tiscali.it writes ("[PATCH v3] 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.
> Thanks, that's helpful.  What are the security implications ?
>
> In particular, does it mean that when the user has a spice client
> connected to a guest, the guest can spy on the user's clipboard all
> the time ?
>
> Thanks,
> Ian.
>
Can you better explain your concerns about this?


[-- Attachment #1.2: Firma crittografica S/MIME --]
[-- Type: application/pkcs7-signature, Size: 4510 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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

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

[George Dunlap]

On Tue, May 7, 2013 at 6:06 PM, Ian Jackson <Ian.Jackson@eu.citrix.com> wrote:
> fantonifabio@tiscali.it writes ("[PATCH v3] 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.
>
> Thanks, that's helpful.  What are the security implications ?
>
> In particular, does it mean that when the user has a spice client
> connected to a guest, the guest can spy on the user's clipboard all
> the time ?

If it did, how would that affect your views on the patch?

It seems overall like the decision to run a piece of software on a
local computer which may send data (such as the clipboard) off to a
remote computer is the user's problem.  She has no way of knowing for
sure that what she's connecting to has vdagent enabled or not.  Even
if we don't accept this patch, the administrator can probably enable
it by using the "extra" args in the config file.

 -George