Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-08-10 Thread George Dunlap 
On 08/07/2015 03:31 AM, Chun Yan Liu wrote:
 +(devid, libxl_devid),   
 +(version, integer),   
 +(ports, integer),   
 +(backend_domid, libxl_domid),   
 +(backend_domname, string),   
 +   ])   
 +   
 +libxl_device_usb = Struct(device_usb, [   
 +(ctrl, libxl_devid),   
 +(port, integer),   
 +(hostbus,   integer),   
 +(hostaddr,  integer),   
 +])   
  
 I think we do want to plan for the future here by doing something like this: 
  
 libxl_device_usb = Struct(device_usb, [ 
 (ctrl, libxl_devid), 
 (port, integer), 
 (u, KeyedUnion(None, libxl_device_usb_type, devtype, 
   [(hostdev, Struct(None, [ 
  (hostbus,   integer), 
  (hostaddr,  integer) ])) 
])) 
  ]) 
  
 
 Yes, that's the future look. For pvusb, currenlty with kernel pvusb driver, 
 the
 devtype is not really necessary. But I can add 'devtype' if it is preferred 
 now.

Yes, I think as much as possible we want the interface which is actually
checked in to be forward-compatible.

Thanks!
 -George


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

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-08-07 Thread Chun Yan Liu 


 On 8/7/2015 at 01:21 AM, in message 55c39796.8000...@citrix.com, George
Dunlap george.dun...@citrix.com wrote: 
 On 08/06/2015 04:11 AM, Chun Yan Liu wrote: 
  As 4.6 goes to bug fixing stage, maybe we can pick up this thread? :-) 
   
  Beside to call for your precious review comments and suggestions so that we 
   
 can 
  make progress, I also want to confirm about the previous discussed two TODO 
  things: 
  1) use UDEV name rule to specify usb device uniquely even across reboot.  
 That 
  got consensus. Next thing is exposing that name to some sysfs entry,  
 right? 
  
 So just to be clear, my understanding of the plan was that we try to fix 
 up the current patch series and check it in once the 4.7 window opens; 
 and that having the utility library to convert other names (including 
 this udev-style naming) into something libxl can use would be a separate 
 series. 
  
 I wasn't necessarily expecting you to work on it (since it wasn't your 
 idea), but if you're keen, I'm sure we'd be grateful. :-) 
  
  2) use libusb instead of reading sysfs by ourselves. As George mentioned,  
 using 
  libusb is not simpler than reading sysfs; and if UDEV name is stored to 
   
 some 
  sysfs entry for us to retrieve, then we still need reading sysfs  
 things. Could we 
  get to a final decision? 
  If these are settled down, I can update related code. 
  
 I don't think that libusb would be particularly useful for the current 
 pvusb code, since 
 1. It's already Linux-specific, 
 2. We need to mess around with sysfs anyway. 
  
 The same thing can't be said of the HVM path: I think it fairly likely 
 that the emulated pass-through will Just Work (or nearly so) on BSDs 
 (assuming that qemu itself works on the BSDs). 
  
 I think it we write our utility library for converting 
 vendorid:productid[:serialno], bus-port, c, then it might make sense to 
 use libusb if it makes it more portable. 
  
 Regarding the code: 
  
 Things are looking pretty close.  A couple of comments in-line: 
  
  diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c   
  index 93bb41e..9869a21 100644   
  --- a/tools/libxl/libxl_device.c   
  +++ b/tools/libxl/libxl_device.c   
  @@ -676,6 +676,10 @@ void libxl__devices_destroy(libxl__egc *egc,
  libxl__devices_remove_state *drs)   
   aodev-action = LIBXL__DEVICE_ACTION_REMOVE;   
   aodev-dev = dev;   
   aodev-force = drs-force;   
  +if (dev-backend_kind == LIBXL__DEVICE_KIND_VUSB) {   
  +libxl__initiate_device_usbctrl_remove(egc, aodev);  
   
  +continue;   
  +} 
  
 I take it the reason we need to special-case this is that we need to go 
 through and un-assign all of the devices inside the controller first? 
  
 At some point we probably want to generalize this a bit, so that usb 
 controllers and vscsi controllers look the same (rather than both being 
 special-cased). 
  
 But since this is internal, maybe we can wait for that design until we 
 actually have both types available. 
  
  +static int   
  +libxl__device_usb_assigned_list(libxl__gc *gc,   
  +libxl_device_usb **list, int *num)   
  +{   
  +char **domlist;   
  +unsigned int nd = 0, i, j;   
  +char *be_path;   
  +libxl_device_usb *usb;   
  +   
  +*list = NULL;   
  +*num = 0;   
  +   
  +domlist = libxl__xs_directory(gc, XBT_NULL, /local/domain, nd);  
   
  +be_path = GCSPRINTF(/local/domain/%d/backend/vusb,
  LIBXL_TOOLSTACK_DOMID);   
  
 Hmm, so this had made me realize that I don't think we're doing quite 
 the right thing for non-dom0 backends.  

For non-dom0 backends, here 'be_path' should be replaced with the right backend.

  
 First of all, things are a bit interesting for driver domain backends, 
 because the namespace of hostbus.hostaddr depends on the backend of 
 the virtual controller.  Which wouldn't be particularly interesting, 
 *except* that because the USB space is so dynamic, you normally have to 
 query the devices to find the hostbus.hostaddr; and you can't do any 
 queries from dom0 at the moment (except, for example, by ssh'ing into 
 the other vm).  So to even assign a hostbus.hostaddr device you have to 
 somehow learn, out-of-band, what those numbers are within the domain.

I think you are right. As for the network driver domain, when specifying vif,
we also need to know the bridge name in that driver domain, then we can use:
vif = [ 'bridge=xenbr0, mac=00:16:3E:0d:13:00, model=e1000, backend=domnet' ]

  
 Secondly, if the backend is in another domain, then all the stuff re 
 assigning a usb device to usbback can't be done by libxl in the 
 toolstack domain either.

Yes. I think so. Should be done in driver domain then.

  Which means I'm pretty sure this stuff will 
 fail at the moment for USB driver domains trying to assign a 
 non-existent

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-08-06 Thread Chun Yan Liu 


 On 8/7/2015 at 01:21 AM, in message 55c39796.8000...@citrix.com, George
Dunlap george.dun...@citrix.com wrote: 
 On 08/06/2015 04:11 AM, Chun Yan Liu wrote: 
  As 4.6 goes to bug fixing stage, maybe we can pick up this thread? :-) 
   
  Beside to call for your precious review comments and suggestions so that we 
   
 can 
  make progress, I also want to confirm about the previous discussed two TODO 
  things: 
  1) use UDEV name rule to specify usb device uniquely even across reboot.  
 That 
  got consensus. Next thing is exposing that name to some sysfs entry,  
 right? 
  
 So just to be clear, my understanding of the plan was that we try to fix 
 up the current patch series and check it in once the 4.7 window opens; 
 and that having the utility library to convert other names (including 
 this udev-style naming) into something libxl can use would be a separate 
 series. 

