Re: [PATCH v4 2/2] xl: Add commands for usb hot-plug

["Roger Pau Monné" <roger.pau@citrix.com>]

On 11/04/13 20:51, George Dunlap wrote:
> Signed-off-by: George Dunlap <george.dunlap@eu.citrix.com>
> CC: Ian Jackson <ian.jackson@citrix.com>
> CC: Roger Pau Monne <roger.pau@citrix.com>
> CC: sstanisi@cbnco.com
> ---
>  docs/man/xl.pod.1         |   30 +++++++
>  tools/libxl/xl.h          |    3 +
>  tools/libxl/xl_cmdimpl.c  |  219 +++++++++++++++++++++++++++++++++++++++++++++
>  tools/libxl/xl_cmdtable.c |   15 ++++
>  4 files changed, 267 insertions(+)
> 
> diff --git a/docs/man/xl.pod.1 b/docs/man/xl.pod.1
> index a0e298e..18a8eee 100644
> --- a/docs/man/xl.pod.1
> +++ b/docs/man/xl.pod.1
> @@ -1110,6 +1110,36 @@ List virtual network interfaces for a domain.
>  
>  =back
>  
> +=head2 USB DEVICES
> +
> +=over 4
> +
> +=item B<usb-add> I<-d domain-id> I<-v hosbus.hostaddr>
> +
> +Passes through the host USB device specified by I<hostbus.hostaddr>.  At
> +the moment this will only work for HVM domains via qemu.
> +
> +The best way to find out the information for the device is typically using
> +lsusb.
> +
> +This command is only available for domains using qemu-xen, not
> +qemu-traditional.
> +
> +=item B<usb-remove> I<-d domain-id> I<-v hosbus.hostaddr>
> +
> +Remove the host USB device from I<domain-id> which is specified
> +by <hostbus.hostaddr>.  This command only works for devices added
> +with usb-add; not for those specified in the config file.
> +
> +This command is only available for domains using qemu-xen, not
> +qemu-traditional.
> +
> +=item B<usb-list> I<domain-id>
> +
> +Show host USB devices assigned to the guest.
> +
> +=back
> +
>  =head2 VTPM DEVICES
>  
>  =over 4
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index b881f92..5c39fa2 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -35,6 +35,9 @@ int main_info(int argc, char **argv);
>  int main_sharing(int argc, char **argv);
>  int main_cd_eject(int argc, char **argv);
>  int main_cd_insert(int argc, char **argv);
> +int main_usb_add(int argc, char **argv);
> +int main_usb_remove(int argc, char **argv);
> +int main_usb_list(int argc, char **argv);
>  int main_console(int argc, char **argv);
>  int main_vncviewer(int argc, char **argv);
>  int main_pcilist(int argc, char **argv);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 61f7b96..a690823 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -2600,6 +2600,225 @@ int main_cd_insert(int argc, char **argv)
>      return 0;
>  }
>  
> +
> +
> +static int parse_usb_hostdev_specifier(libxl_device_usb *dev, const char *s)
> +{
> +    const char * hostbus, *hostaddr, *p;
> +
> +    hostbus = s;
> +    hostaddr=NULL;
> +
> +#define is_dec(_c) ((_c) >= '0' && (_c) <= '9')
> +#define is_hex(_c) (is_dec(_c) || ((_c) >= 'a' && (_c) <= 'f'))

This are kind of general macros, that could be used elsewhere, might be
suitable to put them outside of this function and name them CHAR_IS_DEC
and CHAR_IS_HEX.

> +
> +    /* Match [0-9]+\.[0-9] */
> +    if (!is_dec(*hostbus))
> +        return -1;
> +
> +    for(p=s; *p; p++) {
> +        if(*p == '.') {
> +            if ( !hostaddr )
> +                hostaddr = p+1;
> +            else {
> +                return -1;
> +            }
> +        } else if (!is_dec(*p)) {
> +            return -1;
> +        }
> +    }
> +    if (!hostaddr || !is_dec(*hostaddr))
> +        return -1;
> +    dev->u.hostdev.hostbus  = strtoul(hostbus,  NULL, 10);
> +    dev->u.hostdev.hostaddr = strtoul(hostaddr, NULL, 10);
> +#undef is_dec
> +#undef is_hex
> +
> +    return 0;
> +}
> +
> +static int usb_add(uint32_t domid, libxl_device_usb_type type,
> +                            const char * device)
> +{
> +    libxl_device_usb usbdev;
> +    int rc;
> +
> +    libxl_device_usb_init(&usbdev);
> +
> +    usbdev.type = type;
> +
> +    switch(type) {
> +    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
> +        if ( parse_usb_hostdev_specifier(&usbdev, device) < 0 ) {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        break;
> +    default:
> +        fprintf(stderr, "INTERNAL ERROR: Unimplemented type.\n");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    if ( (rc = libxl_device_usb_add(ctx, domid, &usbdev, NULL)) < 0 )
> +        fprintf(stderr, "libxl_usb_add failed.\n");
> +
> +    libxl_device_usb_dispose(&usbdev);
> +
> +out:
> +    return rc;
> +}
> +
> +int main_usb_add(int argc, char **argv)
> +{
> +    uint32_t domid = -1;

You could use INVALID_DOMID that is defined early in the file.

> +    int opt = 0, rc;
> +    const char *device = NULL;
> +    int type = 0;
> +
> +    SWITCH_FOREACH_OPT(opt, "d:v:", NULL, "usb-add", 0) {
> +    case 'd':
> +        domid = find_domain(optarg);
> +        break;
> +    case 'v':
> +        type = LIBXL_DEVICE_USB_TYPE_HOSTDEV;
> +        device = optarg;
> +        break;
> +    }
> +
> +    if ( domid == -1 ) {
> +        fprintf(stderr, "Must specify domid\n\n");
> +        help("usb-add");
> +        return 2;
> +    }
> +
> +    if ( !device ) {
> +        fprintf(stderr, "Must specify a device\n\n");
> +        help("usb-add");
> +        return 2;
> +    }
> +
> +    rc = usb_add(domid, type, device);
> +    if ( rc < 0 )
> +        return 1;
> +    else
> +        return 0;
> +}
> +
> +static int usb_remove(uint32_t domid, libxl_device_usb_type type,
> +                      const char * device)
> +{
> +    libxl_device_usb usbdev;
> +    int rc;
> +
> +    libxl_device_usb_init(&usbdev);
> +
> +    usbdev.type = type;
> +
> +    switch(type) {
> +    case LIBXL_DEVICE_USB_TYPE_HOSTDEV:
> +        if ( parse_usb_hostdev_specifier(&usbdev, device) < 0 ) {
> +            rc = ERROR_FAIL;
> +            goto out;
> +        }
> +        break;
> +    default:
> +        fprintf(stderr, "INTERNAL ERROR: Unimplemented type.\n");
> +        rc = ERROR_FAIL;
> +        goto out;
> +    }
> +
> +    if ( (rc = libxl_device_usb_remove(ctx, domid, &usbdev, NULL)) < 0 )
> +        fprintf(stderr, "libxl_usb_remove failed.\n");
> +
> +    libxl_device_usb_dispose(&usbdev);
> +
> +out:
> +    return rc;
> +}
> +
> +int main_usb_remove(int argc, char **argv)
> +{
> +    uint32_t domid = -1;

INVALID_DOMID

> +    int opt = 0, rc;
> +    const char *device = NULL;
> +    int type = 0;
> +
> +    SWITCH_FOREACH_OPT(opt, "d:v:", NULL, "usb-remove", 0) {
> +    case 'd':
> +        domid = find_domain(optarg);
> +        break;
> +    case 'v':
> +        type = LIBXL_DEVICE_USB_TYPE_HOSTDEV;
> +        device = optarg;
> +        break;
> +    }
> +
> +    if ( domid == -1 ) {
> +        fprintf(stderr, "Must specify domid\n\n");
> +        help("usb-remove");
> +        return 2;
> +    }
> +
> +    if ( !device ) {
> +        fprintf(stderr, "Must specify a device\n\n");
> +        help("usb-remove");
> +        return 2;
> +    }
> +
> +    rc = usb_remove(domid, type, device);
> +    if ( rc < 0 )
> +        return 1;
> +    else
> +        return 0;
> +}
> +
> +static void usb_list(uint32_t domid)
> +{
> +    libxl_device_usb *dev;
> +    int num, i;
> +
> +    dev = libxl_device_usb_list(ctx, domid, &num);
> +    if (dev == NULL)
> +        return;
> +    printf("protocol  backend  type     device\n");
> +    for (i = 0; i < num; i++) {
> +        printf("%8s  ", (dev[i].protocol==LIBXL_USB_PROTOCOL_PV)?"pv":"dm");
> +        printf("%7d  ", dev[i].backend_domid);
> +        printf("%7s  ", (dev[i].type==LIBXL_DEVICE_USB_TYPE_HOSTDEV)?"hostdev":"unknown");
> +        if(dev[i].type == LIBXL_DEVICE_USB_TYPE_HOSTDEV)
> +            printf("%03d.%03d",
> +                   dev[i].u.hostdev.hostbus,
> +                   dev[i].u.hostdev.hostaddr);
> +        printf("\n");
> +    }
> +    free(dev);
> +}
> +
> +
> +int main_usb_list(int argc, char **argv)
> +{
> +    uint32_t domid = -1;

INVALID_DOMID

> +    int opt;
> +
> +    SWITCH_FOREACH_OPT(opt, "d:", NULL, "usb-list", 0) {
> +    case 'd':
> +        domid = find_domain(optarg);
> +        break;
> +    }
> +
> +    if ( domid == -1 ) {
> +        fprintf(stderr, "Must specify domid\n\n");
> +        help("usb-remove");
> +        return 2;
> +    }
> +
> +    usb_list(domid);
> +    return 0;
> +}
> +
> +
> +
>  int main_console(int argc, char **argv)
>  {
>      uint32_t domid;
> diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
> index b4a87ca..3cf8e65 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -187,6 +187,21 @@ struct cmd_spec cmd_table[] = {
>        "Eject a cdrom from a guest's cd drive",
>        "<Domain> <VirtualDevice>",
>      },
> +    { "usb-add",
> +      &main_usb_add, 1, 1,
> +      "Hot-plug a usb device to a domain.",
> +      "-d <Domain> [-v <hostbus.hostaddr>]",
> +    },
> +    { "usb-remove",
> +      &main_usb_remove, 1, 1,
> +      "Hot-unplug a usb device from a domain.",
> +      "-d <Domain> [-v <hostbus.hostaddr>]",
> +    },
> +    { "usb-list",
> +      &main_usb_list, 0, 0,
> +      "List usb devices for a domain",
> +      "<Domain>",
> +    },
>      { "mem-max",
>        &main_memmax, 0, 1,
>        "Set the maximum amount reservation for a domain",
>