Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Stefan]

I've been following this thread with interest. Having USB passthrough 
for xl will certainly be an appreciated feature.

Provided a USB-device structure, enabling PVUSB is almost only a matter 
of writing to the xenstore. Please have a look at this script:
http://www.neobiker.de/wiki/index.php?title=XEN-PVUSB

It's a shell script that does the work. Of course, there's also a python 
implementation in the GIT. Perhaps we could at least have a USB-device 
structure before the feature freeze? Can you make the code that we have 
available? I'd like to see if I can do something with it, instead of 
having to start from nothing.

Stefan S

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap]

On 10/04/13 14:49, Stefan wrote:
> I've been following this thread with interest. Having USB passthrough
> for xl will certainly be an appreciated feature.
>
> Provided a USB-device structure, enabling PVUSB is almost only a matter
> of writing to the xenstore. Please have a look at this script:
> http://www.neobiker.de/wiki/index.php?title=XEN-PVUSB
>
> It's a shell script that does the work. Of course, there's also a python
> implementation in the GIT. Perhaps we could at least have a USB-device
> structure before the feature freeze? Can you make the code that we have
> available? I'd like to see if I can do something with it, instead of
> having to start from nothing.

I'm working hard to try to get "devicemodel" USB hot-plug working, and 
that involves getting a forward-compatible structure and everything 
ready to add PV as an optional protocol.

If things go as planned, then adding PVUSB support should just be a 
matter of:
* Adding a handler for PVUSB in libxl_usb.c:do_usb_add()
* Adding a command-line option to specify PVUSB even for HVM guests

I should have a rough draft ready for you to try to hack within 24 hours.

  -George

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[jacek burghardt]

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

Opensuse kernel source has patches for kernel that include pvusb pvscsi

[-- Attachment #1.2: Type: text/html, Size: 79 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 RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap]

On 10/04/13 15:07, jacek burghardt wrote:
>
> Opensuse kernel source has patches for kernel that include pvusb pvscsi
>

Great!  But ATM libxl doesn't know how to enable that; so someone's only 
option is either to use xend (or to connect stuff manually, as Stefan's 
script does).

  -George

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Stefan]

