Xen devel - Apr 2013 - [PATCH RESEND 1/2] libxl: Allow multiple USB devices on HVM domain creation

This patch allows an HVM domain to be created with multiple USB
devices.

Since the previous interface only allowed the passing of a single
device, this requires us to add a new element to the hvm struct of
libxl_domain_build_info -- usbdevice_list.  For API compatibility, the
old element, usbdevice, remains.

If hvm.usbdevice_list is set, each device listed will cause an extra
"-usbdevice [foo]" to be appended to the qemu command line.

Callers may set either hvm.usbdevice or hvm.usbdevice_list, but not
both; libxl will throw an error if both are set.

In order to allow users of libxl to write software compatible with
older versions of libxl, also define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST.
If this is defined, callers may use either hvm.usbdevice or
hvm.usbdevice_list; otherwise, only hvm.usbdevice will be available.

v3:
 - Duplicate functionality in both "new" and "old", since
we''re not
   unifying the two anymore.
v2:
 - Throw an error if both usbdevice and usbdevice_list are set
 - Update and clarify definition based on feedback
 - Previous patches means this works for both traditional and upstream

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/libxl/libxl.h         |   16 ++++++++++++++++
 tools/libxl/libxl_dm.c      |   38 ++++++++++++++++++++++++++++++++++++--
 tools/libxl/libxl_types.idl |    1 +
 3 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 030aa86..00a68b8 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -273,6 +273,22 @@
 #endif
 #endif
 
+/* 
+ * LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST
+ * 
+ * If this is defined, then the libxl_domain_build_info structure will
+ * contain hvm.usbdevice_list, a libxl_string_list type that contains
+ * a list of USB devices to specify on the qemu command-line.
+ *
+ * If it is set, callers may use either hvm.usbdevice or
+ * hvm.usbdevice_list, but not both; if both are set, libxl will
+ * throw an error.
+ *
+ * If this is not defined, callers can only use hvm.usbdevice.  Note
+ * that this means only one device can be added at domain build time.
+ */
+#define LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST 1
+
 /* Functions annotated with LIBXL_EXTERNAL_CALLERS_ONLY may not be
  * called from within libxl itself. Callers outside libxl, who
  * do not #include libxl_internal.h, are fine. */
