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

Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API




>>> On 5/19/2015 at 02:07 AM, in message
<20150518180724.GH9503@xxxxxxxxxxxxxxxxxxxxx>, Wei Liu <wei.liu2@xxxxxxxxxx>
wrote: 
> (I look at overall code structure this pass. I haven't done a line by 
> line review.) 
>  
> On Sun, Apr 19, 2015 at 11:50:49AM +0800, Chunyan Liu wrote: 
> > Add pvusb APIs, including: 
> >  - attach/detach (create/destroy) virtual usb controller. 
> >  - attach/detach usb device 
> >  - list usb controller and usb devices 
> >  - some other helper functions 
> >  
> > Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx> 
> > Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx> 
> > --- 
> > Changes to v2: 
> >   * remove qemu emulated usb related definitions, keep the work pure pvusb. 
> >     In last patch, will do all refactor work to unify pvusb and qemu  
> emulated 
> >     usb. 
> >   * use bus.addr as user interface instead of busid to do usb-attach|detach 
> >   * remove usb-assignable-list APIs and some other unnecessary APIs 
> >   * reuse libxl_read_file_contents function instead of another new function 
> >     to handle getting sysfs file content 
> >   * fix build on different platforms as pci does 
> >   * fix many coding style problems 
> >   * address other comments in last version 
> >   * adjust codes to let it look better 
> >  
> >  tools/libxl/Makefile                 |    2 +- 
> >  tools/libxl/libxl.c                  |    2 + 
> >  tools/libxl/libxl.h                  |   45 ++ 
> >  tools/libxl/libxl_internal.h         |   11 +- 
> >  tools/libxl/libxl_osdeps.h           |   13 + 
> >  tools/libxl/libxl_pvusb.c            | 1201  
> ++++++++++++++++++++++++++++++++++ 
> >  tools/libxl/libxl_types.idl          |   41 ++ 
> >  tools/libxl/libxl_types_internal.idl |    1 + 
>  
> You also need to document the xenstore keys and values somewhere under 
> docs directory. 

Thanks, will add that.

>  
> And you forgot to update libxl_retrieve_domain_configuration function. 

Updating.

