Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
Wei Liu writes (Re: [PATCH V3 3/6] libxl: add pvusb API): On Mon, May 18, 2015 at 09:20:43PM -0600, Chun Yan Liu wrote: Thanks, I'm updating that. But maybe like pci_add and pci_remove functions, will add a 'starting' flag to indicate hotplug or creation. Looking at DEFINE_DEVICE_ADD and DEFINE_DEVICE_REMOVE, usbctrl_add and usb_add can use DEFINE_DEVICE_ADD; but usbctrl_remove and usb_remove cannot use DEFINE_DEVICE_REMOVE directly, need some extra handling. So, finally turned to not use these macros but refer to pci functions. TBH I prefer to have only one way to deal with devices. I personally prefer the async style that every other devices use. Maybe that's just because I mostly worked with those. I don't know the full history of libxl_pci.c so I will wait for Ian and Ian's comments on which way to go. libxl_pci.c is rather unfortunate in a few respects. It does a number of things things synchronously that it shouldn't. Certainly the external API for all device add/remove functions should be async. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Wed, 2015-05-20 at 10:04 +0100, Wei Liu wrote:
On Mon, May 18, 2015 at 09:20:43PM -0600, Chun Yan Liu wrote:
[...]
+for (;;) {
+rc = libxl__xs_transaction_start(gc, t);
+if (rc) goto out;
+
+rc = libxl__device_exists(gc, t, device);
+if (rc 0) goto out;
+if (rc == 1) {
+/* already exists in xenstore */
+LOG(ERROR, device already exists in xenstore);
+rc = ERROR_DEVICE_EXISTS;
+goto out;
+}
+
+rc = libxl__set_domain_configuration(gc, domid, d_config);
+if (rc) goto out;
+
+libxl__device_generic_add(gc, t, device,
+ libxl__xs_kvs_of_flexarray(gc, back,
+
back-count),
+ libxl__xs_kvs_of_flexarray(gc,
front,
+
front-count),
+ NULL);
+libxl__usbport_add_xenstore(gc, t, domid, usbctrl);
+rc = libxl__xs_transaction_commit(gc, t);
+if (!rc) break;
+if (rc 0) goto out;
+}
+
You don't have aodev so you cannot check update_json. Maybe you need
aodev?
That field update_json is set to true when doing hotplug. It's set to
false during domain creation.
The same comment applies to other add functions as well.
Thanks, I'm updating that. But maybe like pci_add and pci_remove functions,
will add a 'starting' flag to indicate hotplug or creation.
Looking at DEFINE_DEVICE_ADD and DEFINE_DEVICE_REMOVE, usbctrl_add
and usb_add can use DEFINE_DEVICE_ADD; but usbctrl_remove and usb_remove
cannot use DEFINE_DEVICE_REMOVE directly, need some extra handling. So,
finally turned to not use these macros but refer to pci functions.
TBH I prefer to have only one way to deal with devices. I personally
prefer the async style that every other devices use. Maybe that's just
because I mostly worked with those.
I don't know the full history of libxl_pci.c so I will wait for Ian and
Ian's comments on which way to go.
libxl_pci.c should not be used as an example of the right way to do
things (to say the least).
I think one merit of determining whether to use sync or async is that
whether the operation is long running (slow). Long running one should be
asyn. I guess usb passthrough is not slow so we are probably fine in
this regard.
Device add/remove is expected to be async at least at the public API
level , all the others appear to be and it seems logical to me that they
would be.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On 5/20/2015 at 05:04 PM, in message
20150520090407.gw26...@zion.uk.xensource.com, Wei Liu wei.l...@citrix.com
wrote:
On Mon, May 18, 2015 at 09:20:43PM -0600, Chun Yan Liu wrote:
[...]
+for (;;) {
+rc = libxl__xs_transaction_start(gc, t);
+if (rc) goto out;
+
+rc = libxl__device_exists(gc, t, device);
+if (rc 0) goto out;
+if (rc == 1) {
+/* already exists in xenstore */
+LOG(ERROR, device already exists in xenstore);
+rc = ERROR_DEVICE_EXISTS;
+goto out;
+}
+
+rc = libxl__set_domain_configuration(gc, domid, d_config);
+if (rc) goto out;
+
+libxl__device_generic_add(gc, t, device,
+ libxl__xs_kvs_of_flexarray(gc, back,
+
back-count),
+ libxl__xs_kvs_of_flexarray(gc,
front,
+
front-count),
+ NULL);
+libxl__usbport_add_xenstore(gc, t, domid, usbctrl);
+rc = libxl__xs_transaction_commit(gc, t);
+if (!rc) break;
+if (rc 0) goto out;
+}
+
You don't have aodev so you cannot check update_json. Maybe you need
aodev?
That field update_json is set to true when doing hotplug. It's set to
false during domain creation.
The same comment applies to other add functions as well.
Thanks, I'm updating that. But maybe like pci_add and pci_remove functions,
will add a 'starting' flag to indicate hotplug or creation.
Looking at DEFINE_DEVICE_ADD and DEFINE_DEVICE_REMOVE, usbctrl_add
and usb_add can use DEFINE_DEVICE_ADD; but usbctrl_remove and usb_remove
cannot use DEFINE_DEVICE_REMOVE directly, need some extra handling. So,
finally turned to not use these macros but refer to pci functions.
TBH I prefer to have only one way to deal with devices. I personally
prefer the async style that every other devices use. Maybe that's just
because I mostly worked with those.
I don't know the full history of libxl_pci.c so I will wait for Ian and
Ian's comments on which way to go.
I think one merit of determining whether to use sync or async is that
whether the operation is long running (slow). Long running one should be
asyn. I guess usb passthrough is not slow so we are probably fine in
this regard.
BTW if you find the macros can't meet your need you can either extend
them or not use them.
Got you and Ian. I'll update codes then.
Chunyan
+out:
+if (lock) libxl__unlock_domain_userdata(lock);
+libxl_device_usbctrl_dispose(usbctrl_saved);
+libxl_domain_config_dispose(d_config);
+return rc;
+}
+
+int libxl__device_usbctrl_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_usbctrl *usbctrl)
+{
+int rc = 0;
+
+rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl);
+if (rc 0) goto out;
+
+if (usbctrl-devid == -1) {
+usbctrl-devid = libxl__device_nextid(gc, domid, vusb);
+if (usbctrl-devid 0) {
+rc = ERROR_FAIL;
+goto out;
+}
+}
+
+if (libxl__usbctrl_add_xenstore(gc, domid, usbctrl) 0){
+rc = ERROR_FAIL;
+goto out;
+}
+
+out:
+return rc;
+}
+
+int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+{
+AO_CREATE(ctx, domid, ao_how);
+int rc;
+
+rc = libxl__device_usbctrl_add(gc, domid, usbctrl);
Hmm... Your remove function is async while this one is sync, why?
Hmm, maybe not proper here, just referred to pci_add implementation.
Current calling places only use sync mode.
Yeah, I only ask for consistency.
Wei.
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Tue, May 19, 2015 at 4:20 AM, Chun Yan Liu cy...@suse.com wrote:
+static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
+libxl_device_usbctrl *usbctrl)
+{
+int rc;
+
+if (!usbctrl-version)
+usbctrl-version = 2;
+
How hard would it be to implement USB 3? I assume this depends on QEMU's
support? I.e. we just need to specify the version to 3 and it should
just work? Just curious.
This implementation is coworked with PVUSB driver, so it depends on PVUSB
driver's support. If PVUSB driver supports that, just specify the version to
3 and
can work.
I sort of got the idea that from the pvusb driver's perspective, the
1/2/3 thing was mostly just a number -- one that's important to the
USB device itself and the USB driver for those devices in the guest,
but less so for the pvusb layer. Is that not the case?
-George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On 05/19/2015 12:20 PM, George Dunlap wrote:
On Tue, May 19, 2015 at 4:20 AM, Chun Yan Liu cy...@suse.com wrote:
+static int libxl__device_usbctrl_setdefault(libxl__gc *gc, uint32_t domid,
+libxl_device_usbctrl *usbctrl)
+{
+int rc;
+
+if (!usbctrl-version)
+usbctrl-version = 2;
+
How hard would it be to implement USB 3? I assume this depends on QEMU's
support? I.e. we just need to specify the version to 3 and it should
just work? Just curious.
This implementation is coworked with PVUSB driver, so it depends on PVUSB
driver's support. If PVUSB driver supports that, just specify the version to 3
and
can work.
I sort of got the idea that from the pvusb driver's perspective, the
1/2/3 thing was mostly just a number -- one that's important to the
USB device itself and the USB driver for those devices in the guest,
but less so for the pvusb layer. Is that not the case?
I think at least for support of USB3.0 streams there is some work to do.
I haven't done any thorough research on this topic, but as libusb had to
be changed for support of USB3.0 as well, I'd expect this to be the case
for the pvusb interface, too.
Juergen
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Tue, 2015-05-19 at 19:06 +0100, George Dunlap wrote: diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h index 6bc75c5..cbe3519 100644 --- a/tools/libxl/libxl.h +++ b/tools/libxl/libxl.h @@ -114,6 +114,12 @@ #define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1 /* + * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of + * USB devices through pvusb. + */ +#define LIBXL_HAVE_PVUSB 1 It seems like we should document somewhere how we expect these to be used -- namely the connection between usbctrl and usb devices. In particular, that you can either add a usbctrl, and then add a usb device to it specifically; or if you don't specify a usbctrl when calling usb_add, it will find the most reasonable one to add it to, even creating one for you if you didn't have one. (I've not checked if this exists in the series yet, but...) I'd like to see some docs added to libxl.h as an addendum to the existing: * Devices * === * * Each device is represented by a libxl_device_TYPE data structure ... stuff which describes the general form of interfaces used for those classes of device which distinguish the controller from the devices themselves in the libxl API. The new docs should then be applicable to e..g the pvscsi stuff as well. The goal is to avoid having numerous subtly different style of device interaction in the libxl API by grouping them into the least number of logical groups (thus far 2: ones with explicit controllers and those without) each with a consistent API. Ian. ___ Xen-devel mailing list Xen-devel@lists.xen.org http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Sun, Apr 19, 2015 at 4:50 AM, Chunyan Liu cy...@suse.com wrote:
+int libxl_device_usb_getinfo(libxl_ctx *ctx, char *busid, libxl_usbinfo
*usbinfo)
+{
Sorry, missed something. This function is wrong. It gives you
information about a *host* USB device; but should properly give you
information about a *guest* USB device. The
libxl_device_disk_getinfo() counterpart, for example, takes a domid
and a virtual device (from within a libxl_disk structure) and returns
information about that virtual device.
Hrm... which makes me wonder whether we should use ctrl,port as a
unique handle for usb devices in the interface, or have a separate
devid for the devices themselves.
-George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Sun, Apr 19, 2015 at 4:50 AM, 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
OK, getting closer. :-)
A number of comments:
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..cbe3519 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -114,6 +114,12 @@
#define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
/*
+ * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of
+ * USB devices through pvusb.
+ */
+#define LIBXL_HAVE_PVUSB 1
It seems like we should document somewhere how we expect these to be
used -- namely the connection between usbctrl and usb devices. In
particular, that you can either add a usbctrl, and then add a usb
device to it specifically; or if you don't specify a usbctrl when
calling usb_add, it will find the most reasonable one to add it to,
even creating one for you if you didn't have one.
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
new file mode 100644
index 000..4e4975a
--- /dev/null
+++ b/tools/libxl/libxl_pvusb.c
+int libxl__device_usbctrl_add(libxl__gc *gc, uint32_t domid,
+ libxl_device_usbctrl *usbctrl)
+{
+int rc = 0;
+
+rc = libxl__device_usbctrl_setdefault(gc, domid, usbctrl);
+if (rc 0) goto out;
+
+if (usbctrl-devid == -1) {
Hmm, I was doing to say that this shouldn't be a magic constant, but
it looks like that's what all the other devices do :-/
+static bool is_usb_in_array(libxl_device_usb *usbs, int num,
+libxl_device_usb *usb)
+{
+int i;
+
+for (i = 0; i num; i++) {
+if (!strcmp(usbs[i].busid, usb-busid) )
Here (and elsewhere) you should probably use the COMPARE_USB() macro
you define in this patch.
+/* check if USB device type is assignable */
+static bool is_usb_assignable(libxl__gc *gc, libxl_device_usb *usb)
+{
+libxl_ctx *ctx = libxl__gc_owner(gc);
+int classcode;
+char *filename;
+void *buf;
+
+assert(usb-busid);
+
+filename = GCSPRINTF(SYSFS_USB_DEV/%s/bDeviceClass, usb-busid);
+if (libxl_read_file_contents(ctx, filename, buf, NULL) 0)
+return false;
+
+sscanf(buf, %x, classcode);
+if (classcode == USBHUB_CLASS_CODE)
+return false;
+
+return true;
Just checking, is it really the case that *all* USB classes are
assignable, *except* for hubs?
This is a minor thing, but this might be simper to do this:
return classcode != USBHUB_CLASS_CODE;
+/* get usb devices under certain usb controller */
+static int libxl__device_usb_list(libxl__gc *gc, uint32_t domid, int usbctrl,
+ libxl_device_usb **usbs, int *num)
+{
+char *be_path, *num_devs;
+int n, i;
+
+*usbs = NULL;
+*num = 0;
+
+be_path = GCSPRINTF(%s/backend/vusb/%d/%d,
+libxl__xs_get_dompath(gc, 0), domid, usbctrl);
+num_devs = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF(%s/num-ports, be_path));
+if (!num_devs)
+return 0;
+
+n = atoi(num_devs);
+*usbs = calloc(n, sizeof(libxl_device_usb));
+
+for (i = 0; i n; i++) {
+char *busid;
+libxl_device_usb *usb = NULL;
+
+busid = libxl__xs_read(gc, XBT_NULL,
+ GCSPRINTF(%s/port/%d, be_path, i + 1));
+if (busid strcmp(busid, )) {
+usb = *usbs + *num;
+usb-ctrl = usbctrl;
+usb-port = i + 1;
+usb-busid = strdup(busid);
This needs to populate the hostbus / hostaddr as well; busid is pretty
useless to users / external callers.
+/* get all usb devices of the domain */
+static libxl_device_usb *
+libxl_device_usb_list_all(libxl__gc *gc, uint32_t domid, int *num)
+{
+char **usbctrls;
+unsigned int nd, i, j;
+char *be_path;
+int rc;
+libxl_device_usb *usbs = NULL;
+
+*num = 0;
+
+be_path = GCSPRINTF(/local/domain/%d/backend/vusb/%d,
+LIBXL_TOOLSTACK_DOMID, domid);
+usbctrls = libxl__xs_directory(gc, XBT_NULL, be_path, nd);
+
+for (i = 0; i nd; i++) {
+int nc = 0;
+libxl_device_usb *tmp = NULL;
+rc = libxl__device_usb_list(gc, domid, atoi(usbctrls[i]), tmp, nc);
+if (!nc) continue;
+
+usbs = realloc(usbs, sizeof(libxl_device_usb)*((*num) + nc));
+for(j = 0; j nc; j++) {
+usbs[*num].ctrl = tmp[j].ctrl;
+usbs[*num].port = tmp[j].port;
+usbs[*num].busid = strdup(tmp[j].busid);
This needs to copy the hostaddr and busaddr as well, as these are
primarily what an external caller will want.
+/* find first unused
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Sun, Apr 19, 2015 at 4:50 AM, Chunyan Liu cy...@suse.com wrote:
+int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
+libxl_device_usbctrl *usbctrl,
+libxl_usbctrlinfo *usbctrlinfo)
+{
+GC_INIT(ctx);
+char *dompath, *usbctrlpath;
+char *val;
+int rc = 0;
+
+usbctrlinfo-devid = usbctrl-devid;
+usbctrlinfo-ports = usbctrl-ports;
+usbctrlinfo-version = usbctrl-version;
...and following on from the previous mail, it looks like the other
device_*_getinfo() functions *only* read devid -- which means you just
have to make the struct and fill in devid, and getinfo() will tell
*you* the ports, the version, and so on.
-George
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On Sun, Apr 19, Chunyan Liu wrote:
+static int libxl__usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
+ libxl_device_usbctrl *usbctrl)
+{
+flexarray_append_pair(back, state, 1);
+flexarray_append_pair(front, state, 1);
This (and perhaps other places) should be converted to xenbus_state, see
commit 25519c75b7e05fd82d7f2959aaa85518b5564cc3 (libxl: convert strings
and ints to xenbus_state).
Olaf
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
(I look at overall code structure this pass. I haven't done a line by
line review.)
On Sun, Apr 19, 2015 at 11:50:49AM +0800, 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
---
Changes to v2:
* remove qemu emulated usb related definitions, keep the work pure pvusb.
In last patch, will do all refactor work to unify pvusb and qemu emulated
usb.
* use bus.addr as user interface instead of busid to do usb-attach|detach
* remove usb-assignable-list APIs and some other unnecessary APIs
* reuse libxl_read_file_contents function instead of another new function
to handle getting sysfs file content
* fix build on different platforms as pci does
* fix many coding style problems
* address other comments in last version
* adjust codes to let it look better
tools/libxl/Makefile |2 +-
tools/libxl/libxl.c |2 +
tools/libxl/libxl.h | 45 ++
tools/libxl/libxl_internal.h | 11 +-
tools/libxl/libxl_osdeps.h | 13 +
tools/libxl/libxl_pvusb.c| 1201
++
tools/libxl/libxl_types.idl | 41 ++
tools/libxl/libxl_types_internal.idl |1 +
You also need to document the xenstore keys and values somewhere under
docs directory.
And you forgot to update libxl_retrieve_domain_configuration function.
8 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 tools/libxl/libxl_pvusb.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 1b16598..d52281f 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 b05d18b..a7c81d9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1611,6 +1611,8 @@ void libxl__destroy_domid(libxl__egc *egc,
libxl__destroy_domid_state *dis)
if (libxl__device_pci_destroy_all(gc, domid) 0)
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, pci shutdown failed for domid
%d, domid);
+if (libxl__device_usb_destroy_all(gc, domid) 0)
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, usb shutdown failed for domid
%d, domid);
rc = xc_domain_pause(ctx-xch, domid);
if (rc 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, xc_domain_pause
failed for %d, domid);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..cbe3519 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -114,6 +114,12 @@
#define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
/*
+ * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of
+ * USB devices through pvusb.
+ */
+#define LIBXL_HAVE_PVUSB 1
+
+/*
* LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
* libxl_vendor_device field is present in the hvm sections of
* libxl_domain_build_info. This field tells libxl which
@@ -1224,6 +1230,45 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
+/* USB Controllers*/
+int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_usbctrl *libxl_device_usbctrl_list(libxl_ctx *ctx,
+uint32_t domid, int *num);
+
+
+int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
+libxl_device_usbctrl *usbctrl,
+libxl_usbctrlinfo
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On 5/19/2015 at 02:07 AM, in message
20150518180724.gh9...@zion.uk.xensource.com, Wei Liu wei.l...@citrix.com
wrote:
(I look at overall code structure this pass. I haven't done a line by
line review.)
On Sun, Apr 19, 2015 at 11:50:49AM +0800, 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
---
Changes to v2:
* remove qemu emulated usb related definitions, keep the work pure pvusb.
In last patch, will do all refactor work to unify pvusb and qemu
emulated
usb.
* use bus.addr as user interface instead of busid to do usb-attach|detach
* remove usb-assignable-list APIs and some other unnecessary APIs
* reuse libxl_read_file_contents function instead of another new function
to handle getting sysfs file content
* fix build on different platforms as pci does
* fix many coding style problems
* address other comments in last version
* adjust codes to let it look better
tools/libxl/Makefile |2 +-
tools/libxl/libxl.c |2 +
tools/libxl/libxl.h | 45 ++
tools/libxl/libxl_internal.h | 11 +-
tools/libxl/libxl_osdeps.h | 13 +
tools/libxl/libxl_pvusb.c| 1201
++
tools/libxl/libxl_types.idl | 41 ++
tools/libxl/libxl_types_internal.idl |1 +
You also need to document the xenstore keys and values somewhere under
docs directory.
Thanks, will add that.
And you forgot to update libxl_retrieve_domain_configuration function.
Updating.
8 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 tools/libxl/libxl_pvusb.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 1b16598..d52281f 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 b05d18b..a7c81d9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1611,6 +1611,8 @@ void libxl__destroy_domid(libxl__egc *egc,
libxl__destroy_domid_state *dis)
if (libxl__device_pci_destroy_all(gc, domid) 0)
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, pci shutdown failed for domid
%d, domid);
+if (libxl__device_usb_destroy_all(gc, domid) 0)
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, usb shutdown failed for domid
%d, domid);
rc = xc_domain_pause(ctx-xch, domid);
if (rc 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, xc_domain_pause
failed for %d, domid);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..cbe3519 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -114,6 +114,12 @@
#define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
/*
+ * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of
+ * USB devices through pvusb.
+ */
+#define LIBXL_HAVE_PVUSB 1
+
+/*
* LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
* libxl_vendor_device field is present in the hvm sections of
* libxl_domain_build_info. This field tells libxl which
@@ -1224,6 +1230,45 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t
domid, libxl_device_disk *disk,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
+/* USB Controllers*/
+int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+
[Xen-devel] [PATCH V3 3/6] 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
---
Changes to v2:
* remove qemu emulated usb related definitions, keep the work pure pvusb.
In last patch, will do all refactor work to unify pvusb and qemu emulated
usb.
* use bus.addr as user interface instead of busid to do usb-attach|detach
* remove usb-assignable-list APIs and some other unnecessary APIs
* reuse libxl_read_file_contents function instead of another new function
to handle getting sysfs file content
* fix build on different platforms as pci does
* fix many coding style problems
* address other comments in last version
* adjust codes to let it look better
tools/libxl/Makefile |2 +-
tools/libxl/libxl.c |2 +
tools/libxl/libxl.h | 45 ++
tools/libxl/libxl_internal.h | 11 +-
tools/libxl/libxl_osdeps.h | 13 +
tools/libxl/libxl_pvusb.c| 1201 ++
tools/libxl/libxl_types.idl | 41 ++
tools/libxl/libxl_types_internal.idl |1 +
8 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 tools/libxl/libxl_pvusb.c
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
index 1b16598..d52281f 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 b05d18b..a7c81d9 100644
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1611,6 +1611,8 @@ void libxl__destroy_domid(libxl__egc *egc,
libxl__destroy_domid_state *dis)
if (libxl__device_pci_destroy_all(gc, domid) 0)
LIBXL__LOG(ctx, LIBXL__LOG_ERROR, pci shutdown failed for domid %d,
domid);
+if (libxl__device_usb_destroy_all(gc, domid) 0)
+ LIBXL__LOG(ctx, LIBXL__LOG_ERROR, usb shutdown failed for domid %d,
domid);
rc = xc_domain_pause(ctx-xch, domid);
if (rc 0) {
LIBXL__LOG_ERRNOVAL(ctx, LIBXL__LOG_ERROR, rc, xc_domain_pause failed
for %d, domid);
diff --git a/tools/libxl/libxl.h b/tools/libxl/libxl.h
index 6bc75c5..cbe3519 100644
--- a/tools/libxl/libxl.h
+++ b/tools/libxl/libxl.h
@@ -114,6 +114,12 @@
#define LIBXL_HAVE_DOMAIN_NODEAFFINITY 1
/*
+ * LIBXL_HAVE_PVUSB indicates the functions for doing hot-plug of
+ * USB devices through pvusb.
+ */
+#define LIBXL_HAVE_PVUSB 1
+
+/*
* LIBXL_HAVE_BUILDINFO_HVM_VENDOR_DEVICE indicates that the
* libxl_vendor_device field is present in the hvm sections of
* libxl_domain_build_info. This field tells libxl which
@@ -1224,6 +1230,45 @@ int libxl_cdrom_insert(libxl_ctx *ctx, uint32_t domid,
libxl_device_disk *disk,
const libxl_asyncop_how *ao_how)
LIBXL_EXTERNAL_CALLERS_ONLY;
+/* USB Controllers*/
+int libxl_device_usbctrl_add(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_remove(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+int libxl_device_usbctrl_destroy(libxl_ctx *ctx, uint32_t domid,
+ libxl_device_usbctrl *usbctrl,
+ const libxl_asyncop_how *ao_how)
+ LIBXL_EXTERNAL_CALLERS_ONLY;
+
+libxl_device_usbctrl *libxl_device_usbctrl_list(libxl_ctx *ctx,
+uint32_t domid, int *num);
+
+
+int libxl_device_usbctrl_getinfo(libxl_ctx *ctx, uint32_t domid,
+libxl_device_usbctrl *usbctrl,
+libxl_usbctrlinfo *usbctrlinfo)
+LIBXL_EXTERNAL_CALLERS_ONLY;
+
+/* 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,
+
Re: [Xen-devel] [PATCH V3 3/6] libxl: add pvusb API
On 04/19/2015 05:50 AM, 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
---
Changes to v2:
* remove qemu emulated usb related definitions, keep the work pure pvusb.
In last patch, will do all refactor work to unify pvusb and qemu emulated
usb.
* use bus.addr as user interface instead of busid to do usb-attach|detach
* remove usb-assignable-list APIs and some other unnecessary APIs
* reuse libxl_read_file_contents function instead of another new function
to handle getting sysfs file content
* fix build on different platforms as pci does
* fix many coding style problems
* address other comments in last version
* adjust codes to let it look better
tools/libxl/Makefile |2 +-
tools/libxl/libxl.c |2 +
tools/libxl/libxl.h | 45 ++
tools/libxl/libxl_internal.h | 11 +-
tools/libxl/libxl_osdeps.h | 13 +
tools/libxl/libxl_pvusb.c| 1201 ++
tools/libxl/libxl_types.idl | 41 ++
tools/libxl/libxl_types_internal.idl |1 +
8 files changed, 1314 insertions(+), 2 deletions(-)
create mode 100644 tools/libxl/libxl_pvusb.c
...
diff --git a/tools/libxl/libxl_pvusb.c b/tools/libxl/libxl_pvusb.c
new file mode 100644
index 000..4e4975a
--- /dev/null
+++ b/tools/libxl/libxl_pvusb.c
...
+static int libxl__usbctrl_add_xenstore(libxl__gc *gc, uint32_t domid,
+ libxl_device_usbctrl *usbctrl)
+{
+flexarray_t *front;
+flexarray_t *back;
+libxl__device *device;
+xs_transaction_t t = XBT_NULL;
+int rc = 0;
+libxl_domain_config d_config;
+libxl_device_usbctrl usbctrl_saved;
+libxl__domain_userdata_lock *lock = NULL;
+
+libxl_domain_config_init(d_config);
+libxl_device_usbctrl_init(usbctrl_saved);
+libxl_device_usbctrl_copy(CTX, usbctrl_saved, usbctrl);
+
+GCNEW(device);
+libxl__device_from_usbctrl(gc, domid, usbctrl, device);
+
+front = flexarray_make(gc, 4, 1);
+back = flexarray_make(gc, 12, 1);
+
+flexarray_append_pair(back, frontend-id, GCSPRINTF(%d, domid));
+flexarray_append_pair(back, online, 1);
+flexarray_append_pair(back, state, 1);
+flexarray_append_pair(back, usb-ver, GCSPRINTF(%d, usbctrl-version));
+flexarray_append_pair(back, num-ports, GCSPRINTF(%d, usbctrl-ports));
+flexarray_append_pair(front, backend-id, GCSPRINTF(%d,
usbctrl-backend_domid));
+flexarray_append_pair(front, state, 1);
+
+lock = libxl__lock_domain_userdata(gc, domid);
+if (!lock) {
+rc = ERROR_LOCK_FAIL;
+goto out;
+}
+
+rc = libxl__get_domain_configuration(gc, domid, d_config);
+if (rc) goto out;
+
+DEVICE_ADD(usbctrl, usbctrls, domid, usbctrl_saved, COMPARE_USBCTRL,
d_config);
+
+for (;;) {
+rc = libxl__xs_transaction_start(gc, t);
+if (rc) goto out;
+
+rc = libxl__device_exists(gc, t, device);
+if (rc 0) goto out;
+if (rc == 1) {
+/* already exists in xenstore */
+LOG(ERROR, device already exists in xenstore);
+rc = ERROR_DEVICE_EXISTS;
+goto out;
+}
+
+rc = libxl__set_domain_configuration(gc, domid, d_config);
+if (rc) goto out;
+
+libxl__device_generic_add(gc, t, device,
+ libxl__xs_kvs_of_flexarray(gc, back,
+ back-count),
+ libxl__xs_kvs_of_flexarray(gc, front,
+ front-count),
+ NULL);
+libxl__usbport_add_xenstore(gc, t, domid, usbctrl);
Still no rc check.
+rc = libxl__xs_transaction_commit(gc, t);
+if (!rc) break;
+if (rc 0) goto out;
+}
+
+out:
+if (lock) libxl__unlock_domain_userdata(lock);
+libxl_device_usbctrl_dispose(usbctrl_saved);
+libxl_domain_config_dispose(d_config);
+return rc;
+}
...
+static int
+libxl__device_usbctrl_remove_common(libxl_ctx *ctx, uint32_t domid,
+libxl_device_usbctrl *usbctrl,
+const libxl_asyncop_how *ao_how,
+int force)
+{
+AO_CREATE(ctx, domid, ao_how);
+libxl__device *device;
+libxl__ao_device *aodev;
+libxl_device_usbctrl *usbctrls = NULL;
+libxl_device_usbctrl *usbctrl_find = NULL;
+int numctrl = 0;
+libxl_device_usb *usbs = NULL;
+int numusb = 0;
+int i, rc;
+
+assert(usbctrl-devid != -1);
+
+usbctrls = libxl_device_usbctrl_list(ctx, domid, numctrl);
+if