Got it.

  
 I wasn't necessarily expecting you to work on it (since it wasn't your 
 idea), but if you're keen, I'm sure we'd be grateful. :-) 
  
  2) use libusb instead of reading sysfs by ourselves. As George mentioned,  
 using 
  libusb is not simpler than reading sysfs; and if UDEV name is stored to 
   
 some 
  sysfs entry for us to retrieve, then we still need reading sysfs  
 things. Could we 
  get to a final decision? 
  If these are settled down, I can update related code. 
  
 I don't think that libusb would be particularly useful for the current 
 pvusb code, since 
 1. It's already Linux-specific, 
 2. We need to mess around with sysfs anyway. 
  
 The same thing can't be said of the HVM path: I think it fairly likely 
 that the emulated pass-through will Just Work (or nearly so) on BSDs 
 (assuming that qemu itself works on the BSDs). 
  
 I think it we write our utility library for converting 
 vendorid:productid[:serialno], bus-port, c, then it might make sense to 
 use libusb if it makes it more portable. 
  
 Regarding the code: 
  
 Things are looking pretty close.  A couple of comments in-line: 
  
  diff --git a/tools/libxl/libxl_device.c b/tools/libxl/libxl_device.c   
  index 93bb41e..9869a21 100644   
  --- a/tools/libxl/libxl_device.c   
  +++ b/tools/libxl/libxl_device.c   
  @@ -676,6 +676,10 @@ void libxl__devices_destroy(libxl__egc *egc,
  libxl__devices_remove_state *drs)   
   aodev-action = LIBXL__DEVICE_ACTION_REMOVE;   
   aodev-dev = dev;   
   aodev-force = drs-force;   
  +if (dev-backend_kind == LIBXL__DEVICE_KIND_VUSB) {   
  +libxl__initiate_device_usbctrl_remove(egc, aodev);  
   
  +continue;   
  +} 
  
 I take it the reason we need to special-case this is that we need to go 
 through and un-assign all of the devices inside the controller first? 

