[PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[George Dunlap]

# HG changeset patch
# User George Dunlap <george.dunlap@eu.citrix.com>
# Date 1354101445 0
# Node ID 538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
# Parent  ae6fb202b233af815466055d9f1a635802a50855
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 b_info->hvm.usbdevice is non-NULL, then it will be used exclusively;
otherwise, libxl will check for b_info->hvm.usbdevice_list instead.
Each device listed will cause an extra "-usbdevice [foo]" to be appended
to the qemu command line.

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 present, the caller should use b_info->hvm.usbdevice_list; otherwise
they should use b_info->hvm.usbdevice.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -266,6 +266,23 @@
 #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, only
+ * hvm.usbdevice will be used.
+ *
+ * If this is not defined, callers should 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
--- a/tools/libxl/libxl_dm.c
+++ b/tools/libxl/libxl_dm.c
@@ -191,6 +191,15 @@ static char ** libxl__build_device_model
             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
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -326,6 +326,7 @@ libxl_domain_build_info = Struct("domain
                                        ("usbdevice",        string),
                                        ("soundhw",          string),
                                        ("xen_platform_pci", libxl_defbool),
+                                       ("usbdevice_list",   libxl_string_list),
                                        ])),
                  ("pv", Struct(None, [("kernel", string),
                                       ("slack_memkb", MemKB),

Re: [PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[Ian Campbell]

On Wed, 2012-11-28 at 12:16 +0000, George Dunlap wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@eu.citrix.com>
> # Date 1354101445 0
> # Node ID 538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
> # Parent  ae6fb202b233af815466055d9f1a635802a50855
> 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 b_info->hvm.usbdevice is non-NULL, then it will be used exclusively;
> otherwise, libxl will check for b_info->hvm.usbdevice_list instead.

Is this the right way round? If the caller knows about usbdevice_list
enough to have set it to something then surely that's the one it wanted?

> Each device listed will cause an extra "-usbdevice [foo]" to be appended
> to the qemu command line.
> 
> 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 present, the caller should use b_info->hvm.usbdevice_list; otherwise
> they should use b_info->hvm.usbdevice.

Actually it's a both stricter and looser than that, if
LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined then the caller can
choose to use usbdevice_list but can also use usbdevice if they wish
(but not both). If it is not present then they must not use
usbdevice_list at all.

I wonder if this LIBXL_HAVE should also be contained in a #ifdef
LIBXL_API_VERSION >= 0x040300?

> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

> 
> diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
> --- a/tools/libxl/libxl.h
> +++ b/tools/libxl/libxl.h
> @@ -266,6 +266,23 @@
>  #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, only
> + * hvm.usbdevice will be used.
> + *
> + * If this is not defined, callers should 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
> --- a/tools/libxl/libxl_dm.c
> +++ b/tools/libxl/libxl_dm.c
> @@ -191,6 +191,15 @@ static char ** libxl__build_device_model
>              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
> --- a/tools/libxl/libxl_types.idl
> +++ b/tools/libxl/libxl_types.idl
> @@ -326,6 +326,7 @@ libxl_domain_build_info = Struct("domain
>                                         ("usbdevice",        string),
>                                         ("soundhw",          string),
>                                         ("xen_platform_pci", libxl_defbool),
> +                                       ("usbdevice_list",   libxl_string_list),
>                                         ])),
>                   ("pv", Struct(None, [("kernel", string),
>                                        ("slack_memkb", MemKB),
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

Re: [PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[George Dunlap]

On 29/11/12 11:16, Ian Campbell wrote:
> On Wed, 2012-11-28 at 12:16 +0000, George Dunlap wrote:
>> # HG changeset patch
>> # User George Dunlap <george.dunlap@eu.citrix.com>
>> # Date 1354101445 0
>> # Node ID 538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
>> # Parent  ae6fb202b233af815466055d9f1a635802a50855
>> 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 b_info->hvm.usbdevice is non-NULL, then it will be used exclusively;
>> otherwise, libxl will check for b_info->hvm.usbdevice_list instead.
> Is this the right way round? If the caller knows about usbdevice_list
> enough to have set it to something then surely that's the one it wanted?

  Well, I had considered returning an error if both were set. :-)  I 
suppose actually that's probably the way that is likely to flag up bugs 
in the toolstack the best -- only doing one or the other will cause 
weird buggy behavior that will be hard to track down.  Does that sound 
good to you?

>> Each device listed will cause an extra "-usbdevice [foo]" to be appended
>> to the qemu command line.
>>
>> 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 present, the caller should use b_info->hvm.usbdevice_list; otherwise
>> they should use b_info->hvm.usbdevice.
> Actually it's a both stricter and looser than that, if
> LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined then the caller can
> choose to use usbdevice_list but can also use usbdevice if they wish
> (but not both). If it is not present then they must not use
> usbdevice_list at all.

Hmm, I think I wrote this first and then changed it and forgot to update 
it.  :-)  I also forgot that in this kind of "API spec" document, 
"should" and "must" are technical terms with very specific definitions.  
I'll revise it when I re-spin.

>
> I wonder if this LIBXL_HAVE should also be contained in a #ifdef
> LIBXL_API_VERSION >= 0x040300?

What would be the purpose of that?

  -George

Re: [PATCH 1 of 2] libxl: Allow multiple USB devices on HVM domain creation

[Ian Campbell]

On Thu, 2012-11-29 at 12:01 +0000, George Dunlap wrote:
> On 29/11/12 11:16, Ian Campbell wrote:
> > On Wed, 2012-11-28 at 12:16 +0000, George Dunlap wrote:
> >> # HG changeset patch
> >> # User George Dunlap <george.dunlap@eu.citrix.com>
> >> # Date 1354101445 0
> >> # Node ID 538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
> >> # Parent  ae6fb202b233af815466055d9f1a635802a50855
> >> 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 b_info->hvm.usbdevice is non-NULL, then it will be used exclusively;
> >> otherwise, libxl will check for b_info->hvm.usbdevice_list instead.
> > Is this the right way round? If the caller knows about usbdevice_list
> > enough to have set it to something then surely that's the one it wanted?
> 
>   Well, I had considered returning an error if both were set. :-)  I 
> suppose actually that's probably the way that is likely to flag up bugs 
> in the toolstack the best -- only doing one or the other will cause 
> weird buggy behavior that will be hard to track down.  Does that sound 
> good to you?
> 
> >> Each device listed will cause an extra "-usbdevice [foo]" to be appended
> >> to the qemu command line.
> >>
> >> 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 present, the caller should use b_info->hvm.usbdevice_list; otherwise
> >> they should use b_info->hvm.usbdevice.
> > Actually it's a both stricter and looser than that, if
> > LIBXL_HAVE_BUILDINFO_USBDEVICE_LIST is defined then the caller can
> > choose to use usbdevice_list but can also use usbdevice if they wish
> > (but not both). If it is not present then they must not use
> > usbdevice_list at all.
> 
> Hmm, I think I wrote this first and then changed it and forgot to update 
> it.  :-)  I also forgot that in this kind of "API spec" document, 
> "should" and "must" are technical terms with very specific definitions.  
> I'll revise it when I re-spin.

I don't think we actually use the RFC-whichever definitions of should
and must in general (and they would be SHOULD and MUST anyhow), it's
probably me being picky.

> > I wonder if this LIBXL_HAVE should also be contained in a #ifdef
> > LIBXL_API_VERSION >= 0x040300?
> 
> What would be the purpose of that?

LIBXL_API_VERSION and LIBXL_HAVE_* are two different mechanisms for
helping us to do API compatibility, I hadn't properly thought about the
interaction between them until now.

Adding the ifdef would expose the new feature only to those claiming
compatibility with the newer API.

More generally the purpose if LIBXL_API_VERSION generally is to allow us
to deprecate interfaces over time, which isn't quite the case here I
suppose.

Ian.

[PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[George Dunlap]

# HG changeset patch
# User George Dunlap <george.dunlap@eu.citrix.com>
# Date 1354104851 0
# Node ID e87d8fad28e00090591b65b2682dea9112f8830d
# Parent  538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
xl: Accept a list for usbdevice in config file

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.

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>

diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
--- a/docs/man/xl.cfg.pod.5
+++ b/docs/man/xl.cfg.pod.5
@@ -1103,15 +1103,22 @@ device.
 
 Enables or disables a 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.
 
 =back
 
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -1485,8 +1485,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);

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[Pasi Kärkkäinen]

On Wed, Nov 28, 2012 at 12:16:06PM +0000, George Dunlap wrote:
> # HG changeset patch
> # User George Dunlap <george.dunlap@eu.citrix.com>
> # Date 1354104851 0
> # Node ID e87d8fad28e00090591b65b2682dea9112f8830d
> # Parent  538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
> xl: Accept a list for usbdevice in config file
> 
> 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.
> 
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> 
> diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> --- a/docs/man/xl.cfg.pod.5
> +++ b/docs/man/xl.cfg.pod.5
> @@ -1103,15 +1103,22 @@ device.
>  
>  Enables or disables a USB bus in the guest.
>

Should we add "HVM" there ?

  
> -=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.
>  

Should we mention the difference between qemu-traditional and qemu-upstream here?

qemu-traditional only supports USB 1.1 devices, while qemu-upstream offers USB 2.0 (and 3.0) emulation.
Is there currently a way to control to which virtual usb controller the host device gets added to?

See here for help how to configure that with qemu/kvm:
http://www.linux-kvm.com/content/qemu-kvm-11-adds-experimental-support-usb-30

-- Pasi

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[George Dunlap]

[-- Attachment #1.1: Type: text/plain, Size: 3407 bytes --]

On Wed, Nov 28, 2012 at 3:11 PM, Pasi Kärkkäinen <pasik@iki.fi> wrote:

> On Wed, Nov 28, 2012 at 12:16:06PM +0000, George Dunlap wrote:
> > # HG changeset patch
> > # User George Dunlap <george.dunlap@eu.citrix.com>
> > # Date 1354104851 0
> > # Node ID e87d8fad28e00090591b65b2682dea9112f8830d
> > # Parent  538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
> > xl: Accept a list for usbdevice in config file
> >
> > 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.
> >
> > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> >
> > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
> > --- a/docs/man/xl.cfg.pod.5
> > +++ b/docs/man/xl.cfg.pod.5
> > @@ -1103,15 +1103,22 @@ device.
> >
> >  Enables or disables a USB bus in the guest.
> >
>
> Should we add "HVM" there ?
>

I could make this say, "...in an HVM guest".  (I'll wait to see if I get
other comments before re-sending the patch.)


>
>
> > -=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.
> >
>
> Should we mention the difference between qemu-traditional and
> qemu-upstream here?
>
> qemu-traditional only supports USB 1.1 devices, while qemu-upstream offers
> USB 2.0 (and 3.0) emulation.
> Is there currently a way to control to which virtual usb controller the
> host device gets added to?
>
> See here for help how to configure that with qemu/kvm:
>
> http://www.linux-kvm.com/content/qemu-kvm-11-adds-experimental-support-usb-30
>

I think this level of detail is probably too high for a man page -- this
kind of thing should be on a wiki somewhere I think.

In general though, I think xl's approach to USB stuff is really
simplistic.  It would be nice to have it be more fully-featured, but I'm
not sure that's on anyone's priority list.

 -George

[-- Attachment #1.2: Type: text/html, Size: 4464 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[Ian Campbell]

On Wed, 2012-11-28 at 16:52 +0000, George Dunlap wrote:
> On Wed, Nov 28, 2012 at 3:11 PM, Pasi Kärkkäinen <pasik@iki.fi> wrote:
>         On Wed, Nov 28, 2012 at 12:16:06PM +0000, George Dunlap wrote:
>         > # HG changeset patch
>         > # User George Dunlap <george.dunlap@eu.citrix.com>
>         > # Date 1354104851 0
>         > # Node ID e87d8fad28e00090591b65b2682dea9112f8830d
>         > # Parent  538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
>         > xl: Accept a list for usbdevice in config file
>         >
>         > 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.
>         >
>         > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>         >
>         > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>         > --- a/docs/man/xl.cfg.pod.5
>         > +++ b/docs/man/xl.cfg.pod.5
>         > @@ -1103,15 +1103,22 @@ device.
>         >
>         >  Enables or disables a USB bus in the guest.
>         >
>         
>         
>         Should we add "HVM" there ?
> 
> I could make this say, "...in an HVM guest".  (I'll wait to see if I
> get other comments before re-sending the patch.)

FWIW other similar options simply begin the paragraph with "(HVM only)"

But actually this particular option is already underneath a section
which begins "The following options apply only to HVM guests." and we
don't seem to reiterate that in the other options.

>         > -=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>

Are there other non-host* options?

> I think this level of detail is probably too high for a man page --
> this kind of thing should be on a wiki somewhere I think.

Its a bit of a cop-out but we do reference qemu(1) in some other places,
which might suffice here?

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[George Dunlap]

On 29/11/12 11:06, Ian Campbell wrote:
> On Wed, 2012-11-28 at 16:52 +0000, George Dunlap wrote:
>> On Wed, Nov 28, 2012 at 3:11 PM, Pasi Kärkkäinen <pasik@iki.fi> wrote:
>>          On Wed, Nov 28, 2012 at 12:16:06PM +0000, George Dunlap wrote:
>>          > # HG changeset patch
>>          > # User George Dunlap <george.dunlap@eu.citrix.com>
>>          > # Date 1354104851 0
>>          > # Node ID e87d8fad28e00090591b65b2682dea9112f8830d
>>          > # Parent  538d9ffbd71b41e8cf6d7da0ded9e0a0b07f3c0d
>>          > xl: Accept a list for usbdevice in config file
>>          >
>>          > 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.
>>          >
>>          > Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
>>          >
>>          > diff --git a/docs/man/xl.cfg.pod.5 b/docs/man/xl.cfg.pod.5
>>          > --- a/docs/man/xl.cfg.pod.5
>>          > +++ b/docs/man/xl.cfg.pod.5
>>          > @@ -1103,15 +1103,22 @@ device.
>>          >
>>          >  Enables or disables a USB bus in the guest.
>>          >
>>          
>>          
>>          Should we add "HVM" there ?
>>
>> I could make this say, "...in an HVM guest".  (I'll wait to see if I
>> get other comments before re-sending the patch.)
> FWIW other similar options simply begin the paragraph with "(HVM only)"
>
> But actually this particular option is already underneath a section
> which begins "The following options apply only to HVM guests." and we
> don't seem to reiterate that in the other options.

Ah, right -- now I remember, I thought about it, saw that it was in an 
"HVM only" section, and decided that was sufficient. :-)

>
>>          > -=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>
> Are there other non-host* options?

qemu.git/vl.c suggests other options might include, "disk:", "serial:", 
"net:" and "bt:"; other non-colon-including options include "keyboard", 
"mouse", "wacom-tablet", and "braile".

On the other hand, a number of these seem to require supplementary 
options to work well; e.g., network and bt (bluetooth) require one 
argument to hook up a USB device, and another option to say how it's 
connected to what.

It's also interesting to note that the KVM documentation doesnt' mention 
using "-usb" and "-usbdevice" at all -- they seem to prefer using 
"-device" to specify hubs &c.

But still, it might be useful to say that these are passed via 
"usbdevice" to the qemu command line, and so you can find more 
information on the qemu man page.

  -George

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[George Dunlap]

[-- Attachment #1.1: Type: text/plain, Size: 1241 bytes --]

On Thu, Nov 29, 2012 at 11:44 AM, George Dunlap <george.dunlap@eu.citrix.com
> wrote:

> qemu.git/vl.c suggests other options might include, "disk:", "serial:",
> "net:" and "bt:"; other non-colon-including options include "keyboard",
> "mouse", "wacom-tablet", and "braile".
>
> On the other hand, a number of these seem to require supplementary options
> to work well; e.g., network and bt (bluetooth) require one argument to hook
> up a USB device, and another option to say how it's connected to what.
>
> It's also interesting to note that the KVM documentation doesnt' mention
> using "-usb" and "-usbdevice" at all -- they seem to prefer using "-device"
> to specify hubs &c.
>

Speaking of which, the KVM docs refer to "-usb" and "-usbdevice" as "legacy
interfaces", which will only get you piix3.  It seems like exposing the
full capabilities of qemu would mean either 1) coming up with a full
specifciation which we can then translate into qemu directives, as libvirt
seems to do, or 2) just recommend people construct their own qemu
command-line options to pass through.

It's not 100% clear to me that 1 is actually less complicated from a user
perspective in the long run than 2, and it's certainly a lot more work...

 -George

[-- Attachment #1.2: Type: text/html, Size: 1719 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[Ian Campbell]

On Thu, 2012-11-29 at 12:20 +0000, George Dunlap wrote:
> On Thu, Nov 29, 2012 at 11:44 AM, George Dunlap
> <george.dunlap@eu.citrix.com> wrote:
>         qemu.git/vl.c suggests other options might include, "disk:",
>         "serial:", "net:" and "bt:"; other non-colon-including options
>         include "keyboard", "mouse", "wacom-tablet", and "braile".
>         
>         On the other hand, a number of these seem to require
>         supplementary options to work well; e.g., network and bt
>         (bluetooth) require one argument to hook up a USB device, and
>         another option to say how it's connected to what.
>         
>         It's also interesting to note that the KVM documentation
>         doesnt' mention using "-usb" and "-usbdevice" at all -- they
>         seem to prefer using "-device" to specify hubs &c.
> 
> Speaking of which, the KVM docs refer to "-usb" and "-usbdevice" as
> "legacy interfaces", which will only get you piix3.

This is a concern for other types of device too. We seem to invoke new
qemu with a mixture of legacy and new-style interfaces.

>   It seems like exposing the full capabilities of qemu would mean
> either 1) coming up with a full specifciation which we can then
> translate into qemu directives, as libvirt seems to do, or 2) just
> recommend people construct their own qemu command-line options to pass
> through.

Does libvirt let you care about topologies or just it just automatically
create a new USB controller for every N devices you add and hook things
up in some order?

> It's not 100% clear to me that 1 is actually less complicated from a
> user perspective in the long run than 2, and it's certainly a lot more
> work...
> 
>  -George
> 

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[George Dunlap]

On 29/11/12 12:25, Ian Campbell wrote:
> On Thu, 2012-11-29 at 12:20 +0000, George Dunlap wrote:
>> On Thu, Nov 29, 2012 at 11:44 AM, George Dunlap
>> <george.dunlap@eu.citrix.com> wrote:
>>          qemu.git/vl.c suggests other options might include, "disk:",
>>          "serial:", "net:" and "bt:"; other non-colon-including options
>>          include "keyboard", "mouse", "wacom-tablet", and "braile".
>>          
>>          On the other hand, a number of these seem to require
>>          supplementary options to work well; e.g., network and bt
>>          (bluetooth) require one argument to hook up a USB device, and
>>          another option to say how it's connected to what.
>>          
>>          It's also interesting to note that the KVM documentation
>>          doesnt' mention using "-usb" and "-usbdevice" at all -- they
>>          seem to prefer using "-device" to specify hubs &c.
>>
>> Speaking of which, the KVM docs refer to "-usb" and "-usbdevice" as
>> "legacy interfaces", which will only get you piix3.
> This is a concern for other types of device too. We seem to invoke new
> qemu with a mixture of legacy and new-style interfaces.
>
>>    It seems like exposing the full capabilities of qemu would mean
>> either 1) coming up with a full specifciation which we can then
>> translate into qemu directives, as libvirt seems to do, or 2) just
>> recommend people construct their own qemu command-line options to pass
>> through.
> Does libvirt let you care about topologies or just it just automatically
> create a new USB controller for every N devices you add and hook things
> up in some order?

