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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

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@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.
> 
> 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.

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