Yes.

  
 At some point we probably want to generalize this a bit, so that usb 
 controllers and vscsi controllers look the same (rather than both being 
 special-cased). 
  
 But since this is internal, maybe we can wait for that design until we 
 actually have both types available. 
  
  +static int   
  +libxl__device_usb_assigned_list(libxl__gc *gc,   
  +libxl_device_usb **list, int *num)   
  +{   
  +char **domlist;   
  +unsigned int nd = 0, i, j;   
  +char *be_path;   
  +libxl_device_usb *usb;   
  +   
  +*list = NULL;   
  +*num = 0;   
  +   
  +domlist = libxl__xs_directory(gc, XBT_NULL, /local/domain, nd);  
   
  +be_path = GCSPRINTF(/local/domain/%d/backend/vusb,
  LIBXL_TOOLSTACK_DOMID);   
  
 Hmm, so this had made me realize that I don't think we're doing quite 
 the right thing for non-dom0 backends. 
  
 First of all, things are a bit interesting for driver domain backends, 
 because the namespace of hostbus.hostaddr depends on the backend of 
 the virtual controller.  Which wouldn't be particularly interesting, 
 *except* that because the USB space is so dynamic, you normally have to 
 query the devices to find the hostbus.hostaddr; and you can't do any 
 queries from dom0 at the moment (except, for example, by ssh'ing into 
 the other vm).  So to even assign a hostbus.hostaddr device you have to 
 somehow learn, out-of-band, what those numbers are within the domain. 
  
 Secondly, if the backend is in another domain, then all the stuff re 
 assigning a usb device to usbback can't be done by libxl in the 
 toolstack domain either.  Which means I'm pretty sure this stuff will 
 fail at the moment for USB driver domains trying to assign a 
 non-existent hostbus.hostaddr to the (possibly non-existent) dom0 usbback. 
  
 A couple of ways forward: 
  
 1. Delete backend_domid and backend_name; add them back later when we 
 decide to implement proper USB driver domain support 
  
 2. Leave backend_domid and backend_name, but return an error if they are 
 not 0 and NULL respectively. 
  
 3. If backend_domid !=

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-08-05 Thread Chun Yan Liu 
As 4.6 goes to bug fixing stage, maybe we can pick up this thread? :-)

Beside to call for your precious review comments and suggestions so that we can
make progress, I also want to confirm about the previous discussed two TODO
things:
1) use UDEV name rule to specify usb device uniquely even across reboot. That
got consensus. Next thing is exposing that name to some sysfs entry, right?
2) use libusb instead of reading sysfs by ourselves. As George mentioned, using
libusb is not simpler than reading sysfs; and if UDEV name is stored to some
sysfs entry for us to retrieve, then we still need reading sysfs things. 
Could we
get to a final decision?
If these are settled down, I can update related code.

Thanks,
Chunyan

 On 7/7/2015 at 05:57 PM, in message 559ba270.4000...@eu.citrix.com, George
Dunlap george.dun...@eu.citrix.com wrote: 
 On 07/07/2015 02:25 AM, Chun Yan Liu wrote: 
  Any comments on the implementation? If there is anything improper, I can  
 update. 
  
 Since we decided to wait until the next release, I've been prioritizing 
 reviewing patch series which might make it into 4.6.  I'll come back to 
 it once the freeze starts. 
  
 Believe me, I'd much rather be working on the USB stuff. :-) 
  
  -George 
  
   
  Thanks, 
  Chunyan 
   
  On 6/25/2015 at 06:07 PM, in message 
  1435226838-3067-4-git-send-email-cy...@suse.com, Chunyan Liu 
  cy...@suse.com 
  wrote:  
  Add pvusb APIs, including:  
   - attach/detach (create/destroy) virtual usb controller.  
   - attach/detach usb device  
   - list usb controller and usb devices  
   - some other helper functions  

  Signed-off-by: Chunyan Liu cy...@suse.com  
  Signed-off-by: Simon Cao caobosi...@gmail.com  
  ---  
  changes:  
