[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] libxl: usb2 and usb3 controller support for upstream qemu



On Mon, Jul 08, 2013 at 11:13:00AM +0200, Fabio Fantoni wrote:
> Usage: usb=0|1|2|3 (default=0)
> Enables and specifies the type of an emulated USB bus in the guest.
> 0 for none, 1 for usb1, 2 for usb2 and 3 for usb3. Usb2 and usb3
> only for upstream qemu.
> 
> Signed-off-by: Fabio Fantoni <fabio.fantoni@xxxxxxx>
> ---
>  docs/man/xl.cfg.pod.5       |    6 ++++--
>  tools/libxl/libxl_create.c  |    1 -
>  tools/libxl/libxl_dm.c      |   36 ++++++++++++++++++++++++++++++++----
>  tools/libxl/libxl_types.idl |    2 +-
>  tools/libxl/xl_cmdimpl.c    |    3 ++-
>  tools/libxl/xl_sxp.c        |    2 +-
>  6 files changed, 40 insertions(+), 10 deletions(-)
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> index 48c2669..b1f190a 100644
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1158,9 +1158,11 @@ devices are defined by the device model configuration, 
> please see the
>  L<qemu(1)> manpage for details. The default is not to export any sound
>  device.
>  
> -=item B<usb=BOOLEAN>
> +=item B<usb=NUMBER>
>  

Are you breaking working things with a change like this? Consider that
some other toolstack that might be using this parameter.

(See the SXP output comment below...)

