Re: [PATCH v4] libxl: spice usbredirection support for upstream qemu

[Fabio Fantoni <fabio.fantoni@m2r.biz>]

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 <fabio.fantoni@m2r.biz>
> 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<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
>> 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
>>