- Use macros DEFINE_DEVICE_ADD and DEFINE_DEVICES_ADD for adding  
  usbctrl and usb, update all related codes.  
- Use an extend macro DEFINE_DEVICE_REMOVE_EXT for removimg usbctrl 
  update all related codes.  
- Remove busid from libxl_device_usb definition, keep bus.addr only,  
  update all related codes.  
- Remove documentation since it's mostly about design, move to  
  cover-letter. Some parts are moved to code comments.  
- Address some other comments  

   tools/libxl/Makefile |2 +-  
   tools/libxl/libxl.c  |   53 ++  
   tools/libxl/libxl.h  |   64 ++  
   tools/libxl/libxl_device.c   |4 +  
   tools/libxl/libxl_internal.h |   20 +-  
   tools/libxl/libxl_osdeps.h   |   13 +  
   tools/libxl/libxl_pvusb.c| 1305   
  ++  
   tools/libxl/libxl_types.idl  |   50 ++  
   tools/libxl/libxl_types_internal.idl |1 +  
   tools/libxl/libxl_utils.c|   16 +  
   tools/libxl/libxl_utils.h|5 +  
   11 files changed, 1531 insertions(+), 2 deletions(-)  
   create mode 100644 tools/libxl/libxl_pvusb.c  

  diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile  
  index cc9c152..b820105 100644  
  --- a/tools/libxl/Makefile  
  +++ b/tools/libxl/Makefile  
  @@ -95,7 +95,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o 
  libxl_dm.o  
   
  libxl_pci.o \  
 libxl_internal.o libxl_utils.o libxl_uuid.o \  
 libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o 
  \  
 libxl_save_callout.o _libxl_save_msgs_callout.o \  
  -  libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)  
  +  libxl_qmp.o libxl_event.o libxl_fork.o libxl_pvusb.o 
  $(LIBXL_OBJS-y)  
   LIBXL_OBJS += libxl_genid.o  
   LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o  
 
  diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c  
  index 6570476..6542127 100644  
  --- a/tools/libxl/libxl.c  
  +++ b/tools/libxl/libxl.c  
  @@ -4233,11 +4233,54 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)  
 
 
   
 / 
  
  
  **/  
 
  +/* Macro for defining device remove/destroy functions for usbctrl */  
  +/* Following functions are defined:  
  + * libxl_device_usbctrl_remove  
  + * libxl_device_usbctrl_destroy  
  + */  
  +  
  +#define DEFINE_DEVICE_REMOVE_EXT(type, removedestroy, f)\ 
   
  +int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \ 
   
  +uint32_t domid, libxl_device_##type *type,  \ 
   
  +const libxl_asyncop_how *ao_how)\ 
   
  +{   \ 
   
  +AO_CREATE(ctx, domid, ao_how);  \ 
   
  +libxl__device *device;  \ 
   
  +libxl__ao_device *aodev;\ 
   
  +int rc;

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-07-16 Thread Juergen Gross 

On 06/25/2015 12:07 PM, Chunyan Liu wrote:

Add pvusb APIs, including:
  - attach/detach (create/destroy) virtual usb controller.
  - attach/detach usb device
  - list usb controller and usb devices
  - some other helper functions

Signed-off-by: Chunyan Liu cy...@suse.com
Signed-off-by: Simon Cao caobosi...@gmail.com


Sorry, found another error: You changed too many format specifiers
from %d to %x:

...


+static char *usb_busaddr_to_busid(libxl__gc *gc, int bus, int addr)
+{
+libxl_ctx *ctx = CTX;
+struct dirent *de;
+DIR *dir;
+char *busid = NULL;
+
+assert(bus  0  addr  0);
+
+if (!(dir = opendir(SYSFS_USB_DEV)))
+return NULL;
+
+while((de = readdir(dir))) {
+char *filename;
+void *buf;
+int busnum = -1;
+int devnum = -1;
+
+if (!de-d_name)
+continue;
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/devnum, de-d_name);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, devnum);


That's a decimal number. Use %d, please.


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/busnum, de-d_name);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, busnum);


Same here.


