在 2:06 的 2015/5/20 上，在讯息
caflbxzzhffwo6tm7602y3+x5kx65-w4obfs1vdvb8kqnzda...@mail.gmail.com 中，George
Dunlap george.dun...@eu.citrix.com 写入：
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:
Hi, George, thanks very much!
I'm so sorry for missing the message and not reply immediately.
Before sending new version, I'm answering some of your questions here.
And there are a couple of comments, I still have some hesitation to follow.
All others, I agree and will update as you suggested.
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?
Referring to xm pvusb implementation, only HUB is excluded, so I
just keep the same.
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.
I thought about that when implementing, but finally not added to codes
considering:
* for all libxl pvusb internal usage, busid is enough.
* for toolstack usage, if we want to expose users useful information about
bus:addr,
vendorID:devieID, we have libxl_device_usb_getinfo API, with this API callers
can get
all information including hostbus, hostaddr.
If that couldn't satisfy qemu usage, I can add a translating to
