qemu-devel
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: usb2 and usb3 controller sup

From: Fabio Fantoni
Subject: Re: [Qemu-devel] [Xen-devel] [PATCH] libxl: usb2 and usb3 controller support for upstream qemu
Date: Mon, 08 Jul 2013 12:01:47 +0200
User-agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:17.0) Gecko/20130620 Thunderbird/17.0.7 
Il 08/07/2013 11:44, Wei Liu ha scritto:

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 <address@hidden>
---
  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.


Sorry, you are right.
Can be good the second option of my mail of last week?

- keeping usb=0|1
- adding usb_version=1|2|3 parameter, 2 as default can be good?




(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
address@hidden
http://lists.xen.org/xen-devel
reply via email to
[Prev in Thread] Current Thread [Next in Thread]