+
+if (bus == busnum  addr == devnum) {
+busid = libxl__strdup(NOGC, de-d_name);
+break;
+}
+}
+
+closedir(dir);
+return busid;
+}
+
+static void usb_busaddr_from_busid(libxl__gc *gc, char *busid,
+   int *bus, int *addr)
+{
+libxl_ctx *ctx = CTX;
+char *filename;
+void *buf;
+
+assert(busid);
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/busnum, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, bus);


And here.


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/devnum, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, addr);


And here.


+}


...


+
+/* check if USB device type is assignable */
+static bool is_usb_assignable(libxl__gc *gc, libxl_device_usb *usb)
+{
+libxl_ctx *ctx = CTX;
+int classcode;
+char *filename;
+void *buf = NULL;
+char *busid = NULL;
+
+assert(usb-hostbus  0  usb-hostaddr  0);
+busid = usb_busaddr_to_busid(gc, usb-hostbus, usb-hostaddr);
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/bDeviceClass, busid);
+if (libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+return false;
+
+sscanf(buf, %x, classcode);


This one, too.


+return classcode != USBHUB_CLASS_CODE;
+}


...


+int libxl_device_usb_getinfo(libxl_ctx *ctx, libxl_device_usb *usb,
+ libxl_usbinfo *usbinfo)
+{
+GC_INIT(ctx);
+char *filename;
+char *busid;
+void *buf = NULL;
+int buflen, rc;
+
+usbinfo-ctrl = usb-ctrl;
+usbinfo-port = usb-port;
+
+busid = usb_busaddr_to_busid(gc, usb-hostbus, usb-hostaddr);
+if (!busid) {
+rc = ERROR_FAIL;
+goto out;
+}
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/devnum, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, usbinfo-devnum);


Again.


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/busnum, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, usbinfo-busnum);


And here.


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/idVendor, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, usbinfo-idVendor);


This one is correct!


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/idProduct, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, NULL))
+sscanf(buf, %x, usbinfo-idProduct);


Correct, too.


+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/manufacturer, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, buflen) 
+buflen  0) {
+/* replace \n to \0 */
+if (((char *)buf)[buflen - 1] == '\n')
+((char *)buf)[buflen - 1] = '\0';
+usbinfo-manuf = libxl__strdup(NOGC, buf);
+   }
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/product, busid);
+if (!libxl_read_sysfs_file_contents(ctx, filename, buf, buflen) 
+buflen  0) {
+/* replace \n to \0 */
+if (((char *)buf)[buflen - 1] == '\n')
+((char *)buf)[buflen - 1] = '\0';
+usbinfo-prod = libxl__strdup(NOGC, buf);
+}
+
+rc = 0;
+
+out:
+GC_FREE;
+return rc;
+}



Juergen


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

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-07-06 Thread Chun Yan Liu 
Any comments on the implementation? If there is anything improper, I can update.

Thanks,
Chunyan

 On 6/25/2015 at 06:07 PM, in message