diff --git a/tools/libxl/libxl_dm.c b/tools/libxl/libxl_dm.c
index 82e30df..d10a58f 100644
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -198,11 +198,28 @@ 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) ||
b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb)
+            || b_info->u.hvm.usbdevice
+            || b_info->u.hvm.usbdevice_list) {
+            if ( b_info->u.hvm.usbdevice &&
b_info->u.hvm.usbdevice_list )
+            {
+                LOG(ERROR, "%s: Both usbdevice and usbdevice_list
set",
+                    __func__);
+                return NULL;
+            }
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
                                   "-usbdevice",
b_info->u.hvm.usbdevice, NULL);
+            } else if (b_info->u.hvm.usbdevice_list) {
+                char **p;
+                for (p = b_info->u.hvm.usbdevice_list;
+                     *p;
+                     p++) {
+                    flexarray_vappend(dm_args,
+                                      "-usbdevice",
+                                      *p, NULL);
+                }
             }
         }
         if (b_info->u.hvm.soundhw) {
@@ -479,11 +496,28 @@ 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) ||
b_info->u.hvm.usbdevice) {
+        if (libxl_defbool_val(b_info->u.hvm.usb)
+            || b_info->u.hvm.usbdevice
+            || b_info->u.hvm.usbdevice_list) {
+            if ( b_info->u.hvm.usbdevice &&
b_info->u.hvm.usbdevice_list )
+            {
+                LOG(ERROR, "%s: Both usbdevice and usbdevice_list
set",
+                    __func__);
+                return NULL;
+            }
             flexarray_append(dm_args, "-usb");
             if (b_info->u.hvm.usbdevice) {
                 flexarray_vappend(dm_args,
                                   "-usbdevice",
b_info->u.hvm.usbdevice, NULL);
+            } else if (b_info->u.hvm.usbdevice_list) {
+                char **p;
+                for (p = b_info->u.hvm.usbdevice_list;
+                     *p;
+                     p++) {
+                    flexarray_vappend(dm_args,
+                                      "-usbdevice",
+                                      *p, NULL);
+                }
             }
         }
         if (b_info->u.hvm.soundhw) {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f3c212b..6cb6de6 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -330,6 +330,7 @@ libxl_domain_build_info =
Struct("domain_build_info",[
                                        ("usbdevice",        string),
                                        ("soundhw",          string),
                                        ("xen_platform_pci",
libxl_defbool),
+                                       ("usbdevice_list",  
libxl_string_list),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),
-- 
1.7.9.5

Allow the "usbdevice" key to accept a list of USB devices, and pass
them in using the new usbdevice_list domain build element.

For backwards compatibility, still accept singleton values.

Also update the xl.cfg manpage, adding information about how to pass
through host devices.

v2:
 - Add some verbiage to make it clear that "usb" is for emulated
devices
 - Reference qemu manual for more usbdevice options

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 docs/man/xl.cfg.pod.5    |   28 +++++++++++++++++++---------
 tools/libxl/xl_cmdimpl.c |   19 +++++++++++++++++--
 2 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
index 8db24d7..bc2bb11 100644
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1145,17 +1145,27 @@ device.
 
 =item B<usb=BOOLEAN>
 
-Enables or disables a USB bus in the guest.
+Enables or disables an emulated USB bus in the guest.
 
-=item B<usbdevice=DEVICE>
+=item B<usbdevice=[ "DEVICE", "DEVICE", ...]>
 
-Adds B<DEVICE> to the USB bus. The USB bus must also be enabled using
-B<usb=1>. The most common use for this option is
B<usbdevice=tablet>
-which adds pointer device using absolute coordinates. Such devices
-function better than relative coordinate devices (such as a standard
-mouse) since many methods of exporting guest graphics (such as VNC)
-work better in this mode. Note that this is independent of the actual
-pointer device you are using on the host/client side.
+Adds B<DEVICE>s to the emulated USB bus. The USB bus must also be
+enabled using B<usb=1>. The most common use for this option is
+B<usbdevice=[''tablet'']> which adds pointer device using
absolute
+coordinates. Such devices function better than relative coordinate
+devices (such as a standard mouse) since many methods of exporting
+guest graphics (such as VNC) work better in this mode. Note that this
+is independent of the actual pointer device you are using on the
+host/client side.
+
+Host devices can also be passed through in this way, by specifying
+host:USBID, where USBID is of the form xxxx:yyyy.  The USBID can 
+typically be found by using lsusb or usb-devices.
+
+The form usbdevice=DEVICE is also accepted for backwards compatibility.
+
+More valid options can be found in the "usbdevice" section of the
qemu
+documentation.
 
 =back
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 2d40f8f..afbab85 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1533,8 +1533,23 @@ skip_vfb:
         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);
-        xlu_cfg_replace_string (config, "usbdevice",
-                                &b_info->u.hvm.usbdevice, 0);
+        switch (xlu_cfg_get_list_as_string_list(config, "usbdevice",
+                                               
&b_info->u.hvm.usbdevice_list,
+                                                1))
+        {
+
+        case 0: break; /* Success */
+        case ESRCH: break; /* Option not present */
+        case EINVAL:
+            /* If it''s not a valid list, try reading it as an atom,
falling through to
+             * an error if it fails */
+            if (!xlu_cfg_replace_string(config, "usbdevice",
&b_info->u.hvm.usbdevice, 0))
+                break;
+            /* FALLTHRU */
+        default:
+            fprintf(stderr,"xl: Unable to parse usbdevice.\n");
+            exit(-ERROR_FAIL);
+        }
         xlu_cfg_replace_string (config, "soundhw",
&b_info->u.hvm.soundhw, 0);
         xlu_cfg_get_defbool(config, "xen_platform_pci",
                             &b_info->u.hvm.xen_platform_pci, 0);
-- 
1.7.9.5

George Dunlap writes ("[Xen-devel] [PATCH RESEND 1/2] libxl: Allow multiple
USB devices on HVM domain creation"):
> This patch allows an HVM domain to be created with multiple USB
> devices.
Thanks.  I have applied both of these.  I fixed up some trivial
formatting problems:
 - some added trailing whitespace
 - two overly long lines in xl_cmdimpl.c

Thanks,
Ian.