>  
> >  8 files changed, 1314 insertions(+), 2 deletions(-) 
> >  create mode 100644 tools/libxl/libxl_pvusb.c 
> >  
> > diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile 
> > index 1b16598..d52281f 100644 
> > --- a/tools/libxl/Makefile 
> > +++ b/tools/libxl/Makefile 
> > @@ -95,7 +95,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_vnuma.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_pvusb.o 
> > $(LIBXL_OBJS-y) 
> >  LIBXL_OBJS += libxl_genid.o 
> >  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o 
> >   
> > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c 
> > index b05d18b..a7c81d9 100644 
> > --- a/tools/libxl/libxl.c 
> > +++ b/tools/libxl/libxl.c 
> > @@ -1611,6 +1611,8 @@ void libxl__destroy_domid(libxl__egc *egc,  
> libxl__destroy_domid_state *dis) 
> >   
> >      if (libxl__device_pci_destroy_all(gc, domid) < 0) 
> >          LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "pci shutdown failed for domid  
> %d", domid); 
> > +    if (libxl__device_usb_destroy_all(gc, domid) < 0) 
> > +         LIBXL__LOG(ctx, LIBXL__LOG_ERROR, "usb shutdown failed for domid  
> %d", domid); 
> >      rc = xc_domain_pause(ctx->xch, domid); 
> >      if (rc < 0) { 
> >          LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, "xc_domain_pause  
> failed for %d", domid); 
> > diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h 
> > index 6bc75c5..cbe3519 100644 
> > --- a/tools/libxl/libxl.h 
> > +++ b/tools/libxl/libxl.h 
> > @@ -114,6 +114,12 @@ 
> >  #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 
> >   
> >  /* 
> > + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of 
> > + * USB devices through pvusb. 
> > + */ 
> > +#define LIBXL_HAVE_PVUSB 1 
> > + 
> > +/* 
> >   * LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the 
> >   * libxl_vendor_device field is present in the hvm sections of 
> >   * libxl_domain_build_info. This field tells libxl which 
> > @@ -1224,6 +1230,45 @@ 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 Controllers*/ 
> > +int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid, 
> > +                         libxl_device_usbctrl *usbctrl, 
> > +                         const libxl_asyncop_how *ao_how) 
> > +                         LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +int libxl_device_usbctrl_remove(libxl_ctx *ctx, uint32_t domid, 
> > +                         libxl_device_usbctrl *usbctrl, 
> > +                         const libxl_asyncop_how *ao_how) 
> > +                         LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +int libxl_device_usbctrl_destroy(libxl_ctx *ctx, uint32_t domid, 
> > +                         libxl_device_usbctrl *usbctrl, 
> > +                         const libxl_asyncop_how *ao_how) 
> > +                         LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +libxl_device_usbctrl *libxl_device_usbctrl_list(libxl_ctx *ctx, 
> > +                            uint32_t domid, int *num); 
> > + 
> > + 
> > +int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid, 
> > +                                libxl_device_usbctrl *usbctrl, 
> > +                                libxl_usbctrlinfo *usbctrlinfo) 
> > +                                LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +/* USB Devices */ 
> > +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_usb  
> *usb, 
> > +                         const libxl_asyncop_how *ao_how) 
> > +                         LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid,  
> libxl_device_usb *usb, 
> > +                            const libxl_asyncop_how *ao_how) 
> > +                            LIBXL_EXTERNAL_CALLERS_ONLY; 
> > + 
> > +libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid, 
> > +                                        int usbctrl, int *num); 
> > + 
> > +int libxl_device_usb_getinfo(libxl_ctx *ctx, char *intf, libxl_usbinfo  
> *usbinfo) 
> > +                             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 42eb1b9..f426ed8 100644 
> > --- a/tools/libxl/libxl_internal.h 
> > +++ b/tools/libxl/libxl_internal.h 
> > @@ -2422,6 +2422,13 @@ _hidden void libxl__device_vtpm_add(libxl__egc *egc, 
> >  
> uint32_t domid, 
> >                                     libxl_device_vtpm *vtpm, 
> >                                     libxl__ao_device *aodev); 
> >   
> > +/* from libxl_usb */ 
> > +_hidden int libxl__device_usbctrl_add(libxl__gc *gc, uint32_t domid, 
> > +                                      libxl_device_usbctrl *usbctrl); 
> > +_hidden int libxl__device_usb_add(libxl__gc *gc, uint32_t domid, 
> > +                            libxl_device_usb *usb); 
> > +_hidden int libxl__device_usb_destroy_all(libxl__gc *gc, uint32_t domid); 
> > + 
> >  /* Internal function to connect a vkb device */ 
> >  _hidden int libxl__device_vkb_add(libxl__gc *gc, uint32_t domid, 
> >                                    libxl_device_vkb *vkb); 
> > @@ -3628,7 +3635,9 @@ static inline void 
> > libxl__update_config_vtpm(libxl__gc  
> *gc, 
> >  #define COMPARE_PCI(a, b) ((a)->func == (b)->func &&    \ 
> >                             (a)->bus == (b)->bus &&      \ 
> >                             (a)->dev == (b)->dev) 
> > - 
> > +#define COMPARE_USB(a, b) (!strcmp((a)->busid, (b)->busid)) 
> > +#define COMPARE_USBCTRL(a, b) ((a)->devid == (b)->devid) 
> > +  
> >  /* DEVICE_ADD 
> >   * 
> >   * Add a device in libxl_domain_config structure 
> > diff --git a/tools/libxl/libxl_osdeps.h b/tools/libxl/libxl_osdeps.h 
> > index 08eaf0c..55caf71 100644 
> > --- a/tools/libxl/libxl_osdeps.h 
> > +++ b/tools/libxl/libxl_osdeps.h 
> > @@ -24,6 +24,8 @@ 
> >  #define _GNU_SOURCE 
> >   
> >  #if defined(__NetBSD__) 
> > +#define SYSFS_USB_DEV          "/sys/bus/usb/devices" 
> > +#define SYSFS_USBBACK_DRIVER   "/kern/xen/usb" 
> >  #define SYSFS_PCI_DEV          "/sys/bus/pci/devices" 
> >  #define SYSFS_PCIBACK_DRIVER   "/kern/xen/pci" 
> >  #define NETBACK_NIC_NAME       "xvif%ui%d" 
> > @@ -31,6 +33,8 @@ 
> >  #elif defined(__OpenBSD__) 
> >  #include <util.h> 
> >  #elif defined(__linux__) 
> > +#define SYSFS_USB_DEV          "/sys/bus/usb/devices" 
> > +#define SYSFS_USBBACK_DRIVER   "/sys/bus/usb/drivers/usbback" 
> >  #define SYSFS_PCI_DEV          "/sys/bus/pci/devices" 
> >  #define SYSFS_PCIBACK_DRIVER   "/sys/bus/pci/drivers/pciback" 
> >  #define NETBACK_NIC_NAME       "vif%u.%d" 
> > @@ -38,12 +42,21 @@ 
> >  #elif defined(__sun__) 
> >  #include <stropts.h> 
> >  #elif defined(__FreeBSD__) 
> > +#define SYSFS_USB_DEV          "/dev/null" 
> > +#define SYSFS_USBBACK_DRIVER   "/dev/null" 
> >  #define SYSFS_PCI_DEV          "/dev/null" 
> >  #define SYSFS_PCIBACK_DRIVER   "/dev/null" 
> >  #define NETBACK_NIC_NAME       "xnb%u.%d" 
> >  #include <libutil.h> 
> >  #endif 
> >   
> > +#ifndef SYSFS_USBBACK_DRIVER 
> > +#error define SYSFS_USBBACK_DRIVER for your platform 
> > +#endif 
> > +#ifndef SYSFS_USB_DEV 
> > +#error define SYSFS_USB_DEV for your platform 
> > +#endif 
> > + 
> >  #ifndef SYSFS_PCIBACK_DRIVER 
> >  #error define SYSFS_PCIBACK_DRIVER for your platform 
> >  #endif 
> > diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c 
> > new file mode 100644 
> > index 0000000..4e4975a 
> > --- /dev/null 
> > +++ b/tools/libxl/libxl_pvusb.c 
> > @@ -0,0 +1,1201 @@ 
> > +/* 
> > + * 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" 
> > + 
> > +#define USBBACK_INFO_PATH "/libxl/usbback" 
> > + 
> > +#define USBHUB_CLASS_CODE 0x09 
> > + 
>  
> I'm not very familiar with how USB is supposed to work. Can you explain 
> why this particular value is chosen? 

0x09 is the USB class code for USB HUB defined in USB Spec. USB Class code
is  stored under Linux sysfs too. Since USB HUB could not be assigned to guest,
we need to check that.

>  
> > +static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid, 
> > +                                            libxl_device_usbctrl *usbctrl) 
> > +{ 
> > +    int rc; 
> > + 
> > +    if (!usbctrl->version) 
> > +        usbctrl->version = 2; 
> > + 
>  
> How hard would it be to implement USB 3? I assume this depends on QEMU's 
> support? I.e. we just need to specify the version to 3 and it should 
> just work? Just curious. 

This implementation is coworked with PVUSB  driver, so it depends on PVUSB
driver's support. If PVUSB driver supports that, just specify the version to 3 
and
can work.

QEMU emulated USB device depends on QEMU's support.

>  
> > +    if (!usbctrl->ports) 
> > +        usbctrl->ports = 8; 
> > + 
> > +    rc = libxl__resolve_domid(gc, usbctrl->backend_domname, 
> > +                              &usbctrl->backend_domid); 
> > +    return rc; 
> > +} 
> > + 
> > +static void libxl__device_from_usbctrl(libxl__gc *gc, uint32_t domid, 
> > +                                       libxl_device_usbctrl *usbctrl, 
> > +                                       libxl__device *device) 
> > +{ 
> > +    device->backend_devid   = usbctrl->devid; 
> > +    device->backend_domid   = usbctrl->backend_domid; 
> > +    device->backend_kind    = LIBXL__DEVICE_KIND_VUSB; 
> > +    device->devid           = usbctrl->devid; 
> > +    device->domid           = domid; 
> > +    device->kind            = LIBXL__DEVICE_KIND_VUSB; 
> > +} 
> > + 
> > +static int libxl__usbport_add_xenstore(libxl__gc *gc, 
> > +                                       xs_transaction_t tran, 
> > +                                       uint32_t domid, 
> > +                                       libxl_device_usbctrl *usbctrl) 
> > +{ 
> > +    char *path; 
> > +    int i; 
> > + 
> > +    path = GCSPRINTF("%s/backend/vusb/%d/%d/port", 
> > +                     libxl__xs_get_dompath(gc, 0), domid, usbctrl->devid); 
> > + 
> > +    libxl__xs_mkdir(gc, tran, path, NULL, 0); 
> > + 
> > +    for (i = 1; i <= usbctrl->ports; i++) { 
> > +        if (libxl__xs_write_checked(gc, tran, GCSPRINTF("%s/%d", path, i), 
> >  
> "")) 
> > +            return ERROR_FAIL; 
> > +    } 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +static int libxl__usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid, 
> > +                                       libxl_device_usbctrl *usbctrl) 
> > +{ 
> > +    flexarray_t *front; 
> > +    flexarray_t *back; 
> > +    libxl__device *device; 
> > +    xs_transaction_t t = XBT_NULL; 
> > +    int rc = 0; 
> > +    libxl_domain_config d_config; 
> > +    libxl_device_usbctrl usbctrl_saved; 
> > +    libxl__domain_userdata_lock *lock = NULL; 
> > + 
> > +    libxl_domain_config_init(&d_config); 
> > +    libxl_device_usbctrl_init(&usbctrl_saved); 
> > +    libxl_device_usbctrl_copy(CTX, &usbctrl_saved, usbctrl); 
> > + 
> > +    GCNEW(device); 
> > +    libxl__device_from_usbctrl(gc, domid, usbctrl, device); 
> > + 
> > +    front = flexarray_make(gc, 4, 1); 
> > +    back = flexarray_make(gc, 12, 1); 
> > + 
> > +    flexarray_append_pair(back, "frontend-id", GCSPRINTF("%d", domid)); 
> > +    flexarray_append_pair(back, "online", "1"); 
> > +    flexarray_append_pair(back, "state", "1"); 
> > +    flexarray_append_pair(back, "usb-ver", GCSPRINTF("%d",  
> usbctrl->version)); 
> > +    flexarray_append_pair(back, "num-ports", GCSPRINTF("%d",  
> usbctrl->ports)); 
> > +    flexarray_append_pair(front, "backend-id", GCSPRINTF("%d",  
> usbctrl->backend_domid)); 
>  
> Line too long. 
>  
> > +    flexarray_append_pair(front, "state", "1"); 
> > + 
> > +    lock = libxl__lock_domain_userdata(gc, domid); 
> > +    if (!lock) { 
> > +        rc = ERROR_LOCK_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    rc = libxl__get_domain_configuration(gc, domid, &d_config); 
> > +    if (rc) goto out; 
> > + 
> > +    DEVICE_ADD(usbctrl, usbctrls, domid, &usbctrl_saved, COMPARE_USBCTRL,  
> &d_config); 
> > + 
>  
> Line too long. 
>  
> > +    for (;;) { 
> > +        rc = libxl__xs_transaction_start(gc, &t); 
> > +        if (rc) goto out; 
> > + 
> > +        rc = libxl__device_exists(gc, t, device); 
> > +        if (rc < 0) goto out; 
> > +        if (rc == 1) { 
> > +            /* already exists in xenstore */ 
> > +            LOG(ERROR, "device already exists in xenstore"); 
> > +            rc = ERROR_DEVICE_EXISTS; 
> > +            goto out; 
> > +        } 
> > + 
> > +        rc = libxl__set_domain_configuration(gc, domid, &d_config); 
> > +        if (rc) goto out; 
> > + 
> > +        libxl__device_generic_add(gc, t, device, 
> > +                                  libxl__xs_kvs_of_flexarray(gc, back, 
> > +                                                             back->count), 
> > +                                  libxl__xs_kvs_of_flexarray(gc, front, 
> > +                                                             
> > front->count), 
> > +                                  NULL); 
> > +        libxl__usbport_add_xenstore(gc, t, domid, usbctrl); 
> > +        rc = libxl__xs_transaction_commit(gc, &t); 
> > +        if (!rc) break; 
> > +        if (rc < 0) goto out; 
> > +    } 
> > + 
>  
> You don't have aodev so you cannot check update_json. Maybe you need 
> aodev? 
>  
> That field update_json is set to true when doing hotplug. It's set to 
> false during domain creation. 
>  
> The same comment applies to other add functions as well.

