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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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

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.


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