From mboxrd@z Thu Jan 1 00:00:00 1970 From: Fabio Fantoni Subject: Re: [PATCH v4] libxl: spice usbredirection support for upstream qemu Date: Fri, 15 Nov 2013 11:32:53 +0100 Message-ID: <5285F855.3040301@m2r.biz> References: <1381502301-4047-1-git-send-email-fabio.fantoni@m2r.biz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Stefano Stabellini Cc: George.Dunlap@eu.citrix.com, anthony.perard@citrix.com, xen-devel@lists.xensource.com, Ian.Jackson@eu.citrix.com, Ian.Campbell@citrix.com List-Id: xen-devel@lists.xenproject.org Il 12/11/2013 13:41, Stefano Stabellini ha scritto: > On Fri, 11 Oct 2013, Fabio Fantoni wrote: >> Usage: spiceusbredirection=NUMBER (default=0) >> >> 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 will automatically adds >> an usb2 controller. >> >> Changes from v3: >> - fixed condition that enable usbversion if it isn't defined in presence >> of usbredirection enabled >> >> Changes from v2: >> - updated for usbversion patch v7 >> - now usbredirection cannot be used with usb and usbdevice parameters >> - if usbversion is undefined it will creates an usb2 controller >> >> Changes from v1: >> - Now can be setted the number of redirection channels. >> - Various code improvements. >> >> Signed-off-by: Fabio Fantoni > It looks correct from the QEMU arguments POV Thanks for reply, could someone review it and give approval for xen 4.4 if possible? > > >> docs/man/xl.cfg.pod.5 | 7 +++++++ >> tools/libxl/libxl.h | 11 +++++++++++ >> tools/libxl/libxl_create.c | 10 +++++++--- >> tools/libxl/libxl_dm.c | 12 ++++++++++++ >> tools/libxl/libxl_types.idl | 1 + >> tools/libxl/xl_cmdimpl.c | 2 ++ >> 6 files changed, 40 insertions(+), 3 deletions(-) >> >> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5 >> index 71359e8..800ef0b 100644 >> --- a/docs/man/xl.cfg.pod.5 >> +++ b/docs/man/xl.cfg.pod.5 >> @@ -1160,6 +1160,13 @@ requires vdagent service installed on domU o.s. to work. The default is 0. >> Enables Spice clipboard sharing (copy/paste). It requires spicevdagent >> enabled. The default is false (0). >> >> +=item B >> + >> +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 >> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h >> index 26c7c3b..fcea77c 100644 >> --- a/tools/libxl/libxl.h >> +++ b/tools/libxl/libxl.h >> @@ -371,6 +371,17 @@ >> #define LIBXL_HAVE_SPICE_VDAGENT 1 >> >> /* >> + * LIBXL_HAVE_SPICE_USBREDIRECTION >> + * >> + * If defined, then the libxl_spice_info structure will contain an integer type >> + * field: usbredirection. This value defines if Spice usbredirection is enabled >> + * and with how much channels. >> + * >> + * If this is not defined, the Spice usbredirection support is ignored. >> + */ >> +#define LIBXL_HAVE_SPICE_USBREDIREDIRECTION 1 >> + >> +/* >> * LIBXL_HAVE_DOMAIN_CREATE_RESTORE_PARAMS 1 >> * >> * If this is defined, libxl_domain_create_restore()'s API has changed to >> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c >> index 8ee31bd..8e1bfbd 100644 >> --- a/tools/libxl/libxl_create.c >> +++ b/tools/libxl/libxl_create.c >> @@ -262,12 +262,16 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc, >> libxl_defbool_setdefault(&b_info->u.hvm.usb, false); >> libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci, true); >> >> - if (b_info->u.hvm.usbversion && >> + if (!b_info->u.hvm.usbversion && >> + (b_info->u.hvm.spice.usbredirection > 0) ) >> + b_info->u.hvm.usbversion = 2; >> + >> + if ((b_info->u.hvm.usbversion || b_info->u.hvm.spice.usbredirection) && >> ( libxl_defbool_val(b_info->u.hvm.usb) >> || b_info->u.hvm.usbdevice_list >> || b_info->u.hvm.usbdevice) ){ >> - LOG(ERROR,"usbversion cannot be enabled with usb or" >> - "usbdevice parameters."); >> + LOG(ERROR,"usbversion and/or usbredirection cannot be " >> + "enabled with usb and/or usbdevice parameters."); >> return ERROR_INVAL; >> } >> >> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c >> index 2dd5dc1..4d00c19 100644 >> --- a/tools/libxl/libxl_dm.c >> +++ b/tools/libxl/libxl_dm.c >> @@ -559,6 +559,18 @@ static char ** libxl__build_device_model_args_new(libxl__gc *gc, >> "must be between 1 and 3", __func__); >> return NULL; >> } >> + if (b_info->u.hvm.spice.usbredirection >= 0 && >> + b_info->u.hvm.spice.usbredirection < 5) { >> + for (i = 1; i <= b_info->u.hvm.spice.usbredirection; i++) >> + flexarray_vappend(dm_args, "-chardev", libxl__sprintf(gc, >> + "spicevmc,name=usbredir,id=usbrc%d", i), "-device", >> + libxl__sprintf(gc, "usb-redir,chardev=usbrc%d," >> + "id=usbrc%d", i, i), NULL); >> + } else { >> + LOG(ERROR, "%s: usbredirection parameter is invalid, " >> + "it must be between 1 and 4", __func__); >> + return NULL; >> + } >> } >> if (b_info->u.hvm.soundhw) { >> flexarray_vappend(dm_args, "-soundhw", b_info->u.hvm.soundhw, NULL); >> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl >> index 5e0c32a..65eb33b 100644 >> --- a/tools/libxl/libxl_types.idl >> +++ b/tools/libxl/libxl_types.idl >> @@ -184,6 +184,7 @@ libxl_spice_info = Struct("spice_info", [ >> ("agent_mouse", libxl_defbool), >> ("vdagent", libxl_defbool), >> ("clipboard_sharing", libxl_defbool), >> + ("usbredirection", integer), >> ]) >> >> libxl_sdl_info = Struct("sdl_info", [ >> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c >> index a30bf1d..6dd5cd8 100644 >> --- a/tools/libxl/xl_cmdimpl.c >> +++ b/tools/libxl/xl_cmdimpl.c >> @@ -1533,6 +1533,8 @@ skip_vfb: >> &b_info->u.hvm.spice.vdagent, 0); >> xlu_cfg_get_defbool(config, "spice_clipboard_sharing", >> &b_info->u.hvm.spice.clipboard_sharing, 0); >> + if (!xlu_cfg_get_long (config, "spiceusbredirection", &l, 0)) >> + b_info->u.hvm.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); >> -- >> 1.7.9.5 >>