> -Enables or disables an emulated USB bus in the guest.
> +Enables and specifies the type of an emulated USB bus in the guest.
> +0 for none, 1 for usb1, 2 for usb2 and 3 for usb3. Usb2 and usb3 only
> +for upstream qemu. Default is 0.
>  
>  =item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
>  
> diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
> index cb9c822..292bc27 100644
> --- a/tools/libxl/libxl_create.c
> +++ b/tools/libxl/libxl_create.c
> @@ -259,7 +259,6 @@ int libxl__domain_build_info_setdefault(libxl__gc *gc,
>          libxl_defbool_setdefault(&b_info->u.hvm.hpet,               true);
>          libxl_defbool_setdefault(&b_info->u.hvm.vpt_align,          true);
>          libxl_defbool_setdefault(&b_info->u.hvm.nested_hvm,         false);
> -        libxl_defbool_setdefault(&b_info->u.hvm.usb,                false);

Should probably set the new parameter to default value explicitly. I
know that b_info is memset to zero by at the beginning, but it should be
better to be explicit.

>          libxl_defbool_setdefault(&b_info->u.hvm.xen_platform_pci,   true);
>  
>          if (!b_info->u.hvm.boot) {
> diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
> index a53eceb..561a579 100644
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -198,7 +198,7 @@ static char ** 
> libxl__build_device_model_args_old(libxl__gc *gc,
>          if (b_info->u.hvm.boot) {
>              flexarray_vappend(dm_args, "-boot", b_info->u.hvm.boot, NULL);
>          }
> -        if (libxl_defbool_val(b_info->u.hvm.usb)
> +        if ( b_info->u.hvm.usb != 0
>              || b_info->u.hvm.usbdevice
>              || b_info->u.hvm.usbdevice_list) {
>              if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
> @@ -207,7 +207,14 @@ static char ** 
> libxl__build_device_model_args_old(libxl__gc *gc,
>                      __func__);
>                  return NULL;
>              }
> -            flexarray_append(dm_args, "-usb");
> +            switch (b_info->u.hvm.usb) {
> +            case 1:
> +                flexarray_append(dm_args, "-usb");
> +                break;
> +            default:
> +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                    "usb parameter is invalid must be between 0 and 1");
> +            }
>              if (b_info->u.hvm.usbdevice) {
>                  flexarray_vappend(dm_args,
>                                    "-usbdevice", b_info->u.hvm.usbdevice, 
> NULL);
> @@ -494,7 +501,7 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>              flexarray_vappend(dm_args, "-boot",
>                      libxl__sprintf(gc, "order=%s", b_info->u.hvm.boot), 
> NULL);
>          }
> -        if (libxl_defbool_val(b_info->u.hvm.usb)
> +        if ( b_info->u.hvm.usb != 0
>              || b_info->u.hvm.usbdevice
>              || b_info->u.hvm.usbdevice_list) {
>              if ( b_info->u.hvm.usbdevice && b_info->u.hvm.usbdevice_list )
> @@ -503,7 +510,28 @@ static char ** 
> libxl__build_device_model_args_new(libxl__gc *gc,
>                      __func__);
>                  return NULL;
>              }
> -            flexarray_append(dm_args, "-usb");
> +            switch (b_info->u.hvm.usb) {
> +            case 1:
> +                flexarray_vappend(dm_args,
> +                    "-device", "piix3-usb-uhci,id=usb", NULL);
> +                break;
> +            case 2:
> +                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", NULL);
> +                break;
> +            case 3:
> +                flexarray_vappend(dm_args,
> +                    "-device", "nec-usb-xhci,id=usb", NULL);
> +                break;
> +            default:
> +                LIBXL__LOG(CTX, LIBXL__LOG_ERROR,
> +                    "usb parameter is invalid must be between 0 and 3");
> +            }
>              if (b_info->u.hvm.usbdevice) {
>                  flexarray_vappend(dm_args,
>                                    "-usbdevice", b_info->u.hvm.usbdevice, 
> NULL);
> diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
> index 8262cba..b87e7bf 100644
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -325,7 +325,7 @@ libxl_domain_build_info = Struct("domain_build_info",[
>                                         
>                                         ("serial",           string),
>                                         ("boot",             string),
> -                                       ("usb",              libxl_defbool),
> +                                       ("usb",              integer),
>                                         # usbdevice:
>                                         # - "tablet" for absolute mouse,
>                                         # - "mouse" for PS/2 protocol 
> relative mouse
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c5292f4..253de71 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -1496,7 +1496,8 @@ skip_vfb:
>                              &b_info->u.hvm.gfx_passthru, 0);
>          xlu_cfg_replace_string (config, "serial", &b_info->u.hvm.serial, 0);
>          xlu_cfg_replace_string (config, "boot", &b_info->u.hvm.boot, 0);
> -        xlu_cfg_get_defbool(config, "usb", &b_info->u.hvm.usb, 0);
> +        if (!xlu_cfg_get_long (config, "usb", &l, 0))
> +            b_info->u.hvm.usb = l;
>          switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
>                                                  
> &b_info->u.hvm.usbdevice_list,
>                                                  1))
> diff --git a/tools/libxl/xl_sxp.c b/tools/libxl/xl_sxp.c
> index a16a025..e08f484 100644
> --- a/tools/libxl/xl_sxp.c
> +++ b/tools/libxl/xl_sxp.c
> @@ -141,7 +141,7 @@ void printf_info_sexp(int domid, libxl_domain_config 
> *d_config)
>                 libxl_defbool_to_string(b_info->u.hvm.gfx_passthru));
>          printf("\t\t\t(serial %s)\n", b_info->u.hvm.serial);
>          printf("\t\t\t(boot %s)\n", b_info->u.hvm.boot);
> -        printf("\t\t\t(usb %s)\n", 
> libxl_defbool_to_string(b_info->u.hvm.usb));
> +        printf("\t\t\t(usb %d)\n", b_info->u.hvm.usb);

If I'm not mistaken SXP is for compatibility use now. So it implies that
you might break other toolstack that relies on this parameter.

>          printf("\t\t\t(usbdevice %s)\n", b_info->u.hvm.usbdevice);
>          printf("\t\t)\n");
>          break;
> -- 
> 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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.