Re: [Xen-devel] [PATCH V4 5/7] xl: add pvusb commands

[Juergen Gross <jgross@xxxxxxxx>]

On 06/10/2015 05:20 AM, Chunyan Liu wrote:
> Add pvusb commands: usb-ctrl-attach, usb-ctrl-detach, usb-list,
> usb-attach and usb-detach.
>
> To attach a usb device to guest through pvusb, one could follow
> following example:
>
>   #xl usb-ctrl-attach test_vm version=1 num_ports=8
>
>   #xl usb-list test_vm
>   will show the usb controllers and port usage under the domain.
>
>   #xl usb-assignable-list
>   will list assignable USB devices
xl usb-assignable-list is not part of this patch. Either merge this
patch and the following one, or describe the command in the next patch.

>   #xl usb-attach test_vm 1.6
>   will find the first usable controller:port, and attach usb
>   device whose bus address is 1.6 (busnum is 1, devnum is 6)
>   to it. One could also specify which <controller> and which <port>.
>
>   #xl usb-detach test_vm 1.6
>
>   #xl usb-ctrl-detach test_vm dev_id
>   will destroy the controller with specified dev_id. Dev_id
>   can be traced in usb-list info.
>
> Signed-off-by: Chunyan Liu <cyliu@xxxxxxxx>
> Signed-off-by: Simon Cao <caobosimon@xxxxxxxxx>
> ---
>   docs/man/xl.pod.1         |  38 +++++++
>   tools/libxl/xl.h          |   5 +
>   tools/libxl/xl_cmdimpl.c  | 251 ++++++++++++++++++++++++++++++++++++++++++++++
>   tools/libxl/xl_cmdtable.c |  25 +++++
>   4 files changed, 319 insertions(+)
...

> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index c858068..b29d0fc 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3219,6 +3219,257 @@ int main_cd_insert(int argc, char **argv)
>       return 0;
>   }
>
> +static void usbinfo_print(libxl_device_usb *usbs, int num) {
> +    int i;
Blank line missing.

> +    if (usbs == NULL)
> +         return;
> +    for (i = 0; i < num; i++) {
> +        libxl_usbinfo usbinfo;
Blank line missing.

> +        libxl_usbinfo_init(&usbinfo);
> +
> +        if (usbs[i].port)
> +            printf("  Port %d:", usbs[i].port);
> +        if (!libxl_device_usb_getinfo(ctx, &usbs[i], &usbinfo)) {
> +            printf(" Bus %03x Device %03x: ID %04x:%04x %s %s\n",
> +                    usbinfo.busnum, usbinfo.devnum,
> +                    usbinfo.idVendor, usbinfo.idProduct,
> +                    usbinfo.manuf ?: "", usbinfo.prod ?: "");
Is it really possible for a device to be assigned but without a port
number?

I'd rather combine the two if's and printf statements. This would avoid
the case where "  Port 1: Port 2: ..." is printed due to a failing
libxl_device_usb_getinfo() for port 1.

> +        }
> +        libxl_usbinfo_dispose(&usbinfo);
> +    }
> +}
> +
> +int main_usbctrl_attach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt;
> +    char *oparg;
> +    libxl_device_usbctrl usbctrl;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-ctrl-attach", 1) {
> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind++]);
> +
> +    libxl_device_usbctrl_init(&usbctrl);
> +
> +    while (argc > optind) {
> +        if (MATCH_OPTION("type", argv[optind], oparg)) {
> +            if (!strcmp(oparg, "pv")) {
> +               usbctrl.protocol = LIBXL_USB_PROTOCOL_PV;
> +            } else {
> +               fprintf(stderr, "unsupported type `%s'\n", oparg);
> +               exit(-1);
> +            }
> +        } else if (MATCH_OPTION("version", argv[optind], oparg)) {
> +            usbctrl.version = atoi(oparg);
Shouldn't you check for valid versions?

> +        } else if (MATCH_OPTION("ports", argv[optind], oparg)) {
> +            usbctrl.ports = atoi(oparg);
Same here for number of ports. Otherwise you could blow up xenstore by
e.g. specifying 2 billion ports here.

> +        } else {
> +            fprintf(stderr, "unrecognized argument `%s'\n", argv[optind]);
> +            exit(-1);
> +        }
> +        optind++;
> +    }
> +
> +    if (usbctrl.protocol == LIBXL_USB_PROTOCOL_AUTO)
> +        usbctrl.protocol = LIBXL_USB_PROTOCOL_PV;
Is this really necessary? You do it in libxl, too.


Juergen

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