* [PATCH v3] libxl: add basic spice support for pv domUs
@ 2014-04-11 8:45 Fabio Fantoni
2014-04-22 9:44 ` Fabio Fantoni
2014-04-23 10:46 ` Ian Campbell
0 siblings, 2 replies; 5+ messages in thread
From: Fabio Fantoni @ 2014-04-11 8:45 UTC (permalink / raw
To: xen-devel
Cc: anthony.perard, Fabio Fantoni, Ian.Jackson, Ian.Campbell,
Stefano.Stabellini
This patch adds basic spice support for pv domUs.
The qemu parameters are the same as the hvm ones and they works.
Therefore xl cfg paramters are the same as the hvm ones except that
features not supported yet by pv domUs (vdagent and usbredirection)
are kept disabled by default.
It also enables vfb and vkb required to have basic spice working.
Note: this patch is only a draft and needs to be improved.
Patch is tested and working, except for the pointer not
visible but working.
Any feedback is appreciated.
Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
---
Changes in v3:
- xl.cfg.pod.5: moved spice out of hvm section and specified
the features for now hvm only.
- libxl_types.idl: added spice struct out of keyedunion hvm only.
- use new generic spice struct instead of hvm only ones.
Changes in v2:
- xl_cmdimpl.c: always set vnc and sdl toplevel parameters in &vfb
with vnc or spice enabled on pv domUs otherwise in some cases it
would fail with error for one bool default value missing.
- libxl_dm.c: do not add -nographic if spice is enabled, even though
-nographic seems buggy in upstream qemu.
---
docs/man/xl.cfg.pod.5 | 133 ++++++++++++++++++++++---------------------
tools/libxl/libxl_create.c | 22 ++++---
tools/libxl/libxl_dm.c | 33 ++++++-----
tools/libxl/libxl_types.idl | 3 +-
tools/libxl/xl_cmdimpl.c | 39 +++++++------
tools/libxl/xl_sxp.c | 12 ++--
6 files changed, 124 insertions(+), 118 deletions(-)
diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 146b461..8cb8c57 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1163,71 +1163,6 @@ it.
=back
-=head3 Spice Graphics Support
-
-The following options control the features of SPICE.
-
-=over 4
-
-=item B<spice=BOOLEAN>
-
-Allow access to the display via the SPICE protocol. This enables the
-other SPICE-related settings.
-
-=item B<spicehost="ADDRESS">
-
-Specify the interface address to listen on if given, otherwise any
-interface.
-
-=item B<spiceport=NUMBER>
-
-Specify the port to listen on by the SPICE server if the SPICE is
-enabled.
-
-=item B<spicetls_port=NUMBER>
-
-Specify the secure port to listen on by the SPICE server if the SPICE
-is enabled. At least one of the spiceport or spicetls_port must be
-given if SPICE is enabled. NB. the options depending on spicetls_port
-have not been supported.
-
-=item B<spicedisable_ticketing=BOOLEAN>
-
-Enable client connection without password. When disabled, spicepasswd
-must be set. The default is false (0).
-
-=item B<spicepasswd="PASSWORD">
-
-Specify the ticket password which is used by a client for connection.
-
-=item B<spiceagent_mouse=BOOLEAN>
-
-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.
-
-=item B<spice_clipboard_sharing=BOOLEAN>
-
-Enables Spice clipboard sharing (copy/paste). It requires spicevdagent
-enabled. The default is false (0).
-
-=item B<spiceusbredirection=NUMBER>
-
-Enables spice usbredirection. Creates NUMBER usbredirection channels
-for redirection of up to 4 usb devices from spice client to domU's qemu.
-It requires an usb controller and if not defined it will automatically adds
-an usb2 controller. The default is disabled (0).
-
-=back
-
=head3 Miscellaneous Emulated Hardware
=over 4
@@ -1376,6 +1311,74 @@ option to the device-model.
=back
+=head2 Spice Graphics Support
+
+The following options control the features of SPICE.
+
+=over 4
+
+=item B<spice=BOOLEAN>
+
+Allow access to the display via the SPICE protocol. This enables the
+other SPICE-related settings.
+
+=item B<spicehost="ADDRESS">
+
+Specify the interface address to listen on if given, otherwise any
+interface.
+
+=item B<spiceport=NUMBER>
+
+Specify the port to listen on by the SPICE server if the SPICE is
+enabled.
+
+=item B<spicetls_port=NUMBER>
+
+Specify the secure port to listen on by the SPICE server if the SPICE
+is enabled. At least one of the spiceport or spicetls_port must be
+given if SPICE is enabled. NB. the options depending on spicetls_port
+have not been supported.
+
+=item B<spicedisable_ticketing=BOOLEAN>
+
+Enable client connection without password. When disabled, spicepasswd
+must be set. The default is false (0).
+
+=item B<spicepasswd="PASSWORD">
+
+Specify the ticket password which is used by a client for connection.
+
+=item B<spiceagent_mouse=BOOLEAN>
+
+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.
+This feature is actually supported only by hvm domUs.
+
+=item B<spice_clipboard_sharing=BOOLEAN>
+
+Enables Spice clipboard sharing (copy/paste). It requires spicevdagent
+enabled. The default is false (0).
+This feature is actually supported only by hvm domUs.
+
+=item B<spiceusbredirection=NUMBER>
+
+Enables spice usbredirection. Creates NUMBER usbredirection channels
+for redirection of up to 4 usb devices from spice client to domU's qemu.
+It requires an usb controller and if not defined it will automatically adds
+an usb2 controller. The default is disabled (0).
+This feature is actually supported only by hvm domUs.
+
+=back
+
=head2 Keymaps
The keymaps available are defined by the device-model which you are
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 3376b5c..dc54c9b 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
if (!b_info->event_channels)
b_info->event_channels = 1023;
+ libxl_defbool_setdefault(&b_info->spice.enable, false);
+ if (libxl_defbool_val(b_info->spice.enable)) {
+ libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
+ libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
+ libxl_defbool_setdefault(&b_info->spice.vdagent, false);
+ libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
+ }
+
switch (b_info->type) {
case LIBXL_DOMAIN_TYPE_HVM:
if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
@@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
if (!b_info->u.hvm.usbversion &&
- (b_info->u.hvm.spice.usbredirection > 0) )
+ (b_info->spice.usbredirection > 0) )
b_info->u.hvm.usbversion = 2;
- if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) &&
+ if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
( libxl_defbool_val(b_info->u.hvm.usb)
|| b_info->u.hvm.usbdevice_list
|| b_info->u.hvm.usbdevice) ){
@@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
}
- libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
- if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
- 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.spice.clipboard_sharing,
- false);
- }
-
libxl_defbool_setdefault(&b_info->u.hvm.nographic, false);
libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru, false);
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index e87f606..d4bbdb1 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -478,6 +478,21 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_vappend(dm_args, "-k", keymap, NULL);
}
+ if (libxl_defbool_val(b_info->spice.enable)) {
+ const libxl_spice_info *spice = &b_info->spice;
+ char *spiceoptions = dm_spice_options(gc, spice);
+ if (!spiceoptions)
+ return NULL;
+
+ flexarray_append(dm_args, "-spice");
+ flexarray_append(dm_args, spiceoptions);
+ if (libxl_defbool_val(b_info->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);
+ }
+ }
+
if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
int ioemu_nics = 0;
@@ -489,22 +504,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, "-nographic");
}
- if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
- const libxl_spice_info *spice = &b_info->u.hvm.spice;
- char *spiceoptions = dm_spice_options(gc, spice);
- if (!spiceoptions)
- return NULL;
-
- 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) {
case LIBXL_VGA_INTERFACE_TYPE_STD:
flexarray_append_pair(dm_args, "-device", "VGA");
@@ -639,7 +638,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
flexarray_append(dm_args, "-gfx_passthru");
}
} else {
- if (!sdl && !vnc) {
+ if (!sdl && !vnc && !libxl_defbool_val(b_info->spice.enable)) {
flexarray_append(dm_args, "-nographic");
}
}
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index 2b0faed..4f6c237 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -333,8 +333,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
("ioports", Array(libxl_ioport_range, "num_ioports")),
("irqs", Array(uint32, "num_irqs")),
("iomem", Array(libxl_iomem_range, "num_iomem")),
- ("claim_mode", libxl_defbool),
+ ("claim_mode", libxl_defbool),
("event_channels", uint32),
+ ("spice", libxl_spice_info),
("u", KeyedUnion(None, libxl_domain_type, "type",
[("hvm", Struct(None, [("firmware", string),
("bios", libxl_bios_type),
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index e856e25..c9718f9 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1647,6 +1647,8 @@ skip_vfb:
#undef parse_extra_args
+ xlu_cfg_get_defbool (config, "spice", &b_info->spice.enable, 0);
+
/* If we've already got vfb=[] for PV guest then ignore top level
* VNC config. */
if (c_info->type == LIBXL_DOMAIN_TYPE_PV && !d_config->num_vfbs) {
@@ -1655,7 +1657,7 @@ skip_vfb:
if (!xlu_cfg_get_long (config, "vnc", &l, 0))
vnc_enabled = l;
- if (vnc_enabled) {
+ if (vnc_enabled || libxl_defbool_val(b_info->spice.enable)) {
libxl_device_vfb *vfb;
libxl_device_vkb *vkb;
@@ -1674,6 +1676,21 @@ skip_vfb:
parse_top_level_sdl_options(config, &b_info->u.hvm.sdl);
}
+ if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
+ b_info->spice.port = l;
+ if (!xlu_cfg_get_long (config, "spicetls_port", &l, 0))
+ b_info->spice.tls_port = l;
+ xlu_cfg_replace_string (config, "spicehost",
+ &b_info->spice.host, 0);
+ xlu_cfg_get_defbool(config, "spicedisable_ticketing",
+ &b_info->spice.disable_ticketing, 0);
+ xlu_cfg_replace_string(config, "spicepasswd",
+ &b_info->spice.passwd, 0);
+ xlu_cfg_get_defbool(config, "spiceagent_mouse",
+ &b_info->spice.agent_mouse, 0);
+ /* Some spice features are missed because not supported by domU pv now */
+ b_info->spice.usbredirection = 0;
+
if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
if (!xlu_cfg_get_string (config, "vga", &buf, 0)) {
if (!strcmp(buf, "stdvga")) {
@@ -1693,25 +1710,13 @@ skip_vfb:
LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
- xlu_cfg_get_defbool (config, "spice", &b_info->u.hvm.spice.enable, 0);
- if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
- b_info->u.hvm.spice.port = l;
- if (!xlu_cfg_get_long (config, "spicetls_port", &l, 0))
- b_info->u.hvm.spice.tls_port = l;
- xlu_cfg_replace_string (config, "spicehost",
- &b_info->u.hvm.spice.host, 0);
- xlu_cfg_get_defbool(config, "spicedisable_ticketing",
- &b_info->u.hvm.spice.disable_ticketing, 0);
- xlu_cfg_replace_string (config, "spicepasswd",
- &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);
+ &b_info->spice.vdagent, 0);
xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
- &b_info->u.hvm.spice.clipboard_sharing, 0);
+ &b_info->spice.clipboard_sharing, 0);
if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
- b_info->u.hvm.spice.usbredirection = l;
+ b_info->spice.usbredirection = l;
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);
diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
index a16a025..e9eed99 100644
--- a/tools/libxl/xl_sxp.c
+++ b/tools/libxl/xl_sxp.c
@@ -127,14 +127,14 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
printf("\t\t\t(nographic %s)\n",
libxl_defbool_to_string(b_info->u.hvm.nographic));
printf("\t\t\t(spice %s)\n",
- libxl_defbool_to_string(b_info->u.hvm.spice.enable));
- printf("\t\t\t(spiceport %d)\n", b_info->u.hvm.spice.port);
- printf("\t\t\t(spicetls_port %d)\n", b_info->u.hvm.spice.tls_port);
- printf("\t\t\t(spicehost %s)\n", b_info->u.hvm.spice.host);
+ libxl_defbool_to_string(b_info->spice.enable));
+ printf("\t\t\t(spiceport %d)\n", b_info->spice.port);
+ printf("\t\t\t(spicetls_port %d)\n", b_info->spice.tls_port);
+ printf("\t\t\t(spicehost %s)\n", b_info->spice.host);
printf("\t\t\t(spicedisable_ticketing %s)\n",
- libxl_defbool_to_string(b_info->u.hvm.spice.disable_ticketing));
+ libxl_defbool_to_string(b_info->spice.disable_ticketing));
printf("\t\t\t(spiceagent_mouse %s)\n",
- libxl_defbool_to_string(b_info->u.hvm.spice.agent_mouse));
+ libxl_defbool_to_string(b_info->spice.agent_mouse));
printf("\t\t\t(device_model %s)\n", b_info->device_model ? : "default");
printf("\t\t\t(gfx_passthru %s)\n",
--
1.7.9.5
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] libxl: add basic spice support for pv domUs
2014-04-11 8:45 [PATCH v3] libxl: add basic spice support for pv domUs Fabio Fantoni
@ 2014-04-22 9:44 ` Fabio Fantoni
2014-04-23 10:46 ` Ian Campbell
1 sibling, 0 replies; 5+ messages in thread
From: Fabio Fantoni @ 2014-04-22 9:44 UTC (permalink / raw
To: xen-devel; +Cc: anthony.perard, Ian.Jackson, Ian.Campbell, Stefano.Stabellini
Il 11/04/2014 10:45, Fabio Fantoni ha scritto:
> This patch adds basic spice support for pv domUs.
> The qemu parameters are the same as the hvm ones and they works.
> Therefore xl cfg paramters are the same as the hvm ones except that
> features not supported yet by pv domUs (vdagent and usbredirection)
> are kept disabled by default.
> It also enables vfb and vkb required to have basic spice working.
>
> Note: this patch is only a draft and needs to be improved.
> Patch is tested and working, except for the pointer not
> visible but working.
> Any feedback is appreciated.
Ping
>
> Signed-off-by: Fabio Fantoni <fabio.fantoni@m2r.biz>
>
> ---
>
> Changes in v3:
> - xl.cfg.pod.5: moved spice out of hvm section and specified
> the features for now hvm only.
> - libxl_types.idl: added spice struct out of keyedunion hvm only.
> - use new generic spice struct instead of hvm only ones.
>
> Changes in v2:
> - xl_cmdimpl.c: always set vnc and sdl toplevel parameters in &vfb
> with vnc or spice enabled on pv domUs otherwise in some cases it
> would fail with error for one bool default value missing.
> - libxl_dm.c: do not add -nographic if spice is enabled, even though
> -nographic seems buggy in upstream qemu.
> ---
> docs/man/xl.cfg.pod.5 | 133 ++++++++++++++++++++++---------------------
> tools/libxl/libxl_create.c | 22 ++++---
> tools/libxl/libxl_dm.c | 33 ++++++-----
> tools/libxl/libxl_types.idl | 3 +-
> tools/libxl/xl_cmdimpl.c | 39 +++++++------
> tools/libxl/xl_sxp.c | 12 ++--
> 6 files changed, 124 insertions(+), 118 deletions(-)
>
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 146b461..8cb8c57 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1163,71 +1163,6 @@ it.
>
> =back
>
> -=head3 Spice Graphics Support
> -
> -The following options control the features of SPICE.
> -
> -=over 4
> -
> -=item B<spice=BOOLEAN>
> -
> -Allow access to the display via the SPICE protocol. This enables the
> -other SPICE-related settings.
> -
> -=item B<spicehost="ADDRESS">
> -
> -Specify the interface address to listen on if given, otherwise any
> -interface.
> -
> -=item B<spiceport=NUMBER>
> -
> -Specify the port to listen on by the SPICE server if the SPICE is
> -enabled.
> -
> -=item B<spicetls_port=NUMBER>
> -
> -Specify the secure port to listen on by the SPICE server if the SPICE
> -is enabled. At least one of the spiceport or spicetls_port must be
> -given if SPICE is enabled. NB. the options depending on spicetls_port
> -have not been supported.
> -
> -=item B<spicedisable_ticketing=BOOLEAN>
> -
> -Enable client connection without password. When disabled, spicepasswd
> -must be set. The default is false (0).
> -
> -=item B<spicepasswd="PASSWORD">
> -
> -Specify the ticket password which is used by a client for connection.
> -
> -=item B<spiceagent_mouse=BOOLEAN>
> -
> -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.
> -
> -=item B<spice_clipboard_sharing=BOOLEAN>
> -
> -Enables Spice clipboard sharing (copy/paste). It requires spicevdagent
> -enabled. The default is false (0).
> -
> -=item B<spiceusbredirection=NUMBER>
> -
> -Enables spice usbredirection. Creates NUMBER usbredirection channels
> -for redirection of up to 4 usb devices from spice client to domU's qemu.
> -It requires an usb controller and if not defined it will automatically adds
> -an usb2 controller. The default is disabled (0).
> -
> -=back
> -
> =head3 Miscellaneous Emulated Hardware
>
> =over 4
> @@ -1376,6 +1311,74 @@ option to the device-model.
>
> =back
>
> +=head2 Spice Graphics Support
> +
> +The following options control the features of SPICE.
> +
> +=over 4
> +
> +=item B<spice=BOOLEAN>
> +
> +Allow access to the display via the SPICE protocol. This enables the
> +other SPICE-related settings.
> +
> +=item B<spicehost="ADDRESS">
> +
> +Specify the interface address to listen on if given, otherwise any
> +interface.
> +
> +=item B<spiceport=NUMBER>
> +
> +Specify the port to listen on by the SPICE server if the SPICE is
> +enabled.
> +
> +=item B<spicetls_port=NUMBER>
> +
> +Specify the secure port to listen on by the SPICE server if the SPICE
> +is enabled. At least one of the spiceport or spicetls_port must be
> +given if SPICE is enabled. NB. the options depending on spicetls_port
> +have not been supported.
> +
> +=item B<spicedisable_ticketing=BOOLEAN>
> +
> +Enable client connection without password. When disabled, spicepasswd
> +must be set. The default is false (0).
> +
> +=item B<spicepasswd="PASSWORD">
> +
> +Specify the ticket password which is used by a client for connection.
> +
> +=item B<spiceagent_mouse=BOOLEAN>
> +
> +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.
> +This feature is actually supported only by hvm domUs.
> +
> +=item B<spice_clipboard_sharing=BOOLEAN>
> +
> +Enables Spice clipboard sharing (copy/paste). It requires spicevdagent
> +enabled. The default is false (0).
> +This feature is actually supported only by hvm domUs.
> +
> +=item B<spiceusbredirection=NUMBER>
> +
> +Enables spice usbredirection. Creates NUMBER usbredirection channels
> +for redirection of up to 4 usb devices from spice client to domU's qemu.
> +It requires an usb controller and if not defined it will automatically adds
> +an usb2 controller. The default is disabled (0).
> +This feature is actually supported only by hvm domUs.
> +
> +=back
> +
> =head2 Keymaps
>
> The keymaps available are defined by the device-model which you are
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 3376b5c..dc54c9b 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> if (!b_info->event_channels)
> b_info->event_channels = 1023;
>
> + libxl_defbool_setdefault(&b_info->spice.enable, false);
> + if (libxl_defbool_val(b_info->spice.enable)) {
> + libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
> + libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
> + libxl_defbool_setdefault(&b_info->spice.vdagent, false);
> + libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
> + }
> +
> switch (b_info->type) {
> case LIBXL_DOMAIN_TYPE_HVM:
> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
>
> if (!b_info->u.hvm.usbversion &&
> - (b_info->u.hvm.spice.usbredirection > 0) )
> + (b_info->spice.usbredirection > 0) )
> b_info->u.hvm.usbversion = 2;
>
> - if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) &&
> + if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
> ( libxl_defbool_val(b_info->u.hvm.usb)
> || b_info->u.hvm.usbdevice_list
> || b_info->u.hvm.usbdevice) ){
> @@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
> }
>
> - libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
> - if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
> - 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.spice.clipboard_sharing,
> - false);
> - }
> -
> libxl_defbool_setdefault(&b_info->u.hvm.nographic, false);
>
> libxl_defbool_setdefault(&b_info->u.hvm.gfx_passthru, false);
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index e87f606..d4bbdb1 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -478,6 +478,21 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> flexarray_vappend(dm_args, "-k", keymap, NULL);
> }
>
> + if (libxl_defbool_val(b_info->spice.enable)) {
> + const libxl_spice_info *spice = &b_info->spice;
> + char *spiceoptions = dm_spice_options(gc, spice);
> + if (!spiceoptions)
> + return NULL;
> +
> + flexarray_append(dm_args, "-spice");
> + flexarray_append(dm_args, spiceoptions);
> + if (libxl_defbool_val(b_info->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);
> + }
> + }
> +
> if (b_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> int ioemu_nics = 0;
>
> @@ -489,22 +504,6 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> flexarray_append(dm_args, "-nographic");
> }
>
> - if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
> - const libxl_spice_info *spice = &b_info->u.hvm.spice;
> - char *spiceoptions = dm_spice_options(gc, spice);
> - if (!spiceoptions)
> - return NULL;
> -
> - 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) {
> case LIBXL_VGA_INTERFACE_TYPE_STD:
> flexarray_append_pair(dm_args, "-device", "VGA");
> @@ -639,7 +638,7 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc,
> flexarray_append(dm_args, "-gfx_passthru");
> }
> } else {
> - if (!sdl && !vnc) {
> + if (!sdl && !vnc && !libxl_defbool_val(b_info->spice.enable)) {
> flexarray_append(dm_args, "-nographic");
> }
> }
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 2b0faed..4f6c237 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -333,8 +333,9 @@ libxl_domain_build_info = Struct("domain_build_info",[
> ("ioports", Array(libxl_ioport_range, "num_ioports")),
> ("irqs", Array(uint32, "num_irqs")),
> ("iomem", Array(libxl_iomem_range, "num_iomem")),
> - ("claim_mode", libxl_defbool),
> + ("claim_mode", libxl_defbool),
> ("event_channels", uint32),
> + ("spice", libxl_spice_info),
> ("u", KeyedUnion(None, libxl_domain_type, "type",
> [("hvm", Struct(None, [("firmware", string),
> ("bios", libxl_bios_type),
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index e856e25..c9718f9 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1647,6 +1647,8 @@ skip_vfb:
>
> #undef parse_extra_args
>
> + xlu_cfg_get_defbool (config, "spice", &b_info->spice.enable, 0);
> +
> /* If we've already got vfb=[] for PV guest then ignore top level
> * VNC config. */
> if (c_info->type == LIBXL_DOMAIN_TYPE_PV && !d_config->num_vfbs) {
> @@ -1655,7 +1657,7 @@ skip_vfb:
> if (!xlu_cfg_get_long (config, "vnc", &l, 0))
> vnc_enabled = l;
>
> - if (vnc_enabled) {
> + if (vnc_enabled || libxl_defbool_val(b_info->spice.enable)) {
> libxl_device_vfb *vfb;
> libxl_device_vkb *vkb;
>
> @@ -1674,6 +1676,21 @@ skip_vfb:
> parse_top_level_sdl_options(config, &b_info->u.hvm.sdl);
> }
>
> + if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
> + b_info->spice.port = l;
> + if (!xlu_cfg_get_long (config, "spicetls_port", &l, 0))
> + b_info->spice.tls_port = l;
> + xlu_cfg_replace_string (config, "spicehost",
> + &b_info->spice.host, 0);
> + xlu_cfg_get_defbool(config, "spicedisable_ticketing",
> + &b_info->spice.disable_ticketing, 0);
> + xlu_cfg_replace_string(config, "spicepasswd",
> + &b_info->spice.passwd, 0);
> + xlu_cfg_get_defbool(config, "spiceagent_mouse",
> + &b_info->spice.agent_mouse, 0);
> + /* Some spice features are missed because not supported by domU pv now */
> + b_info->spice.usbredirection = 0;
> +
> if (c_info->type == LIBXL_DOMAIN_TYPE_HVM) {
> if (!xlu_cfg_get_string (config, "vga", &buf, 0)) {
> if (!strcmp(buf, "stdvga")) {
> @@ -1693,25 +1710,13 @@ skip_vfb:
> LIBXL_VGA_INTERFACE_TYPE_CIRRUS;
>
> xlu_cfg_replace_string (config, "keymap", &b_info->u.hvm.keymap, 0);
> - xlu_cfg_get_defbool (config, "spice", &b_info->u.hvm.spice.enable, 0);
> - if (!xlu_cfg_get_long (config, "spiceport", &l, 0))
> - b_info->u.hvm.spice.port = l;
> - if (!xlu_cfg_get_long (config, "spicetls_port", &l, 0))
> - b_info->u.hvm.spice.tls_port = l;
> - xlu_cfg_replace_string (config, "spicehost",
> - &b_info->u.hvm.spice.host, 0);
> - xlu_cfg_get_defbool(config, "spicedisable_ticketing",
> - &b_info->u.hvm.spice.disable_ticketing, 0);
> - xlu_cfg_replace_string (config, "spicepasswd",
> - &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);
> + &b_info->spice.vdagent, 0);
> xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
> - &b_info->u.hvm.spice.clipboard_sharing, 0);
> + &b_info->spice.clipboard_sharing, 0);
> if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
> - b_info->u.hvm.spice.usbredirection = l;
> + b_info->spice.usbredirection = l;
> 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);
> diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
> index a16a025..e9eed99 100644
> --- a/tools/libxl/xl_sxp.c
> +++ b/tools/libxl/xl_sxp.c
> @@ -127,14 +127,14 @@ void printf_info_sexp(int domid, libxl_domain_config *d_config)
> printf("\t\t\t(nographic %s)\n",
> libxl_defbool_to_string(b_info->u.hvm.nographic));
> printf("\t\t\t(spice %s)\n",
> - libxl_defbool_to_string(b_info->u.hvm.spice.enable));
> - printf("\t\t\t(spiceport %d)\n", b_info->u.hvm.spice.port);
> - printf("\t\t\t(spicetls_port %d)\n", b_info->u.hvm.spice.tls_port);
> - printf("\t\t\t(spicehost %s)\n", b_info->u.hvm.spice.host);
> + libxl_defbool_to_string(b_info->spice.enable));
> + printf("\t\t\t(spiceport %d)\n", b_info->spice.port);
> + printf("\t\t\t(spicetls_port %d)\n", b_info->spice.tls_port);
> + printf("\t\t\t(spicehost %s)\n", b_info->spice.host);
> printf("\t\t\t(spicedisable_ticketing %s)\n",
> - libxl_defbool_to_string(b_info->u.hvm.spice.disable_ticketing));
> + libxl_defbool_to_string(b_info->spice.disable_ticketing));
> printf("\t\t\t(spiceagent_mouse %s)\n",
> - libxl_defbool_to_string(b_info->u.hvm.spice.agent_mouse));
> + libxl_defbool_to_string(b_info->spice.agent_mouse));
>
> printf("\t\t\t(device_model %s)\n", b_info->device_model ? : "default");
> printf("\t\t\t(gfx_passthru %s)\n",
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] libxl: add basic spice support for pv domUs
2014-04-11 8:45 [PATCH v3] libxl: add basic spice support for pv domUs Fabio Fantoni
2014-04-22 9:44 ` Fabio Fantoni
@ 2014-04-23 10:46 ` Ian Campbell
2014-04-29 8:45 ` Fabio Fantoni
1 sibling, 1 reply; 5+ messages in thread
From: Ian Campbell @ 2014-04-23 10:46 UTC (permalink / raw
To: Fabio Fantoni; +Cc: anthony.perard, xen-devel, Ian.Jackson, Stefano.Stabellini
On Fri, 2014-04-11 at 10:45 +0200, Fabio Fantoni wrote:
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index 3376b5c..dc54c9b 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> if (!b_info->event_channels)
> b_info->event_channels = 1023;
>
> + libxl_defbool_setdefault(&b_info->spice.enable, false);
> + if (libxl_defbool_val(b_info->spice.enable)) {
> + libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
> + libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
> + libxl_defbool_setdefault(&b_info->spice.vdagent, false);
> + libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
> + }
> +
> switch (b_info->type) {
> case LIBXL_DOMAIN_TYPE_HVM:
> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> @@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
>
> if (!b_info->u.hvm.usbversion &&
> - (b_info->u.hvm.spice.usbredirection > 0) )
> + (b_info->spice.usbredirection > 0) )
> b_info->u.hvm.usbversion = 2;
>
> - if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) &&
> + if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
> ( libxl_defbool_val(b_info->u.hvm.usb)
> || b_info->u.hvm.usbdevice_list
> || b_info->u.hvm.usbdevice) ){
> @@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
> }
>
> - libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
> - if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
> - 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.spice.clipboard_sharing,
> - false);
> - }
libxl appear to now ignore b_info->u.hvm.spice completely. Unfortunately
this will break backwards compatibility.
It should be possible to build a application written against old libxl
against this new libxl and have everything continue to work.
Unfortunately this means you need to jump through a few hoops in the
library code. You should define some sane precedence between
b_info->spice and b_info->u.hvm.spice and copy things from u.hvm.spice
to spice as necessary. I think it makes sense to say that ->spice takes
precedence over ->u.hvm.spice. IOW if ->spice.enable == false and
->u.hvm.spice.enable is true then copy ->u.hvm.spice into ->spice. You
should think through the consequences of this though in case I am
confused.
You also need to announce the presence of this new field via a
LIBXL_HAVE #define in libxl.h (there are plenty of existing examples).
You should document the precedence which you decide on as part of the
associated comment.
> xlu_cfg_get_defbool(config, "spicevdagent",
> - &b_info->u.hvm.spice.vdagent, 0);
> + &b_info->spice.vdagent, 0);
> xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
> - &b_info->u.hvm.spice.clipboard_sharing, 0);
> + &b_info->spice.clipboard_sharing, 0);
> if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
> - b_info->u.hvm.spice.usbredirection = l;
> + b_info->spice.usbredirection = l;
So it appears that some members of b_info->spice are not relevant to PV
guests?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] libxl: add basic spice support for pv domUs
2014-04-23 10:46 ` Ian Campbell
@ 2014-04-29 8:45 ` Fabio Fantoni
2014-04-29 9:02 ` Ian Campbell
0 siblings, 1 reply; 5+ messages in thread
From: Fabio Fantoni @ 2014-04-29 8:45 UTC (permalink / raw
To: Ian Campbell; +Cc: anthony.perard, xen-devel, Ian.Jackson, Stefano.Stabellini
Il 23/04/2014 12:46, Ian Campbell ha scritto:
> On Fri, 2014-04-11 at 10:45 +0200, Fabio Fantoni wrote:
>> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
>> index 3376b5c..dc54c9b 100644
>> --- a/tools/libxl/libxl_create.c
>> +++ b/tools/libxl/libxl_create.c
>> @@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>> if (!b_info->event_channels)
>> b_info->event_channels = 1023;
>>
>> + libxl_defbool_setdefault(&b_info->spice.enable, false);
>> + if (libxl_defbool_val(b_info->spice.enable)) {
>> + libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
>> + libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
>> + libxl_defbool_setdefault(&b_info->spice.vdagent, false);
>> + libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
>> + }
>> +
>> switch (b_info->type) {
>> case LIBXL_DOMAIN_TYPE_HVM:
>> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
>> @@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
>>
>> if (!b_info->u.hvm.usbversion &&
>> - (b_info->u.hvm.spice.usbredirection > 0) )
>> + (b_info->spice.usbredirection > 0) )
>> b_info->u.hvm.usbversion = 2;
>>
>> - if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) &&
>> + if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
>> ( libxl_defbool_val(b_info->u.hvm.usb)
>> || b_info->u.hvm.usbdevice_list
>> || b_info->u.hvm.usbdevice) ){
>> @@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>> libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
>> }
>>
>> - libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
>> - if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
>> - 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.spice.clipboard_sharing,
>> - false);
>> - }
> libxl appear to now ignore b_info->u.hvm.spice completely. Unfortunately
> this will break backwards compatibility.
>
> It should be possible to build a application written against old libxl
> against this new libxl and have everything continue to work.
> Unfortunately this means you need to jump through a few hoops in the
> library code. You should define some sane precedence between
> b_info->spice and b_info->u.hvm.spice and copy things from u.hvm.spice
> to spice as necessary. I think it makes sense to say that ->spice takes
> precedence over ->u.hvm.spice. IOW if ->spice.enable == false and
> ->u.hvm.spice.enable is true then copy ->u.hvm.spice into ->spice. You
> should think through the consequences of this though in case I am
> confused.
Where is the correct place to check for ->u.hvm.spice and copy it in
->spice if enabled?
>
> You also need to announce the presence of this new field via a
> LIBXL_HAVE #define in libxl.h (there are plenty of existing examples).
> You should document the precedence which you decide on as part of the
> associated comment.
Sorry to forgot it, I'll add it on next version.
May I mark ->u.hvm.spice as deprecated?
>
>> xlu_cfg_get_defbool(config, "spicevdagent",
>> - &b_info->u.hvm.spice.vdagent, 0);
>> + &b_info->spice.vdagent, 0);
>> xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
>> - &b_info->u.hvm.spice.clipboard_sharing, 0);
>> + &b_info->spice.clipboard_sharing, 0);
>> if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
>> - b_info->u.hvm.spice.usbredirection = l;
>> + b_info->spice.usbredirection = l;
> So it appears that some members of b_info->spice are not relevant to PV
> guests?
>
>
At the moment the pv domUs don't support virtio and emulated usb, so I
leave such features disabled on them.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3] libxl: add basic spice support for pv domUs
2014-04-29 8:45 ` Fabio Fantoni
@ 2014-04-29 9:02 ` Ian Campbell
0 siblings, 0 replies; 5+ messages in thread
From: Ian Campbell @ 2014-04-29 9:02 UTC (permalink / raw
To: Fabio Fantoni; +Cc: anthony.perard, xen-devel, Ian.Jackson, Stefano.Stabellini
On Tue, 2014-04-29 at 10:45 +0200, Fabio Fantoni wrote:
> Il 23/04/2014 12:46, Ian Campbell ha scritto:
> > On Fri, 2014-04-11 at 10:45 +0200, Fabio Fantoni wrote:
> >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >> index 3376b5c..dc54c9b 100644
> >> --- a/tools/libxl/libxl_create.c
> >> +++ b/tools/libxl/libxl_create.c
> >> @@ -215,6 +215,14 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> >> if (!b_info->event_channels)
> >> b_info->event_channels = 1023;
> >>
> >> + libxl_defbool_setdefault(&b_info->spice.enable, false);
> >> + if (libxl_defbool_val(b_info->spice.enable)) {
> >> + libxl_defbool_setdefault(&b_info->spice.disable_ticketing, false);
> >> + libxl_defbool_setdefault(&b_info->spice.agent_mouse, true);
> >> + libxl_defbool_setdefault(&b_info->spice.vdagent, false);
> >> + libxl_defbool_setdefault(&b_info->spice.clipboard_sharing, false);
> >> + }
> >> +
> >> switch (b_info->type) {
> >> case LIBXL_DOMAIN_TYPE_HVM:
> >> if (b_info->shadow_memkb == LIBXL_MEMKB_DEFAULT)
> >> @@ -306,10 +314,10 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> >> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true);
> >>
> >> if (!b_info->u.hvm.usbversion &&
> >> - (b_info->u.hvm.spice.usbredirection > 0) )
> >> + (b_info->spice.usbredirection > 0) )
> >> b_info->u.hvm.usbversion = 2;
> >>
> >> - if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) &&
> >> + if ((b_info->u.hvm.usbversion || b_info->spice.usbredirection) &&
> >> ( libxl_defbool_val(b_info->u.hvm.usb)
> >> || b_info->u.hvm.usbdevice_list
> >> || b_info->u.hvm.usbdevice) ){
> >> @@ -337,16 +345,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> >> libxl_defbool_setdefault(&b_info->u.hvm.sdl.opengl, false);
> >> }
> >>
> >> - libxl_defbool_setdefault(&b_info->u.hvm.spice.enable, false);
> >> - if (libxl_defbool_val(b_info->u.hvm.spice.enable)) {
> >> - 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.spice.clipboard_sharing,
> >> - false);
> >> - }
> > libxl appear to now ignore b_info->u.hvm.spice completely. Unfortunately
> > this will break backwards compatibility.
> >
> > It should be possible to build a application written against old libxl
> > against this new libxl and have everything continue to work.
> > Unfortunately this means you need to jump through a few hoops in the
> > library code. You should define some sane precedence between
> > b_info->spice and b_info->u.hvm.spice and copy things from u.hvm.spice
> > to spice as necessary. I think it makes sense to say that ->spice takes
> > precedence over ->u.hvm.spice. IOW if ->spice.enable == false and
> > ->u.hvm.spice.enable is true then copy ->u.hvm.spice into ->spice. You
> > should think through the consequences of this though in case I am
> > confused.
>
> Where is the correct place to check for ->u.hvm.spice and copy it in
> ->spice if enabled?
In the relevant libxl__._setdefault I think.
> >
> > You also need to announce the presence of this new field via a
> > LIBXL_HAVE #define in libxl.h (there are plenty of existing examples).
> > You should document the precedence which you decide on as part of the
> > associated comment.
>
> Sorry to forgot it, I'll add it on next version.
> May I mark ->u.hvm.spice as deprecated?
You can say that ->spice is preferred for sure.
> >> xlu_cfg_get_defbool(config, "spicevdagent",
> >> - &b_info->u.hvm.spice.vdagent, 0);
> >> + &b_info->spice.vdagent, 0);
> >> xlu_cfg_get_defbool(config, "spice_clipboard_sharing",
> >> - &b_info->u.hvm.spice.clipboard_sharing, 0);
> >> + &b_info->spice.clipboard_sharing, 0);
> >> if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0))
> >> - b_info->u.hvm.spice.usbredirection = l;
> >> + b_info->spice.usbredirection = l;
> > So it appears that some members of b_info->spice are not relevant to PV
> > guests?
> >
> >
>
> At the moment the pv domUs don't support virtio and emulated usb, so I
> leave such features disabled on them.
"At the moment" implies that this might change, have you given any
thought to the API stability implications of adding this. Perhaps there
are steps you can take now while introducing the basic level of support
which will simplify the future work?
Ian.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-04-29 9:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-11 8:45 [PATCH v3] libxl: add basic spice support for pv domUs Fabio Fantoni
2014-04-22 9:44 ` Fabio Fantoni
2014-04-23 10:46 ` Ian Campbell
2014-04-29 8:45 ` Fabio Fantoni
2014-04-29 9:02 ` Ian Campbell
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.