| Message ID | 1523698174-22832-1-git-send-email-agk@godking.net |
|---|---|
| State | New |
| Headers | show |
| Series | qapi: [PATCH v2] Implement query-usbhost QMP command | expand |
On Sat, Apr 14, 2018 at 02:29:34AM -0700, Alexander Kappner wrote: > Implement a QMP command similar to the HMP's "info usbhost" command. The hmp version of the command should be switched over to call the qmp_query_usbhost() and pretty-print the qapi struct then, to make sure hmp and qmp versions of the command deliver consistent results. > This allows a QMP client to query which USB devices may be available > for redirection. I'm wondering what the use case is? At least libvirt sandboxes qemu, for security reasons, and you wouldn't get any useful results because qemu hasn't the permissions needed to scan the usb bus. > ## > +# @query-usbhost: > +# > +# Returns information about USB devices available on the host > +# > +# Returns: a [UsbDeviceInfo]. Returns an error if compiled without > +# CONFIG_USB_LIBUSB > +# > +# Since: TODO (maintainer insert version number if mainlined) Will not make it into 2.12, so 2.13. > +# @UsbDeviceInfo: > +# > +# @speed: the speed Use an enum for this? > +# @id_vendor: idVendor field from device descriptor > +# > +# @id_product: idProduct field from device descriptor > +# > +# @str_product: string descriptor referenced by iProduct index, if any > +# > +# @str_manufacturer: string descriptor referenced by iManufacturer index, if any > +# > +# @dev_addr: address on bus that device is connected to > +# > +# @bus_num: bus number device is connected to No port? cheers, Gerd
Hi Gerd, thanks for reviewing. I'll follow up with a v3 of the patch addressing your proposed changes. >> This allows a QMP client to query which USB devices may be available >> for redirection. > At least libvirt sandboxes qemu, for security reasons, and you wouldn't > get any useful results because qemu hasn't the permissions needed to > scan the usb bus. Then at least this tells you that you can't make anything available via redirection either, which is also useful information. > I'm wondering what the use case is? I'm working on a smartphone client that talks to a QEMU instance via QMP, and that allows the user to switch USB devices between host and QEMU guest. This removes the need for a separate set of input devices permanently assigned to the host (since if I pass through a keyboard/mouse to the VM, I can't use that same keyboard/mouse to run a command that re-attaches those devices to the host, so I need some external channel). Basically using the phone as a KVM switch. Best Alexander On 04/16/2018 01:29 AM, Gerd Hoffmann wrote: > On Sat, Apr 14, 2018 at 02:29:34AM -0700, Alexander Kappner wrote: >> Implement a QMP command similar to the HMP's "info usbhost" command. > > The hmp version of the command should be switched over to call the > qmp_query_usbhost() and pretty-print the qapi struct then, to make > sure hmp and qmp versions of the command deliver consistent results. > >> This allows a QMP client to query which USB devices may be available >> for redirection. > > I'm wondering what the use case is? > > At least libvirt sandboxes qemu, for security reasons, and you wouldn't > get any useful results because qemu hasn't the permissions needed to > scan the usb bus. > >> ## >> +# @query-usbhost: >> +# >> +# Returns information about USB devices available on the host >> +# >> +# Returns: a [UsbDeviceInfo]. Returns an error if compiled without >> +# CONFIG_USB_LIBUSB >> +# >> +# Since: TODO (maintainer insert version number if mainlined) > > Will not make it into 2.12, so 2.13. > >> +# @UsbDeviceInfo: >> +# >> +# @speed: the speed > > Use an enum for this? > >> +# @id_vendor: idVendor field from device descriptor >> +# >> +# @id_product: idProduct field from device descriptor >> +# >> +# @str_product: string descriptor referenced by iProduct index, if any >> +# >> +# @str_manufacturer: string descriptor referenced by iManufacturer index, if any >> +# >> +# @dev_addr: address on bus that device is connected to >> +# >> +# @bus_num: bus number device is connected to > > No port? > > cheers, > Gerd >
On Mon, Apr 16, 2018 at 03:52:03PM -0700, Alexander Kappner wrote: > Hi Gerd, > > thanks for reviewing. I'll follow up with a v3 of the patch addressing your proposed changes. > > >> This allows a QMP client to query which USB devices may be available > >> for redirection. > > > At least libvirt sandboxes qemu, for security reasons, and you wouldn't > > get any useful results because qemu hasn't the permissions needed to > > scan the usb bus. > Then at least this tells you that you can't make anything available via redirection either, which is also useful information. Well, you can, but you have to go through libvirt instead. Libvirt will whitelist the usb device you want assign then (and only that one) so qemu can access it. > > I'm wondering what the use case is? > I'm working on a smartphone client that talks to a QEMU instance via > QMP, and that allows the user to switch USB devices between host and > QEMU guest. This removes the need for a separate set of input devices > permanently assigned to the host (since if I pass through a > keyboard/mouse to the VM, I can't use that same keyboard/mouse to run > a command that re-attaches those devices to the host, so I need some > external channel). Basically using the phone as a KVM switch. That wouldn't work for any libvirt-managed guests. Also: Do you know input-linux (see ui/input-linux.c)? That allows switching input devices between host and (one) guest by keyboard hotkey. cheers, Gerd
On 04/14/2018 04:29 AM, Alexander Kappner wrote: > Implement a QMP command similar to the HMP's "info usbhost" command. > This allows a QMP client to query which USB devices may be available > for redirection. Because the availability of the command needs to > depend on the target's (not the build host's) USB configuration, > a stub function in host-stub.c is provided for targets without USB support. > > v2 of this patch resolves build failure under some configurations > without libusb. > > Signed-off-by: Alexander Kappner <agk@godking.net> > --- > hw/usb/host-libusb.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ > hw/usb/host-stub.c | 9 ++++++++ > qapi/misc.json | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 135 insertions(+) > > +++ b/qapi/misc.json > @@ -270,6 +270,46 @@ > { 'command': 'query-kvm', 'returns': 'KvmInfo' } > > ## > +# @query-usbhost: > +# > +# Returns information about USB devices available on the host > +# > +# Returns: a [UsbDeviceInfo]. Returns an error if compiled without > +# CONFIG_USB_LIBUSB s/a [UsbDeviceInfo]/a list of UsbDeviceInfo > +# > +# Since: TODO (maintainer insert version number if mainlined) 2.13 (it's easier to guess the next version, on the likely chance that it does not need tweaking, than it is to force the maintainer to have to tweak the patch on acceptance) > +## > +{ 'command': 'query-usbhost', 'returns': ['UsbDeviceInfo'] } > + > +## > # @UuidInfo: > # > # Guest UUID information (Universally Unique Identifier). > @@ -876,6 +916,28 @@ > 'regions': ['PciMemoryRegion']} } > > ## > +# @UsbDeviceInfo: > +# > +# @speed: the speed > +# > +# @id_vendor: idVendor field from device descriptor Please, new QMP interfaces should favor '-' over '_' in naming; so this should be id-vendor, and so on. > +# > +# @id_product: idProduct field from device descriptor > +# > +# @str_product: string descriptor referenced by iProduct index, if any What's the difference between idProduct and iProduct in the text here? > +# > +# @str_manufacturer: string descriptor referenced by iManufacturer index, if any Is the 'str_' (or my preferred switch to 'str-') prefix necessary; or can this field just be 'manufacturer'? > +# > +# @dev_addr: address on bus that device is connected to > +# > +# @bus_num: bus number device is connected to > +## > +{ 'struct': 'UsbDeviceInfo', > + 'data': > + {'speed': 'int', 'id_vendor': 'int', 'id_product' : 'int', 'str_product': 'str', > + 'b_device_class': 'int', 'str_manufacturer' : 'str', 'dev_addr' : 'int', 'bus_num' : 'int'} } Long line; please fit things within 80 columns.
diff --git a/hw/usb/host-libusb.c b/hw/usb/host-libusb.c index 1b0be07..efdf577 100644 --- a/hw/usb/host-libusb.c +++ b/hw/usb/host-libusb.c @@ -40,6 +40,7 @@ #include <libusb.h> #include "qapi/error.h" +#include "qapi/qapi-commands-misc.h" #include "qemu-common.h" #include "monitor/monitor.h" #include "qemu/error-report.h" @@ -1743,6 +1744,69 @@ bool usb_host_dev_is_scsi_storage(USBDevice *ud) return is_scsi_storage; } +UsbDeviceInfoList *qmp_query_usbhost(Error **errp) +{ + UsbDeviceInfoList *usb_list = NULL; + UsbDeviceInfoList *info; + libusb_device **devs = NULL; + struct libusb_device_descriptor ddesc; + char port[16]; + int i, n; + + if (usb_host_init() != 0) { + return NULL; + } + n = libusb_get_device_list(ctx, &devs); + + for (i = 0; i < n; i++) { + if (libusb_get_device_descriptor(devs[i], &ddesc) != 0) { + continue; + } + if (ddesc.bDeviceClass == LIBUSB_CLASS_HUB) { + continue; + } + usb_host_get_port(devs[i], port, sizeof(port)); + info = g_new0(UsbDeviceInfoList, 1); + info->value = g_new0(UsbDeviceInfo, 1); + info->value->id_vendor = ddesc.idVendor; + info->value->bus_num = libusb_get_bus_number(devs[i]); + info->value->dev_addr = libusb_get_device_address(devs[i]); + info->value->id_product = ddesc.idProduct; + info->value->b_device_class = ddesc.bDeviceClass; + info->value->speed = libusb_get_device_speed(devs[i]); + info->next = usb_list; + usb_list = info; + + if (ddesc.iProduct) { + libusb_device_handle *handle; + if (libusb_open(devs[i], &handle) == 0) { + unsigned char name[64] = ""; + libusb_get_string_descriptor_ascii(handle, + ddesc.iProduct, + name, sizeof(name)); + libusb_close(handle); + info->value->str_product = g_strdup((gchar *)name); + } + } else + info->value->str_product = NULL; + + if (ddesc.iManufacturer) { + libusb_device_handle *handle; + if (libusb_open(devs[i], &handle) == 0) { + unsigned char name[64] = ""; + libusb_get_string_descriptor_ascii(handle, + ddesc.iManufacturer, + name, sizeof(name)); + libusb_close(handle); + info->value->str_manufacturer = g_strdup((gchar *)name); + } + } else + info->value->str_manufacturer = NULL; + } + libusb_free_device_list(devs, 1); + return usb_list; +} + void hmp_info_usbhost(Monitor *mon, const QDict *qdict) { libusb_device **devs = NULL; diff --git a/hw/usb/host-stub.c b/hw/usb/host-stub.c index 41d93ec..fd227c4 100644 --- a/hw/usb/host-stub.c +++ b/hw/usb/host-stub.c @@ -35,6 +35,9 @@ #include "ui/console.h" #include "hw/usb.h" #include "monitor/monitor.h" +#include "qapi/error.h" +#include "qapi/qapi-commands-misc.h" +#include "qapi/qmp/qerror.h" void hmp_info_usbhost(Monitor *mon, const QDict *qdict) { @@ -45,3 +48,9 @@ bool usb_host_dev_is_scsi_storage(USBDevice *ud) { return false; } + +UsbDeviceInfoList *qmp_query_usbhost(Error **errp) +{ + error_setg(errp, QERR_FEATURE_DISABLED, "usb"); + return NULL; +}; diff --git a/qapi/misc.json b/qapi/misc.json index 5636f4a..19a1453 100644 --- a/qapi/misc.json +++ b/qapi/misc.json @@ -270,6 +270,46 @@ { 'command': 'query-kvm', 'returns': 'KvmInfo' } ## +# @query-usbhost: +# +# Returns information about USB devices available on the host +# +# Returns: a [UsbDeviceInfo]. Returns an error if compiled without +# CONFIG_USB_LIBUSB +# +# Since: TODO (maintainer insert version number if mainlined) +# +# Example: +# +# -> {"execute": "query-usbhost" } +# { +# "return": [ +# { +# "b_device_class": 239, +# "id_product": 793, +# "id_vendor": 3468, +# "speed": 3, +# "str_manufacturer": "Schiit Audio", +# "str_product": "Schiit Modi Uber" +# "bus_num": 5, +# "dev_addr": 21 +# }, +# { +# "b_device_class": 0, +# "id_product": 6944, +# "id_vendor": 6940, +# "speed": 2, +# "str_manufacturer": "Corsair", +# "str_product": "Corsair STRAFE RGB Gaming Keyboard" +# "bus_num": 5, +# "dev_addr": 35 +# } +# ] +# } +## +{ 'command': 'query-usbhost', 'returns': ['UsbDeviceInfo'] } + +## # @UuidInfo: # # Guest UUID information (Universally Unique Identifier). @@ -876,6 +916,28 @@ 'regions': ['PciMemoryRegion']} } ## +# @UsbDeviceInfo: +# +# @speed: the speed +# +# @id_vendor: idVendor field from device descriptor +# +# @id_product: idProduct field from device descriptor +# +# @str_product: string descriptor referenced by iProduct index, if any +# +# @str_manufacturer: string descriptor referenced by iManufacturer index, if any +# +# @dev_addr: address on bus that device is connected to +# +# @bus_num: bus number device is connected to +## +{ 'struct': 'UsbDeviceInfo', + 'data': + {'speed': 'int', 'id_vendor': 'int', 'id_product' : 'int', 'str_product': 'str', + 'b_device_class': 'int', 'str_manufacturer' : 'str', 'dev_addr' : 'int', 'bus_num' : 'int'} } + +## # @PciInfo: # # Information about a PCI bus
Implement a QMP command similar to the HMP's "info usbhost" command. This allows a QMP client to query which USB devices may be available for redirection. Because the availability of the command needs to depend on the target's (not the build host's) USB configuration, a stub function in host-stub.c is provided for targets without USB support. v2 of this patch resolves build failure under some configurations without libusb. Signed-off-by: Alexander Kappner <agk@godking.net> --- hw/usb/host-libusb.c | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++ hw/usb/host-stub.c | 9 ++++++++ qapi/misc.json | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 135 insertions(+)