Thanks, I'm updating that. But maybe like pci_add and pci_remove functions,
will add a 'starting' flag to indicate hotplug or creation.
Looking at DEFINE_DEVICE_ADD and DEFINE_DEVICE_REMOVE, usbctrl_add
and usb_add can use DEFINE_DEVICE_ADD; but usbctrl_remove and usb_remove
cannot use DEFINE_DEVICE_REMOVE directly, need some extra handling. So,
finally turned to not use these macros but refer to pci functions.

>  
> > +out: 
> > +    if (lock) libxl__unlock_domain_userdata(lock); 
> > +    libxl_device_usbctrl_dispose(&usbctrl_saved); 
> > +    libxl_domain_config_dispose(&d_config); 
> > +    return rc; 
> > +} 
> > + 
> > +int libxl__device_usbctrl_add(libxl__gc *gc, uint32_t domid, 
> > +                              libxl_device_usbctrl *usbctrl) 
> > +{ 
> > +    int rc = 0; 
> > + 
> > +    rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl); 
> > +    if (rc < 0) goto out; 
> > + 
> > +    if (usbctrl->devid == -1) { 
> > +        usbctrl->devid = libxl__device_nextid(gc, domid, "vusb"); 
> > +        if (usbctrl->devid < 0) { 
> > +            rc = ERROR_FAIL; 
> > +            goto out; 
> > +        } 
> > +    } 
> > + 
> > +    if (libxl__usbctrl_add_xenstore(gc, domid, usbctrl) < 0){ 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +out: 
> > +    return rc; 
> > +} 
> > + 
> > +int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid, 
> > +                             libxl_device_usbctrl *usbctrl, 
> > +                             const libxl_asyncop_how *ao_how) 
> > +{ 
> > +    AO_CREATE(ctx, domid, ao_how); 
> > +    int rc; 
> > + 
> > +    rc = libxl__device_usbctrl_add(gc, domid, usbctrl); 
>  
> Hmm... Your remove function is async while this one is sync, why?

Hmm, maybe not proper here, just referred to pci_add implementation.
Current calling places only use sync mode.
 
>  
> > +    libxl__ao_complete(egc, ao, rc); 
> > +    return AO_INPROGRESS; 
> > +} 
> > + 
> > +static int libxl__device_usb_list(libxl__gc *gc, uint32_t domid, int  
> usbctrl, 
> > +                                  libxl_device_usb **usbs, int *num); 
> > + 
> > +static int do_usb_remove(libxl__gc *gc, uint32_t domid, 
> > +                         libxl_device_usb *usb); 
> > + 
> > +static int 
> > +libxl__device_usbctrl_remove_common(libxl_ctx *ctx, uint32_t domid, 
> > +                                    libxl_device_usbctrl *usbctrl, 
> > +                                    const libxl_asyncop_how *ao_how, 
> > +                                    int force) 
> > +{ 
> > +    AO_CREATE(ctx, domid, ao_how); 
> > +    libxl__device *device; 
> > +    libxl__ao_device *aodev; 
> > +    libxl_device_usbctrl *usbctrls = NULL; 
> > +    libxl_device_usbctrl *usbctrl_find = NULL; 
> > +    int numctrl = 0; 
> > +    libxl_device_usb *usbs = NULL; 
> > +    int numusb = 0; 
> > +    int i, rc; 
> > + 
> > +    assert(usbctrl->devid != -1); 
> > + 
> > +    usbctrls = libxl_device_usbctrl_list(ctx, domid, &numctrl); 
> > +    if (!numctrl) { 
> > +        LOG(ERROR, "No USB controller exists in this domain"); 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    for (i = 0; i < numctrl; i++) { 
> > +        if (usbctrl->devid == usbctrls[i].devid) { 
> > +            usbctrl_find = usbctrls + i; 
> > +            break; 
> > +        } 
> > +    } 
> > + 
> > +    if (!usbctrl_find) { 
> > +        LOG(ERROR, "USB controller %d is not attached to this domain", 
> > +            usbctrl->devid); 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    usbctrl = usbctrl_find; 
> > + 
> > +    GCNEW(device); 
> > +    libxl__device_from_usbctrl(gc, domid, usbctrl, device); 
> > + 
> > +    /* Remove usb devives first */ 
> > +    rc  = libxl__device_usb_list(gc, domid, usbctrl->devid, &usbs, 
> > &numusb); 
> > +    if (rc) goto out; 
> > +    for (i = 0; i < numusb; i++) { 
> > +        if (do_usb_remove(gc, domid, &usbs[i])) { 
> > +            LOG(ERROR, "do_usb_remove failed"); 
> > +            rc = ERROR_FAIL; 
> > +            goto out; 
> > +        } 
> > +    } 
> > +    /* remove usbctrl */ 
> > +    GCNEW(aodev); 
> > +    libxl__prepare_ao_device(ao, aodev); 
> > +    aodev->action = LIBXL__DEVICE_ACTION_REMOVE; 
> > +    aodev->dev = device; 
> > +    aodev->callback = device_addrm_aocomplete; 
> > +    aodev->force = force; 
> > +    libxl__initiate_device_remove(egc, aodev); 
> > + 
> > +out: 
> > +    free(usbctrls); 
> > +    free(usbs); 
> > +    if(rc) return AO_ABORT(rc); 
> > +    return AO_INPROGRESS; 
> > +} 
> > + 
> > +int libxl_device_usbctrl_remove(libxl_ctx *ctx, uint32_t domid, 
> > +                                libxl_device_usbctrl *usbctrl, 
> > +                                const libxl_asyncop_how *ao_how) 
> > +{ 
> > +    return libxl__device_usbctrl_remove_common(ctx, domid, usbctrl,  
> ao_how, 0); 
> > +} 
> > + 
> > +int libxl_device_usbctrl_destroy(libxl_ctx *ctx, uint32_t domid, 
> > +                                 libxl_device_usbctrl *usbctrl, 
> > +                                 const libxl_asyncop_how *ao_how) 
> > +{ 
> > +    return libxl__device_usbctrl_remove_common(ctx, domid, usbctrl,  
> ao_how, 1); 
> > +} 
> > + 
> > +libxl_device_usbctrl * 
> > +libxl_device_usbctrl_list(libxl_ctx *ctx, uint32_t domid, int *num) 
> > +{ 
> > +    GC_INIT(ctx); 
> > +    libxl_device_usbctrl *usbctrls = NULL; 
> > +    char *fe_path = NULL; 
> > +    char **dir = NULL; 
> > +    unsigned int ndirs = 0; 
> > + 
> > +    *num = 0; 
> > + 
> > +    fe_path = GCSPRINTF("%s/device/vusb", 
> > +                        libxl__xs_get_dompath(gc, domid)); 
> > +    dir = libxl__xs_directory(gc, XBT_NULL, fe_path, &ndirs); 
> > + 
> > +    if (dir && ndirs) { 
> > +        usbctrls = malloc(sizeof(*usbctrls) * ndirs); 
> > +        libxl_device_usbctrl* usbctrl; 
> > +        libxl_device_usbctrl* end = usbctrls + ndirs; 
> > +        for(usbctrl = usbctrls; usbctrl < end; usbctrl++, dir++, (*num)++) 
> >  
> { 
> > +            char *tmp; 
> > +            const char *be_path = libxl__xs_read(gc, XBT_NULL, 
> > +                                    GCSPRINTF("%s/%s/backend", fe_path,  
> *dir)); 
> > + 
> > +            libxl_device_usbctrl_init(usbctrl); 
> > +            usbctrl->devid = atoi(*dir); 
> > + 
> > +            tmp = libxl__xs_read(gc, XBT_NULL, 
> > +                                 GCSPRINTF("%s/%s/backend-id", fe_path,  
> *dir)); 
> > +            if (!tmp) goto outerr; 
> > +            usbctrl->backend_domid = atoi(tmp); 
> > + 
> > +            tmp = libxl__xs_read(gc, XBT_NULL, 
> > +                                 GCSPRINTF("%s/usb-ver", be_path)); 
> > +            if (!tmp) goto outerr; 
> > +            usbctrl->version = atoi(tmp); 
> > + 
> > +            tmp = libxl__xs_read(gc, XBT_NULL, 
> > +                                 GCSPRINTF("%s/num-ports", be_path)); 
> > +            if (!tmp) goto outerr; 
> > +            usbctrl->ports = atoi(tmp); 
> > +       } 
> > +    } 
> > + 
> > +    return usbctrls; 
> > + 
> > +outerr: 
> > +    LOG(ERROR, "Unable to list USB Controllers"); 
> > +    for (int i = 0; i < *num; i++) { 
> > +        libxl_device_usbctrl_dispose(usbctrls + i); 
> > +    } 
> > +    free(usbctrls); 
>  
> It might be useful to provide a function to free the whole list of 
> device. There are similar functions in libxl. 

Yeah, that would be useful.

>  
> > +    *num = 0; 
> > +    return NULL; 
> > +} 
> > + 
> > +int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid, 
> > +                                libxl_device_usbctrl *usbctrl, 
> > +                                libxl_usbctrlinfo *usbctrlinfo) 
> > +{ 
> > +    GC_INIT(ctx); 
> > +    char *dompath, *usbctrlpath; 
> > +    char *val; 
> > +    int rc = 0; 
> > + 
> > +    usbctrlinfo->devid = usbctrl->devid; 
> > +    usbctrlinfo->ports = usbctrl->ports; 
> > +    usbctrlinfo->version = usbctrl->version; 
> > + 
> > +    dompath = libxl__xs_get_dompath(gc, domid); 
> > +    usbctrlpath = GCSPRINTF("%s/device/vusb/%d", dompath,  
> usbctrlinfo->devid); 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/backend", usbctrlpath)); 
> > +    usbctrlinfo->backend = libxl__strdup(NOGC, val); 
> > +    if (!usbctrlinfo->backend) { 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/backend-id", usbctrlpath)); 
> > +    usbctrlinfo->backend_id = val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/state", usbctrlpath)); 
> > +    usbctrlinfo->state = val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/event-channel", usbctrlpath)); 
> > +    usbctrlinfo->evtch = val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/urb-ring-ref", usbctrlpath)); 
> > +    usbctrlinfo->ref_urb = val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/conn-ring-ref", usbctrlpath)); 
> > +    usbctrlinfo->ref_conn= val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/frontend", usbctrlinfo->backend)); 
> > +    usbctrlinfo->frontend = libxl__strdup(NOGC, val); 
> > + 
> > +    val = libxl__xs_read(gc, XBT_NULL, 
> > +                         GCSPRINTF("%s/frontend-id", 
> > usbctrlinfo->backend)); 
> > +    usbctrlinfo->frontend_id = val ? strtoul(val, NULL, 10) : -1; 
> > + 
> > +out: 
> > +    GC_FREE; 
> > +    return rc; 
> > +} 
> > + 
> > +/* usb device functions */ 
> > + 
> > +static int 
> > +libxl__device_usb_assigned_list(libxl__gc *gc, 
> > +                                libxl_device_usb **list, int *num) 
> > +{ 
> > +    char **domlist; 
> > +    unsigned int nd = 0, i, j; 
> > +    char *be_path; 
> > +    libxl_device_usb *usb; 
> > + 
> > +    *list = NULL; 
> > +    *num = 0; 
> > + 
> > +    domlist = libxl__xs_directory(gc, XBT_NULL, "/local/domain", &nd); 
> > +    be_path = GCSPRINTF("/local/domain/%d/backend/vusb",  
> LIBXL_TOOLSTACK_DOMID); 
> > +    for (i = 0; i < nd; i++) { 
> > +        char *path, *num_ports, **ctrl_list; 
> > +        unsigned int nc = 0; 
> > +        path = GCSPRINTF("%s/%s", be_path, domlist[i]); 
> > +        ctrl_list = libxl__xs_directory(gc, XBT_NULL, path , &nc); 
> > + 
> > +        for (j = 0; j < nc; j++) { 
> > +            path = GCSPRINTF("%s/%s/%s/num-ports", be_path, 
> > +                             domlist[i], ctrl_list[j]); 
> > +            num_ports = libxl__xs_read(gc, XBT_NULL, path); 
> > +            if (num_ports) { 
> > +                int nport = atoi(num_ports), k; 
> > +                char *devpath, *busid; 
> > + 
> > +                for (k = 1; k <= nport; k++) { 
> > +                    devpath = GCSPRINTF("%s/%s/%s/port/%u", be_path, 
> > +                                        domlist[i], ctrl_list[j], k); 
> > +                    busid = libxl__xs_read(gc, XBT_NULL, devpath); 
> > +                    /* If there are USB device attached, add it to list */ 
> > +                    if (busid && strcmp(busid, "") ) { 
> > +                        *list = realloc(*list, 
> > +                                  sizeof(libxl_device_usb) * ((*num) +  
> 1)); 
> > +                        if (*list == NULL) 
> > +                            return ERROR_NOMEM; 
>  
> Use GCREALLOC_ARRAY. 

Thanks, will update.

>  
> > +                        usb = *list + *num; 
> > +                        usb->ctrl = atoi(ctrl_list[j]); 
> > +                        usb->port = k; 
> > +                        usb->busid = strdup(busid); 
> > +                        (*num)++; 
> > +                    } 
> > +                } 
> > +            } 
> > +        } 
> > +    } 
> > +    libxl__ptr_add(gc, *list); 
> > + 
>  
> No need to do this if you use GCREALLOC_ARRAY. 
>  
> > +    return 0; 
> > +} 
> > + 
> > +static bool is_usb_in_array(libxl_device_usb *usbs, int num, 
> > +                            libxl_device_usb *usb) 
> > +{ 
> > +    int i; 
> > + 
> > +    for (i = 0; i < num; i++) { 
> > +        if (!strcmp(usbs[i].busid, usb->busid) ) 
> > +            return true; 
> > +    } 
> > + 
> > +    return false; 
> > +} 
> > + 
> > +/* check if USB device is already assigned to a domain */ 
> > +static bool is_usb_assigned(libxl__gc *gc, libxl_device_usb *usb) 
> > +{ 
> > +    libxl_device_usb *usbs; 
> > +    int rc, num; 
> > + 
> > +    rc = libxl__device_usb_assigned_list(gc, &usbs, &num); 
> > +    if (rc) { 
> > +        LOG(ERROR, "Fail to get assigned usb list"); 
> > +        return true; 
> > +    } 
> > + 
> > +    if (is_usb_in_array(usbs, num, usb)) 
> > +        return true; 
> > + 
> > +    return false; 
> > +} 
> > + 
> > +/* check if USB device type is assignable */ 
> > +static bool is_usb_assignable(libxl__gc *gc, libxl_device_usb *usb) 
> > +{ 
> > +    libxl_ctx *ctx = libxl__gc_owner(gc); 
> > +    int classcode; 
> > +    char *filename; 
> > +    void *buf; 
> > + 
> > +    assert(usb->busid); 
> > + 
> > +    filename = GCSPRINTF(SYSFS_USB_DEV"/%s/bDeviceClass", usb->busid); 
> > +    if (libxl_read_file_contents(ctx, filename, &buf, NULL) < 0) 
> > +        return false; 
> > + 
> > +    sscanf(buf, "%x", &classcode); 
> > +    if (classcode == USBHUB_CLASS_CODE) 
> > +        return false; 
> > + 
> > +    return true; 
> > +} 
> > + 
> > +/* get usb devices under certain usb controller */ 
> > +static int libxl__device_usb_list(libxl__gc *gc, uint32_t domid, int  
> usbctrl, 
> > +                                  libxl_device_usb **usbs, int *num) 
> > +{ 
> > +    char *be_path, *num_devs; 
> > +    int n, i; 
> > + 
> > +    *usbs = NULL; 
> > +    *num = 0; 
> > + 
> > +    be_path = GCSPRINTF("%s/backend/vusb/%d/%d", 
> > +                        libxl__xs_get_dompath(gc, 0), domid, usbctrl); 
> > +    num_devs = libxl__xs_read(gc, XBT_NULL, 
> > +                              GCSPRINTF("%s/num-ports", be_path)); 
> > +    if (!num_devs) 
> > +        return 0; 
> > + 
> > +    n = atoi(num_devs); 
> > +    *usbs = calloc(n, sizeof(libxl_device_usb)); 
> > + 
>  
> libxl__calloc(NOGC, ...) 
Will update.
>  
> > +    for (i = 0; i < n; i++) { 
> > +        char *busid; 
> > +        libxl_device_usb *usb = NULL; 
> > + 
> > +        busid = libxl__xs_read(gc, XBT_NULL, 
> > +                               GCSPRINTF("%s/port/%d", be_path, i + 1)); 
> > +        if (busid && strcmp(busid, "")) { 
> > +            usb = *usbs + *num; 
> > +            usb->ctrl = usbctrl; 
> > +            usb->port = i + 1; 
> > +            usb->busid = strdup(busid); 
> > +            (*num)++; 
> > +        } 
> > +    } 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +/* get all usb devices of the domain */ 
> > +static libxl_device_usb * 
> > +libxl_device_usb_list_all(libxl__gc *gc, uint32_t domid, int *num) 
> > +{ 
> > +    char **usbctrls; 
> > +    unsigned int nd, i, j; 
> > +    char *be_path; 
> > +    int rc; 
> > +    libxl_device_usb *usbs = NULL; 
> > + 
> > +    *num = 0; 
> > + 
> > +    be_path = GCSPRINTF("/local/domain/%d/backend/vusb/%d", 
> > +                        LIBXL_TOOLSTACK_DOMID, domid); 
> > +    usbctrls = libxl__xs_directory(gc, XBT_NULL, be_path, &nd); 
> > + 
> > +    for (i = 0; i < nd; i++) { 
> > +        int nc = 0; 
> > +        libxl_device_usb *tmp = NULL; 
> > +        rc = libxl__device_usb_list(gc, domid, atoi(usbctrls[i]), &tmp,  
> &nc); 
> > +        if (!nc) continue; 
> > + 
> > +        usbs = realloc(usbs, sizeof(libxl_device_usb)*((*num) + nc)); 
>  
> libxl__realloc 
>  
> > +        for(j = 0; j < nc; j++) { 
> > +            usbs[*num].ctrl = tmp[j].ctrl; 
> > +            usbs[*num].port = tmp[j].port; 
> > +            usbs[*num].busid = strdup(tmp[j].busid); 
> > +            (*num)++; 
> > +        } 
> > +        free(tmp); 
> > +    } 
> > +    return usbs; 
> > +} 
> > + 
> > +libxl_device_usb *libxl_device_usb_list(libxl_ctx *ctx, uint32_t domid, 
> > +                                        int usbctrl, int *num) 
> > +{ 
> > +    GC_INIT(ctx); 
> > +    libxl_device_usb *usbs = NULL; 
> > + 
> > +    libxl__device_usb_list(gc, domid, usbctrl, &usbs, num); 
> > + 
> > +    GC_FREE; 
> > +    return usbs; 
> > +} 
> > + 
> > +/* set default value */ 
> > +static char *usb_busaddr_to_busid(libxl__gc *gc, int bus, int addr) 
> > +{ 
> > +    libxl_ctx *ctx = libxl__gc_owner(gc); 
> > +    struct dirent *de; 
> > +    DIR *dir; 
> > +    char *busid = NULL; 
> > + 
> > +    if (bus < 1 || addr < 1) 
> > +        return NULL; 
> > + 
> > +    if (!(dir = opendir(SYSFS_USB_DEV))) 
> > +        return NULL; 
> > + 
> > +    while((de = readdir(dir))) { 
> > +        char *filename; 
> > +        void *buf; 
> > +        int busnum = -1; 
> > +        int devnum = -1; 
> > + 
> > +        if (!de->d_name) 
> > +            continue; 
> > + 
> > +        filename = GCSPRINTF(SYSFS_USB_DEV"/%s/devnum", de->d_name); 
> > +        if (!libxl_read_file_contents(ctx, filename, &buf, NULL)) 
> > +            sscanf(buf, "%x", &devnum); 
> > + 
> > +        filename = GCSPRINTF(SYSFS_USB_DEV"/%s/busnum", de->d_name); 
> > +        if (!libxl_read_file_contents(ctx, filename, &buf, NULL)) 
> > +            sscanf(buf, "%x", &busnum); 
> > + 
> > +        if (bus == busnum && addr == devnum) { 
> > +            busid = strdup(de->d_name); 
> > +            break; 
> > +        } 
> > +    } 
> > + 
> > +    closedir(dir); 
> > +    return busid; 
> > +} 
> > + 
> > +/* find first unused controller:port and give that to usb device */ 
> > +static int 
> > +libxl__device_usb_set_default_usbctrl(libxl__gc *gc, uint32_t domid, 
> > +                                      libxl_device_usb *usb) 
> > +{ 
> > +    libxl_ctx *ctx = CTX; 
> > +    libxl_device_usbctrl *usbctrls; 
> > +    libxl_device_usb *usbs = NULL; 
> > +    int numctrl, numusb, i, j, rc = -1; 
> > +    char *be_path, *tmp; 
> > + 
> > +    usbctrls = libxl_device_usbctrl_list(ctx, domid, &numctrl); 
> > +    if ( !numctrl) 
> > +        goto out; 
> > + 
> > +    for (i = 0; i < numctrl; i++) { 
> > +        rc = libxl__device_usb_list(gc, domid, usbctrls[i].devid, 
> > +                                    &usbs, &numusb); 
> > +        if (rc) continue; 
> > + 
> > +        if (!usbctrls[i].ports || numusb == usbctrls[i].ports) 
> > +            continue; 
> > + 
> > +        for (j = 1; i <= numusb; j++) { 
> > +            be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> > +                                libxl__xs_get_dompath(gc, 0), domid, 
> > +                                usbctrls[i].devid, j); 
> > +            tmp = libxl__xs_read(gc, XBT_NULL, be_path); 
> > +            if (tmp && !strcmp( tmp, "")) { 
> > +                usb->ctrl = usbctrls[i].devid; 
> > +                usb->port = j; 
> > +                break; 
> > +            } 
> > +        } 
> > +    } 
> > + 
> > +    rc = 0; 
> > + 
> > +out: 
> > +    if (usbctrls) 
> > +        free(usbctrls); 
> > +    if (usbs) 
> > +        free(usbs); 
> > +    return rc; 
> > +} 
> > + 
> > +static int libxl__device_usb_setdefault(libxl__gc *gc, uint32_t domid, 
> > +                                        libxl_device_usb *usb) 
> > +{ 
> > +    char *be_path, *tmp; 
> > + 
> > +    if (usb->ctrl == -1) { 
> > +        int ret = libxl__device_usb_set_default_usbctrl(gc, domid, usb); 
> > +        /* If no existing ctrl to host this usb device, setup a new one */ 
> > +        if (ret) { 
> > +            libxl_device_usbctrl usbctrl; 
> > +            libxl_device_usbctrl_init(&usbctrl); 
> > +            libxl__device_usbctrl_add(gc, domid, &usbctrl); 
> > +            usb->ctrl = usbctrl.devid; 
> > +            usb->port = 1; 
> > +            libxl_device_usbctrl_dispose(&usbctrl); 
> > +        } 
> > +    } 
> > + 
> > +    be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> > +                        libxl__xs_get_dompath(gc, 0), 
> > +                        domid, usb->ctrl, usb->port); 
> > +    tmp = libxl__xs_read(gc, XBT_NULL, be_path); 
> > +    if (!tmp || strcmp(tmp, "") ){ 
> > +        LOG(ERROR, "The controller port isn't available"); 
> > +        return ERROR_INVAL; 
> > +    } 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +/* xenstore usb data */ 
> > +static int libxl__device_usb_add_xenstore(libxl__gc *gc, uint32_t domid, 
> > +                                          libxl_device_usb *usb) 
> > +{ 
> > +    char *be_path; 
> > +    int rc; 
> > +    libxl_domain_config d_config; 
> > +    libxl_device_usb usb_saved; 
> > +    libxl__domain_userdata_lock *lock = NULL; 
> > + 
> > +    libxl_domain_config_init(&d_config); 
> > +    libxl_device_usb_init(&usb_saved); 
> > +    libxl_device_usb_copy(CTX, &usb_saved, usb); 
> > + 
> > +    lock = libxl__lock_domain_userdata(gc, domid); 
> > +    if (!lock) { 
> > +        rc = ERROR_LOCK_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    rc = libxl__get_domain_configuration(gc, domid, &d_config); 
> > +    if (rc) goto out; 
> > + 
> > +    DEVICE_ADD(usb, usbs, domid, &usb_saved, COMPARE_USB, &d_config); 
> > + 
> > +    rc = libxl__set_domain_configuration(gc, domid, &d_config); 
> > +    if (rc) goto out; 
> > + 
> > +    be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> > +                  libxl__xs_get_dompath(gc, 0), domid, usb->ctrl, 
> > usb->port); 
> > +    LOG(DEBUG, "Adding new usb device to xenstore"); 
> > +    if (libxl__xs_write_checked(gc, XBT_NULL, be_path, usb->busid)) { 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
>  
> This is not right. See libxl_internal.h, search for "lock json 
> config" to see the pattern. In short, you should use a xenstore 
> transaction instead of libxl__xs_write_checked with XBT_NULL. 
>  
> Also you need to use aodev->update_json. 
>  
> > +    rc = 0; 
> > + 
> > +out: 
> > +    if (lock) libxl__unlock_domain_userdata(lock); 
> > +    libxl_device_usb_dispose(&usb_saved); 
> > +    libxl_domain_config_dispose(&d_config); 
> > +    return rc; 
> > +} 
> > + 
> > +static int libxl__device_usb_remove_xenstore(libxl__gc *gc, uint32_t  
> domid, 
> > +                                             libxl_device_usb *usb) 
> > +{ 
> > +    char *be_path; 
> > + 
> > +    be_path = GCSPRINTF("%s/backend/vusb/%d/%d/port/%d", 
> > +                        libxl__xs_get_dompath(gc, 0), 
> > +                        domid, usb->ctrl, usb->port); 
> > +    LOG(DEBUG, "Removing USB device from xenstore"); 
> > +    if (libxl__xs_write_checked(gc,XBT_NULL, be_path, "")) 
> > +        return ERROR_FAIL; 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +/* bind/unbind usb device interface */ 
> > +static int unbind_usb_intf(libxl__gc *gc, char *intf, char **drvpath) 
> > +{ 
> > +    char *path, *spath, *dp = NULL; 
> > +    int fd = -1; 
> > +    int rc = 0; 
> > +    struct stat st; 
> > + 
> > +    spath = GCSPRINTF(SYSFS_USB_DEV"/%s/driver", intf); 
> > +    if (!lstat(spath, &st)) { 
> > +        /* Find the canonical path to the driver. */ 
> > +        dp = libxl__zalloc(gc, PATH_MAX); 
> > +        dp = realpath(spath, dp); 
> > + 
> > +        path = GCSPRINTF("%s/unbind", spath); 
> > +        fd = open(path, O_WRONLY); 
> > +        if (fd < 0) 
> > +            return ERROR_FAIL; 
> > +        rc = write(fd, intf, strlen(intf)); 
> > +        close(fd); 
> > +        if (rc < 0) 
> > +            return ERROR_FAIL; 
> > +    } 
> > + 
> > +    if (drvpath) 
> > +        *drvpath = dp; 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +static int bind_usb_intf(libxl__gc *gc, char *intf, char *drvpath) 
> > +{ 
> > +    char *path; 
> > +    struct stat st; 
> > +    int fd, rc = 0; 
> > + 
> > +    path = GCSPRINTF("%s/%s", drvpath, intf); 
> > +    rc = lstat(path, &st); 
> > +    /* already bind, return */ 
> > +    if(rc == 0) 
> > +        return 0; 
> > + 
> > +    path = GCSPRINTF("%s/bind", drvpath); 
> > +    fd = open(path, O_WRONLY); 
> > +    if (fd < 0) 
> > +        return ERROR_FAIL; 
> > + 
> > +    rc = write(fd, intf, strlen(intf)); 
> > +    close(fd); 
> > +    if (rc < 0) 
> > +        return ERROR_FAIL; 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +/* Is usb interface bound to usbback? */ 
> > +static int usb_intf_is_assigned(libxl__gc *gc, char *intf) 
> > +{ 
> > +    char *spath; 
> > +    int rc; 
> > +    struct stat st; 
> > + 
> > +    spath = GCSPRINTF(SYSFS_USBBACK_DRIVER"/%s", intf); 
> > +    rc = lstat(spath, &st); 
> > + 
> > +    if(rc == 0) 
> > +        return 1; 
> > +    if (rc < 0 && errno == ENOENT) 
> > +        return 0; 
> > +    LOGE(ERROR, "Accessing %s", spath); 
> > +    return -1; 
> > +} 
> > + 
> > +static int usb_get_all_interfaces(libxl__gc *gc, libxl_device_usb *usb, 
> > +                                  char ***intfs, int *num) 
> > +{ 
> > +    DIR *dir; 
> > +    struct dirent *entry; 
> > +    char *buf; 
> > +    int rc = 0; 
> > + 
> > +    *intfs = NULL; 
> > +    *num = 0; 
> > + 
> > +    buf = GCSPRINTF("%s:", usb->busid); 
> > + 
> > +    if (!(dir = opendir(SYSFS_USB_DEV))) { 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    while ((entry = readdir(dir)) != NULL) { 
> > +        if (!strncmp(entry->d_name, buf, strlen(buf))){ 
> > +            *intfs = realloc(*intfs, sizeof(char *) * (*num + 1)); 
> > +            if (*intfs == NULL) { 
> > +                rc = ERROR_FAIL; 
> > +                goto out; 
> > +            } 
> > +            (*intfs)[*num] = strdup(entry->d_name); 
> > +            (*num)++; 
> > +        } 
> > +    } 
> > + 
> > +    closedir(dir); 
> > + 
> > +out: 
> > +    return rc; 
> > +} 
> > + 
> > +/* Cann't write '.' or ':' into Xenstore as key. So, change '.' to '_', 
>  
> "Can't" 
>  
> And this behaviour needs to be documented. 
>  
> > + * change ':' to '-'. 
> > + */ 
> > +static char *usb_interface_encode(char *busid) 
> > +{ 
> > +    char *str = strdup(busid); 
> > +    int i, len = strlen(str); 
> > + 
> > +    for (i = 0; i < len; i++) { 
> > +        if (str[i] == '.') 
> > +            str[i] = '_'; 
> > +         if (str[i] == ':') 
>  
> Indentation. 
>  
> > +            str[i] = '-'; 
> > +    } 
> > +    return str; 
> > +} 
> > + 
> > +/* unbind usb device from usbback driver, if there are many interfaces 
> > + * under the usb device, then check each interface, unbind from usbback 
> > + * driver and rebind to original driver 
> > + */ 
> > +static int usbback_dev_unassign(libxl__gc *gc, libxl_device_usb *usb) 
> > +{ 
> > +    char **intfs = NULL; 
> > +    char *path; 
> > +    int num = 0, i; 
> > +    int rc = 0; 
> > +    char *usb_encode = NULL; 
> > + 
> > +    if (usb_get_all_interfaces(gc, usb, &intfs, &num) < 0) 
> > +        return ERROR_FAIL; 
> > + 
> > +    usb_encode = usb_interface_encode(usb->busid); 
> > + 
> > +    for (i = 0; i < num; i++){ 
> > +        char *intf = intfs[i]; 
> > +        char *drvpath = NULL; 
> > + 
> > +        if (usb_intf_is_assigned(gc, intf) > 0) { 
> > +            /* unbind interface from usbback driver */ 
> > +            if (unbind_usb_intf(gc, intf, NULL) < 0) { 
> > +                rc = ERROR_FAIL; 
> > +                goto out; 
> > +            } 
> > +        } 
> > + 
> > +        /* bind interface to its originial driver */ 
> > +        drvpath = libxl__xs_read(gc, XBT_NULL, 
> > +                  GCSPRINTF(USBBACK_INFO_PATH"/%s/%s/driver_path", 
> > +                  usb_encode, usb_interface_encode(intf))); 
> > +        if (drvpath) { 
> > +            if (bind_usb_intf(gc, intf, drvpath) < 0) { 
> > +                LOGE(ERROR, "Couldn't bind %s to %s", intf, drvpath); 
> > +                rc = ERROR_FAIL; 
> > +                goto out; 
> > +            } 
> > +        } 
> > +    } 
> > + 
> > +    /* finally, remove xs driver path */ 
> > +    path = GCSPRINTF(USBBACK_INFO_PATH"/%s", usb_encode); 
> > +    libxl__xs_rm_checked(gc, XBT_NULL, path); 
> > + 
> > +out: 
> > +    if (intfs) { 
> > +        for (i = 0; i < num; i++) 
> > +            free(intfs[i]); 
> > +        free(intfs); 
> > +    } 
> > +    free(usb_encode); 
> > +    return rc; 
> > +} 
> > + 
> > +/* bind usb device to "usbback" driver, if there are many interfaces 
> > + * under the usb device, check each interface, unbind from original 
> > + * driver and bind to usbback driver. 
> > + */ 
> > +static int usbback_dev_assign(libxl__gc *gc, libxl_device_usb *usb) 
> > +{ 
> > +    char **intfs = NULL; 
> > +    int num = 0, i; 
> > +    int rc = 0; 
> > +    char *usb_encode = NULL; 
> > + 
> > +    if (usb_get_all_interfaces(gc, usb, &intfs, &num) < 0) 
> > +        return ERROR_FAIL; 
> > + 
> > +    usb_encode = usb_interface_encode(usb->busid); 
> > + 
> > +    for (i = 0; i < num; i++){ 
> > +        char *intf = intfs[i]; 
> > +        char *path = NULL; 
> > +        char *drvpath = NULL; 
> > + 
> > +        /* already assigned to usbback */ 
> > +        if (usb_intf_is_assigned(gc, intf) > 0) 
> > +            continue; 
> > + 
> > +        /* unbind interface from original driver */ 
> > +        if (unbind_usb_intf(gc, intf, &drvpath) < 0) { 
> > +            rc = ERROR_FAIL; 
> > +            goto out_rebind; 
> > +        } 
> > + 
> > +        if (drvpath) { 
> > +            /* write driver path to xenstore for later rebinding */ 
> > +            path = GCSPRINTF(USBBACK_INFO_PATH"/%s/%s/driver_path", 
> > +                             usb_encode, usb_interface_encode(intf)); 
> > +            if (libxl__xs_write_checked(gc, XBT_NULL, path, drvpath) < 0) 
> > { 
> > +                LOG(WARN, "Write of %s to node %s failed", drvpath, path); 
> > +            } 
> > +        } 
> > + 
> > +        /* bind interface to usbback */ 
> > +        if (bind_usb_intf(gc, intf, SYSFS_USBBACK_DRIVER) < 0){ 
> > +            LOGE(ERROR, "Couldn't bind %s to %s", intf,  
> SYSFS_USBBACK_DRIVER); 
> > +            rc = ERROR_FAIL; 
> > +            goto out_rebind; 
> > +        } 
> > +    } 
> > + 
> > +    goto out; 
> > + 
> > +out_rebind: 
> > +    /* some interfaces might be bound to usbback, unbind it then and 
> > +     * rebind to its original driver 
> > +     */ 
> > +    usbback_dev_unassign(gc, usb); 
> > +out: 
> > +    if (intfs) { 
> > +        for (i = 0; i < num; i++) 
> > +            free(intfs[i]); 
> > +        free(intfs); 
> > +    } 
> > +    free(usb_encode); 
> > +    return rc; 
> > +} 
> > + 
> > +static int do_usb_add(libxl__gc *gc, uint32_t domid, libxl_device_usb  
> *usb) 
> > +{ 
> > +    int rc = 0; 
> > + 
> > +    rc = libxl__device_usb_add_xenstore(gc, domid, usb); 
> > +    if (rc) goto out; 
> > + 
> > +    rc = usbback_dev_assign(gc, usb); 
> > +    if (rc) 
> > +        libxl__device_usb_remove_xenstore(gc, domid, usb); 
> > + 
> > +out: 
> > +    return rc; 
> > +} 
> > + 
> > +int libxl__device_usb_add(libxl__gc *gc, uint32_t domid, libxl_device_usb  
> *usb) 
> > +{ 
> > +    int rc; 
> > + 
> > +    usb->busid = usb_busaddr_to_busid(gc, usb->hostbus, usb->hostaddr); 
> > +    if (!usb->busid) { 
> > +        LOG(ERROR, "USB device doesn't exist in sysfs"); 
> > +        return ERROR_INVAL; 
> > +    } 
> > + 
> > +    if (!is_usb_assignable(gc, usb)) { 
> > +        LOG(ERROR, "USB device is not assignable."); 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    /* check usb device is already assigned by pvusb */ 
> > +    if (is_usb_assigned(gc, usb)) { 
> > +        LOG(ERROR, "USB device is already attached to a domain."); 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    rc = libxl__device_usb_setdefault(gc, domid, usb); 
> > +    if (rc) goto out; 
> > + 
> > +    rc = do_usb_add(gc, domid, usb); 
> > + 
> > +out: 
> > +    return rc; 
> > +} 
> > + 
> > +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid, 
> > +                         libxl_device_usb *usb, 
> > +                         const libxl_asyncop_how *ao_how) 
> > +{ 
> > +    AO_CREATE(ctx, domid, ao_how); 
> > +    int rc; 
> > + 
> > +    rc = libxl__device_usb_add(gc, domid, usb); 
>  
> Hmm... Your remove function is async and this one is sync. See comment 
> of usbctrl_add. 
>  
> > +    libxl__ao_complete(egc, ao, rc); 
> > +    return AO_INPROGRESS; 
> > +} 
> > + 
> > +static int do_usb_remove(libxl__gc *gc, uint32_t domid, 
> > +                         libxl_device_usb *usb) 
> > +{ 
> > +    if (libxl__device_usb_remove_xenstore(gc, domid, usb)) 
> > +        return -1; 
> > + 
> > +    usbback_dev_unassign(gc, usb); 
> > + 
> > +    return 0; 
> > +} 
> > + 
> > +static int libxl__device_usb_remove(libxl__gc *gc, uint32_t domid, 
> > +                                    libxl_device_usb *usb) 
> > +{ 
> > +    libxl_device_usb *usbs = NULL; 
> > +    libxl_device_usb *usb_find = NULL; 
> > +    int i, num, rc; 
> > + 
> > +    usb->busid = usb_busaddr_to_busid(gc, usb->hostbus, usb->hostaddr); 
> > +    if (!usb->busid) { 
> > +        LOG(ERROR, "USB device doesn't exist in sysfs"); 
> > +        return ERROR_INVAL; 
> > +    } 
> > + 
> > +    usbs = libxl_device_usb_list_all(gc, domid, &num); 
> > +    if (!usbs) { 
> > +       LOG(ERROR, "No USB device attached to this domain"); 
> > +       return ERROR_FAIL; 
> > +    } 
> > + 
> > +    for (i = 0; i < num; i++) { 
> > +        if (!strcmp(usb->busid, usbs[i].busid)) { 
> > +            usb_find = usbs + i; 
> > +            break; 
> > +        } 
> > +    } 
> > + 
> > +    /* doesn't find the usb device in domain's usb device list*/ 
> > +    if (!usb_find) { 
> > +        LOG(ERROR, "USB device %x.%x is not attached to this domain", 
> > +            usb->hostbus, usb->hostaddr); 
> > +        rc = ERROR_FAIL; 
> > +        goto out; 
> > +    } 
> > + 
> > +    rc = do_usb_remove(gc, domid, usb_find); 
> > + 
> > +out: 
> > +    free(usbs); 
> > +    return rc; 
> > +} 
> > + 
> > +int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid, 
> > +                            libxl_device_usb *usb, 
> > +                            const libxl_asyncop_how *ao_how) 
> > + 
> > +{ 
> > +    AO_CREATE(ctx, domid, ao_how); 
> > +    int rc; 
> > + 
> > +    rc = libxl__device_usb_remove(gc, domid, usb); 
> > + 
>  
> Same here. 
>  
> Wei. 
>  
>  


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.