From mboxrd@z Thu Jan  1 00:00:00 1970
From: =?windows-1252?Q?J=FCrgen_Gro=DF?= <jgross@suse.com>
Subject: Re: [PATCH RFC V2 4/5] xl: add pvusb commands
Date: Tue, 10 Feb 2015 07:25:28 +0100
Message-ID: <54D9A458.5060603@suse.com>
References: <1421656131-19366-1-git-send-email-cyliu@suse.com>
	<1421656131-19366-5-git-send-email-cyliu@suse.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"; Format="flowed"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1421656131-19366-5-git-send-email-cyliu@suse.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Chunyan Liu <cyliu@suse.com>, xen-devel@lists.xen.org
Cc: george.dunlap@eu.citrix.com, lars.kurth@citrix.com, caobosimon@gmail.com, ian.campbell@citrix.com, ian.jackson@citrix.com
List-Id: xen-devel@lists.xenproject.org

On 01/19/2015 09:28 AM, Chunyan Liu wrote:
> Add pvusb commands.
>
> 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 all assignable usb devices now in host, with their
>   sysfs interface. (This is very useful since later we will use
>   sysfs interface to attach a usb devie to guest)
>
>   #xl usb-attach test_vm 2-1.1
>   will find the first usable controller:port, and attach usb
>   device with sysfs interface 2-1.1 (sys/bus/usb/devices/2-1.1)
>   to it. One could also specify which <controller> and which <port>
>
>   #xl usb-detach test_vm 2-1.1
>
>   #xl usb-ctrl-detach test_vm dev_id
>   will destroy the controller with dev_id as specified. Dev_id
>   can be traced in usb-list info

Sorry for late review.

Some comments inline.

>
> Signed-off-by: Chunyan Liu <cyliu@suse.com>
> Signed-off-by: Simon Cao <caobosimon@gmail.com>
> ---
>   tools/libxl/xl.h          |   6 ++
>   tools/libxl/xl_cmdimpl.c  | 256 ++++++++++++++++++++++++++++++++++++++++++++++
>   tools/libxl/xl_cmdtable.c |  30 ++++++
>   3 files changed, 292 insertions(+)
>
> diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
> index 5bc138c..f37a99f 100644
> --- a/tools/libxl/xl.h
> +++ b/tools/libxl/xl.h
> @@ -86,6 +86,12 @@ int main_blockdetach(int argc, char **argv);
>   int main_vtpmattach(int argc, char **argv);
>   int main_vtpmlist(int argc, char **argv);
>   int main_vtpmdetach(int argc, char **argv);
> +int main_usbassignable_list(int argc, char **argv);
> +int main_usbctrl_attach(int argc, char **argv);
> +int main_usbctrl_detach(int argc, char **argv);
> +int main_usbattach(int argc, char **argv);
> +int main_usbdetach(int argc, char **argv);
> +int main_usblist(int argc, char **argv);
>   int main_uptime(int argc, char **argv);
>   int main_claims(int argc, char **argv);
>   int main_tmem_list(int argc, char **argv);
> diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
> index 0b02a6c..a28f460 100644
> --- a/tools/libxl/xl_cmdimpl.c
> +++ b/tools/libxl/xl_cmdimpl.c
> @@ -3000,6 +3000,262 @@ int main_cd_insert(int argc, char **argv)
>       return 0;
>   }
>
> +static void usbinfo_print(libxl_device_usb *usbs, int num) {
> +    int i;
> +    if ( usbs == NULL )
> +         return;
> +    libxl_usbinfo usbinfo;
> +    for (i = 0; i < num; i++) {
> +        /* TO BE Improved */
> +        if (usbs[i].port )
> +            printf("Port %d:", usbs[i].port);
> +        printf("Interface %8s ", usbs[i].intf);
> +        if (!libxl_device_usb_getinfo(ctx, usbs[i].intf, &usbinfo)) {
> +            printf("Bus %03d Dev %03d: %04d:%04d %s %s\n",
> +                    usbinfo.bus, usbinfo.devnum, usbinfo.idVendor,
> +                    usbinfo.idProduct, usbinfo.manuf, usbinfo.prod);
> +        }
> +        libxl_usbinfo_dispose(&usbinfo);
> +    }
> +}
> +
> +
> +static void usb_assignable_list(void)
> +{
> +    libxl_device_usb *usbs;
> +    int num;
> +
> +    usbs = libxl_device_usb_assignable_list(ctx, &num);
> +
> +    usbinfo_print(usbs, num);
> +
> +    free(usbs);
> +}
> +
> +int main_usbassignable_list(int argc, char **argv)
> +{
> +    int opt;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-assignable-list", 0) {
> +        /* No options */
> +    }
> +
> +    usb_assignable_list();
> +    return 0;
> +}
> +
> +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-controller-attach", 1) {

