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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Thu, Jul 04, 2013 at 12:16:43PM +0200, Fabio Fantoni wrote:
> Il 04/07/2013 12:12, Wei Liu ha scritto:
> >On Thu, Jul 04, 2013 at 12:05:59PM +0200, Fabio Fantoni wrote:
> >>Usage: spiceusbredirection=1|0 (default=0)
> >>Enables spice usbredirection. The Spice usbredirection creates usb2
> >>controller and 4 usbredirection channels for redirection of up to 4
> >>usb devices from spice client to domU's qemu.
> >>
> >>Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx>
> >>---
> >>  docs/man/xl.cfg.pod.5       |    8 ++++++++
> >>  tools/libxl/libxl_create.c  |    1 +
> >>  tools/libxl/libxl_dm.c      |   18 ++++++++++++++++++
> >>  tools/libxl/libxl_types.idl |    1 +
> >>  tools/libxl/xl_cmdimpl.c    |    2 ++
> >>  5 files changed, 30 insertions(+)
> >>
> >>diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> >>index 766862d..a450800 100644
> >>--- a/docs/man/xl.cfg.pod.5
> >>+++ b/docs/man/xl.cfg.pod.5
> >>@@ -1134,6 +1134,14 @@ requires vdagent service installed on domU o.s. to 
> >>work. The default is 0.
> >>  =back
> >>+=item B<spiceusbredirection=BOOLEAN>
> >>+
> >>+Enables spice usbredirection. The Spice usbredirection creates usb2
> >>+controller and 4 usbredirection channels for redirection of up to 4 usb
> >>+devices from spice client to domU's qemu. The default is 0.
> >>+
> >>+=back
> >>+
> >>  =head3 Miscellaneous Emulated Hardware
> >>  =over 4
> >>diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> >>index 8db5460..58df106 100644
> >>--- a/tools/libxl/libxl_create.c
> >>+++ b/tools/libxl/libxl_create.c
> >>@@ -289,6 +289,7 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
> >>                                       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.usbredirection, 
> >>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 bc605e4..4f625e0 100644
> >>--- a/tools/libxl/libxl_dm.c
> >>+++ b/tools/libxl/libxl_dm.c
> >>@@ -471,6 +471,24 @@ static char ** 
> >>libxl__build_device_model_args_new(libxl__gc *gc,
> >>                      
> >> "virtserialport,chardev=vdagent,name=com.redhat.spice.0",
> >>                      NULL);
> >>              }
> >>+
> >>+            if (libxl_defbool_val(b_info->u.hvm.spice.usbredirection)) {
> >>+                flexarray_vappend(dm_args, 
> >>"-device","ich9-usb-ehci1,id=usb,"
> >>+                    "bus=pci.0,addr=0x1d.0x7", "-device","ich9-usb-uhci1,"
> >>+                    
> >>"masterbus=usb.0,firstport=0,bus=pci.0,multifunction=on,"
> >>+                    "addr=0x1d.0x0", 
> >>"-device","ich9-usb-uhci2,masterbus=usb.0,"
> >>+                    "firstport=2,bus=pci.0,addr=0x1d.0x1", "-device",
> >>+                    "ich9-usb-uhci3,masterbus=usb.0,firstport=4,bus=pci.0,"
> >>+                    "addr=0x1d.0x2", "-chardev","spicevmc,name=usbredir,"
> >>+                    
> >>"id=usbrc1","-device","usb-redir,chardev=usbrc1,id=usbrc1,"
> >>+                    "bus=usb.0", 
> >>"-chardev","spicevmc,name=usbredir,id=usbrc2",
> >>+                    
> >>"-device","usb-redir,chardev=usbrc2,id=usbrc2,bus=usb.0",
> >>+                    
> >>"-chardev","spicevmc,name=usbredir,id=usbrc3","-device",
> >>+                    "usb-redir,chardev=usbrc3,id=usbrc3,bus=usb.0", 
> >>"-chardev",
> >>+                    
> >>"spicevmc,name=usbredir,id=usbrc4","-device","usb-redir,"
> >>+                    "chardev=usbrc4,id=usbrc4,bus=usb.0", NULL);
> >Any reason for so many hardcoded options?
> 
> I searched and requested for one year on spice-devel and qemu-devel
> about alternative methods but nothing found for now.
> Already tried usb=1 which creates usb1 controller that is not
> working with usb redirection.
> 

What if QEMU upstream changes and these options don't work any more? In
that case this functionality is broken and users have no way to
workaround it. IMHO unless they are clearly documented we should not
consider adding in theses hardcoded options in libxl.


> >
> >>+            }
> >>+
> >>          }
> >>          switch (b_info->u.hvm.vga.kind) {
> >>diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> >>index 14425d1..80aa2b8 100644
> >>--- a/tools/libxl/libxl_types.idl
> >>+++ b/tools/libxl/libxl_types.idl
> >>@@ -173,6 +173,7 @@ libxl_spice_info = Struct("spice_info", [
> >>      ("passwd",      string),
> >>      ("agent_mouse", libxl_defbool),
> >>      ("vdagent",     libxl_defbool),
> >>+    ("usbredirection", libxl_defbool),
> >>      ])
> >>  libxl_sdl_info = Struct("sdl_info", [
> >>diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> >>index 44a632c..5dab898 100644
> >>--- a/tools/libxl/xl_cmdimpl.c
> >>+++ b/tools/libxl/xl_cmdimpl.c
> >>@@ -1493,6 +1493,8 @@ skip_vfb:
> >>                              &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, "spiceusbredirection",
> >>+                            &b_info->u.hvm.spice.usbredirection, 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
> >>
> >>
> >>_______________________________________________
> >>Xen-devel mailing list
> >>Xen-devel@xxxxxxxxxxxxx
> >>http://lists.xen.org/xen-devel

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