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

2015-06-12 Thread Chun Yan Liu 


 On 6/12/2015 at 03:37 PM, in message 557a8c57.1040...@suse.com, Juergen 
 Gross
jgr...@suse.com wrote: 
 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. 

Oh, yes, I forget to split.

  
  
#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 cy...@suse.com 
  Signed-off-by: Simon Cao caobosi...@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? 

For assigned usb device, it's not possible. But this function will be called
in two places, one is to list assigned usb devices in 'xl usb-list'; another
is to list assignable usb devices in 'xl usb-assignable-list'. If
'usb-assignable-list' is not taken, this could be improved.

  
 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? 

OK. Will check.

  
  +} 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. 

Will add check. What's the supported max ports? 

  
  +} 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. 
Yes, seems not necessary. Anyway (from config file or hotplug), it will
call libxl_device_usbctrl_setdefault and do that.

Thanks,
Chunyan
  
  
 Juergen 
  
 ___ 
 Xen-devel mailing list 
 Xen-devel@lists.xen.org 
 http://lists.xen.org/xen-devel 
  
  


___
Xen-devel mailing list

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

2015-06-12 Thread Juergen Gross 

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

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

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

2015-06-12 Thread Juergen Gross 

On 06/12/2015 10:03 AM, Chun Yan Liu wrote:




On 6/12/2015 at 03:37 PM, in message 557a8c57.1040...@suse.com, Juergen Gross

jgr...@suse.com wrote:

On 06/10/2015 05:20 AM, Chunyan Liu wrote:


...


+} 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.


Will add check. What's the supported max ports?


31. I'm about to send a patch updating the pvusb interface description
which will then contain a define for that (USBIF_MAX_PORTNR).


Juergen


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

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

2015-06-09 Thread Chunyan Liu 
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-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 cy...@suse.com
Signed-off-by: Simon Cao caobosi...@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/docs/man/xl.pod.1 b/docs/man/xl.pod.1
index 4eb929d..0259b43 100644
--- a/docs/man/xl.pod.1
+++ b/docs/man/xl.pod.1
@@ -1387,6 +1387,44 @@ List pass-through pci devices for a domain.
 
 =back
 
+=head1 USB PASS-THROUGH
+
+=over 4
+
+=item Busb-ctrl-attach Idomain-id [Iversion=val] [Iports=number]
+
+Create a new USB controller for the specified domain.
+Bversion=val is the usb controller version, could be 1 (USB1.1) or 2 
(USB2.0).
+Bports=number is the total ports of the usb controller.
+By default, it will create a USB2.0 controller with 8 ports.
+
+=item Busb-ctrl-detach Idomain-id Idevid
+
+Destroy a USB controller from the specified domain.
+Bdevid is devid of the USB controller.
+
+=item Busb-attach Idomain-id Ibus.addr [Icontroller=devid 
[Iport=number]]
+
+Hot-plug a new pass-through USB device to the specified domain.
+Bbus.addr is the busnum.devnum of the physical USB device to pass-through.
+Bcontroller=devid Bport=number is the USB controller:port to hotplug the
+USB device to. By default, it will find the first available controller:port
+and use it; if there is no controller, it will create one.
+
+=item Busb-detach Idomain-id Ibus.addr
+
+Hot-unplug a previously assigned USB device from a domain. Bbus.addr is
+busnum.devnum of the physical USB device to be removed from the guest domain.
+
+If B-f is specified, Bxl is going to forcefully remove the device even
+without guest's collaboration.
+
+=item Busb-list Idomain-id
+
+List pass-through usb devices for a domain.
+
+=back
+
 =head1 TMEM
 
 =over 4
diff --git a/tools/libxl/xl.h b/tools/libxl/xl.h
index 5bc138c..2d57ee3 100644
--- a/tools/libxl/xl.h
+++ b/tools/libxl/xl.h
@@ -86,6 +86,11 @@ 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_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 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;
+if (usbs == NULL)
+ return;
+for (i = 0; i  num; i++) {
+libxl_usbinfo usbinfo;
+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 ?: );
+}
+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);
+} else if (MATCH_OPTION(ports, argv[optind],