From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH V4 5/7] xl: add pvusb commands Date: Fri, 12 Jun 2015 09:37:59 +0200 Message-ID: <557A8C57.1040108@suse.com> References: <1433906441-3280-1-git-send-email-cyliu@suse.com> <1433906441-3280-6-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: In-Reply-To: <1433906441-3280-6-git-send-email-cyliu@suse.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Chunyan Liu , xen-devel@lists.xen.org Cc: george.dunlap@eu.citrix.com, Ian.Jackson@eu.citrix.com, Simon Cao , wei.liu2@citrix.com, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org 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 and which . > > #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 > Signed-off-by: Simon Cao > --- > 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