[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH v2 3/4] libxl: add HVM usb passthrough support
On 19/09/16 17:31, George Dunlap wrote: > On 19/09/16 13:40, Juergen Gross wrote: >> Add HVM usb passthrough support to libxl by using qemu's capability >> to emulate standard USB controllers. >> >> A USB controller is added via qmp command to the emulated hardware >> when a usbctrl device of type DEVICEMODEL is requested. Depending on >> the requested speed the appropriate hardware type is selected. A host >> USB device can then be added to the emulated USB controller via qmp >> command. >> >> Removing of the devices is done via qmp commands, too. >> >> Signed-off-by: Juergen Gross <jgross@xxxxxxxx> > > Thanks Juergen! That was a pretty great turn-around time. :-) > > Overall everything looks reasonable, with just a few nits... > >> @@ -278,13 +460,6 @@ static void libxl__device_usbctrl_add(libxl__egc *egc, >> uint32_t domid, >> } >> } >> >> - if (usbctrl->type != LIBXL_USBCTRL_TYPE_PV && >> - usbctrl->type != LIBXL_USBCTRL_TYPE_QUSB) { >> - LOG(ERROR, "Unsupported USB controller type"); >> - rc = ERROR_FAIL; >> - goto out; >> - } >> - >> rc = libxl__device_usbctrl_add_xenstore(gc, domid, usbctrl, >> aodev->update_json); >> if (rc) goto out; >> @@ -293,6 +468,12 @@ static void libxl__device_usbctrl_add(libxl__egc *egc, >> uint32_t domid, >> rc = libxl__device_from_usbctrl(gc, domid, usbctrl, device); >> if (rc) goto outrm; >> >> + if (device->backend_kind == LIBXL__DEVICE_KIND_NONE) { >> + rc = libxl__device_usbctrl_add_hvm(gc, domid, usbctrl); >> + if (rc) goto outrm; >> + goto out; >> + } > > This is sort of minor, but this seems like the wrong conditional. Is > there a reason you did't check for usbctrl->type == HVM here (as I think > you did on the usbdev_add path)? No, I don't think so. I don't want to wait for the backend to connect in case it isn't even there, so the conditional seems to be just the right one. In the usbdev_add path I have to take different measures for each of the possible three controller types (visb, qusb or devicemodel), so doing a switch over usbctrl->type seems to be the natural choice. > >> +static char *pvusb_get_port_path(libxl__gc *gc, uint32_t domid, >> + libxl_usbctrl_type type, int ctrl, int >> port) >> +{ >> + char *path; >> + >> + if (type == LIBXL_USBCTRL_TYPE_DEVICEMODEL) >> + path = GCSPRINTF("%s/device/vusb", libxl__xs_libxl_path(gc, domid)); >> + else >> + path = GCSPRINTF("%s/backend/%s/%d", >> + libxl__xs_get_dompath(gc, LIBXL_TOOLSTACK_DOMID), >> + pvusb_get_device_type(type), domid); >> + >> + return GCSPRINTF("%s/%d/port/%d", path, ctrl, port); >> +} > > Should this function be named "vusb_get..." rather than "pvusb_get...""? > After all, it returns the port even for non-PV usb ports. Yes, you are right. > I think that's all I had on this at a quick pass-through. > > The one other thing to bring up is how this new emulated usb interface > in the config file (usbctrl and usbdev) will interact with the old > emulated usb interface (usb, usbversion, and usbdevice). If we enable a > reasonable set of emulated-only devices (the tablet in particular), I > think we should deprecate the old interfaces (though continue supporting > them for backwards compatibility, of course). Thoughts? The old interface is available for domain config only, right? I think we should try to convert this stuff to the new interface in order to be able to use it for a running domain, too. But I think this will be a 4.9 topic. Juergen _______________________________________________ Xen-devel mailing list Xen-devel@xxxxxxxxxxxxx https://lists.xen.org/xen-devel
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |