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

Re: [libvirt] [v2 13/13] Add usb-redir device



On Fri, Aug 26, 2011 at 01:44:29AM +0300, Marc-André Lureau wrote:
> diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in
> index e35b76b..0ab08de 100644
> --- a/docs/formatdomain.html.in
> +++ b/docs/formatdomain.html.in
> @@ -1314,6 +1314,12 @@
>        0.4.4 for USB and 0.6.0 for PCI (KVM only)</span>:
>      </p>
>  
> +    <p>
> +      Device redirection through a character device is
> +      supported <span class="since">since after 0.9.5 for USB (KVM
> +      only)</span>:
> +    </p>
> +
>  <pre>
>    ...
>    &lt;devices&gt;
> @@ -1348,14 +1354,19 @@
>          "subsystem" and <code>type</code> is "usb" for a USB device and "pci"
>          for a PCI device. When <code>managed</code> is "yes" for a PCI
>          device, it is detached from the host before being passed on to
> -        the guest.</dd>
> +        the guest. Redirection through a character device is enable by
> +        specifying the <code>redirection</code> character device
> +        type.</dd>
>        <dt><code>source</code></dt>
>        <dd>The source element describes the device as seen from the host.
>        The USB device can either be addressed by vendor / product id using the
>        <code>vendor</code> and <code>product</code> elements or by the device's
>        address on the hosts using the <code>address</code> element.
>        PCI devices on the other hand can only be described by their
> -      <code>address</code></dd>
> +      <code>address</code>
> +      In case of device redirection, the source element describes the
> +      character device to redirect from.
> +      </dd>
>        <dt><code>vendor</code>, <code>product</code></dt>
>        <dd>The <code>vendor</code> and <code>product</code> elements each have an
>        <code>id</code> attribute that specifies the USB vendor and product id.


> diff --git a/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml
> new file mode 100644
> index 0000000..bb50b81
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/qemuxml2argv-usb-redir.xml
> @@ -0,0 +1,33 @@
> +<domain type='qemu'>
> +  <name>QEMUGuest1</name>
> +  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>

[snip]

> +    <hostdev mode='subsystem' type='usb' redirection='tcp'>
> +      <source mode='connect' host='localhost' service='4000'/>
> +    </hostdev>
> +    <memballoon model='virtio'/>
> +  </devices>
> +</domain>

I'm not convinced that this is the right way to be modelling access
to remote USB devices.

The <hostdev> element is used to refer to devices that are local
to the virtualization host, being assigned. Existing applications
using libvirt, expect the child elements in <hostdev> to vary
based on the mode+type attributes. This change is making the
child elements vary based on mode+type+redirection. This means
that any existing application using libvirt is going to get
confused when it sees one of these redirected hostdevs, because
it will not be expecting this new element contents.

As seen earlier in the patch, we've also had to special case
all the internal libvirt code to look for the new 'redirection'
attribute as well.


I think we need to take one of a few possible alternative
approaches

 1. Introduce a new value for the 'type' parameter, eg

      <hostdev mode='subsystem' type='usbredir'>
        ...

 2. Introduce an entirely new device type

     <redirdev type='usb'>
          ...
     </redirdev>


  3. Same as 1, but s/redir/remote/

  4. Same as 2, but s/redir/remote/


I have a slight preference for 2/4 at this point, because I
feel that the handling/processing of redirected USB devices
has essentially zero code overlap with handling of host device
assignment, both for libvirt internally and for client apps.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|


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