All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Juergen Gross <jgross@suse.com>
To: Chunyan Liu <cyliu@suse.com>, xen-devel@lists.xen.org
Cc: george.dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com,
	Simon Cao <caobosimon@gmail.com>,
	wei.liu2@citrix.com, ian.campbell@citrix.com
Subject: Re: [PATCH V4 5/7] xl: add pvusb commands
Date: Fri, 12 Jun 2015 09:37:59 +0200	[thread overview]
Message-ID: <557A8C57.1040108@suse.com> (raw)
In-Reply-To: <1433906441-3280-6-git-send-email-cyliu@suse.com>

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@suse.com>
> Signed-off-by: Simon Cao <caobosimon@gmail.com>
> ---
>   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

  reply	other threads:[~2015-06-12  7:37 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-10  3:20 [PATCH V4 0/7] xen pvusb toolstack work Chunyan Liu
2015-06-10  3:20 ` [PATCH V4 1/7] libxl: export some functions for pvusb use Chunyan Liu
2015-06-11 16:08   ` Ian Jackson
2015-06-11 16:28     ` Wei Liu
2015-06-12 15:14       ` Ian Jackson
2015-06-10  3:20 ` [PATCH V4 2/7] libxl_read_file_contents: add new entry to read sysfs file Chunyan Liu
2015-06-11 16:16   ` Ian Jackson
2015-06-12  7:00     ` Chun Yan Liu
2015-06-12 15:11       ` Ian Jackson
2015-06-10  3:20 ` [PATCH V4 3/7] libxl: add pvusb API Chunyan Liu
2015-06-11 15:00   ` Juergen Gross
2015-06-11 16:07     ` Ian Jackson
2015-06-11 16:42   ` Ian Jackson
2015-06-12  7:39     ` Chun Yan Liu
2015-06-12  8:06       ` Chun Yan Liu
2015-06-12 11:22       ` Ian Jackson
2015-06-15 14:17   ` George Dunlap
2015-06-15 14:25     ` Jürgen Groß
2015-06-15 14:34       ` George Dunlap
2015-06-15 18:26         ` Juergen Gross
2015-06-16 10:30           ` George Dunlap
2015-06-16 10:51             ` Juergen Gross
2015-06-16 11:11               ` George Dunlap
2015-06-16 11:19                 ` Juergen Gross
2015-06-16 11:23                   ` George Dunlap
2015-06-16 11:44                     ` Ian Jackson
2015-06-17 11:24                       ` Ian Campbell
2015-06-18 11:50                         ` George Dunlap
2015-06-18 12:08                         ` George Dunlap
2015-06-18 13:03                           ` Juergen Gross
2015-06-22 13:29                           ` Proposed plan for libxl USB interface (was Re: [PATCH V4 3/7] libxl: add pvusb API) George Dunlap
2015-06-22 14:14                             ` Juergen Gross
2015-06-22 14:22                             ` Ian Jackson
2015-06-23  2:42                             ` Chun Yan Liu
2015-06-23  2:43                             ` Chun Yan Liu
2015-06-23  2:44                             ` Chun Yan Liu
2015-06-16 10:41     ` [PATCH V4 3/7] libxl: add pvusb API Ian Jackson
2015-06-16 10:56       ` Jürgen Groß
2015-06-16 11:03         ` George Dunlap
2015-06-16 11:10         ` Ian Jackson
2015-06-16 11:25           ` Juergen Gross
2015-06-16 11:45             ` George Dunlap
2015-06-16 12:02               ` Ian Jackson
2015-06-16 13:19                 ` George Dunlap
2015-06-16 13:32                   ` Juergen Gross
2015-06-16 13:37                   ` [PATCH V4 3/7] libxl: add pvusb API [and 1 more messages] Ian Jackson
2015-06-16 14:41                     ` George Dunlap
2015-06-16 15:58                       ` Sander Eikelenboom
2015-06-16 15:59                       ` Ian Jackson
2015-06-16 16:34                         ` George Dunlap
2015-06-17  3:59                           ` Juergen Gross
2015-06-17 10:27                             ` George Dunlap
2015-06-18  6:24                               ` Chun Yan Liu
2015-06-16 11:45             ` [PATCH V4 3/7] libxl: add pvusb API Ian Jackson
2015-06-16 13:06               ` Juergen Gross
2015-06-16 13:09                 ` George Dunlap
2015-06-16 13:23                   ` Juergen Gross
2015-06-16 13:29                     ` George Dunlap
2015-06-16 13:49                       ` Juergen Gross
2015-06-16 14:06                         ` George Dunlap
2015-06-16 14:20                           ` Juergen Gross
2015-06-16 14:37                             ` George Dunlap
2015-06-17 11:34                             ` Ian Campbell
2015-06-17 11:40                               ` Juergen Gross
2015-06-18  6:20                               ` Chun Yan Liu
2015-06-18  7:02                                 ` Juergen Gross
2015-06-18  8:50                                   ` Ian Campbell
2015-06-18 13:02                                     ` Juergen Gross
2015-06-16 15:38             ` George Dunlap
2015-06-16 11:01       ` George Dunlap
2015-06-16 11:12         ` Ian Jackson
2015-06-16 11:21           ` George Dunlap
2015-06-16 16:32             ` Ian Jackson
2015-06-16 16:39               ` George Dunlap
2015-06-16 16:51                 ` Ross Philipson
2015-06-17  4:03                   ` Jürgen Groß
2015-06-17 13:48                     ` Ross Philipson
2015-06-15 17:47   ` George Dunlap
2015-06-23 10:18     ` Chun Yan Liu
2015-06-23 11:29       ` George Dunlap
2015-06-24  2:26         ` Chun Yan Liu
2015-06-10  3:20 ` [PATCH V4 4/7] libxl: add libxl_device_usb_assignable_list API Chunyan Liu
2015-06-10  3:20 ` [PATCH V4 5/7] xl: add pvusb commands Chunyan Liu
2015-06-12  7:37   ` Juergen Gross [this message]
2015-06-12  8:03     ` Chun Yan Liu
2015-06-12  8:22       ` Juergen Gross
2015-06-10  3:20 ` [PATCH V4 6/7] xl: add usb-assignable-list command Chunyan Liu
2015-06-10  3:20 ` [PATCH V4 7/7] domcreate: support pvusb in configuration file Chunyan Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=557A8C57.1040108@suse.com \
    --to=jgross@suse.com \
    --cc=Ian.Jackson@eu.citrix.com \
    --cc=caobosimon@gmail.com \
    --cc=cyliu@suse.com \
    --cc=george.dunlap@eu.citrix.com \
    --cc=ian.campbell@citrix.com \
    --cc=wei.liu2@citrix.com \
    --cc=xen-devel@lists.xen.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.