Re: [Qemu-devel] qapi: [PATCH v2] Implement query-usbhost QMP command

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2880 bytes --]

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.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]