No idea -- I took a quick glance through the libvirt docs and I didn't 
see anything particular about a topology.  I don't have a system set up 
with libvirt, so I can't try it and see what the resulting qemu command 
line looks like :-)

http://libvirt.org/formatdomain.html#elementsUSB

  -George

Re: [PATCH 2 of 2] xl: Accept a list for usbdevice in config file

[Pasi Kärkkäinen]

On Thu, Nov 29, 2012 at 12:25:56PM +0000, Ian Campbell wrote:
> On Thu, 2012-11-29 at 12:20 +0000, George Dunlap wrote:
> > On Thu, Nov 29, 2012 at 11:44 AM, George Dunlap
> > <george.dunlap@eu.citrix.com> wrote:
> >         qemu.git/vl.c suggests other options might include, "disk:",
> >         "serial:", "net:" and "bt:"; other non-colon-including options
> >         include "keyboard", "mouse", "wacom-tablet", and "braile".
> >         
> >         On the other hand, a number of these seem to require
> >         supplementary options to work well; e.g., network and bt
> >         (bluetooth) require one argument to hook up a USB device, and
> >         another option to say how it's connected to what.
> >         
> >         It's also interesting to note that the KVM documentation
> >         doesnt' mention using "-usb" and "-usbdevice" at all -- they
> >         seem to prefer using "-device" to specify hubs &c.
> > 
> > Speaking of which, the KVM docs refer to "-usb" and "-usbdevice" as
> > "legacy interfaces", which will only get you piix3.
> 
> This is a concern for other types of device too. We seem to invoke new
> qemu with a mixture of legacy and new-style interfaces.
> 
> >   It seems like exposing the full capabilities of qemu would mean
> > either 1) coming up with a full specifciation which we can then
> > translate into qemu directives, as libvirt seems to do, or 2) just
> > recommend people construct their own qemu command-line options to pass
> > through.
> 
> Does libvirt let you care about topologies or just it just automatically
> create a new USB controller for every N devices you add and hook things
> up in some order?
> 

Not sure about libvirt.. quick googling suggests that libvirt doesn't allow specifying the usb controller.

with qemu-kvm you can assign host usb devices to specific usb controllers
using the "bus=" option:
http://www.linux-kvm.com/content/qemu-kvm-11-adds-experimental-support-usb-30

qemu-kvm -device nec-usb-xhci,id=xhci -device usb-storage,bus=xhci.0,drive=usbstick


-- Pasi