fantonifabio@tiscali.it
2013-Apr-30 14:40 UTC
[PATCH v2] libxl: Spice vdagent support for upstream qemu
Usage: spicevdagent=1|0 (default=0)
Enables spice vdagent. The default is 0.
Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
docs/man/xl.cfg.pod.5 | 4 ++++
tools/libxl/libxl_dm.c | 6 ++++++
tools/libxl/libxl_types.idl | 1 +
tools/libxl/xl_cmdimpl.c | 2 ++
4 files changed, 13 insertions(+)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index f8b4576..8f95bfe 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1123,6 +1123,10 @@ 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 default is 0.
+
=back
=head3 Miscellaneous Emulated Hardware
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..8a33444 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
Stefano Stabellini
2013-May-01 11:28 UTC
Re: [PATCH v2] libxl: Spice vdagent support for upstream qemu
On Tue, 30 Apr 2013, fantonifabio@tiscali.it wrote:> Usage: spicevdagent=1|0 (default=0) > Enables spice vdagent. The default is 0. > > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>The patch looks reasonable but I am unable to test it because it requires a spice-enabled QEMU. Trusting Fabio that this patch makes it work for him, I think that I would accept it.> docs/man/xl.cfg.pod.5 | 4 ++++ > tools/libxl/libxl_dm.c | 6 ++++++ > tools/libxl/libxl_types.idl | 1 + > tools/libxl/xl_cmdimpl.c | 2 ++ > 4 files changed, 13 insertions(+) > > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 > index f8b4576..8f95bfe 100644 > --- a/docs/man/xl.cfg.pod.5 > +++ b/docs/man/xl.cfg.pod.5 > @@ -1123,6 +1123,10 @@ 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 default is 0. > + > =back > > =head3 Miscellaneous Emulated Hardware > 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..8a33444 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 >
Ian Campbell
2013-May-01 11:39 UTC
Re: [PATCH v2] libxl: Spice vdagent support for upstream qemu
On Wed, 2013-05-01 at 12:28 +0100, Stefano Stabellini wrote:> On Tue, 30 Apr 2013, fantonifabio@tiscali.it wrote: > > Usage: spicevdagent=1|0 (default=0) > > Enables spice vdagent. The default is 0. > > > > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz> > > The patch looks reasonable but I am unable to test it because it > requires a spice-enabled QEMU. > > Trusting Fabio that this patch makes it work for him, I think that I > would accept it.For 4.3? It''s hard for me to determine if it is worth the risk, it''s not clear what the wider implications of enabling this seemingly innocuous option are. It would be useful, both in making this determination and for our users, if the changelog and/or documentation included some details on what the spice vdagent actually is, what it does, what features it enables and what is required from the guest side in order to use it.> > 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)) {Something somewhere needs to call libxl_defbool_setdefault on this, otherwise users who don''t ask for something explicit will get an abort(). libxl__domain_build_info_setdefault looks like the right place for that.> > + 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..8a33444 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),Please indent to match the others.> > ]) > > > > 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 > >
George Dunlap
2013-May-01 15:17 UTC
Re: [PATCH v2] libxl: Spice vdagent support for upstream qemu
On Wed, May 1, 2013 at 12:39 PM, Ian Campbell <Ian.Campbell@citrix.com> wrote:> On Wed, 2013-05-01 at 12:28 +0100, Stefano Stabellini wrote: >> On Tue, 30 Apr 2013, fantonifabio@tiscali.it wrote: >> > Usage: spicevdagent=1|0 (default=0) >> > Enables spice vdagent. The default is 0. >> > >> > Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz> >> >> The patch looks reasonable but I am unable to test it because it >> requires a spice-enabled QEMU. >> >> Trusting Fabio that this patch makes it work for him, I think that I >> would accept it. > > For 4.3? It''s hard for me to determine if it is worth the risk, it''s not > clear what the wider implications of enabling this seemingly innocuous > option are.Yes, we''re coming up on a release soon. Unless Fabio''s got a really amazing argument to support it, I''m inclined to play it safe and say wait until 4.4. -George
Ian Jackson
2013-May-02 14:55 UTC
Re: [PATCH v2] libxl: Spice vdagent support for upstream qemu
fantonifabio@tiscali.it writes ("[PATCH v2] libxl: Spice vdagent support
for upstream qemu"):> Usage: spicevdagent=1|0 (default=0)
> Enables spice vdagent. The default is 0.
...> +=item B<spicevdagent=BOOLEAN>
> +
> +Enables spice vdagent. The default is 0.
I think this doc hunk needs an explantion of what a "spice vdagent" is
and why you might want to turn it on or off.
After I see that explanation I might have more of an opinion about the
rest of the patch.
Ian.