Re: [Xen-devel] [PATCH 1/2] libxl: Introduce functions to add and remove host USB devices to an HVM guest

[George Dunlap <George.Dunlap@xxxxxxxxxxxxx>]

On Tue, Mar 19, 2013 at 5:55 PM, Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>> I had thought about something like that, but the basic problem is that
>> HVM guests can use PVUSB as well. I don't think libxl is the right place
>> to be deciding whether to use qemu or PVUSB if both are available.
>
> I guess using both (PVUSB + Qemu) for HVM, and let the guest decide
> won't work, right?

How is the guest in a position to decide?  One requires libxl to talk
to qmp, the other requires libxl to do a bunch of xenstore writes.
The second requires not only a funcitonal PVUSB front-end in the
guest, but also a functional PVUSB back-end; and apparently the PVUSB
back-end is decent in the SuSE "classic Xen" kernels, but fairly well
broken in the pvops kernels at the moment.  The guest is in no
position to know whether the PVUSB back-end is reliable or not, and
thus no position to decide for itself.

> Since libxl doesn't support PVUSB right now you could always name this
> libxl_device_usb_add and sort the problem out once we have PVUSB
> support.

I have to completely disagree with this.  The current xm commands
evolved this way, and the result is that the naming is incredibly
confusing: there are two sets of commands because there have to be
(see below), but it's really difficult to tell which one goes with
which.  Given that we are definitely planning on doing qemu USB and PV
USB, we should take the opportunity to plan ahead what kind of
interface we're shooting for.  If we have to have a different set of
interfaces for each one, they need to be clear and consistent.

> IMHO I would really prefer to only have one public function to
> add USB devices if possible.

That was my goal initially as well.  However, I just don't think it's
practical.  The underlying interfaces to which the commands must map
are too different -- there are extra steps which need to be done for
PVUSB that don't need to be done for qemu-xen and vice versa, and
functionality that is available with one but not the other.  Trying to
shove them into one interface means either 1) having some of the
advertised interface functionality not work for one or the other, or
2) making the interface support only what is supported in both.  #1 is
bad because it makes the interface much more complicated and difficult
for the programmer to use; #2 is bad because we don't make full use of
the functionality available.

There are three sets of interfaces we need to consider:

* qemu-traditional
- Communication method: qemu-monitor
- Available commands:
 usb-add: Accepts "legacy" USB device
 usb-del: Accepts same arguments as above
 usb-list: List existing usb devices
- Unique aspects:
  Can add emulated tablets, mice, keyboard, &c, as well as host USB.
  usb-del accepts same arguments as usb-add, so no need for 'id'
  usb-list available
  Explicit wildcards needed
  Only a single command needed to add
  No way to make USB busses; topology handled automatically

* qemu-xen
 - Communication method: qmp
 - Available commands:
 device-add: Accepts "qdev"-style devices, which are more generic.
Also accepts 'id'
 device-remove: Only accepts 'id'
- Unique aspects:
  Implicit wildcards -- any factor not specified matches anything
  No way to list USB devices
  Must specify an id on creation in order to remove a device
  Only a single command needed to add
  Adding emulated tablets, mice, keyboard &c has a very different
syntax to host-usb, not easy to make the same interface work for both
  No way to make USB busses; topology handled automatically

* PVUSB
 - Communication method: xenbus
 - Available commands:
  usb-hc-create/destroy
  usb-attach/detach
  usb-assignable-devices
- Unique aspects:
  I haven't looked at this in detail, but this is what it looks like:
  You can (must?) create different virtual busses
  You must make a USB device "assignable" before attaching it (?)
  You must specify the USB bus to attach it to
  Devices are removed by vbus.vport rather than by id, or by vendorid:productid

I'm focusing on getting qemu-xen host-usb support working at the
moment.  It's possible that the qemu-xen commands could be mostly
mapped on to qemu-traditional commands; but there's just too much
mismatch between PVUSB and qemu-xen to make it worthwhile, I think.


>> I suppose if we allow the user to pass *dev though, then usb_del could
>> either use dev->id (if it exists), or re-construct the default id and
>> try to remove it if not.
>
> You could pass the same id that you are passing as an argument inside
> libxl_device_usb id field.

Passing a whole structure in when only one element is going to be read
-- requiring the caller to declare the structure, initialize it, set
the element, then tear it down afterwards -- just for a char *?  That
seems incredibly silly to me.

But having the option of just re-specifying the device the same way
you did the first time, rather than having to keep the arbitrary
string around somewhere, does make sense.

 -George

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