On 04/10/2013 10:43 AM, George Dunlap wrote:
> On 10/04/13 15:07, jacek burghardt wrote:
>>
>> Opensuse kernel source has patches for kernel that include pvusb pvscsi
>>
>
> Great!  But ATM libxl doesn't know how to enable that; so someone's 
> only option is either to use xend (or to connect stuff manually, as 
> Stefan's script does).
>
>  -George
Indeed, as long as pvusb modules are available on both Dom0 and DomU...
The script George mentioned was written before PVUSB was implemented in 
XM. I found it linked here:
http://wiki.xen.org/wiki/Xen_USB_Passthrough

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[jacek burghardt]

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

Does xcp supports assignment of USB or xen-api ? I know that spice allows
for USB redirection. Also remote desktop supports redirection of devices. I
like using android devices with xen hosted servers as remote desktop
exports my SD cards to server.


On Wed, Apr 10, 2013 at 9:19 AM, Stefan <sstanisi@cbnco.com> wrote:

> On 04/10/2013 10:43 AM, George Dunlap wrote:
>
>> On 10/04/13 15:07, jacek burghardt wrote:
>>
>>>
>>> Opensuse kernel source has patches for kernel that include pvusb pvscsi
>>>
>>>
>> Great!  But ATM libxl doesn't know how to enable that; so someone's only
>> option is either to use xend (or to connect stuff manually, as Stefan's
>> script does).
>>
>>  -George
>>
> Indeed, as long as pvusb modules are available on both Dom0 and DomU...
> The script George mentioned was written before PVUSB was implemented in
> XM. I found it linked here:
> http://wiki.xen.org/wiki/Xen_**USB_Passthrough<http://wiki.xen.org/wiki/Xen_USB_Passthrough>
>

[-- Attachment #1.2: Type: text/html, Size: 1678 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 RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest"):
> On 09/04/13 17:30, Ian Jackson wrote:
> > I think in principle you could specify a backend domid for a non-stub
> > dm too.
> 
> How is that supposed to work?

It would set up a PV frontend in qemu in dom0.  I think this is a bit
silly because it's hard to see why you'd want to but it's not an
inherently absurd configuration.  I'm not saying this should be
implemented, but it demonstrates that things are more orthogonal than
they seem.

> > libxl can tell whether the guest is using a stub-dm.
> 
> Yes, but the question is about all the extra random plumbing that libxl 
> would be doing, and the extra codepaths that will be in use, and whether 
> doing all that automatically is really a good interface or not.

I think it is.  It's what happens when you ask for emulated block or
network devices in the stubdom case.  I don't see how USB is
different.

> For example, most distro kernels (apparently) have buggy PVUSB 
> back-ends; possibly stubdoms have buggy PVUSB front-ends.

stubdoms are part of the Xen support infrastructure.  If they have
buggy front-ends we should fix them.  We don't have a compatibility
guarantee to uphold.

> And then suppose that he decides he wants security / scalability /
> whatever, and implements stubdoms.  But he doesn't realize the
> implications; so the next time he happens to pass in a device, it
> suddenly starts using the buggy PVUSB path, and hilarity
> ensues.

This is no different with s/USB/block/.

> We could then consider adding "vendorid:productid" as a 
> properly-supported interface for either PVUSB or DEVICEMODEL at some 
> point in the future -- i.e., have libxl look it up, check that it's 
> unique, and translate it into hostbus.hostaddr.

Well you might want something more automatic.

I guess the question now is whether to include vid:pid in the API.
Maybe that would be in a device spec struct form introduced later.

It occurs to me to wonder whether one might want to pass through "any
and all devices with this vid:pid".

Ian.

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap]

On 09/04/13 17:30, Ian Jackson wrote:
> George Dunlap writes ("Re: [Xen-devel] [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest"):
>> The reason for specifying STUBDOM I guess is that STUBDOM is really a
>> special case, where it's really both a PVUSB  and a DEVICEMODEL; so in
>> theory you could specify a backend_domid.
> I think in principle you could specify a backend domid for a non-stub
> dm too.

How is that supposed to work?

>> The question, I guess, is whether we should assume that the caller can
>> be trusted to know whether the VM in question is using a stub domain or
>> not.  It's certainly possible to make the interface only specify PVUSB
>> or DEVICEMODEL, and to make it (someday) so that calling DEVICEMODEL for
>> a stubdom will set up PVUSB for the stubdom, then tell the device model
>> to make the device available via emulation all automatically.
> libxl can tell whether the guest is using a stub-dm.

Yes, but the question is about all the extra random plumbing that libxl 
would be doing, and the extra codepaths that will be in use, and whether 
doing all that automatically is really a good interface or not.

For example, most distro kernels (apparently) have buggy PVUSB 
back-ends; possibly stubdoms have buggy PVUSB front-ends.  Now suppose 
that a user is using HVM domains without stubdoms, and is used to 
(without thinking) just adding in USB devices via qemu.  And then 
suppose that he decides he wants security / scalability / whatever, and 
implements stubdoms.  But he doesn't realize the implications; so the 
next time he happens to pass in a device, it suddenly starts using the 
buggy PVUSB path, and hilarity ensues. Perhaps someday we even unify the 
config file and the hot-plug USB stuff, like it's unified for pci 
devices; and the user just forgets that he has devices passed through 
specified, and hilarity ensues without his even taking an active step to 
cause it.

I agree that we should in general honor the principle of "Ask what the 
caller/user is trying to do, not the technical details of how to do 
it".  But I think we should also honor the "Principle of least 
surprise", and I'm afraid that we may not be able to honor both fully in 
this case.  The question is which one can we fudge while causing the 
least damage.

>> But since we're not implementing stubdom at the moment anyway, we can
>> probably just take out STUBDOM entirely and discuss whether to add a new
>> protocol when we actually decide to implement it.
> It affects the documentation now, I thinki.

Can we not just say that only HVM domains without stub domains are 
supported?  And then later either change the documentation to say, "stub 
domains are supported automatically" or to say "stub domains require a 
different protocol" (whichever we end up deciding)?

>
>>> What are the semantics if multiple host devices match *dev ?
>>> How is the id returned ?
>> I think in general if it matches multiple devices then it should fail.
>> That's what qemu will do, so at the moment that is implemented by
>> default for us.
> This should be documented.

Ack.

>
>>> I think dev should probably be
>>>     const libxl_device_usb *dev
>> Sure.
> You didn't ask my question about ids.

I looked at the other interfaces, and they seem to just use a 
re-specification of the same parameters to specify the same device. (IOW 
I took out the whole "device handle" thing.)

One thing we could do is just get rid of the "vendorid:productid" thing 
entirely, at least for now.  hostbus.hostaddr is unique, it's just 
potentially a bit less convenient if the topology tends to change across 
reboots / sessions (e.g., plugging in your device into a different 
port).  That would make it much more like pci devices, which are 
specified uniquely with domain:bus:device.function.

We could then consider adding "vendorid:productid" as a 
properly-supported interface for either PVUSB or DEVICEMODEL at some 
point in the future -- i.e., have libxl look it up, check that it's 
unique, and translate it into hostbus.hostaddr.

>
>>> This is a lot of enum-handling machinery.  Don't we have something
>>> similar already which could be pressed into service ?  If not then
>>> this should be in a separate file.
> ...
>>> Perhaps this should be generated by the idl compiler ?\
>> I'm not going to be able to make that happen by the feature freeze this
>> Friday.
>>
>> Perhaps just writing the raw numeric values for now?
> If it's just needed for xenstore for the toolstack's own consumption
> then that's fine, as when you upgrade the toolstack you need to
> upgrade xen and thus migrate away all your domains.  Alternatively I
> think the IDL compiler produces long names which might be useable too.

OK, I'll take a look.

>
>>> I stopped reading here because of the wrap damage I'm afraid.
>>> (Reproduced it by adding hard CRs where my MUA wraps it, so you can
>>> see what it looks like.)
>> Well the main purpose of this mail was to see if this could be
>> implemented by Friday, or if it would need to wait until 4.4.  We've
>> uncovered a couple of things which if required would mean waiting --
>> what do you think?  I'm fine with waiting -- that's why I sent the
>> e-mail, so I could stop early if there was little chance of success. :-)
> I don't think my questions need to be answered in a way that means we
> have to wait, so long as we have a route to improving things later.
>
> Ian.

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Ian Jackson]

George Dunlap writes ("Re: [Xen-devel] [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest"):
> The reason for specifying STUBDOM I guess is that STUBDOM is really a 
> special case, where it's really both a PVUSB  and a DEVICEMODEL; so in 
> theory you could specify a backend_domid.

I think in principle you could specify a backend domid for a non-stub
dm too.

> The question, I guess, is whether we should assume that the caller can 
> be trusted to know whether the VM in question is using a stub domain or 
> not.  It's certainly possible to make the interface only specify PVUSB 
> or DEVICEMODEL, and to make it (someday) so that calling DEVICEMODEL for 
> a stubdom will set up PVUSB for the stubdom, then tell the device model 
> to make the device available via emulation all automatically.

libxl can tell whether the guest is using a stub-dm.

> But since we're not implementing stubdom at the moment anyway, we can 
> probably just take out STUBDOM entirely and discuss whether to add a new 
> protocol when we actually decide to implement it.

It affects the documentation now, I thinki.

> > What are the semantics if multiple host devices match *dev ?
> > How is the id returned ?
> 
> I think in general if it matches multiple devices then it should fail.  
> That's what qemu will do, so at the moment that is implemented by 
> default for us.

This should be documented.

> > I think dev should probably be
> >    const libxl_device_usb *dev
> 
> Sure.

You didn't ask my question about ids.

> > This is a lot of enum-handling machinery.  Don't we have something
> > similar already which could be pressed into service ?  If not then
> > this should be in a separate file.
...
> > Perhaps this should be generated by the idl compiler ?\
> 
> I'm not going to be able to make that happen by the feature freeze this 
> Friday.
> 
> Perhaps just writing the raw numeric values for now?

