Re: [PATCH v2 2/3] hw/usb: Introduce x-query-usbhost QMP command

[Philippe Mathieu-Daudé]

On 11/6/24 12:23, Philippe Mathieu-Daudé wrote:
> This is a counterpart to the HMP "info usbhost" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> The existing HMP command is rewritten to call the QMP command.
>
> Since host-libusb.c can be built as part of the 'hw-usb' module,
> we introduce the libusb_register_hmp_info_hrt() helper to allow late
> registration when the module is loaded.
>
> Suggested-by: Daniel P. Berrangé <berrange@redhat.com>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   qapi/machine.json           | 18 ++++++++++++++++
>   hw/usb/host-libusb.h        | 16 ++++++++++++++
>   include/hw/usb.h            |  3 ---
>   hw/usb/bus-stub.c           |  7 +++++-
>   hw/usb/host-libusb-common.c | 31 ++++++++++++++++++++++++++
>   hw/usb/host-libusb.c        | 43 +++++++++++++++++++++++++------------
>   tests/qtest/qmp-cmd-test.c  |  3 +++
>   hmp-commands-info.hx        |  2 ++
>   hw/usb/meson.build          |  1 +
>   9 files changed, 106 insertions(+), 18 deletions(-)
>   create mode 100644 hw/usb/host-libusb.h
>   create mode 100644 hw/usb/host-libusb-common.c


> diff --git a/include/hw/usb.h b/include/hw/usb.h
> index d46d96779a..c0b34af518 100644
> --- a/include/hw/usb.h
> +++ b/include/hw/usb.h
> @@ -465,9 +465,6 @@ void usb_device_reset(USBDevice *dev);
>   void usb_wakeup(USBEndpoint *ep, unsigned int stream);
>   void usb_generic_async_ctrl_complete(USBDevice *s, USBPacket *p);
>   
> -/* usb-linux.c */
> -void hmp_info_usbhost(Monitor *mon, const QDict *qdict);
> -
>   /* usb ports of the VM */
>   
>   #define VM_USB_HUB_SIZE 8
> diff --git a/hw/usb/bus-stub.c b/hw/usb/bus-stub.c
> index fcabe8429e..948429bb33 100644
> --- a/hw/usb/bus-stub.c
> +++ b/hw/usb/bus-stub.c
> @@ -11,7 +11,6 @@
>   #include "qapi/error.h"
>   #include "qapi/qapi-commands-machine.h"
>   #include "sysemu/sysemu.h"
> -#include "monitor/monitor.h"
>   #include "hw/usb.h"
>   
>   USBDevice *usbdevice_create(const char *driver)
> @@ -26,3 +25,9 @@ HumanReadableText *qmp_x_query_usb(Error **errp)
>       error_setg(errp, "Support for USB devices not built-in");
>       return NULL;
>   }
> +
> +HumanReadableText *qmp_x_query_usbhost(Error **errp)
> +{
> +    error_setg(errp, "Support for USB devices not built-in");
> +    return NULL;
> +}
> diff --git a/hw/usb/host-libusb-common.c b/hw/usb/host-libusb-common.c
> new file mode 100644
> index 0000000000..406a2b25be
> --- /dev/null
> +++ b/hw/usb/host-libusb-common.c
> @@ -0,0 +1,31 @@
> +/*
> + * QEMU USB host redirector helpers
> + *
> + * SPDX-FileContributor: Philippe Mathieu-Daudé <philmd@linaro.org>
> + * SPDX-FileCopyrightText: 2024 Linaro Ltd.
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + */
> +
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "qapi/qapi-commands-machine.h"
> +#include "monitor/monitor.h"
> +#include "host-libusb.h"
> +
> +static HumanReadableText *(*qmp_x_query_usbhost_handler)(Error **errp);
> +
> +void libusb_register_hmp_info_hrt(HumanReadableText *(*handler)(Error **errp))
> +{
> +    qmp_x_query_usbhost_handler = handler;
> +    monitor_register_hmp_info_hrt("usbhost", handler);
> +}
> +
> +HumanReadableText *qmp_x_query_usbhost(Error **errp)
> +{
> +    if (module_load("hw-usb-", "host", errp) <= 0) {
> +        return NULL;
> +    }
> +    assert(qmp_x_query_usbhost_handler);
> +    return qmp_x_query_usbhost_handler(errp);
> +}



> diff --git a/hw/usb/meson.build b/hw/usb/meson.build
> index d7de1003e3..af92b504fd 100644
> --- a/hw/usb/meson.build
> +++ b/hw/usb/meson.build
> @@ -7,6 +7,7 @@ system_ss.add(when: 'CONFIG_USB', if_true: files(
>     'core.c',
>     'desc.c',
>     'desc-msos.c',
> +  'host-libusb-common.c',
>     'libhw.c',
>     'pcap.c',
>   ), if_false: files('bus-stub.c'))

Since this files depends on libusb, squashing:

-- >8 --
diff --git a/hw/usb/meson.build b/hw/usb/meson.build
index af92b504fd..89b197fbd8 100644
--- a/hw/usb/meson.build
+++ b/hw/usb/meson.build
@@ -9,3 +9,2 @@ system_ss.add(when: 'CONFIG_USB', if_true: files(
   'desc-msos.c',
-  'host-libusb-common.c',
   'libhw.c',
@@ -86,2 +85,3 @@ endif
 if libusb.found()
+  system_ss.add(when: 'CONFIG_USB', if_true: files('host-libusb-common.c'))
   usbhost_ss = ss.source_set()
---