1435226838-3067-4-git-send-email-cy...@suse.com, Chunyan Liu cy...@suse.com
wrote: 
 Add pvusb APIs, including: 
  - attach/detach (create/destroy) virtual usb controller. 
  - attach/detach usb device 
  - list usb controller and usb devices 
  - some other helper functions 
  
 Signed-off-by: Chunyan Liu cy...@suse.com 
 Signed-off-by: Simon Cao caobosi...@gmail.com 
 --- 
 changes: 
   - Use macros DEFINE_DEVICE_ADD and DEFINE_DEVICES_ADD for adding 
 usbctrl and usb, update all related codes. 
   - Use an extend macro DEFINE_DEVICE_REMOVE_EXT for removimg usbctrl
 update all related codes. 
   - Remove busid from libxl_device_usb definition, keep bus.addr only, 
 update all related codes. 
   - Remove documentation since it's mostly about design, move to 
 cover-letter. Some parts are moved to code comments. 
   - Address some other comments 
  
  tools/libxl/Makefile |2 +- 
  tools/libxl/libxl.c  |   53 ++ 
  tools/libxl/libxl.h  |   64 ++ 
  tools/libxl/libxl_device.c   |4 + 
  tools/libxl/libxl_internal.h |   20 +- 
  tools/libxl/libxl_osdeps.h   |   13 + 
  tools/libxl/libxl_pvusb.c| 1305  
 ++ 
  tools/libxl/libxl_types.idl  |   50 ++ 
  tools/libxl/libxl_types_internal.idl |1 + 
  tools/libxl/libxl_utils.c|   16 + 
  tools/libxl/libxl_utils.h|5 + 
  11 files changed, 1531 insertions(+), 2 deletions(-) 
  create mode 100644 tools/libxl/libxl_pvusb.c 
  
 diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile 
 index cc9c152..b820105 100644 
 --- a/tools/libxl/Makefile 
 +++ b/tools/libxl/Makefile 
 @@ -95,7 +95,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o  
 libxl_pci.o \ 
   libxl_internal.o libxl_utils.o libxl_uuid.o \ 
   libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o 
 \ 
   libxl_save_callout.o _libxl_save_msgs_callout.o \ 
 - libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y) 
 + libxl_qmp.o libxl_event.o libxl_fork.o libxl_pvusb.o 
 $(LIBXL_OBJS-y) 
  LIBXL_OBJS += libxl_genid.o 
  LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o 
   
 diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c 
 index 6570476..6542127 100644 
 --- a/tools/libxl/libxl.c 
 +++ b/tools/libxl/libxl.c 
 @@ -4233,11 +4233,54 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1) 
   
   
 / 
 **/ 
   
 +/* Macro for defining device remove/destroy functions for usbctrl */ 
 +/* Following functions are defined: 
 + * libxl_device_usbctrl_remove 
 + * libxl_device_usbctrl_destroy 
 + */ 
 + 
 +#define DEFINE_DEVICE_REMOVE_EXT(type, removedestroy, f)\ 
 +int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \ 
 +uint32_t domid, libxl_device_##type *type,  \ 
 +const libxl_asyncop_how *ao_how)\ 
 +{   \ 
 +AO_CREATE(ctx, domid, ao_how);  \ 
 +libxl__device *device;  \ 
 +libxl__ao_device *aodev;\ 
 +int rc; \ 
 +\ 
 +GCNEW(device);  \ 
 +rc = libxl__device_from_##type(gc, domid, type, device);\ 
 +if (rc != 0) goto out;  \ 
 +\ 
 +GCNEW(aodev);   \ 
 +libxl__prepare_ao_device(ao, aodev);\ 
 +aodev-action = LIBXL__DEVICE_ACTION_REMOVE;\ 
 +aodev-dev = device;\ 
 +aodev-callback = device_addrm_aocomplete;  \ 
 +aodev-force = f;   \ 
 +libxl__initiate_device_##type##_remove(egc, aodev); \ 
 +\ 
 +out:\ 
 +if (rc) return AO_ABORT(rc);\ 
 +return AO_INPROGRESS;   \ 
 +} 
 + 
 + 
 +DEFINE_DEVICE_REMOVE_EXT(usbctrl, remove, 0) 
 +DEFINE_DEVICE_REMOVE_EXT(usbctrl, destroy, 1) 
 +