If it's just needed for xenstore for the toolstack's own consumption
then that's fine, as when you upgrade the toolstack you need to
upgrade xen and thus migrate away all your domains.  Alternatively I
think the IDL compiler produces long names which might be useable too.

> > I stopped reading here because of the wrap damage I'm afraid.
> > (Reproduced it by adding hard CRs where my MUA wraps it, so you can
> > see what it looks like.)
> 
> Well the main purpose of this mail was to see if this could be 
> implemented by Friday, or if it would need to wait until 4.4.  We've 
> uncovered a couple of things which if required would mean waiting -- 
> what do you think?  I'm fine with waiting -- that's why I sent the 
> e-mail, so I could stop early if there was little chance of success. :-)

I don't think my questions need to be answered in a way that means we
have to wait, so long as we have a route to improving things later.

Ian.

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap]

On 09/04/13 15:14, Ian Jackson wrote:
> George Dunlap writes ("[Xen-devel] [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest"):
>> I'm mailing this intermediate form of v3 to get feedback before I go
>> all the way through the process of implementing the whole thing and
>> ironing out the bugs.
> Thanks.  The basic approach seems plausible.
>
>> For each device removed or added, one of three protocols is available:
>> * PVUSB
>> * qemu (DEVICEMODEL)
>> * stubdom (i.e., stubdom -> pvusb, domain -> stubdom qemu)
> You seem to be making this specifiable separately for each device,
> which doesn't seem like it could be right.
>
> I think it would be best to make the protocol (as specified in the
> libxl api, xl config files etc.), simply specify what the guest sees:
> PV or HVM.  In the case of HVM, that's the stub-dm if the guest has
> one or qemu in dom0 if not.

The dichotomy isn't PV / HVM; it's PVUSB vs qemu (or "devicemodel").  
Remember that HVM guests can have PVUSB front-ends, and thus can use the 
PVUSB protocol.

The reason for specifying STUBDOM I guess is that STUBDOM is really a 
special case, where it's really both a PVUSB  and a DEVICEMODEL; so in 
theory you could specify a backend_domid.

The question, I guess, is whether we should assume that the caller can 
be trusted to know whether the VM in question is using a stub domain or 
not.  It's certainly possible to make the interface only specify PVUSB 
or DEVICEMODEL, and to make it (someday) so that calling DEVICEMODEL for 
a stubdom will set up PVUSB for the stubdom, then tell the device model 
to make the device available via emulation all automatically.

But since we're not implementing stubdom at the moment anyway, we can 
probably just take out STUBDOM entirely and discuss whether to add a new 
protocol when we actually decide to implement it.
>> +#define LIBXL_DEVICE_HOST_USB_ANY (-1)
>> +#define LIBXL_DEVICE_USB_BACKEND_DEFAULT (-1)
>> +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
>> +                         libxl_device_usb *dev,
>> +                         const libxl_asyncop_how *ao_how)
>> +                         LIBXL_EXTERNAL_CALLERS_ONLY;
> What are the semantics if multiple host devices match *dev ?
> How is the id returned ?

I think in general if it matches multiple devices then it should fail.  
That's what qemu will do, so at the moment that is implemented by 
default for us.

> I think dev should probably be
>    const libxl_device_usb *dev

Sure.

>> +    libxl__json_object *args = NULL;
>> +    char *id;
>> +
>> +    id = libxl__sprintf(NOGC, HOST_USB_QDEV_ID,
>> +                        (uint16_t) dev->u.hostdev.hostbus,
>> +                        (uint16_t) dev->u.hostdev.hostaddr,
>> +                        (uint16_t) dev->u.hostdev.vendorid,
>> +                        (uint16_t) dev->u.hostdev.productid);
> If hostbus and hostaddr aren't specified and we pass through multiple
> devices with the same vendor and product id, this won't yield unique
> ids.

If multiple devices match, then qemu will fail to add the device.