Should be "usb-ctrl-attach".

> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind++]);
> +
> +    libxl_device_usbctrl_init(&usbctrl);
> +
> +    while (argc >= optind) {

argc > optind?

> +        if (MATCH_OPTION("type",  argv[optind], oparg)) {
> +            if (!strcmp("pv", oparg)) {
> +                usbctrl.type = LIBXL_USBCTRL_TYPE_PV;
> +            } else if(!strcmp("ioemu", oparg)) {
> +                usbctrl.type = LIBXL_USBCTRL_TYPE_DEVICEMODEL;
> +            } else {
> +                fprintf(stderr, "Invalid parameter `type'.\n");
> +                exit(-1);
> +            }
> +        } else if (MATCH_OPTION("version", argv[optind], oparg)) {
> +            usbctrl.usb_version = atoi(oparg);
> +        } else if (MATCH_OPTION("num_ports", argv[optind], oparg)) {
> +            usbctrl.num_ports = atoi(oparg);
> +        }  else {
> +            fprintf(stderr, "unrecognized argument `%s'\n", argv[optind]);
> +            exit(-1);
> +        }
> +        optind++;
> +    }
> +
> +    if (dryrun_only) {
> +       char* json = libxl_device_usbctrl_to_json(ctx, &usbctrl);
> +       printf("usb controller: %s\n", json);
> +       free(json);
> +       libxl_device_usbctrl_dispose(&usbctrl);
> +       if (ferror(stdout) || fflush(stdout)) {
> +           perror("stdout");
> +           exit(-1);
> +       }
> +       return 0;
> +    }
> +
> +    if (libxl_device_usbctrl_add(ctx, domid, &usbctrl, 0)) {
> +        fprintf(stderr, "libxl_device_usbctrl_add failed.\n");
> +        exit(-1);
> +    }
> +    libxl_device_usbctrl_dispose(&usbctrl);
> +    return 0;
> +}
> +
> +int main_usbctrl_detach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int devid;
> +    int opt;
> +    libxl_device_usbctrl usbctrl;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-controller-detach", 2) {

Should be "usb-ctrl-detach".

> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind]);
> +    devid = atoi(argv[optind+1]);
> +
> +    libxl_device_usbctrl_init(&usbctrl);
> +
> +    if (libxl_devid_to_device_usbctrl(ctx, domid, devid, &usbctrl)) {
> +        fprintf(stderr, "Unknown usb controller %d.\n", devid);
> +        exit(-1);
> +    }
> +
> +    if(libxl_device_usbctrl_remove(ctx, domid, &usbctrl, 0)) {
> +        fprintf(stderr, "libxl_device_usbctrl_add failed.\n");
> +        exit(-1);
> +    }
> +    libxl_device_usbctrl_dispose(&usbctrl);
> +    return 0;
> +
> +}
> +
> +int main_usbattach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt;
> +    char *oparg;
> +    libxl_device_usb usb;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-attach", 2) {
> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind++]);
> +    libxl_device_usb_init(&usb);
> +    replace_string(&usb.intf, argv[optind++]);
> +    while (argc >= optind) {

argc > optind?

> +        if (MATCH_OPTION("controller", argv[optind], oparg)) {
> +            usb.ctrl = atoi(oparg);
> +        } else if (MATCH_OPTION("port", argv[optind], oparg)) {
> +            usb.port = atoi(oparg);
> +        } else {
> +            fprintf(stderr, "unrecognized argument `%s'\n", argv[optind]);
> +            exit(-1);
> +        }
> +        optind++;
> +    }
> +
> +    if (dryrun_only) {
> +        char *json = libxl_device_usb_to_json(ctx, &usb);
> +        printf("usb: %s\n", json);
> +        free(json);
> +        libxl_device_usb_dispose(&usb);
> +        if (ferror(stdout) || fflush(stdout)) {
> +            perror("stdout");
> +            exit(-1);
> +        }
> +        return 0;
> +    }
> +
> +    if (libxl_device_usb_add(ctx, domid, &usb, 0)) {
> +        fprintf(stderr, "libxl_device_usb_add failed.\n");
> +        exit(-1);
> +    }
> +
> +    libxl_device_usb_dispose(&usb);
> +    return 0;
> +}
> +
> +int main_usbdetach(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    int opt;
> +    libxl_device_usb usb;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-detach", 2) {
> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind++]);
> +    libxl_device_usb_init(&usb);
> +    replace_string(&usb.intf, argv[optind++]);
> +
> +    if (argc > optind) {
> +        fprintf(stderr, "Invalid arguments.\n");
> +        exit(-1);
> +    }
> +
> +    if (libxl_intf_to_device_usb(ctx, domid, usb.intf, &usb) ) {
> +        fprintf(stderr, "libxl_intf_to_device_usb failed.\n");
> +        exit(-1);
> +    }
> +
> +    if (libxl_device_usb_remove(ctx, domid, &usb, 0) ) {
> +        fprintf(stderr, "libxl_device_usb_remove failed.\n");
> +        exit(-1);
> +    }
> +    libxl_device_usb_dispose(&usb);
> +    return 0;
> +}
> +
> +int main_usblist(int argc, char **argv)
> +{
> +    uint32_t domid;
> +    libxl_device_usbctrl *usbctrls;
> +    libxl_device_usb *usbs;
> +    libxl_usbctrlinfo usbctrlinfo;
> +    int numctrl, numusb, i, opt;
> +
> +    SWITCH_FOREACH_OPT(opt, "", NULL, "usb-list", 1) {
> +        /* No options */
> +    }
> +
> +    domid = find_domain(argv[optind++]);
> +
> +    if (argc > optind) {
> +        fprintf(stderr, "Invalid arguments.\n");
> +        exit(-1);
> +    }
> +
> +    usbctrls = libxl_device_usbctrl_list(ctx, domid, &numctrl);
> +    if (!usbctrls) {
> +        return 0;
> +    }
> +
> +    for (i = 0; i < numctrl; ++i) {
> +        printf("%-3s %-5s %-3s %-5s %-7s  %-30s\n",
> +                "Idx", "type", "BE", "state", "usb-ver", "BE-path");
> +
> +        if (!libxl_device_usbctrl_getinfo(ctx, domid,
> +                                &usbctrls[i], &usbctrlinfo)) {
> +            printf("%-3d %-5s %-3d %-5d %-7d %-30s\n",
> +                    i, usbctrlinfo.type, usbctrlinfo.backend_id,
> +                    usbctrlinfo.state, usbctrlinfo.version,
> +                    usbctrlinfo.backend );
> +
> +            usbs = libxl_device_usb_list(ctx, domid, usbctrlinfo.devid, &numusb);
> +            usbinfo_print(usbs, numusb);
> +
> +            libxl_usbctrlinfo_dispose(&usbctrlinfo);
> +        }
> +        libxl_device_usbctrl_dispose(&usbctrls[i]);
> +    }
> +
> +    free(usbctrls);
> +    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 4b30d3d..b29bc2e 100644
> --- a/tools/libxl/xl_cmdtable.c
> +++ b/tools/libxl/xl_cmdtable.c
> @@ -541,6 +541,36 @@ struct cmd_spec cmd_table[] = {
>         "\"cache_occupancy\":         Show L3 cache occupancy\n",
>       },
>   #endif
> +    { "usb-ctrl-attach",
> +      &main_usbctrl_attach, 1, 1,
> +      "Create a virtual USB controller for a domain",
> +      "<Domain> [version=<version>] [num_ports=<number>]",
> +    },
> +    { "usb-ctrl-detach",
> +      &main_usbctrl_detach, 0, 1,
> +      "Destory the virtual USB controller specified by <DevId> for a domain",

s/Destory/Destroy/

> +      "<Domain> <DevId>",
> +    },
> +    { "usb-attach",
> +      &main_usbattach, 1, 2,
> +      "Attach a USB device to a domain",
> +      "<Domain> <BusId> [controller=<DevId> port=<port>]",
> +    },
> +    { "usb-detach",
> +      &main_usbdetach, 0, 1,
> +      "Detach a USB device from a domain",
> +      "<Domain> <BusId>",
> +    },
> +    { "usb-list",
> +      &main_usblist, 0, 0,
> +      "List information about USB devices for a domain",
> +      "<Domain>",
> +    },
> +    { "usb-assignable-list",
> +      &main_usbassignable_list, 0, 0,
> +      "List all the assignable USB devices",
> +    },
> +
>   };
>
>   int cmdtable_len = sizeof(cmd_table)/sizeof(struct cmd_spec);
>


Juergen