Re: [Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-06-25 Thread Ian Jackson 
Chunyan Liu writes ([PATCH V5 3/7] libxl: add pvusb API):
 Add pvusb APIs, including:
  - attach/detach (create/destroy) virtual usb controller.
  - attach/detach usb device
  - list usb controller and usb devices
  - some other helper functions
 
 Signed-off-by: Chunyan Liu cy...@suse.com
 Signed-off-by: Simon Cao caobosi...@gmail.com

I have reviewed the new API introduced in this patch and it looks
sensible to me.

I did notice this wrap damage:

 +/* USB Devices */
 +int libxl_device_usb_add(libxl_ctx *ctx, uint32_t domid, libxl_device_usb\
 *usb,
 + const libxl_asyncop_how *ao_how)
 + LIBXL_EXTERNAL_CALLERS_ONLY;
 +
 +int libxl_device_usb_remove(libxl_ctx *ctx, uint32_t domid, libxl_device_\
usb *usb,
 +const libxl_asyncop_how *ao_how)


I haven't reviewed the implementation (yet).

Ian.

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

[Xen-devel] [PATCH V5 3/7] libxl: add pvusb API

2015-06-25 Thread Chunyan Liu 
Add pvusb APIs, including:
 - attach/detach (create/destroy) virtual usb controller.
 - attach/detach usb device
 - list usb controller and usb devices
 - some other helper functions

Signed-off-by: Chunyan Liu cy...@suse.com
Signed-off-by: Simon Cao caobosi...@gmail.com
---
changes:
  - Use macros DEFINE_DEVICE_ADD and DEFINE_DEVICES_ADD for adding
usbctrl and usb, update all related codes.
  - Use an extend macro DEFINE_DEVICE_REMOVE_EXT for removimg usbctrl,
update all related codes.
  - Remove busid from libxl_device_usb definition, keep bus.addr only,
update all related codes.
  - Remove documentation since it's mostly about design, move to
cover-letter. Some parts are moved to code comments.
  - Address some other comments

 tools/libxl/Makefile |2 +-
 tools/libxl/libxl.c  |   53 ++
 tools/libxl/libxl.h  |   64 ++
 tools/libxl/libxl_device.c   |4 +
 tools/libxl/libxl_internal.h |   20 +-
 tools/libxl/libxl_osdeps.h   |   13 +
 tools/libxl/libxl_pvusb.c| 1305 ++
 tools/libxl/libxl_types.idl  |   50 ++
 tools/libxl/libxl_types_internal.idl |1 +
 tools/libxl/libxl_utils.c|   16 +
 tools/libxl/libxl_utils.h|5 +
 11 files changed, 1531 insertions(+), 2 deletions(-)
 create mode 100644 tools/libxl/libxl_pvusb.c

diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index cc9c152..b820105 100644
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -95,7 +95,7 @@ LIBXL_OBJS = flexarray.o libxl.o libxl_create.o libxl_dm.o 
libxl_pci.o \
libxl_internal.o libxl_utils.o libxl_uuid.o \
libxl_json.o libxl_aoutils.o libxl_numa.o libxl_vnuma.o 
\
libxl_save_callout.o _libxl_save_msgs_callout.o \
-   libxl_qmp.o libxl_event.o libxl_fork.o $(LIBXL_OBJS-y)
+   libxl_qmp.o libxl_event.o libxl_fork.o libxl_pvusb.o 
$(LIBXL_OBJS-y)
 LIBXL_OBJS += libxl_genid.o
 LIBXL_OBJS += _libxl_types.o libxl_flask.o _libxl_types_internal.o
 
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
index 6570476..6542127 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -4233,11 +4233,54 @@ DEFINE_DEVICE_REMOVE(vtpm, destroy, 1)
 
 
/**/
 
+/* Macro for defining device remove/destroy functions for usbctrl */
+/* Following functions are defined:
+ * libxl_device_usbctrl_remove
+ * libxl_device_usbctrl_destroy
+ */
+
+#define DEFINE_DEVICE_REMOVE_EXT(type, removedestroy, f)\
+int libxl_device_##type##_##removedestroy(libxl_ctx *ctx,   \
+uint32_t domid, libxl_device_##type *type,  \
+const libxl_asyncop_how *ao_how)\
+{   \
+AO_CREATE(ctx, domid, ao_how);  \
+libxl__device *device;  \
+libxl__ao_device *aodev;\
+int rc; \
+\
+GCNEW(device);  \
+rc = libxl__device_from_##type(gc, domid, type, device);\
+if (rc != 0) goto out;  \
+\
+GCNEW(aodev);   \
+libxl__prepare_ao_device(ao, aodev);\
+aodev-action = LIBXL__DEVICE_ACTION_REMOVE;\
+aodev-dev = device;\
+aodev-callback = device_addrm_aocomplete;  \
+aodev-force = f;   \
+libxl__initiate_device_##type##_remove(egc, aodev); \
+\
+out:\
+if (rc) return AO_ABORT(rc);\
+return AO_INPROGRESS;   \
+}
+
+
+DEFINE_DEVICE_REMOVE_EXT(usbctrl, remove, 0)
+DEFINE_DEVICE_REMOVE_EXT(usbctrl, destroy, 1)
+
+#undef DEFINE_DEVICE_REMOVE_EXT
+
+/**/
+
 /* Macro for defining device addition functions in a compact way */
 /* The following functions are defined:
  * libxl_device_disk_add
  * libxl_device_nic_add
  * libxl_device_vtpm_add
+ * libxl_device_usbctrl_add
+ * libxl_device_usb_add
  */
 
 #define DEFINE_DEVICE_ADD(type)