We could do a lot of this checking in the library itself rather than 
qemu. (In fact for PVUSB I think we're going to have to.)  But again, 
there's no way I'm going to get that coded up by Friday; getting that 
done would mean waiting until 4.4.

>> +struct enum_str {
>> +    int min,max;
>> +    const char * str[];
>> +};
>> +
>> +static char * __enum_to_str(libxl__gc *gc, struct enum_str *e, libxl_usb_protocol t)
> (Trailing whitespace, and needs wrapping.)
>
> This is a lot of enum-handling machinery.  Don't we have something
> similar already which could be pressed into service ?  If not then
> this should be in a separate file.
>
>> +struct enum_str enum_protocol = {
>> +    .min = 1,
>> +    .max = LIBXL_USB_PROTOCOL_STUBDOM,
>> +    .str = {
>> +        [LIBXL_USB_PROTOCOL_PV] = "pv",
>> +        [LIBXL_USB_PROTOCOL_DEVICEMODEL] = "dm",
>> +        [LIBXL_USB_PROTOCOL_STUBDOM] = "stubdom",
> Perhaps this should be generated by the idl compiler ?\

I'm not going to be able to make that happen by the feature freeze this 
Friday.

Perhaps just writing the raw numeric values for now?

>
>
>> +static char * create_hostdev_xenstore_entry(libxl__gc *gc, uint32_t domid, l
> ibxl_device_usb *usbdev, flexarray_t *a)
>> +{
>> +    char * path;
>> +
>> +    path = libxl__sprintf(gc, "/libxl/usb/%d/%s",
>> +                          domid,
>> +                          libxl__sprintf(gc, USB_HOSTDEV_ID,
>> +                                        (uint16_t)usbdev->u.hostdev.hostbus,
>> +                                        (uint16_t)usbdev->u.hostdev.hostaddr
> ,
>> +                                        (uint16_t)usbdev->u.hostdev.vendorid
> ,
>> +                                        (uint16_t)usbdev->u.hostdev.producti
> d));
>
> I stopped reading here because of the wrap damage I'm afraid.
> (Reproduced it by adding hard CRs where my MUA wraps it, so you can
> see what it looks like.)

Well the main purpose of this mail was to see if this could be 
implemented by Friday, or if it would need to wait until 4.4.  We've 
uncovered a couple of things which if required would mean waiting -- 
what do you think?  I'm fine with waiting -- that's why I sent the 
e-mail, so I could stop early if there was little chance of success. :-)

  -George

Re: [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[Ian Jackson]

George Dunlap writes ("[Xen-devel] [PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest"):
> I'm mailing this intermediate form of v3 to get feedback before I go
> all the way through the process of implementing the whole thing and
> ironing out the bugs.

Thanks.  The basic approach seems plausible.

> For each device removed or added, one of three protocols is available:
> * PVUSB
> * qemu (DEVICEMODEL)
> * stubdom (i.e., stubdom -> pvusb, domain -> stubdom qemu)

You seem to be making this specifiable separately for each device,
which doesn't seem like it could be right.

I think it would be best to make the protocol (as specified in the
libxl api, xl config files etc.), simply specify what the guest sees:
PV or HVM.  In the case of HVM, that's the stub-dm if the guest has
one or qemu in dom0 if not.


> +#define LIBXL_DEVICE_HOST_USB_ANY (-1)
> +#define LIBXL_DEVICE_USB_BACKEND_DEFAULT (-1)
> +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
> +                         libxl_device_usb *dev,
> +                         const libxl_asyncop_how *ao_how)
> +                         LIBXL_EXTERNAL_CALLERS_ONLY;

What are the semantics if multiple host devices match *dev ?
How is the id returned ?

I think dev should probably be
  const libxl_device_usb *dev

> +    libxl__json_object *args = NULL;
> +    char *id;
> +
> +    id = libxl__sprintf(NOGC, HOST_USB_QDEV_ID,
> +                        (uint16_t) dev->u.hostdev.hostbus,
> +                        (uint16_t) dev->u.hostdev.hostaddr,
> +                        (uint16_t) dev->u.hostdev.vendorid,
> +                        (uint16_t) dev->u.hostdev.productid);

If hostbus and hostaddr aren't specified and we pass through multiple
devices with the same vendor and product id, this won't yield unique
ids.

> +struct enum_str {
> +    int min,max;
> +    const char * str[];
> +};
> +
> +static char * __enum_to_str(libxl__gc *gc, struct enum_str *e, libxl_usb_protocol t)

(Trailing whitespace, and needs wrapping.)

This is a lot of enum-handling machinery.  Don't we have something
similar already which could be pressed into service ?  If not then
this should be in a separate file.

> +struct enum_str enum_protocol = {
> +    .min = 1,
> +    .max = LIBXL_USB_PROTOCOL_STUBDOM,
> +    .str = {
> +        [LIBXL_USB_PROTOCOL_PV] = "pv",
> +        [LIBXL_USB_PROTOCOL_DEVICEMODEL] = "dm",
> +        [LIBXL_USB_PROTOCOL_STUBDOM] = "stubdom",

Perhaps this should be generated by the idl compiler ?


> +static char * create_hostdev_xenstore_entry(libxl__gc *gc, uint32_t domid, l
ibxl_device_usb *usbdev, flexarray_t *a) 
> +{
> +    char * path;
> +
> +    path = libxl__sprintf(gc, "/libxl/usb/%d/%s",
> +                          domid,
> +                          libxl__sprintf(gc, USB_HOSTDEV_ID,
> +                                        (uint16_t)usbdev->u.hostdev.hostbus,

> +                                        (uint16_t)usbdev->u.hostdev.hostaddr
,
> +                                        (uint16_t)usbdev->u.hostdev.vendorid
,
> +                                        (uint16_t)usbdev->u.hostdev.producti
d));

I stopped reading here because of the wrap damage I'm afraid.
(Reproduced it by adding hard CRs where my MUA wraps it, so you can
see what it looks like.)


Ian.

[PATCH RFC] libxl: Introduce functions to add and remove USB devices to an HVM guest

[George Dunlap]

I'm mailing this intermediate form of v3 to get feedback before I go
all the way through the process of implementing the whole thing and
ironing out the bugs.

THIS IS STILL A PROTOTYPE.

This patch exposes a generic interface which can be expanded in the
future to implement USB for PVUSB, qemu, and stubdoms.  It can also be
extended to include other types of USB other than host USB (for example,
tablets, mice, or keyboards).

For each device removed or added, one of three protocols is available:
* PVUSB
* qemu (DEVICEMODEL)
* stubdom (i.e., stubdom -> pvusb, domain -> stubdom qemu)

The caller can additionally specify "AUTO", in which case the library will
try to determine the best protocol automatically.

At the moment, the only protocol implemented is DEVICEMODEL, and the only
device type impelmented is HOSTDEV.

This uses the qmp functionality, and is thus only available for
qemu-xen, not qemu-traditional.

v3 (RFC):
 - Complete re-write with new generalized protocol
 - Store list of assigned USB devices in xenstore
v2:
 - Make interface async-ready
 - usb_del takes libxl_device_host_usb struct
 - usb_del will re-construct original id given device info if no id is given

Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
---
 tools/libxl/Makefile         |    2 +-
 tools/libxl/libxl.h          |   38 ++++
 tools/libxl/libxl_internal.h |    4 +
 tools/libxl/libxl_qmp.c      |   80 ++++++++
 tools/libxl/libxl_types.idl  |   23 +++
 tools/libxl/libxl_usb.c      |  438 ++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 584 insertions(+), 1 deletion(-)
 create mode 100644 tools/libxl/libxl_usb.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 2984051..866960a 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -74,7 +74,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o libxl_pci.o \
 			libxl_internal.o libxl_utils.o libxl_uuid.o \
 			libxl_json.o libxl_aoutils.o libxl_numa.o \
 			libxl_save_callout.o _libxl_save_msgs_callout.o \
-			libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+			libxl_qmp.o libxl_event.o libxl_fork.o libxl_usb.o $(LIBXL_OBJS-y)
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
 
 $(LIBXL_OBJS): CFLAGS += $(CFLAGS_LIBXL) -include $(XEN_ROOT)/tools/config.h
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 030aa86..47dbef4 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -75,6 +75,11 @@
 #define LIBXL_HAVE_FIRMWARE_PASSTHROUGH 1
 
 /*
+ * FIXME: Update comment
+ */
+#define LIBXL_HAVE_USB 1
+
+/*
  * libxl ABI compatibility
  *
  * The only guarantee which libxl makes regarding ABI compatibility
@@ -719,6 +724,39 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid, libxl_device_disk *disk,
                        const libxl_asyncop_how *ao_how)
                        LIBXL_EXTERNAL_CALLERS_ONLY;
 
+/*
+ * USB
+ * 
+ * For each device removed or added, one of three protocols is available:
+ * - PVUSB
+ * - qemu (DEVICEMODEL)
+ * - stubdom (i.e., stubdom -> pvusb, domain -> stubdom qemu)
+ *
+ * The caller can additionally specify "AUTO", in which case the library will
+ * try to determine the best protocol automatically.
+ *
+ * At the moment, the only protocol implemented is DEVICEMODEL, and the only
+ * device type impelmented is HOSTDEV.
+ *
+ * This uses the qmp functionality, and is thus only available for
+ * qemu-xen, not qemu-traditional.
+ *
+ */
+#define LIBXL_DEVICE_HOST_USB_ANY (-1)
+#define LIBXL_DEVICE_USB_BACKEND_DEFAULT (-1)
+int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb *dev,
+                         const libxl_asyncop_how *ao_how)
+                         LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid, 
+                            libxl_device_usb *dev,
+                            const libxl_asyncop_how *ao_how)
+                            LIBXL_EXTERNAL_CALLERS_ONLY;
+int libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid, 
+                          libxl_device_usb **dev,
+                          const libxl_asyncop_how *ao_how)
+                          LIBXL_EXTERNAL_CALLERS_ONLY;
+
 /* Network Interfaces */
 int libxl_device_nic_add(libxl_ctx *ctx, uint32_t domid, libxl_device_nic *nic,
                          const libxl_asyncop_how *ao_how)
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 3ba3a21..15d70ed 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -1412,6 +1412,10 @@ _hidden int libxl__qmp_save(libxl__gc *gc, int domid, const char *filename);
 /* Set dirty bitmap logging status */
 _hidden int libxl__qmp_set_global_dirty_log(libxl__gc *gc, int domid, bool enable);
 _hidden int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid, const libxl_device_disk *disk);
+_hidden int libxl__qmp_usb_add(libxl__gc *gc, int domid, 
+                               libxl_device_usb *dev);
+_hidden int libxl__qmp_usb_remove(libxl__gc *gc, int domid, 
+                                  libxl_device_usb *dev);
 /* close and free the QMP handler */
 _hidden void libxl__qmp_close(libxl__qmp_handler *qmp);
 /* remove the socket file, if the file has already been removed,
diff --git a/tools/libxl/libxl_qmp.c b/tools/libxl/libxl_qmp.c
index 644d2c0..87b999f 100644
--- a/tools/libxl/libxl_qmp.c
+++ b/tools/libxl/libxl_qmp.c
@@ -42,6 +42,7 @@
 
 #define QMP_RECEIVE_BUFFER_SIZE 4096
 #define PCI_PT_QDEV_ID "pci-pt-%02x_%02x.%01x"
+#define HOST_USB_QDEV_ID "usb-hostdev-%04x.%04x-%04x.%04x"
 
 typedef int (*qmp_callback_t)(libxl__qmp_handler *qmp,
                               const libxl__json_object *tree,
@@ -929,6 +930,85 @@ int libxl__qmp_insert_cdrom(libxl__gc *gc, int domid,
     }
 }
 
+static int libxl__qmp_usb_hostdev_add(libxl__gc *gc, int domid,
+                                   libxl_device_usb *dev)
+{
+    libxl__json_object *args = NULL;
+    char *id;
+
+    id = libxl__sprintf(NOGC, HOST_USB_QDEV_ID,
+                        (uint16_t) dev->u.hostdev.hostbus,
+                        (uint16_t) dev->u.hostdev.hostaddr,
+                        (uint16_t) dev->u.hostdev.vendorid,
+                        (uint16_t) dev->u.hostdev.productid);
+
+    qmp_parameters_add_string(gc, &args, "driver", "usb-host");
+    if(dev->u.hostdev.hostbus != LIBXL_DEVICE_HOST_USB_ANY) {
+        QMP_PARAMETERS_SPRINTF(&args, "hostbus", "0x%x", dev->u.hostdev.hostbus);
+    }
+    if(dev->u.hostdev.hostaddr != LIBXL_DEVICE_HOST_USB_ANY) {
+        QMP_PARAMETERS_SPRINTF(&args, "hostaddr", "0x%x", dev->u.hostdev.hostaddr);
+    }
+    if(dev->u.hostdev.vendorid != LIBXL_DEVICE_HOST_USB_ANY) {
+        QMP_PARAMETERS_SPRINTF(&args, "vendorid", "0x%x", dev->u.hostdev.vendorid);
+    }
+    if(dev->u.hostdev.productid != LIBXL_DEVICE_HOST_USB_ANY) {
+        QMP_PARAMETERS_SPRINTF(&args, "productid", "0x%x", dev->u.hostdev.productid);
+    }
+
+    qmp_parameters_add_string(gc, &args, "id", id);
+
+    return qmp_run_command(gc, domid, "device_add", args, NULL, NULL);
+}
+
+int libxl__qmp_usb_add(libxl__gc *gc, int domid,
+                            libxl_device_usb *usbdev)
+{
+    int rc;
+    switch(usbdev->type) {
+    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+        rc = libxl__qmp_usb_hostdev_add(gc, domid, usbdev);
+        break;
+    default:
+        return ERROR_INVAL;
+    }
+    return rc;
+}
+     
+
+static int libxl__qmp_usb_hostdev_remove(libxl__gc *gc, int domid, 
+                               libxl_device_usb *dev)
+{
+    libxl__json_object *args = NULL;
+    char *id;
+
+    id = libxl__sprintf(NOGC, HOST_USB_QDEV_ID,
+                        (uint16_t) dev->u.hostdev.hostbus,
+                        (uint16_t) dev->u.hostdev.hostaddr,
+                        (uint16_t) dev->u.hostdev.vendorid,
+                        (uint16_t) dev->u.hostdev.productid);
+
+    qmp_parameters_add_string(gc, &args, "id", id);
+
+    return qmp_run_command(gc, domid, "device_del", args, NULL, NULL);
+}
+
+int libxl__qmp_usb_remove(libxl__gc *gc, int domid,
+                          libxl_device_usb *usbdev)
+{
+    int rc;
+    switch(usbdev->type) {
+    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+        rc = libxl__qmp_usb_hostdev_remove(gc, domid, usbdev);
+        break;
+    default:
+        return ERROR_INVAL;
+    }
+    return rc;
+}
+     
+
+
 int libxl__qmp_initializations(libxl__gc *gc, uint32_t domid,
                                const libxl_domain_config *guest_config)
 {
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
index f3c212b..8d9bae7 100644
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -406,6 +406,29 @@ libxl_device_vtpm = Struct("device_vtpm", [
     ("uuid",             libxl_uuid),
 ])
 
+libxl_device_usb_protocol = Enumeration("usb_protocol", [
+    (0, "AUTO"),
+    (1, "PV"),
+    (2, "DEVICEMODEL"),
+    (3, "STUBDOM"),
+    ])
+
+libxl_device_usb_type = Enumeration("device_usb_type", [
+    (1, "HOSTDEV"),
+    ])
+
+libxl_device_usb = Struct("device_usb", [
+        ("protocol", libxl_device_usb_protocol, {'init_val': 'LIBXL_USB_PROTOCOL_AUTO'}),
+        ("backend_domid", libxl_domid, {'init_val': 'LIBXL_DEVICE_USB_BACKEND_DEFAULT'}),
+        ("u", KeyedUnion(None, libxl_device_usb_type, "type",
+                         [("hostdev", Struct(None, [
+                                ("hostbus",   integer, {'init_val': 'LIBXL_DEVICE_HOST_USB_ANY'}),
+                                ("hostaddr",  integer, {'init_val': 'LIBXL_DEVICE_HOST_USB_ANY'}),
+                                ("vendorid",  integer, {'init_val': 'LIBXL_DEVICE_HOST_USB_ANY'}),
+                                ("productid", integer, {'init_val': 'LIBXL_DEVICE_HOST_USB_ANY'}) ]))
+                          ]))
+    ])
+
 libxl_domain_config = Struct("domain_config", [
     ("c_info", libxl_domain_create_info),
     ("b_info", libxl_domain_build_info),
diff --git a/tools/libxl/libxl_usb.c b/tools/libxl/libxl_usb.c
new file mode 100644
index 0000000..e508b85
--- /dev/null
+++ b/tools/libxl/libxl_usb.c
@@ -0,0 +1,438 @@
+/*
+ * Copyright (C) 2009      Citrix Ltd.
+ * Author Vincent Hanquez <vincent.hanquez@eu.citrix.com>
+ * Author Stefano Stabellini <stefano.stabellini@eu.citrix.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published
+ * by the Free Software Foundation; version 2.1 only. with the special
+ * exception on linking described in file LICENSE.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Lesser General Public License for more details.
+ */
+
+#include "libxl_osdeps.h" /* must come before any other headers */
+
+#include "libxl_internal.h"
+
+/*
+ * /libxl/usb/$domain/??/type, protocol, backend, [typeinfo]
+ */
+#define USB_INFO_PATH "/libxl/usb"
+#define USB_HOSTDEV_ID "usb-hostdev-%04x.%04x-%04x.%04x"
+
+struct enum_str {
+    int min,max;
+    const char * str[];
+};
+
+static char * __enum_to_str(libxl__gc *gc, struct enum_str *e, libxl_usb_protocol t) 
+{
+    if (t < e->min || t > e->max)
+        return NULL;
+
+    return libxl__sprintf(gc, "%s", e->str[t]);
+}
+
+static int __str_to_enum(struct enum_str *e, const char * str) 
+{
+    int rc = -1, i;
+    for(i=e->min; i<=e->max; i++)
+        if(!strcmp(str, e->str[i])) {
+            rc = i;
+            break;
+        }
+    return rc;
+}
+
+struct enum_str enum_protocol = {
+    .min = 1,
+    .max = LIBXL_USB_PROTOCOL_STUBDOM,
+    .str = {
+        [LIBXL_USB_PROTOCOL_PV] = "pv",
+        [LIBXL_USB_PROTOCOL_DEVICEMODEL] = "dm",
+        [LIBXL_USB_PROTOCOL_STUBDOM] = "stubdom",
+    }
+};
+#define protocol_to_str(gc, t) __enum_to_str(gc, &enum_protocol, t)
+#define str_to_protocol(s) __str_to_enum(&enum_protocol, s)
+
+struct enum_str enum_type = {
+    .min = 1,
+    .max = LIBXL_DEVICE_USB_TYPE_HOSTDEV,
+    .str = {
+        [LIBXL_DEVICE_USB_TYPE_HOSTDEV]="hostdev",
+    }
+};
+#define type_to_str(gc, t) __enum_to_str(gc, &enum_type, t)
+#define str_to_type(s) __str_to_enum(&enum_type, s)
+
+static char * create_hostdev_xenstore_entry(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev, flexarray_t *a) 
+{
+    char * path;
+
+    path = libxl__sprintf(gc, "/libxl/usb/%d/%s",
+                          domid,
+                          libxl__sprintf(gc, USB_HOSTDEV_ID,
+                                        (uint16_t)usbdev->u.hostdev.hostbus,
+                                        (uint16_t)usbdev->u.hostdev.hostaddr,
+                                        (uint16_t)usbdev->u.hostdev.vendorid,
+                                        (uint16_t)usbdev->u.hostdev.productid));
+    flexarray_append_pair(a, "type", "hostdev");
+    flexarray_append_pair(a, "protocol", protocol_to_str(gc, usbdev->protocol));
+
+    if(usbdev->protocol == LIBXL_USB_PROTOCOL_PV
+       || usbdev->protocol == LIBXL_USB_PROTOCOL_STUBDOM)
+        flexarray_append_pair(a, "backend_domid",
+                              libxl__sprintf(gc, "%d", usbdev->backend_domid));
+
+    if(usbdev->u.hostdev.hostbus != LIBXL_DEVICE_HOST_USB_ANY)
+        flexarray_append_pair(a, "hostbus", 
+                              libxl__sprintf(gc, "%d", usbdev->u.hostdev.hostbus));
+    if(usbdev->u.hostdev.hostaddr != LIBXL_DEVICE_HOST_USB_ANY)
+        flexarray_append_pair(a, "hostaddr", 
+                              libxl__sprintf(gc, "%d", usbdev->u.hostdev.hostaddr));
+    if(usbdev->u.hostdev.vendorid != LIBXL_DEVICE_HOST_USB_ANY)
+        flexarray_append_pair(a, "vendorid", 
+                              libxl__sprintf(gc, "%d", usbdev->u.hostdev.vendorid));
+    if(usbdev->u.hostdev.productid != LIBXL_DEVICE_HOST_USB_ANY)
+        flexarray_append_pair(a, "productid", 
+                              libxl__sprintf(gc, "%d", usbdev->u.hostdev.productid));
+
+    return path;
+}
+
+static int read_hostdev_xenstore_entry(libxl__gc *gc, const char * path, libxl_device_usb *usbdev)
+{
+    char * val;
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/hostbus", path));
+    usbdev->u.hostdev.hostbus = val ? atoi(val) : LIBXL_DEVICE_HOST_USB_ANY;
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/hostaddr", path));
+    usbdev->u.hostdev.hostaddr = val ? atoi(val) : LIBXL_DEVICE_HOST_USB_ANY;
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/vendorid", path));
+    usbdev->u.hostdev.vendorid = val ? atoi(val) : LIBXL_DEVICE_HOST_USB_ANY;
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/productid", path));
+    usbdev->u.hostdev.productid = val ? atoi(val) : LIBXL_DEVICE_HOST_USB_ANY;
+
+    return 0;
+}
+
+static int usb_read_xenstore(libxl__gc *gc, const char * path,
+                                           libxl_device_usb *usbdev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    char *val;
+    int rc = 0;
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/protocol", path));
+    if(!val) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (missing protocol)");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if ((usbdev->protocol = str_to_protocol(val)) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (bad protocol)");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    
+
+    if(usbdev->protocol == LIBXL_USB_PROTOCOL_PV
+       || usbdev->protocol == LIBXL_USB_PROTOCOL_STUBDOM) {
+        val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/backend_domid", path));
+        if(!val) {
+            LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (missing backend_domid)");
+        rc = ERROR_FAIL;
+        goto out;
+        }
+        usbdev->protocol = atoi(val);
+    }
+
+    val = libxl__xs_read(gc, XBT_NULL, libxl__sprintf(gc, "%s/type", path));
+    if(!val) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (missing type)");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+    if ((usbdev->type = str_to_type(val)) < 0) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (bad type)");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    switch(usbdev->type) {
+    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+        read_hostdev_xenstore_entry(gc, path, usbdev);
+        break;
+    default: 
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Internal error (unimplemented type)");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+out:
+    return rc;
+}
+
+static int usb_add_xenstore(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    flexarray_t *dev;
+    char *dev_path;
+    xs_transaction_t t;
+
+    dev = flexarray_make(gc, 16, 1);
+
+    switch(usbdev->type) {
+    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+        dev_path = create_hostdev_xenstore_entry(gc, domid, usbdev, dev);
+        break;
+    default:
+        LOG(ERROR, "Invalid device type");
+        return ERROR_FAIL;
+    }
+
+    LIBXL__LOG(ctx, LIBXL__LOG_DEBUG, "Adding new usb device to xenstore");
+
+retry_transaction:
+    t = xs_transaction_start(ctx->xsh);
+    libxl__xs_writev(gc, t, dev_path,
+                    libxl__xs_kvs_of_flexarray(gc, dev, dev->count));
+    if (!xs_transaction_end(ctx->xsh, t, 0))
+        if (errno == EAGAIN)
+            goto retry_transaction;
+
+    return 0;
+}
+
+static int get_assigned_devices(libxl__gc *gc, uint32_t domid, libxl_device_usb **list, int *num, int do_gc)
+{
+    char **devlist, *dompath;
+    unsigned int nd;
+    int i;
+
+    *list = NULL;
+    *num = 0;
+
+    dompath = libxl__sprintf(gc, "%s/%d", USB_INFO_PATH, domid);
+
+    devlist = libxl__xs_directory(gc, XBT_NULL, dompath, &nd);
+    for(i = 0; i < nd; i++) {
+        char *path;
+        libxl_device_usb t;
+
+        path = libxl__sprintf(gc, "%s/%s", dompath, devlist[i]);
+        if ( !usb_read_xenstore(gc, path, &t) ) {
+            *list = realloc(*list, sizeof(libxl_device_usb) * ((*num) +1));
+            if (*list == NULL)
+                return ERROR_NOMEM;
+            *list[*num] = t;
+            (*num)++;
+        }
+    }
+    if(do_gc)
+        libxl__ptr_add(gc, *list);
+
+    return 0;
+}
+
+static int is_usbdev_type_hostdev_equal(libxl_device_usb *a, libxl_device_usb *b)
+{
+    if ( !memcmp(&a->u.hostdev, &b->u.hostdev, sizeof(a->u.hostdev) ) )
+        return 1;
+    else
+        return 0;
+}
+
+static int is_usbdev_in_array(libxl_device_usb *assigned, int num_assigned,
+                              libxl_device_usb *dev)
+{
+    int i;
+
+    for(i = 0; i < num_assigned; i++) {
+        if (assigned[i].protocol != dev->protocol
+            || assigned[i].type != dev->type)
+            continue;
+
+        switch(dev->type) {
+        case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
+            if (!is_usbdev_type_hostdev_equal(dev, assigned+i))
+                continue;
+        }
+
+        return 1;
+    }
+
+    return 0;
+}
+
+static int do_usb_add(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev)
+{
+    //libxl_ctx *ctx = libxl__gc_owner(gc);
+    int rc;
+
+    switch (usbdev->protocol) {
+    case LIBXL_USB_PROTOCOL_DEVICEMODEL:
+        switch (libxl__device_model_version_running(gc, domid)) {
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN_TRADITIONAL:
+            LOG(ERROR, "usb-add not yet implemented for qemu-traditional");
+            return ERROR_INVAL;
+        case LIBXL_DEVICE_MODEL_VERSION_QEMU_XEN:
+            rc = libxl__qmp_usb_add(gc, domid, usbdev);
+            break;
+        default:
+            return ERROR_INVAL;
+        }
+        break;
+    default:
+        return ERROR_FAIL;
+    }
+
+    rc = usb_add_xenstore(gc, domid, usbdev);
+
+    return rc;
+}
+
+static int libxl__device_usb_setdefault(libxl__gc *gc, libxl_device_usb *usbdev)
+{
+    return 0;
+}
+
+
+/* Either set the protocol automatically based on domid and existence of
+ * stubdom, or check to make sure it's a valid combination. */
+static int check_usb_protocol(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev) {
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl_domain_type domtype = libxl__domain_type(gc, domid);
+    int stubdomid = libxl_get_stubdom_id(ctx, domid);
+    int rc=0;
+
+    switch(usbdev->protocol) {
+    case LIBXL_USB_PROTOCOL_AUTO:
+        if ( domtype == LIBXL_DOMAIN_TYPE_PV ) {
+            usbdev->protocol = LIBXL_USB_PROTOCOL_PV;
+        } else if (domtype == LIBXL_DOMAIN_TYPE_HVM) {
+            if (stubdomid != 0) {
+                usbdev->protocol = LIBXL_USB_PROTOCOL_STUBDOM;
+            } else {
+                /* FIXME: See if we can detect PV frontend */
+                usbdev->protocol = LIBXL_USB_PROTOCOL_DEVICEMODEL;
+            }
+        }
+        break;
+    case LIBXL_USB_PROTOCOL_PV:
+        /* PV protocol is valid for all combinations */
+        break;
+    case LIBXL_USB_PROTOCOL_DEVICEMODEL:
+        if ( domtype != LIBXL_DOMAIN_TYPE_HVM 
+             ||stubdomid != 0 )
+            rc = ERROR_FAIL;
+        break;
+    case LIBXL_USB_PROTOCOL_STUBDOM:
+        if ( domtype != LIBXL_DOMAIN_TYPE_HVM 
+             || stubdomid == 0 )
+            rc = ERROR_FAIL;
+        break;
+    }
+    return rc;
+}
+
+static int libxl__device_usb_add(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev)
+{
+    libxl_ctx *ctx = libxl__gc_owner(gc);
+    libxl_device_usb *assigned;
+    int rc = ERROR_FAIL, num_assigned;
+
+    rc = libxl__device_usb_setdefault(gc, usbdev);
+    if (rc) goto out;
+
+    rc = check_usb_protocol(gc, domid, usbdev);
+    if (rc) goto out;
+
+    if ( usbdev->protocol != LIBXL_USB_PROTOCOL_DEVICEMODEL ) {
+        rc = ERROR_FAIL;
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Protocol not implemented");
+        goto out;
+    }
+
+    rc = get_assigned_devices(gc, domid, &assigned, &num_assigned, 1);
+    if ( rc ) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot determine if device is assigned, refusing to continue");
+        goto out;
+    }
+    if ( is_usbdev_in_array(assigned, num_assigned, usbdev) ) {
+        LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "USB device already attached to a domain");
+        rc = ERROR_FAIL;
+        goto out;
+    }
+
+    if ( do_usb_add(gc, domid, usbdev) )
+        rc = ERROR_FAIL;
+
+out:
+    return rc;
+}
+
+int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid,
+                         libxl_device_usb *usbdev,
+                         const libxl_asyncop_how *ao_how)
+{
+    AO_CREATE(ctx, domid, ao_how);
+    int rc;
+    rc = libxl__device_usb_add(gc, domid, usbdev);
+    libxl__ao_complete(egc, ao, rc);
+    return AO_INPROGRESS;
+}
+
+/* static int libxl__device_usb_remove(libxl__gc *gc, uint32_t domid, libxl_device_usb *usbdev) */
+/* { */
+/*     libxl_ctx *ctx = libxl__gc_owner(gc); */
+/*     libxl_device_usb *assigned; */
+/*     int rc = ERROR_FAIL, num_assigned; */
+
+/*     rc = libxl__device_usb_setdefault(gc, usbdev); */
+/*     if (rc) goto out; */
+
+/*     rc = check_usb_protocol(gc, domid, usbdev); */
+/*     if (rc) goto out; */
+
+/*     if ( usbdev->protocol != LIBXL_USB_PROTOCOL_DEVICEMODEL ) { */
+/*         rc = ERROR_FAIL; */
+/*         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "Protocol not implemented"); */
+/*         goto out; */
+/*     } */
+
+/*     rc = get_assigned_devices(gc, domid, &assigned, &num_assigned, 1); */
+/*     if ( rc ) { */
+/*         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "cannot determine if device is assigned, refusing to continue"); */
+/*         goto out; */
+/*     } */
+/*     if ( !is_usbdev_in_array(assigned, num_assigned, usbdev) ) { */
+/*         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "USB not found"); */
+/*         rc = ERROR_FAIL; */
+/*         goto out; */
+/*     } */
+
+/*     if ( do_usb_remove(gc, domid, usbdev) ) */
+/*         rc = ERROR_FAIL; */
+
+/* out: */
+/*     return rc; */
+/* } */
+
+/* int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid, */
+/*                             libxl_device_usb *usbdev, */
+/*                             const libxl_asyncop_how *ao_how) */
+/* { */
+/*     AO_CREATE(ctx, domid, ao_how); */
+/*     int rc; */
+/*     rc = libxl__device_usb_remove(gc, domid, usbdev); */
+/*     libxl__ao_complete(egc, ao, rc); */
+/*     return AO_INPROGRESS; */
+/* } */
-- 
1.7.9.5