[PATCH v3 00/18] ui: Move and clean up monitor command code

[PATCH v3 00/18] ui: Move and clean up monitor command code

[Markus Armbruster]

This is mainly about splitting off monitor-related code.  There's also
a minimum Spice version bump, and a few UI improvements to HMP
commands sendkey, change vnc, and info spice.

The only reason for keeping new PATCH 12 separate from old PATCH 11 is
preserving PATCH 11's R-bys.  I might squash them together.

v3:
* Rebased, straighforward conflicts with "qapi: Elide redundant
  has_FOO in generated C" (merge commit ae2b87341b5)
* PATCH 09: Commit message pasto fixed
* PATCH 11: Comment tweaked
* PATCH 12: New
* PATCH 13: protocol_table[] made const [Philippe], #include tweaked
* PATCH 16-18: New

Markus Armbruster (18):
  ui: Check numeric part of expire_password argument @time properly
  ui: Fix silent truncation of numeric keys in HMP sendkey
  ui/spice: Require spice-protocol >= 0.14.0
  Revert "hmp: info spice: take out webdav"
  ui/spice: Require spice-server >= 0.14.0
  ui/spice: QXLInterface method set_mm_time() is now dead, drop
  ui/spice: Give hmp_info_spice()'s channel_names[] static linkage
  ui: Clean up a few things checkpatch.pl would flag later on
  ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
  ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c
  ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
  ui: Move more HMP commands from monitor to ui/
  ui: Improve "change vnc" error reporting
  ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c
  ui: Reduce nesting in hmp_change_vnc() slightly
  ui: Split hmp_mouse_set() and move the HMP part to ui/
  ui: Don't check for mode change after mouse_set error
  ui: Simplify control flow in qemu_mouse_set()

 meson.build                   |   4 +-
 hw/display/qxl.h              |   2 -
 include/monitor/hmp.h         |   8 +
 include/monitor/qmp-helpers.h |  26 ++
 include/ui/console.h          |   2 +-
 include/ui/qemu-spice.h       |   8 +-
 include/ui/spice-display.h    |   2 -
 chardev/spice.c               |   2 -
 hw/display/qxl.c              |  26 +-
 monitor/hmp-cmds.c            | 368 +--------------------------
 monitor/misc.c                |  67 -----
 monitor/qmp-cmds.c            | 176 +++----------
 ui/input.c                    |  28 +--
 ui/spice-display.c            |  10 -
 ui/ui-hmp-cmds.c              | 461 ++++++++++++++++++++++++++++++++++
 ui/ui-qmp-cmds.c              | 177 +++++++++++++
 ui/vdagent.c                  |   4 -
 hw/display/trace-events       |   1 -
 ui/meson.build                |   2 +
 19 files changed, 725 insertions(+), 649 deletions(-)
 create mode 100644 include/monitor/qmp-helpers.h
 create mode 100644 ui/ui-hmp-cmds.c
 create mode 100644 ui/ui-qmp-cmds.c

-- 
2.38.1

[PATCH v3 01/18] ui: Check numeric part of expire_password argument @time properly

[Markus Armbruster]

When argument @time isn't 'now' or 'never', we parse it as an integer,
optionally prefixed with '+'.  If parsing fails, we silently assume
zero.  Report an error and fail instead.

While there, use qemu_strtou64() instead of strtoull() so
checkpatch.pl won't complain.

Aside: encoding numbers in strings is bad QMP practice.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/qmp-cmds.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 2932b3f3a5..a1695b6c96 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -201,15 +201,28 @@ void qmp_expire_password(ExpirePasswordOptions *opts, Error **errp)
     time_t when;
     int rc;
     const char *whenstr = opts->time;
+    const char *numstr = NULL;
+    uint64_t num;
 
     if (strcmp(whenstr, "now") == 0) {
         when = 0;
     } else if (strcmp(whenstr, "never") == 0) {
         when = TIME_MAX;
     } else if (whenstr[0] == '+') {
-        when = time(NULL) + strtoull(whenstr+1, NULL, 10);
+        when = time(NULL);
+        numstr = whenstr + 1;
     } else {
-        when = strtoull(whenstr, NULL, 10);
+        when = 0;
+        numstr = whenstr;
+    }
+
+    if (numstr) {
+        if (qemu_strtou64(numstr, NULL, 10, &num) < 0) {
+            error_setg(errp, "Parameter 'time' doesn't take value '%s'",
+                       whenstr);
+            return;
+        }
+        when += num;
     }
 
     if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
-- 
2.38.1

[PATCH v3 02/18] ui: Fix silent truncation of numeric keys in HMP sendkey

[Markus Armbruster]

Keys are int.  HMP sendkey assigns them from the value strtoul(),
silently truncating values greater than INT_MAX.  Fix to reject them.

While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
won't complain.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/hmp-cmds.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index ed78a87ddd..b8e294e6fa 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1549,8 +1549,13 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
         v = g_malloc0(sizeof(*v));
 
         if (strstart(keys, "0x", NULL)) {
-            char *endp;
-            int value = strtoul(keys, &endp, 0);
+            const char *endp;
+            unsigned long value;
+
+            if (qemu_strtoul(keys, &endp, 0, &value) < 0
+                || value >= INT_MAX) {
+                goto err_out;
+            }
             assert(endp <= keys + keyname_len);
             if (endp != keys + keyname_len) {
                 goto err_out;
-- 
2.38.1

[PATCH v3 03/18] ui/spice: Require spice-protocol >= 0.14.0

[Markus Armbruster]

Version 0.14.0 is now old enough to have made it into the major
distributions:

   Debian 11: 0.14.3
   RHEL-8: 0.14.2
   FreeBSD (ports): 0.14.4
   Fedora 35: 0.14.0
   Ubuntu 20.04: 0.14.0
   OpenSUSE Leap 15.3: 0.14.3

Requiring it lets us drop two version checks in ui/vdagent.c.  It also
enables the next commit.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build  | 2 +-
 ui/vdagent.c | 4 ----
 2 files changed, 1 insertion(+), 5 deletions(-)

diff --git a/meson.build b/meson.build
index 5c6b5a1c75..9f27c5cea3 100644
--- a/meson.build
+++ b/meson.build
@@ -740,7 +740,7 @@ endif
 
 spice_protocol = not_found
 if not get_option('spice_protocol').auto() or have_system
-  spice_protocol = dependency('spice-protocol', version: '>=0.12.3',
+  spice_protocol = dependency('spice-protocol', version: '>=0.14.0',
                               required: get_option('spice_protocol'),
                               method: 'pkg-config', kwargs: static_kwargs)
 endif
diff --git a/ui/vdagent.c b/ui/vdagent.c
index 4bf50f0c4d..1f51a78da1 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -87,9 +87,7 @@ static const char *cap_name[] = {
     [VD_AGENT_CAP_MONITORS_CONFIG_POSITION]       = "monitors-config-position",
     [VD_AGENT_CAP_FILE_XFER_DISABLED]             = "file-xfer-disabled",
     [VD_AGENT_CAP_FILE_XFER_DETAILED_ERRORS]      = "file-xfer-detailed-errors",
-#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 0)
     [VD_AGENT_CAP_GRAPHICS_DEVICE_INFO]           = "graphics-device-info",
-#endif
 #if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 1)
     [VD_AGENT_CAP_CLIPBOARD_NO_RELEASE_ON_REGRAB] = "clipboard-no-release-on-regrab",
     [VD_AGENT_CAP_CLIPBOARD_GRAB_SERIAL]          = "clipboard-grab-serial",
@@ -112,9 +110,7 @@ static const char *msg_name[] = {
     [VD_AGENT_CLIENT_DISCONNECTED]   = "client-disconnected",
     [VD_AGENT_MAX_CLIPBOARD]         = "max-clipboard",
     [VD_AGENT_AUDIO_VOLUME_SYNC]     = "audio-volume-sync",
-#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 0)
     [VD_AGENT_GRAPHICS_DEVICE_INFO]  = "graphics-device-info",
-#endif
 };
 
 static const char *sel_name[] = {
-- 
2.38.1

[PATCH v3 04/18] Revert "hmp: info spice: take out webdav"

[Markus Armbruster]

This reverts commit 7c6044a94e52db8aef9a71d616c7a0914adb71ab.

We had to take it out because SPICE_CHANNEL_WEBDAV requires
spice-protocol 0.12.7, but we had only 0.12.3.  We have 0.14.0 now, so
put it back in.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/hmp-cmds.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index b8e294e6fa..664b3454bb 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -622,12 +622,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
         [SPICE_CHANNEL_SMARTCARD] = "smartcard",
         [SPICE_CHANNEL_USBREDIR] = "usbredir",
         [SPICE_CHANNEL_PORT] = "port",
-#if 0
-        /* minimum spice-protocol is 0.12.3, webdav was added in 0.12.7,
-         * no easy way to #ifdef (SPICE_CHANNEL_* is a enum).  Disable
-         * as quick fix for build failures with older versions. */
         [SPICE_CHANNEL_WEBDAV] = "webdav",
-#endif
     };
 
     info = qmp_query_spice(NULL);
-- 
2.38.1

[PATCH v3 05/18] ui/spice: Require spice-server >= 0.14.0

[Markus Armbruster]

Version 0.14.0 is now old enough to have made it into the major
distributions:

     Debian 11: 0.14.3
     RHEL-8: 0.14.3
     FreeBSD (ports): 0.15.0
     Fedora 35: 0.15.0
     Ubuntu 20.04: 0.14.2
     OpenSUSE Leap 15.3: 0.14.3

Requiring it lets us drop a number of version checks.  The next commit
will clean up some more.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build                | 2 +-
 hw/display/qxl.h           | 2 --
 include/ui/qemu-spice.h    | 6 +-----
 include/ui/spice-display.h | 2 --
 chardev/spice.c            | 2 --
 hw/display/qxl.c           | 7 +------
 6 files changed, 3 insertions(+), 18 deletions(-)

diff --git a/meson.build b/meson.build
index 9f27c5cea3..31705f7fe9 100644
--- a/meson.build
+++ b/meson.build
@@ -746,7 +746,7 @@ if not get_option('spice_protocol').auto() or have_system
 endif
 spice = not_found
 if not get_option('spice').auto() or have_system
-  spice = dependency('spice-server', version: '>=0.12.5',
+  spice = dependency('spice-server', version: '>=0.14.0',
                      required: get_option('spice'),
                      method: 'pkg-config', kwargs: static_kwargs)
 endif
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 7894bd5134..5bec25cdd3 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -100,9 +100,7 @@ struct PCIQXLDevice {
     QXLModes           *modes;
     uint32_t           rom_size;
     MemoryRegion       rom_bar;
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
     uint16_t           max_outputs;
-#endif
 
     /* vram pci bar */
     uint64_t           vram_size;
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 21fe195e18..a7a1890b3f 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -34,13 +34,9 @@ int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
-#define SPICE_NEEDS_SET_MM_TIME 1
-#else
 #define SPICE_NEEDS_SET_MM_TIME 0
-#endif
 
-#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
+#if SPICE_SERVER_VERSION >= 0x000f00 /* release 0.15.0 */
 #define SPICE_HAS_ATTACHED_WORKER 1
 #else
 #define SPICE_HAS_ATTACHED_WORKER 0
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index e271e011da..5aa13664d6 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -28,11 +28,9 @@
 #include "ui/console.h"
 
 #if defined(CONFIG_OPENGL) && defined(CONFIG_GBM)
-# if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */
 #  define HAVE_SPICE_GL 1
 #  include "ui/egl-helpers.h"
 #  include "ui/egl-context.h"
-# endif
 #endif
 
 #define NUM_MEMSLOTS 8
diff --git a/chardev/spice.c b/chardev/spice.c
index bbffef4913..e843d961a7 100644
--- a/chardev/spice.c
+++ b/chardev/spice.c
@@ -98,9 +98,7 @@ static SpiceCharDeviceInterface vmc_interface = {
     .write              = vmc_write,
     .read               = vmc_read,
     .event              = vmc_event,
-#if SPICE_SERVER_VERSION >= 0x000c06
     .flags              = SPICE_CHAR_DEVICE_NOTIFY_WRITABLE,
-#endif
 };
 
 
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index 6772849dec..ddca611804 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -260,8 +260,7 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
                     QXL_COOKIE_TYPE_POST_LOAD_MONITORS_CONFIG,
                     0));
     } else {
-/* >= release 0.12.6, < release 0.14.2 */
-#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
+#if SPICE_SERVER_VERSION < 0x000e02 /* release 0.14.2 */
         if (qxl->max_outputs) {
             spice_qxl_set_max_monitors(&qxl->ssd.qxl, qxl->max_outputs);
         }
@@ -1089,12 +1088,10 @@ static int interface_client_monitors_config(QXLInstance *sin,
         return 1;
     }
 
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
     /* limit number of outputs based on setting limit */
     if (qxl->max_outputs && qxl->max_outputs <= max_outputs) {
         max_outputs = qxl->max_outputs;
     }
-#endif
 
     config_changed = qxl_rom_monitors_config_changed(rom,
                                                      monitors_config,
@@ -2487,9 +2484,7 @@ static Property qxl_properties[] = {
         DEFINE_PROP_UINT32("vram64_size_mb", PCIQXLDevice, vram_size_mb, -1),
         DEFINE_PROP_UINT32("vgamem_mb", PCIQXLDevice, vgamem_size_mb, 16),
         DEFINE_PROP_INT32("surfaces", PCIQXLDevice, ssd.num_surfaces, 1024),
-#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
         DEFINE_PROP_UINT16("max_outputs", PCIQXLDevice, max_outputs, 0),
-#endif
         DEFINE_PROP_UINT32("xres", PCIQXLDevice, xres, 0),
         DEFINE_PROP_UINT32("yres", PCIQXLDevice, yres, 0),
         DEFINE_PROP_BOOL("global-vmstate", PCIQXLDevice, vga.global_vmstate, false),
-- 
2.38.1

[PATCH v3 06/18] ui/spice: QXLInterface method set_mm_time() is now dead, drop

[Markus Armbruster]

SPICE_NEEDS_SET_MM_TIME is now always off.  Bury the dead code.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/ui/qemu-spice.h |  2 --
 hw/display/qxl.c        | 19 -------------------
 ui/spice-display.c      | 10 ----------
 hw/display/trace-events |  1 -
 4 files changed, 32 deletions(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index a7a1890b3f..b7d493742c 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -34,8 +34,6 @@ int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-#define SPICE_NEEDS_SET_MM_TIME 0
-
 #if SPICE_SERVER_VERSION >= 0x000f00 /* release 0.15.0 */
 #define SPICE_HAS_ATTACHED_WORKER 1
 #else
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index ddca611804..ec712d3ca2 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -543,22 +543,6 @@ static void interface_set_compression_level(QXLInstance *sin, int level)
     qxl_rom_set_dirty(qxl);
 }
 
-#if SPICE_NEEDS_SET_MM_TIME
-static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
-{
-    PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
-
-    if (!qemu_spice_display_is_running(&qxl->ssd)) {
-        return;
-    }
-
-    trace_qxl_interface_set_mm_time(qxl->id, mm_time);
-    qxl->shadow_rom.mm_clock = cpu_to_le32(mm_time);
-    qxl->rom->mm_clock = cpu_to_le32(mm_time);
-    qxl_rom_set_dirty(qxl);
-}
-#endif
-
 static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
 {
     PCIQXLDevice *qxl = container_of(sin, PCIQXLDevice, ssd.qxl);
@@ -1145,9 +1129,6 @@ static const QXLInterface qxl_interface = {
 #endif
 
     .set_compression_level   = interface_set_compression_level,
-#if SPICE_NEEDS_SET_MM_TIME
-    .set_mm_time             = interface_set_mm_time,
-#endif
     .get_init_info           = interface_get_init_info,
 
     /* the callbacks below are called from spice server thread context */
diff --git a/ui/spice-display.c b/ui/spice-display.c
index 494168e7fe..0616a6982f 100644
--- a/ui/spice-display.c
+++ b/ui/spice-display.c
@@ -517,13 +517,6 @@ static void interface_set_compression_level(QXLInstance *sin, int level)
     /* nothing to do */
 }
 
-#if SPICE_NEEDS_SET_MM_TIME
-static void interface_set_mm_time(QXLInstance *sin, uint32_t mm_time)
-{
-    /* nothing to do */
-}
-#endif
-
 static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
 {
     SimpleSpiceDisplay *ssd = container_of(sin, SimpleSpiceDisplay, qxl);
@@ -715,9 +708,6 @@ static const QXLInterface dpy_interface = {
     .attache_worker          = interface_attach_worker,
 #endif
     .set_compression_level   = interface_set_compression_level,
-#if SPICE_NEEDS_SET_MM_TIME
-    .set_mm_time             = interface_set_mm_time,
-#endif
     .get_init_info           = interface_get_init_info,
 
     /* the callbacks below are called from spice server thread context */
diff --git a/hw/display/trace-events b/hw/display/trace-events
index 0c0ffcbe42..2336a0ca15 100644
--- a/hw/display/trace-events
+++ b/hw/display/trace-events
@@ -55,7 +55,6 @@ virtio_gpu_fence_ctrl(uint64_t fence, uint32_t type) "fence 0x%" PRIx64 ", type
 virtio_gpu_fence_resp(uint64_t fence) "fence 0x%" PRIx64
 
 # qxl.c
-disable qxl_interface_set_mm_time(int qid, uint32_t mm_time) "%d %d"
 disable qxl_io_write_vga(int qid, const char *mode, uint32_t addr, uint32_t val) "%d %s addr=%u val=%u"
 qxl_create_guest_primary(int qid, uint32_t width, uint32_t height, uint64_t mem, uint32_t format, uint32_t position) "%d %ux%u mem=0x%" PRIx64 " %u,%u"
 qxl_create_guest_primary_rest(int qid, int32_t stride, uint32_t type, uint32_t flags) "%d %d,%d,%d"
-- 
2.38.1

[PATCH v3 07/18] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage

[Markus Armbruster]

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 monitor/hmp-cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 664b3454bb..4d57520f82 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -611,7 +611,7 @@ void hmp_info_spice(Monitor *mon, const QDict *qdict)
     SpiceChannelList *chan;
     SpiceInfo *info;
     const char *channel_name;
-    const char * const channel_names[] = {
+    static const char *const channel_names[] = {
         [SPICE_CHANNEL_MAIN] = "main",
         [SPICE_CHANNEL_DISPLAY] = "display",
         [SPICE_CHANNEL_INPUTS] = "inputs",
-- 
2.38.1

[PATCH v3 08/18] ui: Clean up a few things checkpatch.pl would flag later on

[Markus Armbruster]

Fix a few style violations so that checkpatch.pl won't complain when I
move this code.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/hmp-cmds.c |  7 ++++---
 monitor/qmp-cmds.c | 21 +++++++++++----------
 2 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 4d57520f82..5b41225bb9 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -587,9 +587,10 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
         hmp_info_vnc_servers(mon, info->server);
         hmp_info_vnc_clients(mon, info->clients);
         if (!info->server) {
-            /* The server entry displays its auth, we only
-             * need to display in the case of 'reverse' connections
-             * where there's no server.
+            /*
+             * The server entry displays its auth, we only need to
+             * display in the case of 'reverse' connections where
+             * there's no server.
              */
             hmp_info_vnc_authcrypt(mon, "  ", info->auth,
                                info->has_vencrypt ? &info->vencrypt : NULL);
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index a1695b6c96..6d6df86607 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -186,8 +186,10 @@ void qmp_set_password(SetPasswordOptions *opts, Error **errp)
             error_setg(errp, QERR_INVALID_PARAMETER, "connected");
             return;
         }
-        /* Note that setting an empty password will not disable login through
-         * this interface. */
+        /*
+         * Note that setting an empty password will not disable login
+         * through this interface.
+         */
         rc = vnc_display_password(opts->u.vnc.display, opts->password);
     }
 
@@ -272,12 +274,10 @@ void qmp_add_client(const char *protocol, const char *fdname,
             error_setg(errp, "spice failed to add client");
             close(fd);
         }
-        return;
 #ifdef CONFIG_VNC
     } else if (strcmp(protocol, "vnc") == 0) {
         skipauth = has_skipauth ? skipauth : false;
         vnc_display_add_client(NULL, fd, skipauth);
-        return;
 #endif
 #ifdef CONFIG_DBUS_DISPLAY
     } else if (strcmp(protocol, "@dbus-display") == 0) {
@@ -289,19 +289,20 @@ void qmp_add_client(const char *protocol, const char *fdname,
             close(fd);
             return;
         }
-        return;
 #endif
-    } else if ((s = qemu_chr_find(protocol)) != NULL) {
+    } else {
+        s = qemu_chr_find(protocol);
+        if (!s) {
+            error_setg(errp, "protocol '%s' is invalid", protocol);
+            close(fd);
+            return;
+        }
         if (qemu_chr_add_client(s, fd) < 0) {
             error_setg(errp, "failed to add client");
             close(fd);
             return;
         }
-        return;
     }
-
-    error_setg(errp, "protocol '%s' is invalid", protocol);
-    close(fd);
 }
 
 
-- 
2.38.1

[PATCH v3 09/18] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c

[Markus Armbruster]

This moves these commands from MAINTAINERS section "QMP" to
"Graphics".

Command add-client applies to socket character devices in addition to
display devices.  Move it anyway.  Aside: the way @protocol character
device IDs and display types is bad design.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 monitor/qmp-cmds.c | 118 ---------------------------------------
 ui/ui-qmp-cmds.c   | 136 +++++++++++++++++++++++++++++++++++++++++++++
 ui/meson.build     |   1 +
 3 files changed, 137 insertions(+), 118 deletions(-)
 create mode 100644 ui/ui-qmp-cmds.c

diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 6d6df86607..61449f1b58 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -36,9 +36,7 @@
 #include "qapi/qapi-commands-machine.h"
 #include "qapi/qapi-commands-misc.h"
 #include "qapi/qapi-commands-stats.h"
-#include "qapi/qapi-commands-ui.h"
 #include "qapi/type-helpers.h"
-#include "qapi/qmp/qerror.h"
 #include "exec/ramlist.h"
 #include "hw/mem/memory-device.h"
 #include "hw/acpi/acpi_dev_interface.h"
@@ -168,89 +166,6 @@ void qmp_system_wakeup(Error **errp)
     qemu_system_wakeup_request(QEMU_WAKEUP_REASON_OTHER, errp);
 }
 
-void qmp_set_password(SetPasswordOptions *opts, Error **errp)
-{
-    int rc;
-
-    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
-        if (!qemu_using_spice(errp)) {
-            return;
-        }
-        rc = qemu_spice.set_passwd(opts->password,
-                opts->connected == SET_PASSWORD_ACTION_FAIL,
-                opts->connected == SET_PASSWORD_ACTION_DISCONNECT);
-    } else {
-        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
-        if (opts->connected != SET_PASSWORD_ACTION_KEEP) {
-            /* vnc supports "connected=keep" only */
-            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
-            return;
-        }
-        /*
-         * Note that setting an empty password will not disable login
-         * through this interface.
-         */
-        rc = vnc_display_password(opts->u.vnc.display, opts->password);
-    }
-
-    if (rc != 0) {
-        error_setg(errp, "Could not set password");
-    }
-}
-
-void qmp_expire_password(ExpirePasswordOptions *opts, Error **errp)
-{
-    time_t when;
-    int rc;
-    const char *whenstr = opts->time;
-    const char *numstr = NULL;
-    uint64_t num;
-
-    if (strcmp(whenstr, "now") == 0) {
-        when = 0;
-    } else if (strcmp(whenstr, "never") == 0) {
-        when = TIME_MAX;
-    } else if (whenstr[0] == '+') {
-        when = time(NULL);
-        numstr = whenstr + 1;
-    } else {
-        when = 0;
-        numstr = whenstr;
-    }
-
-    if (numstr) {
-        if (qemu_strtou64(numstr, NULL, 10, &num) < 0) {
-            error_setg(errp, "Parameter 'time' doesn't take value '%s'",
-                       whenstr);
-            return;
-        }
-        when += num;
-    }
-
-    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
-        if (!qemu_using_spice(errp)) {
-            return;
-        }
-        rc = qemu_spice.set_pw_expire(when);
-    } else {
-        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
-        rc = vnc_display_pw_expire(opts->u.vnc.display, when);
-    }
-
-    if (rc != 0) {
-        error_setg(errp, "Could not set password expire time");
-    }
-}
-
-#ifdef CONFIG_VNC
-void qmp_change_vnc_password(const char *password, Error **errp)
-{
-    if (vnc_display_password(NULL, password) < 0) {
-        error_setg(errp, "Could not set password");
-    }
-}
-#endif
-
 void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
                     Error **errp)
@@ -305,7 +220,6 @@ void qmp_add_client(const char *protocol, const char *fdname,
     }
 }
 
-
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
 {
     return qmp_memory_device_list();
@@ -344,38 +258,6 @@ MemoryInfo *qmp_query_memory_size_summary(Error **errp)
     return mem_info;
 }
 
-void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
-{
-    switch (arg->type) {
-    case DISPLAY_RELOAD_TYPE_VNC:
-#ifdef CONFIG_VNC
-        if (arg->u.vnc.has_tls_certs && arg->u.vnc.tls_certs) {
-            vnc_display_reload_certs(NULL, errp);
-        }
-#else
-        error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
-#endif
-        break;
-    default:
-        abort();
-    }
-}
-
-void qmp_display_update(DisplayUpdateOptions *arg, Error **errp)
-{
-    switch (arg->type) {
-    case DISPLAY_UPDATE_TYPE_VNC:
-#ifdef CONFIG_VNC
-        vnc_display_update(&arg->u.vnc, errp);
-#else
-        error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
-#endif
-        break;
-    default:
-        abort();
-    }
-}
-
 static int qmp_x_query_rdma_foreach(Object *obj, void *opaque)
 {
     RdmaProvider *rdma;
diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c
new file mode 100644
index 0000000000..c9f92caf1d
--- /dev/null
+++ b/ui/ui-qmp-cmds.c
@@ -0,0 +1,136 @@
+/*
+ * QMP commands related to UI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/qapi-commands-ui.h"
+#include "qapi/qmp/qerror.h"
+#include "qemu/cutils.h"
+#include "ui/console.h"
+#include "ui/qemu-spice.h"
+
+void qmp_set_password(SetPasswordOptions *opts, Error **errp)
+{
+    int rc;
+
+    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
+        if (!qemu_using_spice(errp)) {
+            return;
+        }
+        rc = qemu_spice.set_passwd(opts->password,
+                opts->connected == SET_PASSWORD_ACTION_FAIL,
+                opts->connected == SET_PASSWORD_ACTION_DISCONNECT);
+    } else {
+        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
+        if (opts->connected != SET_PASSWORD_ACTION_KEEP) {
+            /* vnc supports "connected=keep" only */
+            error_setg(errp, QERR_INVALID_PARAMETER, "connected");
+            return;
+        }
+        /*
+         * Note that setting an empty password will not disable login
+         * through this interface.
+         */
+        rc = vnc_display_password(opts->u.vnc.display, opts->password);
+    }
+
+    if (rc != 0) {
+        error_setg(errp, "Could not set password");
+    }
+}
+
+void qmp_expire_password(ExpirePasswordOptions *opts, Error **errp)
+{
+    time_t when;
+    int rc;
+    const char *whenstr = opts->time;
+    const char *numstr = NULL;
+    uint64_t num;
+
+    if (strcmp(whenstr, "now") == 0) {
+        when = 0;
+    } else if (strcmp(whenstr, "never") == 0) {
+        when = TIME_MAX;
+    } else if (whenstr[0] == '+') {
+        when = time(NULL);
+        numstr = whenstr + 1;
+    } else {
+        when = 0;
+        numstr = whenstr;
+    }
+
+    if (numstr) {
+        if (qemu_strtou64(numstr, NULL, 10, &num) < 0) {
+            error_setg(errp, "Parameter 'time' doesn't take value '%s'",
+                       whenstr);
+            return;
+        }
+        when += num;
+    }
+
+    if (opts->protocol == DISPLAY_PROTOCOL_SPICE) {
+        if (!qemu_using_spice(errp)) {
+            return;
+        }
+        rc = qemu_spice.set_pw_expire(when);
+    } else {
+        assert(opts->protocol == DISPLAY_PROTOCOL_VNC);
+        rc = vnc_display_pw_expire(opts->u.vnc.display, when);
+    }
+
+    if (rc != 0) {
+        error_setg(errp, "Could not set password expire time");
+    }
+}
+
+#ifdef CONFIG_VNC
+void qmp_change_vnc_password(const char *password, Error **errp)
+{
+    if (vnc_display_password(NULL, password) < 0) {
+        error_setg(errp, "Could not set password");
+    }
+}
+#endif
+
+void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
+{
+    switch (arg->type) {
+    case DISPLAY_RELOAD_TYPE_VNC:
+#ifdef CONFIG_VNC
+        if (arg->u.vnc.has_tls_certs && arg->u.vnc.tls_certs) {
+            vnc_display_reload_certs(NULL, errp);
+        }
+#else
+        error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
+#endif
+        break;
+    default:
+        abort();
+    }
+}
+
+void qmp_display_update(DisplayUpdateOptions *arg, Error **errp)
+{
+    switch (arg->type) {
+    case DISPLAY_UPDATE_TYPE_VNC:
+#ifdef CONFIG_VNC
+        vnc_display_update(&arg->u.vnc, errp);
+#else
+        error_setg(errp, "vnc is invalid, missing 'CONFIG_VNC'");
+#endif
+        break;
+    default:
+        abort();
+    }
+}
diff --git a/ui/meson.build b/ui/meson.build
index c1b137bf33..9194ea335b 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -14,6 +14,7 @@ softmmu_ss.add(files(
   'kbd-state.c',
   'keymaps.c',
   'qemu-pixman.c',
+  'ui-qmp-cmds.c',
   'util.c',
 ))
 if dbus_display
-- 
2.38.1

[PATCH v3 10/18] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/qmp-helpers.h | 26 ++++++++++++
 monitor/qmp-cmds.c            | 74 ++++++++++++++++-------------------
 ui/ui-qmp-cmds.c              | 41 +++++++++++++++++++
 3 files changed, 100 insertions(+), 41 deletions(-)
 create mode 100644 include/monitor/qmp-helpers.h

diff --git a/include/monitor/qmp-helpers.h b/include/monitor/qmp-helpers.h
new file mode 100644
index 0000000000..4718c63c73
--- /dev/null
+++ b/include/monitor/qmp-helpers.h
@@ -0,0 +1,26 @@
+/*
+ * QMP command helpers
+ *
+ * Copyright (c) 2022 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef MONITOR_QMP_HELPERS_H
+
+bool qmp_add_client_spice(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#ifdef CONFIG_VNC
+bool qmp_add_client_vnc(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
+bool qmp_add_client_dbus_display(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#endif
+
+#endif
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 61449f1b58..b5b736761a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -17,13 +17,11 @@
 #include "qemu/cutils.h"
 #include "qemu/option.h"
 #include "monitor/monitor.h"
+#include "monitor/qmp-helpers.h"
 #include "sysemu/sysemu.h"
 #include "qemu/config-file.h"
 #include "qemu/uuid.h"
 #include "chardev/char.h"
-#include "ui/qemu-spice.h"
-#include "ui/console.h"
-#include "ui/dbus-display.h"
 #include "sysemu/kvm.h"
 #include "sysemu/runstate.h"
 #include "sysemu/runstate-action.h"
@@ -170,54 +168,48 @@ void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
                     Error **errp)
 {
+    static struct {
+        const char *name;
+        bool (*add_client)(int fd, bool has_skipauth, bool skipauth,
+                           bool has_tls, bool tls, Error **errp);
+    } protocol_table[] = {
+        { "spice", qmp_add_client_spice },
+#ifdef CONFIG_VNC
+        { "vnc", qmp_add_client_vnc },
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
+        { "@dbus-display", qmp_add_client_dbus_display },
+#endif
+    };
     Chardev *s;
-    int fd;
+    int fd, i;
 
     fd = monitor_get_fd(monitor_cur(), fdname, errp);
     if (fd < 0) {
         return;
     }
 
-    if (strcmp(protocol, "spice") == 0) {
-        if (!qemu_using_spice(errp)) {
-            close(fd);
-            return;
-        }
-        skipauth = has_skipauth ? skipauth : false;
-        tls = has_tls ? tls : false;
-        if (qemu_spice.display_add_client(fd, skipauth, tls) < 0) {
-            error_setg(errp, "spice failed to add client");
-            close(fd);
-        }
-#ifdef CONFIG_VNC
-    } else if (strcmp(protocol, "vnc") == 0) {
-        skipauth = has_skipauth ? skipauth : false;
-        vnc_display_add_client(NULL, fd, skipauth);
-#endif
-#ifdef CONFIG_DBUS_DISPLAY
-    } else if (strcmp(protocol, "@dbus-display") == 0) {
-        if (!qemu_using_dbus_display(errp)) {
-            close(fd);
-            return;
-        }
-        if (!qemu_dbus_display.add_client(fd, errp)) {
-            close(fd);
-            return;
-        }
-#endif
-    } else {
-        s = qemu_chr_find(protocol);
-        if (!s) {
-            error_setg(errp, "protocol '%s' is invalid", protocol);
-            close(fd);
-            return;
-        }
-        if (qemu_chr_add_client(s, fd) < 0) {
-            error_setg(errp, "failed to add client");
-            close(fd);
+    for (i = 0; i < ARRAY_SIZE(protocol_table); i++) {
+        if (!strcmp(protocol, protocol_table[i].name)) {
+            if (!protocol_table[i].add_client(fd, has_skipauth, skipauth,
+                                              has_tls, tls, errp)) {
+                close(fd);
+            }
             return;
         }
     }
+
+    s = qemu_chr_find(protocol);
+    if (!s) {
+        error_setg(errp, "protocol '%s' is invalid", protocol);
+        close(fd);
+        return;
+    }
+    if (qemu_chr_add_client(s, fd) < 0) {
+        error_setg(errp, "failed to add client");
+        close(fd);
+        return;
+    }
 }
 
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
diff --git a/ui/ui-qmp-cmds.c b/ui/ui-qmp-cmds.c
index c9f92caf1d..dbc4afcd73 100644
--- a/ui/ui-qmp-cmds.c
+++ b/ui/ui-qmp-cmds.c
@@ -14,10 +14,12 @@
  */
 
 #include "qemu/osdep.h"
+#include "monitor/qmp-helpers.h"
 #include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qerror.h"
 #include "qemu/cutils.h"
 #include "ui/console.h"
+#include "ui/dbus-display.h"
 #include "ui/qemu-spice.h"
 
 void qmp_set_password(SetPasswordOptions *opts, Error **errp)
@@ -103,6 +105,45 @@ void qmp_change_vnc_password(const char *password, Error **errp)
 }
 #endif
 
+bool qmp_add_client_spice(int fd, bool has_skipauth, bool skipauth,
+                          bool has_tls, bool tls, Error **errp)
+{
+    if (!qemu_using_spice(errp)) {
+        return false;
+    }
+    skipauth = has_skipauth ? skipauth : false;
+    tls = has_tls ? tls : false;
+    if (qemu_spice.display_add_client(fd, skipauth, tls) < 0) {
+        error_setg(errp, "spice failed to add client");
+        return false;
+    }
+    return true;
+}
+
+#ifdef CONFIG_VNC
+bool qmp_add_client_vnc(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp)
+{
+    skipauth = has_skipauth ? skipauth : false;
+    vnc_display_add_client(NULL, fd, skipauth);
+    return true;
+}
+#endif
+
+#ifdef CONFIG_DBUS_DISPLAY
+bool qmp_add_client_dbus_display(int fd, bool has_skipauth, bool skipauth,
+                                 bool has_tls, bool tls, Error **errp)
+{
+    if (!qemu_using_dbus_display(errp)) {
+        return false;
+    }
+    if (!qemu_dbus_display.add_client(fd, errp)) {
+        return false;
+    }
+    return true;
+}
+#endif
+
 void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
 {
     switch (arg->type) {
-- 
2.38.1

[PATCH v3 11/18] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c

[Markus Armbruster]

This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" to "Graphics".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 monitor/hmp-cmds.c | 339 ------------------------------------------
 ui/ui-hmp-cmds.c   | 357 +++++++++++++++++++++++++++++++++++++++++++++
 ui/meson.build     |   1 +
 3 files changed, 358 insertions(+), 339 deletions(-)
 create mode 100644 ui/ui-hmp-cmds.c

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 5b41225bb9..c4f161a596 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -51,7 +51,6 @@
 #include "qapi/string-input-visitor.h"
 #include "qapi/string-output-visitor.h"
 #include "qom/object_interfaces.h"
-#include "ui/console.h"
 #include "qemu/cutils.h"
 #include "qemu/error-report.h"
 #include "hw/core/cpu.h"
@@ -59,10 +58,6 @@
 #include "migration/snapshot.h"
 #include "migration/misc.h"
 
-#ifdef CONFIG_SPICE
-#include <spice/enums.h>
-#endif
-
 bool hmp_handle_error(Monitor *mon, Error *err)
 {
     if (err) {
@@ -178,26 +173,6 @@ void hmp_info_chardev(Monitor *mon, const QDict *qdict)
     qapi_free_ChardevInfoList(char_info);
 }
 
-void hmp_info_mice(Monitor *mon, const QDict *qdict)
-{
-    MouseInfoList *mice_list, *mouse;
-
-    mice_list = qmp_query_mice(NULL);
-    if (!mice_list) {
-        monitor_printf(mon, "No mouse devices connected\n");
-        return;
-    }
-
-    for (mouse = mice_list; mouse; mouse = mouse->next) {
-        monitor_printf(mon, "%c Mouse #%" PRId64 ": %s%s\n",
-                       mouse->value->current ? '*' : ' ',
-                       mouse->value->index, mouse->value->name,
-                       mouse->value->absolute ? " (absolute)" : "");
-    }
-
-    qapi_free_MouseInfoList(mice_list);
-}
-
 void hmp_info_migrate(Monitor *mon, const QDict *qdict)
 {
     MigrationInfo *info;
@@ -516,168 +491,6 @@ void hmp_info_migrate_parameters(Monitor *mon, const QDict *qdict)
     qapi_free_MigrationParameters(params);
 }
 
-
-#ifdef CONFIG_VNC
-/* Helper for hmp_info_vnc_clients, _servers */
-static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
-                                  const char *name)
-{
-    monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
-                   name,
-                   info->host,
-                   info->service,
-                   NetworkAddressFamily_str(info->family),
-                   info->websocket ? " (Websocket)" : "");
-}
-
-/* Helper displaying and auth and crypt info */
-static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
-                                   VncPrimaryAuth auth,
-                                   VncVencryptSubAuth *vencrypt)
-{
-    monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
-                   VncPrimaryAuth_str(auth),
-                   vencrypt ? VncVencryptSubAuth_str(*vencrypt) : "none");
-}
-
-static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
-{
-    while (client) {
-        VncClientInfo *cinfo = client->value;
-
-        hmp_info_VncBasicInfo(mon, qapi_VncClientInfo_base(cinfo), "Client");
-        monitor_printf(mon, "    x509_dname: %s\n",
-                       cinfo->x509_dname ?: "none");
-        monitor_printf(mon, "    sasl_username: %s\n",
-                       cinfo->sasl_username ?: "none");
-
-        client = client->next;
-    }
-}
-
-static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
-{
-    while (server) {
-        VncServerInfo2 *sinfo = server->value;
-        hmp_info_VncBasicInfo(mon, qapi_VncServerInfo2_base(sinfo), "Server");
-        hmp_info_vnc_authcrypt(mon, "    ", sinfo->auth,
-                               sinfo->has_vencrypt ? &sinfo->vencrypt : NULL);
-        server = server->next;
-    }
-}
-
-void hmp_info_vnc(Monitor *mon, const QDict *qdict)
-{
-    VncInfo2List *info2l, *info2l_head;
-    Error *err = NULL;
-
-    info2l = qmp_query_vnc_servers(&err);
-    info2l_head = info2l;
-    if (hmp_handle_error(mon, err)) {
-        return;
-    }
-    if (!info2l) {
-        monitor_printf(mon, "None\n");
-        return;
-    }
-
-    while (info2l) {
-        VncInfo2 *info = info2l->value;
-        monitor_printf(mon, "%s:\n", info->id);
-        hmp_info_vnc_servers(mon, info->server);
-        hmp_info_vnc_clients(mon, info->clients);
-        if (!info->server) {
-            /*
-             * The server entry displays its auth, we only need to
-             * display in the case of 'reverse' connections where
-             * there's no server.
-             */
-            hmp_info_vnc_authcrypt(mon, "  ", info->auth,
-                               info->has_vencrypt ? &info->vencrypt : NULL);
-        }
-        if (info->display) {
-            monitor_printf(mon, "  Display: %s\n", info->display);
-        }
-        info2l = info2l->next;
-    }
-
-    qapi_free_VncInfo2List(info2l_head);
-
-}
-#endif
-
-#ifdef CONFIG_SPICE
-void hmp_info_spice(Monitor *mon, const QDict *qdict)
-{
-    SpiceChannelList *chan;
-    SpiceInfo *info;
-    const char *channel_name;
-    static const char *const channel_names[] = {
-        [SPICE_CHANNEL_MAIN] = "main",
-        [SPICE_CHANNEL_DISPLAY] = "display",
-        [SPICE_CHANNEL_INPUTS] = "inputs",
-        [SPICE_CHANNEL_CURSOR] = "cursor",
-        [SPICE_CHANNEL_PLAYBACK] = "playback",
-        [SPICE_CHANNEL_RECORD] = "record",
-        [SPICE_CHANNEL_TUNNEL] = "tunnel",
-        [SPICE_CHANNEL_SMARTCARD] = "smartcard",
-        [SPICE_CHANNEL_USBREDIR] = "usbredir",
-        [SPICE_CHANNEL_PORT] = "port",
-        [SPICE_CHANNEL_WEBDAV] = "webdav",
-    };
-
-    info = qmp_query_spice(NULL);
-
-    if (!info->enabled) {
-        monitor_printf(mon, "Server: disabled\n");
-        goto out;
-    }
-
-    monitor_printf(mon, "Server:\n");
-    if (info->has_port) {
-        monitor_printf(mon, "     address: %s:%" PRId64 "\n",
-                       info->host, info->port);
-    }
-    if (info->has_tls_port) {
-        monitor_printf(mon, "     address: %s:%" PRId64 " [tls]\n",
-                       info->host, info->tls_port);
-    }
-    monitor_printf(mon, "    migrated: %s\n",
-                   info->migrated ? "true" : "false");
-    monitor_printf(mon, "        auth: %s\n", info->auth);
-    monitor_printf(mon, "    compiled: %s\n", info->compiled_version);
-    monitor_printf(mon, "  mouse-mode: %s\n",
-                   SpiceQueryMouseMode_str(info->mouse_mode));
-
-    if (!info->has_channels || info->channels == NULL) {
-        monitor_printf(mon, "Channels: none\n");
-    } else {
-        for (chan = info->channels; chan; chan = chan->next) {
-            monitor_printf(mon, "Channel:\n");
-            monitor_printf(mon, "     address: %s:%s%s\n",
-                           chan->value->host, chan->value->port,
-                           chan->value->tls ? " [tls]" : "");
-            monitor_printf(mon, "     session: %" PRId64 "\n",
-                           chan->value->connection_id);
-            monitor_printf(mon, "     channel: %" PRId64 ":%" PRId64 "\n",
-                           chan->value->channel_type, chan->value->channel_id);
-
-            channel_name = "unknown";
-            if (chan->value->channel_type > 0 &&
-                chan->value->channel_type < ARRAY_SIZE(channel_names) &&
-                channel_names[chan->value->channel_type]) {
-                channel_name = channel_names[chan->value->channel_type];
-            }
-
-            monitor_printf(mon, "     channel name: %s\n", channel_name);
-        }
-    }
-
-out:
-    qapi_free_SpiceInfo(info);
-}
-#endif
-
 void hmp_info_balloon(Monitor *mon, const QDict *qdict)
 {
     BalloonInfo *info;
@@ -1262,69 +1075,6 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
-void hmp_set_password(Monitor *mon, const QDict *qdict)
-{
-    const char *protocol  = qdict_get_str(qdict, "protocol");
-    const char *password  = qdict_get_str(qdict, "password");
-    const char *display = qdict_get_try_str(qdict, "display");
-    const char *connected = qdict_get_try_str(qdict, "connected");
-    Error *err = NULL;
-
-    SetPasswordOptions opts = {
-        .password = (char *)password,
-        .has_connected = !!connected,
-    };
-
-    opts.connected = qapi_enum_parse(&SetPasswordAction_lookup, connected,
-                                     SET_PASSWORD_ACTION_KEEP, &err);
-    if (err) {
-        goto out;
-    }
-
-    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
-                                    DISPLAY_PROTOCOL_VNC, &err);
-    if (err) {
-        goto out;
-    }
-
-    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
-        opts.u.vnc.display = (char *)display;
-    }
-
-    qmp_set_password(&opts, &err);
-
-out:
-    hmp_handle_error(mon, err);
-}
-
-void hmp_expire_password(Monitor *mon, const QDict *qdict)
-{
-    const char *protocol  = qdict_get_str(qdict, "protocol");
-    const char *whenstr = qdict_get_str(qdict, "time");
-    const char *display = qdict_get_try_str(qdict, "display");
-    Error *err = NULL;
-
-    ExpirePasswordOptions opts = {
-        .time = (char *)whenstr,
-    };
-
-    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
-                                    DISPLAY_PROTOCOL_VNC, &err);
-    if (err) {
-        goto out;
-    }
-
-    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
-        opts.u.vnc.display = (char *)display;
-    }
-
-    qmp_expire_password(&opts, &err);
-
-out:
-    hmp_handle_error(mon, err);
-}
-
-
 #ifdef CONFIG_VNC
 static void hmp_change_read_arg(void *opaque, const char *password,
                                 void *readline_opaque)
@@ -1521,95 +1271,6 @@ void hmp_closefd(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
-void hmp_sendkey(Monitor *mon, const QDict *qdict)
-{
-    const char *keys = qdict_get_str(qdict, "keys");
-    KeyValue *v = NULL;
-    KeyValueList *head = NULL, **tail = &head;
-    int has_hold_time = qdict_haskey(qdict, "hold-time");
-    int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
-    Error *err = NULL;
-    const char *separator;
-    int keyname_len;
-
-    while (1) {
-        separator = qemu_strchrnul(keys, '-');
-        keyname_len = separator - keys;
-
-        /* Be compatible with old interface, convert user inputted "<" */
-        if (keys[0] == '<' && keyname_len == 1) {
-            keys = "less";
-            keyname_len = 4;
-        }
-
-        v = g_malloc0(sizeof(*v));
-
-        if (strstart(keys, "0x", NULL)) {
-            const char *endp;
-            unsigned long value;
-
-            if (qemu_strtoul(keys, &endp, 0, &value) < 0
-                || value >= INT_MAX) {
-                goto err_out;
-            }
-            assert(endp <= keys + keyname_len);
-            if (endp != keys + keyname_len) {
-                goto err_out;
-            }
-            v->type = KEY_VALUE_KIND_NUMBER;
-            v->u.number.data = value;
-        } else {
-            int idx = index_from_key(keys, keyname_len);
-            if (idx == Q_KEY_CODE__MAX) {
-                goto err_out;
-            }
-            v->type = KEY_VALUE_KIND_QCODE;
-            v->u.qcode.data = idx;
-        }
-        QAPI_LIST_APPEND(tail, v);
-        v = NULL;
-
-        if (!*separator) {
-            break;
-        }
-        keys = separator + 1;
-    }
-
-    qmp_send_key(head, has_hold_time, hold_time, &err);
-    hmp_handle_error(mon, err);
-
-out:
-    qapi_free_KeyValue(v);
-    qapi_free_KeyValueList(head);
-    return;
-
-err_out:
-    monitor_printf(mon, "invalid parameter: %.*s\n", keyname_len, keys);
-    goto out;
-}
-
-void coroutine_fn
-hmp_screendump(Monitor *mon, const QDict *qdict)
-{
-    const char *filename = qdict_get_str(qdict, "filename");
-    const char *id = qdict_get_try_str(qdict, "device");
-    int64_t head = qdict_get_try_int(qdict, "head", 0);
-    const char *input_format  = qdict_get_try_str(qdict, "format");
-    Error *err = NULL;
-    ImageFormat format;
-
-    format = qapi_enum_parse(&ImageFormat_lookup, input_format,
-                              IMAGE_FORMAT_PPM, &err);
-    if (err) {
-        goto end;
-    }
-
-    qmp_screendump(filename, id, id != NULL, head,
-                   input_format != NULL, format, &err);
-end:
-    hmp_handle_error(mon, err);
-}
-
 void hmp_chardev_add(Monitor *mon, const QDict *qdict)
 {
     const char *args = qdict_get_str(qdict, "args");
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
new file mode 100644
index 0000000000..aecd996968
--- /dev/null
+++ b/ui/ui-hmp-cmds.c
@@ -0,0 +1,357 @@
+/*
+ * HMP commands related to UI
+ *
+ * Copyright IBM, Corp. 2011
+ *
+ * Authors:
+ *  Anthony Liguori   <aliguori@us.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ *
+ * Contributions after 2012-01-13 are licensed under the terms of the
+ * GNU GPL, version 2 or (at your option) any later version.
+ */
+
+#include "qemu/osdep.h"
+#ifdef CONFIG_SPICE
+#include <spice/enums.h>
+#endif
+#include "monitor/hmp.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-ui.h"
+#include "qapi/qmp/qdict.h"
+#include "qemu/cutils.h"
+#include "ui/console.h"
+
+void hmp_info_mice(Monitor *mon, const QDict *qdict)
+{
+    MouseInfoList *mice_list, *mouse;
+
+    mice_list = qmp_query_mice(NULL);
+    if (!mice_list) {
+        monitor_printf(mon, "No mouse devices connected\n");
+        return;
+    }
+
+    for (mouse = mice_list; mouse; mouse = mouse->next) {
+        monitor_printf(mon, "%c Mouse #%" PRId64 ": %s%s\n",
+                       mouse->value->current ? '*' : ' ',
+                       mouse->value->index, mouse->value->name,
+                       mouse->value->absolute ? " (absolute)" : "");
+    }
+
+    qapi_free_MouseInfoList(mice_list);
+}
+
+#ifdef CONFIG_VNC
+/* Helper for hmp_info_vnc_clients, _servers */
+static void hmp_info_VncBasicInfo(Monitor *mon, VncBasicInfo *info,
+                                  const char *name)
+{
+    monitor_printf(mon, "  %s: %s:%s (%s%s)\n",
+                   name,
+                   info->host,
+                   info->service,
+                   NetworkAddressFamily_str(info->family),
+                   info->websocket ? " (Websocket)" : "");
+}
+
+/* Helper displaying and auth and crypt info */
+static void hmp_info_vnc_authcrypt(Monitor *mon, const char *indent,
+                                   VncPrimaryAuth auth,
+                                   VncVencryptSubAuth *vencrypt)
+{
+    monitor_printf(mon, "%sAuth: %s (Sub: %s)\n", indent,
+                   VncPrimaryAuth_str(auth),
+                   vencrypt ? VncVencryptSubAuth_str(*vencrypt) : "none");
+}
+
+static void hmp_info_vnc_clients(Monitor *mon, VncClientInfoList *client)
+{
+    while (client) {
+        VncClientInfo *cinfo = client->value;
+
+        hmp_info_VncBasicInfo(mon, qapi_VncClientInfo_base(cinfo), "Client");
+        monitor_printf(mon, "    x509_dname: %s\n",
+                       cinfo->x509_dname ?: "none");
+        monitor_printf(mon, "    sasl_username: %s\n",
+                       cinfo->sasl_username ?: "none");
+
+        client = client->next;
+    }
+}
+
+static void hmp_info_vnc_servers(Monitor *mon, VncServerInfo2List *server)
+{
+    while (server) {
+        VncServerInfo2 *sinfo = server->value;
+        hmp_info_VncBasicInfo(mon, qapi_VncServerInfo2_base(sinfo), "Server");
+        hmp_info_vnc_authcrypt(mon, "    ", sinfo->auth,
+                               sinfo->has_vencrypt ? &sinfo->vencrypt : NULL);
+        server = server->next;
+    }
+}
+
+void hmp_info_vnc(Monitor *mon, const QDict *qdict)
+{
+    VncInfo2List *info2l, *info2l_head;
+    Error *err = NULL;
+
+    info2l = qmp_query_vnc_servers(&err);
+    info2l_head = info2l;
+    if (hmp_handle_error(mon, err)) {
+        return;
+    }
+    if (!info2l) {
+        monitor_printf(mon, "None\n");
+        return;
+    }
+
+    while (info2l) {
+        VncInfo2 *info = info2l->value;
+        monitor_printf(mon, "%s:\n", info->id);
+        hmp_info_vnc_servers(mon, info->server);
+        hmp_info_vnc_clients(mon, info->clients);
+        if (!info->server) {
+            /*
+             * The server entry displays its auth, we only need to
+             * display in the case of 'reverse' connections where
+             * there's no server.
+             */
+            hmp_info_vnc_authcrypt(mon, "  ", info->auth,
+                               info->has_vencrypt ? &info->vencrypt : NULL);
+        }
+        if (info->display) {
+            monitor_printf(mon, "  Display: %s\n", info->display);
+        }
+        info2l = info2l->next;
+    }
+
+    qapi_free_VncInfo2List(info2l_head);
+
+}
+#endif
+
+#ifdef CONFIG_SPICE
+void hmp_info_spice(Monitor *mon, const QDict *qdict)
+{
+    SpiceChannelList *chan;
+    SpiceInfo *info;
+    const char *channel_name;
+    static const char *const channel_names[] = {
+        [SPICE_CHANNEL_MAIN] = "main",
+        [SPICE_CHANNEL_DISPLAY] = "display",
+        [SPICE_CHANNEL_INPUTS] = "inputs",
+        [SPICE_CHANNEL_CURSOR] = "cursor",
+        [SPICE_CHANNEL_PLAYBACK] = "playback",
+        [SPICE_CHANNEL_RECORD] = "record",
+        [SPICE_CHANNEL_TUNNEL] = "tunnel",
+        [SPICE_CHANNEL_SMARTCARD] = "smartcard",
+        [SPICE_CHANNEL_USBREDIR] = "usbredir",
+        [SPICE_CHANNEL_PORT] = "port",
+        [SPICE_CHANNEL_WEBDAV] = "webdav",
+    };
+
+    info = qmp_query_spice(NULL);
+
+    if (!info->enabled) {
+        monitor_printf(mon, "Server: disabled\n");
+        goto out;
+    }
+
+    monitor_printf(mon, "Server:\n");
+    if (info->has_port) {
+        monitor_printf(mon, "     address: %s:%" PRId64 "\n",
+                       info->host, info->port);
+    }
+    if (info->has_tls_port) {
+        monitor_printf(mon, "     address: %s:%" PRId64 " [tls]\n",
+                       info->host, info->tls_port);
+    }
+    monitor_printf(mon, "    migrated: %s\n",
+                   info->migrated ? "true" : "false");
+    monitor_printf(mon, "        auth: %s\n", info->auth);
+    monitor_printf(mon, "    compiled: %s\n", info->compiled_version);
+    monitor_printf(mon, "  mouse-mode: %s\n",
+                   SpiceQueryMouseMode_str(info->mouse_mode));
+
+    if (!info->has_channels || info->channels == NULL) {
+        monitor_printf(mon, "Channels: none\n");
+    } else {
+        for (chan = info->channels; chan; chan = chan->next) {
+            monitor_printf(mon, "Channel:\n");
+            monitor_printf(mon, "     address: %s:%s%s\n",
+                           chan->value->host, chan->value->port,
+                           chan->value->tls ? " [tls]" : "");
+            monitor_printf(mon, "     session: %" PRId64 "\n",
+                           chan->value->connection_id);
+            monitor_printf(mon, "     channel: %" PRId64 ":%" PRId64 "\n",
+                           chan->value->channel_type, chan->value->channel_id);
+
+            channel_name = "unknown";
+            if (chan->value->channel_type > 0 &&
+                chan->value->channel_type < ARRAY_SIZE(channel_names) &&
+                channel_names[chan->value->channel_type]) {
+                channel_name = channel_names[chan->value->channel_type];
+            }
+
+            monitor_printf(mon, "     channel name: %s\n", channel_name);
+        }
+    }
+
+out:
+    qapi_free_SpiceInfo(info);
+}
+#endif
+
+void hmp_set_password(Monitor *mon, const QDict *qdict)
+{
+    const char *protocol  = qdict_get_str(qdict, "protocol");
+    const char *password  = qdict_get_str(qdict, "password");
+    const char *display = qdict_get_try_str(qdict, "display");
+    const char *connected = qdict_get_try_str(qdict, "connected");
+    Error *err = NULL;
+
+    SetPasswordOptions opts = {
+        .password = (char *)password,
+        .has_connected = !!connected,
+    };
+
+    opts.connected = qapi_enum_parse(&SetPasswordAction_lookup, connected,
+                                     SET_PASSWORD_ACTION_KEEP, &err);
+    if (err) {
+        goto out;
+    }
+
+    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
+                                    DISPLAY_PROTOCOL_VNC, &err);
+    if (err) {
+        goto out;
+    }
+
+    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
+        opts.u.vnc.display = (char *)display;
+    }
+
+    qmp_set_password(&opts, &err);
+
+out:
+    hmp_handle_error(mon, err);
+}
+
+void hmp_expire_password(Monitor *mon, const QDict *qdict)
+{
+    const char *protocol  = qdict_get_str(qdict, "protocol");
+    const char *whenstr = qdict_get_str(qdict, "time");
+    const char *display = qdict_get_try_str(qdict, "display");
+    Error *err = NULL;
+
+    ExpirePasswordOptions opts = {
+        .time = (char *)whenstr,
+    };
+
+    opts.protocol = qapi_enum_parse(&DisplayProtocol_lookup, protocol,
+                                    DISPLAY_PROTOCOL_VNC, &err);
+    if (err) {
+        goto out;
+    }
+
+    if (opts.protocol == DISPLAY_PROTOCOL_VNC) {
+        opts.u.vnc.display = (char *)display;
+    }
+
+    qmp_expire_password(&opts, &err);
+
+out:
+    hmp_handle_error(mon, err);
+}
+
+void hmp_sendkey(Monitor *mon, const QDict *qdict)
+{
+    const char *keys = qdict_get_str(qdict, "keys");
+    KeyValue *v = NULL;
+    KeyValueList *head = NULL, **tail = &head;
+    int has_hold_time = qdict_haskey(qdict, "hold-time");
+    int hold_time = qdict_get_try_int(qdict, "hold-time", -1);
+    Error *err = NULL;
+    const char *separator;
+    int keyname_len;
+
+    while (1) {
+        separator = qemu_strchrnul(keys, '-');
+        keyname_len = separator - keys;
+
+        /* Be compatible with old interface, convert user inputted "<" */
+        if (keys[0] == '<' && keyname_len == 1) {
+            keys = "less";
+            keyname_len = 4;
+        }
+
+        v = g_malloc0(sizeof(*v));
+
+        if (strstart(keys, "0x", NULL)) {
+            const char *endp;
+            unsigned long value;
+
+            if (qemu_strtoul(keys, &endp, 0, &value) < 0
+                || value >= INT_MAX) {
+                goto err_out;
+            }
+            assert(endp <= keys + keyname_len);
+            if (endp != keys + keyname_len) {
+                goto err_out;
+            }
+            v->type = KEY_VALUE_KIND_NUMBER;
+            v->u.number.data = value;
+        } else {
+            int idx = index_from_key(keys, keyname_len);
+            if (idx == Q_KEY_CODE__MAX) {
+                goto err_out;
+            }
+            v->type = KEY_VALUE_KIND_QCODE;
+            v->u.qcode.data = idx;
+        }
+        QAPI_LIST_APPEND(tail, v);
+        v = NULL;
+
+        if (!*separator) {
+            break;
+        }
+        keys = separator + 1;
+    }
+
+    qmp_send_key(head, has_hold_time, hold_time, &err);
+    hmp_handle_error(mon, err);
+
+out:
+    qapi_free_KeyValue(v);
+    qapi_free_KeyValueList(head);
+    return;
+
+err_out:
+    monitor_printf(mon, "invalid parameter: %.*s\n", keyname_len, keys);
+    goto out;
+}
+
+void coroutine_fn
+hmp_screendump(Monitor *mon, const QDict *qdict)
+{
+    const char *filename = qdict_get_str(qdict, "filename");
+    const char *id = qdict_get_try_str(qdict, "device");
+    int64_t head = qdict_get_try_int(qdict, "head", 0);
+    const char *input_format  = qdict_get_try_str(qdict, "format");
+    Error *err = NULL;
+    ImageFormat format;
+
+    format = qapi_enum_parse(&ImageFormat_lookup, input_format,
+                              IMAGE_FORMAT_PPM, &err);
+    if (err) {
+        goto end;
+    }
+
+    qmp_screendump(filename, id, id != NULL, head,
+                   input_format != NULL, format, &err);
+end:
+    hmp_handle_error(mon, err);
+}
diff --git a/ui/meson.build b/ui/meson.build
index 9194ea335b..612ea2325b 100644
--- a/ui/meson.build
+++ b/ui/meson.build
@@ -14,6 +14,7 @@ softmmu_ss.add(files(
   'kbd-state.c',
   'keymaps.c',
   'qemu-pixman.c',
+  'ui-hmp-cmds.c',
   'ui-qmp-cmds.c',
   'util.c',
 ))
-- 
2.38.1

[PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Markus Armbruster]

This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" to "Graphics".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/monitor/hmp.h |  2 ++
 monitor/misc.c        | 66 -------------------------------------------
 ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 68 insertions(+), 66 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 27f86399f7..b228a406f3 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -81,6 +81,8 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
 void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
+void hmp_mouse_move(Monitor *mon, const QDict *qdict);
+void hmp_mouse_button(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void coroutine_fn hmp_screendump(Monitor *mon, const QDict *qdict);
 void hmp_chardev_add(Monitor *mon, const QDict *qdict);
diff --git a/monitor/misc.c b/monitor/misc.c
index bf3f1c67ca..3d68940d28 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -34,7 +34,6 @@
 #include "qemu/config-file.h"
 #include "qemu/ctype.h"
 #include "ui/console.h"
-#include "ui/input.h"
 #include "audio/audio.h"
 #include "disas/disas.h"
 #include "qemu/timer.h"
@@ -825,49 +824,6 @@ static void hmp_sum(Monitor *mon, const QDict *qdict)
     monitor_printf(mon, "%05d\n", sum);
 }
 
-static int mouse_button_state;
-
-static void hmp_mouse_move(Monitor *mon, const QDict *qdict)
-{
-    int dx, dy, dz, button;
-    const char *dx_str = qdict_get_str(qdict, "dx_str");
-    const char *dy_str = qdict_get_str(qdict, "dy_str");
-    const char *dz_str = qdict_get_try_str(qdict, "dz_str");
-
-    dx = strtol(dx_str, NULL, 0);
-    dy = strtol(dy_str, NULL, 0);
-    qemu_input_queue_rel(NULL, INPUT_AXIS_X, dx);
-    qemu_input_queue_rel(NULL, INPUT_AXIS_Y, dy);
-
-    if (dz_str) {
-        dz = strtol(dz_str, NULL, 0);
-        if (dz != 0) {
-            button = (dz > 0) ? INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN;
-            qemu_input_queue_btn(NULL, button, true);
-            qemu_input_event_sync();
-            qemu_input_queue_btn(NULL, button, false);
-        }
-    }
-    qemu_input_event_sync();
-}
-
-static void hmp_mouse_button(Monitor *mon, const QDict *qdict)
-{
-    static uint32_t bmap[INPUT_BUTTON__MAX] = {
-        [INPUT_BUTTON_LEFT]       = MOUSE_EVENT_LBUTTON,
-        [INPUT_BUTTON_MIDDLE]     = MOUSE_EVENT_MBUTTON,
-        [INPUT_BUTTON_RIGHT]      = MOUSE_EVENT_RBUTTON,
-    };
-    int button_state = qdict_get_int(qdict, "button_state");
-
-    if (mouse_button_state == button_state) {
-        return;
-    }
-    qemu_input_update_buttons(NULL, bmap, mouse_button_state, button_state);
-    qemu_input_event_sync();
-    mouse_button_state = button_state;
-}
-
 static void hmp_ioport_read(Monitor *mon, const QDict *qdict)
 {
     int size = qdict_get_int(qdict, "size");
@@ -1700,28 +1656,6 @@ void object_del_completion(ReadLineState *rs, int nb_args, const char *str)
     qapi_free_ObjectPropertyInfoList(start);
 }
 
-void sendkey_completion(ReadLineState *rs, int nb_args, const char *str)
-{
-    int i;
-    char *sep;
-    size_t len;
-
-    if (nb_args != 2) {
-        return;
-    }
-    sep = strrchr(str, '-');
-    if (sep) {
-        str = sep + 1;
-    }
-    len = strlen(str);
-    readline_set_completion_index(rs, len);
-    for (i = 0; i < Q_KEY_CODE__MAX; i++) {
-        if (!strncmp(str, QKeyCode_str(i), len)) {
-            readline_add_completion(rs, QKeyCode_str(i));
-        }
-    }
-}
-
 void set_link_completion(ReadLineState *rs, int nb_args, const char *str)
 {
     size_t len;
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index aecd996968..95abd4693f 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -23,6 +23,50 @@
 #include "qapi/qmp/qdict.h"
 #include "qemu/cutils.h"
 #include "ui/console.h"
+#include "ui/input.h"
+
+static int mouse_button_state;
+
+void hmp_mouse_move(Monitor *mon, const QDict *qdict)
+{
+    int dx, dy, dz, button;
+    const char *dx_str = qdict_get_str(qdict, "dx_str");
+    const char *dy_str = qdict_get_str(qdict, "dy_str");
+    const char *dz_str = qdict_get_try_str(qdict, "dz_str");
+
+    dx = strtol(dx_str, NULL, 0);
+    dy = strtol(dy_str, NULL, 0);
+    qemu_input_queue_rel(NULL, INPUT_AXIS_X, dx);
+    qemu_input_queue_rel(NULL, INPUT_AXIS_Y, dy);
+
+    if (dz_str) {
+        dz = strtol(dz_str, NULL, 0);
+        if (dz != 0) {
+            button = (dz > 0) ? INPUT_BUTTON_WHEEL_UP : INPUT_BUTTON_WHEEL_DOWN;
+            qemu_input_queue_btn(NULL, button, true);
+            qemu_input_event_sync();
+            qemu_input_queue_btn(NULL, button, false);
+        }
+    }
+    qemu_input_event_sync();
+}
+
+void hmp_mouse_button(Monitor *mon, const QDict *qdict)
+{
+    static uint32_t bmap[INPUT_BUTTON__MAX] = {
+        [INPUT_BUTTON_LEFT]       = MOUSE_EVENT_LBUTTON,
+        [INPUT_BUTTON_MIDDLE]     = MOUSE_EVENT_MBUTTON,
+        [INPUT_BUTTON_RIGHT]      = MOUSE_EVENT_RBUTTON,
+    };
+    int button_state = qdict_get_int(qdict, "button_state");
+
+    if (mouse_button_state == button_state) {
+        return;
+    }
+    qemu_input_update_buttons(NULL, bmap, mouse_button_state, button_state);
+    qemu_input_event_sync();
+    mouse_button_state = button_state;
+}
 
 void hmp_info_mice(Monitor *mon, const QDict *qdict)
 {
@@ -334,6 +378,28 @@ err_out:
     goto out;
 }
 
+void sendkey_completion(ReadLineState *rs, int nb_args, const char *str)
+{
+    int i;
+    char *sep;
+    size_t len;
+
+    if (nb_args != 2) {
+        return;
+    }
+    sep = strrchr(str, '-');
+    if (sep) {
+        str = sep + 1;
+    }
+    len = strlen(str);
+    readline_set_completion_index(rs, len);
+    for (i = 0; i < Q_KEY_CODE__MAX; i++) {
+        if (!strncmp(str, QKeyCode_str(i), len)) {
+            readline_add_completion(rs, QKeyCode_str(i));
+        }
+    }
+}
+
 void coroutine_fn
 hmp_screendump(Monitor *mon, const QDict *qdict)
 {
-- 
2.38.1

[PATCH v3 13/18] ui: Improve "change vnc" error reporting

[Markus Armbruster]

Switch from monitor_printf() to error_setg() and hmp_handle_error().
This makes "this is an error" more obvious both in the source and in
the monitor, where hmp_handle_error() prefixes the message with
"Error: ".

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 monitor/hmp-cmds.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index c4f161a596..4340e71c90 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1097,9 +1097,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 #ifdef CONFIG_VNC
     if (strcmp(device, "vnc") == 0) {
         if (read_only) {
-            monitor_printf(mon,
-                           "Parameter 'read-only-mode' is invalid for VNC\n");
-            return;
+            error_setg(&err, "Parameter 'read-only-mode' is invalid for VNC");
+            goto end;
         }
         if (strcmp(target, "passwd") == 0 ||
             strcmp(target, "password") == 0) {
@@ -1111,7 +1110,8 @@ void hmp_change(Monitor *mon, const QDict *qdict)
                 qmp_change_vnc_password(arg, &err);
             }
         } else {
-            monitor_printf(mon, "Expected 'password' after 'vnc'\n");
+            error_setg(&err, "Expected 'password' after 'vnc'");
+            goto end;
         }
     } else
 #endif
-- 
2.38.1

[PATCH v3 14/18] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 include/monitor/hmp.h |  5 +++++
 monitor/hmp-cmds.c    | 30 ++----------------------------
 monitor/qmp-cmds.c    |  2 +-
 ui/ui-hmp-cmds.c      | 35 ++++++++++++++++++++++++++++++++++-
 4 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index b228a406f3..df89eac22a 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -73,6 +73,11 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict);
 void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
 void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VNC
+void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
+                    const char *arg, const char *read_only, bool force,
+                    Error **errp);
+#endif
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_add(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 4340e71c90..1dba973092 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -25,7 +25,7 @@
 #include "qemu/timer.h"
 #include "qemu/sockets.h"
 #include "qemu/help_option.h"
-#include "monitor/monitor-internal.h"
+#include "monitor/monitor.h"
 #include "qapi/error.h"
 #include "qapi/clone-visitor.h"
 #include "qapi/opts-visitor.h"
@@ -41,7 +41,6 @@
 #include "qapi/qapi-commands-run-state.h"
 #include "qapi/qapi-commands-stats.h"
 #include "qapi/qapi-commands-tpm.h"
-#include "qapi/qapi-commands-ui.h"
 #include "qapi/qapi-commands-virtio.h"
 #include "qapi/qapi-visit-virtio.h"
 #include "qapi/qapi-visit-net.h"
@@ -1075,15 +1074,6 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict)
     hmp_handle_error(mon, err);
 }
 
-#ifdef CONFIG_VNC
-static void hmp_change_read_arg(void *opaque, const char *password,
-                                void *readline_opaque)
-{
-    qmp_change_vnc_password(password, NULL);
-    monitor_read_command(opaque, 1);
-}
-#endif
-
 void hmp_change(Monitor *mon, const QDict *qdict)
 {
     const char *device = qdict_get_str(qdict, "device");
@@ -1096,23 +1086,7 @@ void hmp_change(Monitor *mon, const QDict *qdict)
 
 #ifdef CONFIG_VNC
     if (strcmp(device, "vnc") == 0) {
-        if (read_only) {
-            error_setg(&err, "Parameter 'read-only-mode' is invalid for VNC");
-            goto end;
-        }
-        if (strcmp(target, "passwd") == 0 ||
-            strcmp(target, "password") == 0) {
-            if (!arg) {
-                MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
-                monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
-                return;
-            } else {
-                qmp_change_vnc_password(arg, &err);
-            }
-        } else {
-            error_setg(&err, "Expected 'password' after 'vnc'");
-            goto end;
-        }
+        hmp_change_vnc(mon, device, target, arg, read_only, force, &err);
     } else
 #endif
     {
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index b5b736761a..14f0f78e51 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -168,7 +168,7 @@ void qmp_add_client(const char *protocol, const char *fdname,
                     bool has_skipauth, bool skipauth, bool has_tls, bool tls,
                     Error **errp)
 {
-    static struct {
+    static const struct {
         const char *name;
         bool (*add_client)(int fd, bool has_skipauth, bool skipauth,
                            bool has_tls, bool tls, Error **errp);
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index 95abd4693f..41d934acb9 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -18,7 +18,8 @@
 #include <spice/enums.h>
 #endif
 #include "monitor/hmp.h"
-#include "monitor/monitor.h"
+#include "monitor/monitor-internal.h"
+#include "qapi/error.h"
 #include "qapi/qapi-commands-ui.h"
 #include "qapi/qmp/qdict.h"
 #include "qemu/cutils.h"
@@ -311,6 +312,38 @@ out:
     hmp_handle_error(mon, err);
 }
 
+#ifdef CONFIG_VNC
+static void hmp_change_read_arg(void *opaque, const char *password,
+                                void *readline_opaque)
+{
+    qmp_change_vnc_password(password, NULL);
+    monitor_read_command(opaque, 1);
+}
+
+void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
+                    const char *arg, const char *read_only, bool force,
+                    Error **errp)
+{
+    if (read_only) {
+        error_setg(errp, "Parameter 'read-only-mode' is invalid for VNC");
+        return;
+    }
+    if (strcmp(target, "passwd") == 0 ||
+        strcmp(target, "password") == 0) {
+        if (!arg) {
+            MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
+            monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
+            return;
+        } else {
+            qmp_change_vnc_password(arg, errp);
+        }
+    } else {
+        error_setg(errp, "Expected 'password' after 'vnc'");
+        return;
+    }
+}
+#endif
+
 void hmp_sendkey(Monitor *mon, const QDict *qdict)
 {
     const char *keys = qdict_get_str(qdict, "keys");
-- 
2.38.1

[PATCH v3 15/18] ui: Reduce nesting in hmp_change_vnc() slightly

[Markus Armbruster]

Transform

    if (good) {
        do stuff
    } else {
        handle error
    }

to

    if (!good) {
        handle error
	return;
    }
    do stuff

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 ui/ui-hmp-cmds.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index 41d934acb9..adea425d35 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -328,19 +328,16 @@ void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
         error_setg(errp, "Parameter 'read-only-mode' is invalid for VNC");
         return;
     }
-    if (strcmp(target, "passwd") == 0 ||
-        strcmp(target, "password") == 0) {
-        if (!arg) {
-            MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
-            monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
-            return;
-        } else {
-            qmp_change_vnc_password(arg, errp);
-        }
-    } else {
+    if (strcmp(target, "passwd") && strcmp(target, "password")) {
         error_setg(errp, "Expected 'password' after 'vnc'");
         return;
     }
+    if (!arg) {
+        MonitorHMP *hmp_mon = container_of(mon, MonitorHMP, common);
+        monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
+    } else {
+        qmp_change_vnc_password(arg, errp);
+    }
 }
 #endif
 
-- 
2.38.1

[PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/monitor/hmp.h | 1 +
 include/ui/console.h  | 2 +-
 monitor/misc.c        | 1 -
 ui/input.c            | 5 +----
 ui/ui-hmp-cmds.c      | 8 ++++++++
 5 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index df89eac22a..8688769a27 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -88,6 +88,7 @@ void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
 void hmp_mouse_move(Monitor *mon, const QDict *qdict);
 void hmp_mouse_button(Monitor *mon, const QDict *qdict);
+void hmp_mouse_set(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void coroutine_fn hmp_screendump(Monitor *mon, const QDict *qdict);
 void hmp_chardev_add(Monitor *mon, const QDict *qdict);
diff --git a/include/ui/console.h b/include/ui/console.h
index e400ee9fa7..e7f375312c 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry);
 
 void kbd_put_ledstate(int ledstate);
 
-void hmp_mouse_set(Monitor *mon, const QDict *qdict);
+void qemu_mouse_set(int index, Error **err);
 
 /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx
    constants) */
diff --git a/monitor/misc.c b/monitor/misc.c
index 3d68940d28..50cb9f008b 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -33,7 +33,6 @@
 #include "ui/qemu-spice.h"
 #include "qemu/config-file.h"
 #include "qemu/ctype.h"
-#include "ui/console.h"
 #include "audio/audio.h"
 #include "disas/disas.h"
 #include "qemu/timer.h"
diff --git a/ui/input.c b/ui/input.c
index 8f4a87d1d7..7bece94e79 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -2,8 +2,6 @@
 #include "sysemu/sysemu.h"
 #include "qapi/error.h"
 #include "qapi/qapi-commands-ui.h"
-#include "qapi/qmp/qdict.h"
-#include "qemu/error-report.h"
 #include "trace.h"
 #include "ui/input.h"
 #include "ui/console.h"
@@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp)
     return mice_list;
 }
 
-void hmp_mouse_set(Monitor *mon, const QDict *qdict)
+void qemu_mouse_set(int index, Error **err)
 {
     QemuInputHandlerState *s;
-    int index = qdict_get_int(qdict, "index");
     int found = 0;
 
     QTAILQ_FOREACH(s, &handlers, node) {
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index adea425d35..ad6a2a52e2 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -69,6 +69,14 @@ void hmp_mouse_button(Monitor *mon, const QDict *qdict)
     mouse_button_state = button_state;
 }
 
+void hmp_mouse_set(Monitor *mon, const QDict *qdict)
+{
+    Error *err = NULL;
+
+    qemu_mouse_set(qdict_get_int(qdict, "index"), &err);
+    hmp_handle_error(mon, err);
+}
+
 void hmp_info_mice(Monitor *mon, const QDict *qdict)
 {
     MouseInfoList *mice_list, *mouse;
-- 
2.38.1

[PATCH v3 17/18] ui: Don't check for mode change after mouse_set error

[Markus Armbruster]

hmp_mouse_set() doesn't bail out when it can't find a mouse.
Harmless, since qemu_input_check_mode_change() should be a no-op then.
Clean it up anyway.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/input.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ui/input.c b/ui/input.c
index 7bece94e79..99e52c938e 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -613,6 +613,7 @@ void qemu_mouse_set(int index, Error **err)
 
     if (!found) {
         error_report("Mouse at index '%d' not found", index);
+        return;
     }
 
     qemu_input_check_mode_change();
-- 
2.38.1

[PATCH v3 18/18] ui: Simplify control flow in qemu_mouse_set()

[Markus Armbruster]

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 ui/input.c | 22 ++++++++++------------
 1 file changed, 10 insertions(+), 12 deletions(-)

diff --git a/ui/input.c b/ui/input.c
index 99e52c938e..df2f54cb9a 100644
--- a/ui/input.c
+++ b/ui/input.c
@@ -595,26 +595,24 @@ MouseInfoList *qmp_query_mice(Error **errp)
 void qemu_mouse_set(int index, Error **err)
 {
     QemuInputHandlerState *s;
-    int found = 0;
 
     QTAILQ_FOREACH(s, &handlers, node) {
-        if (s->id != index) {
-            continue;
+        if (s->id == index) {
+            break;
         }
-        if (!(s->handler->mask & (INPUT_EVENT_MASK_REL |
-                                  INPUT_EVENT_MASK_ABS))) {
-            error_report("Input device '%s' is not a mouse", s->handler->name);
-            return;
-        }
-        found = 1;
-        qemu_input_handler_activate(s);
-        break;
     }
 
-    if (!found) {
+    if (!s) {
         error_report("Mouse at index '%d' not found", index);
         return;
     }
 
+    if (!(s->handler->mask & (INPUT_EVENT_MASK_REL |
+                              INPUT_EVENT_MASK_ABS))) {
+        error_report("Input device '%s' is not a mouse", s->handler->name);
+        return;
+    }
+
+    qemu_input_handler_activate(s);
     qemu_input_check_mode_change();
 }
-- 
2.38.1

Re: [PATCH v3 01/18] ui: Check numeric part of expire_password argument @time properly

[Daniel P. Berrangé]

On Tue, Dec 20, 2022 at 10:06:28AM +0100, Markus Armbruster wrote:
> When argument @time isn't 'now' or 'never', we parse it as an integer,
> optionally prefixed with '+'.  If parsing fails, we silently assume
> zero.  Report an error and fail instead.
> 
> While there, use qemu_strtou64() instead of strtoull() so
> checkpatch.pl won't complain.
> 
> Aside: encoding numbers in strings is bad QMP practice.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  monitor/qmp-cmds.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>



With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Daniel P. Berrangé]

On Tue, Dec 20, 2022 at 10:06:39AM +0100, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "Graphics".
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/monitor/hmp.h |  2 ++
>  monitor/misc.c        | 66 -------------------------------------------
>  ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 68 insertions(+), 66 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/

[Daniel P. Berrangé]

On Tue, Dec 20, 2022 at 10:06:43AM +0100, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  include/monitor/hmp.h | 1 +
>  include/ui/console.h  | 2 +-
>  monitor/misc.c        | 1 -
>  ui/input.c            | 5 +----
>  ui/ui-hmp-cmds.c      | 8 ++++++++
>  5 files changed, 11 insertions(+), 6 deletions(-)

>  
> -void hmp_mouse_set(Monitor *mon, const QDict *qdict)
> +void qemu_mouse_set(int index, Error **err)

This is adding a Error parameter, nit s/err/errp/

>  {
>      QemuInputHandlerState *s;
> -    int index = qdict_get_int(qdict, "index");
>      int found = 0;
>  
>      QTAILQ_FOREACH(s, &handlers, node) {

But not changing either error_report() call to error_setg(),
so the errp is unused.

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 17/18] ui: Don't check for mode change after mouse_set error

[Daniel P. Berrangé]

On Tue, Dec 20, 2022 at 10:06:44AM +0100, Markus Armbruster wrote:
> hmp_mouse_set() doesn't bail out when it can't find a mouse.
> Harmless, since qemu_input_check_mode_change() should be a no-op then.
> Clean it up anyway.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/input.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 18/18] ui: Simplify control flow in qemu_mouse_set()

[Daniel P. Berrangé]

On Tue, Dec 20, 2022 at 10:06:45AM +0100, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  ui/input.c | 22 ++++++++++------------
>  1 file changed, 10 insertions(+), 12 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: [PATCH v3 00/18] ui: Move and clean up monitor command code

[Philippe Mathieu-Daudé]

On 20/12/22 10:06, Markus Armbruster wrote:
> This is mainly about splitting off monitor-related code.  There's also
> a minimum Spice version bump, and a few UI improvements to HMP
> commands sendkey, change vnc, and info spice.

Possibly related, use of InputEvent in replay:
https://lore.kernel.org/qemu-devel/20221219170806.60580-5-philmd@linaro.org/

Re: [PATCH v3 08/18] ui: Clean up a few things checkpatch.pl would flag later on

[Philippe Mathieu-Daudé]

On 20/12/22 10:06, Markus Armbruster wrote:
> Fix a few style violations so that checkpatch.pl won't complain when I
> move this code.
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   monitor/hmp-cmds.c |  7 ++++---
>   monitor/qmp-cmds.c | 21 +++++++++++----------
>   2 files changed, 15 insertions(+), 13 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 00/18] ui: Move and clean up monitor command code

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 20/12/22 10:06, Markus Armbruster wrote:
>> This is mainly about splitting off monitor-related code.  There's also
>> a minimum Spice version bump, and a few UI improvements to HMP
>> commands sendkey, change vnc, and info spice.
>
> Possibly related, use of InputEvent in replay:
> https://lore.kernel.org/qemu-devel/20221219170806.60580-5-philmd@linaro.org/

Independent, as far as I can tell.  My series changes very little in
include/, see appended diff.


diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 27f86399f7..8688769a27 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -73,6 +73,11 @@ void hmp_x_colo_lost_heartbeat(Monitor *mon, const QDict *qdict);
 void hmp_set_password(Monitor *mon, const QDict *qdict);
 void hmp_expire_password(Monitor *mon, const QDict *qdict);
 void hmp_change(Monitor *mon, const QDict *qdict);
+#ifdef CONFIG_VNC
+void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
+                    const char *arg, const char *read_only, bool force,
+                    Error **errp);
+#endif
 void hmp_migrate(Monitor *mon, const QDict *qdict);
 void hmp_device_add(Monitor *mon, const QDict *qdict);
 void hmp_device_del(Monitor *mon, const QDict *qdict);
@@ -81,6 +86,9 @@ void hmp_netdev_add(Monitor *mon, const QDict *qdict);
 void hmp_netdev_del(Monitor *mon, const QDict *qdict);
 void hmp_getfd(Monitor *mon, const QDict *qdict);
 void hmp_closefd(Monitor *mon, const QDict *qdict);
+void hmp_mouse_move(Monitor *mon, const QDict *qdict);
+void hmp_mouse_button(Monitor *mon, const QDict *qdict);
+void hmp_mouse_set(Monitor *mon, const QDict *qdict);
 void hmp_sendkey(Monitor *mon, const QDict *qdict);
 void coroutine_fn hmp_screendump(Monitor *mon, const QDict *qdict);
 void hmp_chardev_add(Monitor *mon, const QDict *qdict);
diff --git a/include/monitor/qmp-helpers.h b/include/monitor/qmp-helpers.h
new file mode 100644
index 0000000000..4718c63c73
--- /dev/null
+++ b/include/monitor/qmp-helpers.h
@@ -0,0 +1,26 @@
+/*
+ * QMP command helpers
+ *
+ * Copyright (c) 2022 Red Hat Inc.
+ *
+ * Authors:
+ *  Markus Armbruster <armbru@redhat.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2.  See
+ * the COPYING file in the top-level directory.
+ */
+
+#ifndef MONITOR_QMP_HELPERS_H
+
+bool qmp_add_client_spice(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#ifdef CONFIG_VNC
+bool qmp_add_client_vnc(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#endif
+#ifdef CONFIG_DBUS_DISPLAY
+bool qmp_add_client_dbus_display(int fd, bool has_skipauth, bool skipauth,
+                        bool has_tls, bool tls, Error **errp);
+#endif
+
+#endif
diff --git a/include/ui/console.h b/include/ui/console.h
index e400ee9fa7..e7f375312c 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry);
 
 void kbd_put_ledstate(int ledstate);
 
-void hmp_mouse_set(Monitor *mon, const QDict *qdict);
+void qemu_mouse_set(int index, Error **err);
 
 /* keysym is a unicode code except for special keys (see QEMU_KEY_xxx
    constants) */
diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index 21fe195e18..b7d493742c 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -34,13 +34,7 @@ int qemu_spice_add_display_interface(QXLInstance *qxlin, QemuConsole *con);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
-#define SPICE_NEEDS_SET_MM_TIME 1
-#else
-#define SPICE_NEEDS_SET_MM_TIME 0
-#endif
-
-#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
+#if SPICE_SERVER_VERSION >= 0x000f00 /* release 0.15.0 */
 #define SPICE_HAS_ATTACHED_WORKER 1
 #else
 #define SPICE_HAS_ATTACHED_WORKER 0
diff --git a/include/ui/spice-display.h b/include/ui/spice-display.h
index e271e011da..5aa13664d6 100644
--- a/include/ui/spice-display.h
+++ b/include/ui/spice-display.h
@@ -28,11 +28,9 @@
 #include "ui/console.h"
 
 #if defined(CONFIG_OPENGL) && defined(CONFIG_GBM)
-# if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */
 #  define HAVE_SPICE_GL 1
 #  include "ui/egl-helpers.h"
 #  include "ui/egl-context.h"
-# endif
 #endif
 
 #define NUM_MEMSLOTS 8

Re: [PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/

[Philippe Mathieu-Daudé]

On 20/12/22 10:06, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/monitor/hmp.h | 1 +
>   include/ui/console.h  | 2 +-
>   monitor/misc.c        | 1 -
>   ui/input.c            | 5 +----
>   ui/ui-hmp-cmds.c      | 8 ++++++++
>   5 files changed, 11 insertions(+), 6 deletions(-)


> diff --git a/include/ui/console.h b/include/ui/console.h
> index e400ee9fa7..e7f375312c 100644
> --- a/include/ui/console.h
> +++ b/include/ui/console.h
> @@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry);
>   
>   void kbd_put_ledstate(int ledstate);
>   
> -void hmp_mouse_set(Monitor *mon, const QDict *qdict);
> +void qemu_mouse_set(int index, Error **err);

Can we return a boolean to indicate eventual failure?

> @@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp)
>       return mice_list;
>   }
>   
> -void hmp_mouse_set(Monitor *mon, const QDict *qdict)
> +void qemu_mouse_set(int index, Error **err)
>   {
>       QemuInputHandlerState *s;
> -    int index = qdict_get_int(qdict, "index");
>       int found = 0;
>   
>       QTAILQ_FOREACH(s, &handlers, node) {
However no failure is reported, errp isn't used... Do we really
want to pass it along?

Re: [PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/

[Markus Armbruster]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Tue, Dec 20, 2022 at 10:06:43AM +0100, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>  include/monitor/hmp.h | 1 +
>>  include/ui/console.h  | 2 +-
>>  monitor/misc.c        | 1 -
>>  ui/input.c            | 5 +----
>>  ui/ui-hmp-cmds.c      | 8 ++++++++
>>  5 files changed, 11 insertions(+), 6 deletions(-)
>
>>  
>> -void hmp_mouse_set(Monitor *mon, const QDict *qdict)
>> +void qemu_mouse_set(int index, Error **err)
>
> This is adding a Error parameter, nit s/err/errp/

Yes.

>>  {
>>      QemuInputHandlerState *s;
>> -    int index = qdict_get_int(qdict, "index");
>>      int found = 0;
>>  
>>      QTAILQ_FOREACH(s, &handlers, node) {
>
> But not changing either error_report() call to error_setg(),
> so the errp is unused.

I suspect I some distraction hit my flu-addled brain in just the right
moment, and I forgot to flip to error_setg().  Will fix, thanks!

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Philippe Mathieu-Daudé]

On 20/12/22 10:06, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "Graphics".
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/monitor/hmp.h |  2 ++
>   monitor/misc.c        | 66 -------------------------------------------
>   ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 68 insertions(+), 66 deletions(-)

You forgot to move hmp_sendkey() along.

Re: [PATCH v3 13/18] ui: Improve "change vnc" error reporting

[Philippe Mathieu-Daudé]

On 20/12/22 10:06, Markus Armbruster wrote:
> Switch from monitor_printf() to error_setg() and hmp_handle_error().
> This makes "this is an error" more obvious both in the source and in
> the monitor, where hmp_handle_error() prefixes the message with
> "Error: ".
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>   monitor/hmp-cmds.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 20/12/22 10:06, Markus Armbruster wrote:
>> This moves these commands from MAINTAINERS section "Human
>> Monitor (HMP)" to "Graphics".
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   include/monitor/hmp.h |  2 ++
>>   monitor/misc.c        | 66 -------------------------------------------
>>   ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 68 insertions(+), 66 deletions(-)
>
> You forgot to move hmp_sendkey() along.

Moved in the previous patch.

If I get your R-by, I'll squash the two patches together.

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Philippe Mathieu-Daudé]

On 20/12/22 12:49, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 20/12/22 10:06, Markus Armbruster wrote:
>>> This moves these commands from MAINTAINERS section "Human
>>> Monitor (HMP)" to "Graphics".
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>>    include/monitor/hmp.h |  2 ++
>>>    monitor/misc.c        | 66 -------------------------------------------
>>>    ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>>>    3 files changed, 68 insertions(+), 66 deletions(-)
>>
>> You forgot to move hmp_sendkey() along.
> 
> Moved in the previous patch.

Oops :)

> If I get your R-by, I'll squash the two patches together.

Hmm not needed, the previous patch is already big enough.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 20/12/22 12:49, Markus Armbruster wrote:
>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>> 
>>> On 20/12/22 10:06, Markus Armbruster wrote:
>>>> This moves these commands from MAINTAINERS section "Human
>>>> Monitor (HMP)" to "Graphics".
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>>    include/monitor/hmp.h |  2 ++
>>>>    monitor/misc.c        | 66 -------------------------------------------
>>>>    ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>>>>    3 files changed, 68 insertions(+), 66 deletions(-)
>>>
>>> You forgot to move hmp_sendkey() along.
>>
>> Moved in the previous patch.
>
> Oops :)
>
>> If I get your R-by, I'll squash the two patches together.
>
> Hmm not needed, the previous patch is already big enough.

Yes, but it's just code motion, and the split between the two parts
feels arbitrary.  It came to be by accident: I missed a bunch of HMP
commands hiding in yet another file :)

> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!

Re: [PATCH v3 12/18] ui: Move more HMP commands from monitor to ui/

[Philippe Mathieu-Daudé]

On 20/12/22 16:30, Markus Armbruster wrote:
> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> 
>> On 20/12/22 12:49, Markus Armbruster wrote:
>>> Philippe Mathieu-Daudé <philmd@linaro.org> writes:
>>>
>>>> On 20/12/22 10:06, Markus Armbruster wrote:
>>>>> This moves these commands from MAINTAINERS section "Human
>>>>> Monitor (HMP)" to "Graphics".
>>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>>> ---
>>>>>     include/monitor/hmp.h |  2 ++
>>>>>     monitor/misc.c        | 66 -------------------------------------------
>>>>>     ui/ui-hmp-cmds.c      | 66 +++++++++++++++++++++++++++++++++++++++++++
>>>>>     3 files changed, 68 insertions(+), 66 deletions(-)
>>>>
>>>> You forgot to move hmp_sendkey() along.
>>>
>>> Moved in the previous patch.
>>
>> Oops :)
>>
>>> If I get your R-by, I'll squash the two patches together.
>>
>> Hmm not needed, the previous patch is already big enough.
> 
> Yes, but it's just code motion, and the split between the two parts
> feels arbitrary.  It came to be by accident: I missed a bunch of HMP
> commands hiding in yet another file :)

I see, OK then.

Re: [PATCH v3 02/18] ui: Fix silent truncation of numeric keys in HMP sendkey

[Dr. David Alan Gilbert]

* Markus Armbruster (armbru@redhat.com) wrote:
> Keys are int.  HMP sendkey assigns them from the value strtoul(),
> silently truncating values greater than INT_MAX.  Fix to reject them.
> 
> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
> won't complain.

Last time through you said you could switch to qemu_strtoui, but
I just noticed we've actually got a qemu_strto*i* - that
would remove the value comparison

Dave

> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  monitor/hmp-cmds.c | 9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index ed78a87ddd..b8e294e6fa 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1549,8 +1549,13 @@ void hmp_sendkey(Monitor *mon, const QDict *qdict)
>          v = g_malloc0(sizeof(*v));
>  
>          if (strstart(keys, "0x", NULL)) {
> -            char *endp;
> -            int value = strtoul(keys, &endp, 0);
> +            const char *endp;
> +            unsigned long value;
> +
> +            if (qemu_strtoul(keys, &endp, 0, &value) < 0
> +                || value >= INT_MAX) {
> +                goto err_out;
> +            }
>              assert(endp <= keys + keyname_len);
>              if (endp != keys + keyname_len) {
>                  goto err_out;
> -- 
> 2.38.1
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK

Re: [PATCH v3 02/18] ui: Fix silent truncation of numeric keys in HMP sendkey

[Markus Armbruster]

"Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:

> * Markus Armbruster (armbru@redhat.com) wrote:
>> Keys are int.  HMP sendkey assigns them from the value strtoul(),
>> silently truncating values greater than INT_MAX.  Fix to reject them.
>> 
>> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
>> won't complain.
>
> Last time through you said you could switch to qemu_strtoui, but

Did I?  Oh, I did in review of

    [PATCH 08/12] pci: Fix silent truncation of pcie_aer_inject_error argument

but failed to do it here, too.

> I just noticed we've actually got a qemu_strto*i* - that
> would remove the value comparison

Thanks!

Re: [PATCH v3 16/18] ui: Split hmp_mouse_set() and move the HMP part to ui/

[Markus Armbruster]

Philippe Mathieu-Daudé <philmd@linaro.org> writes:

> On 20/12/22 10:06, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   include/monitor/hmp.h | 1 +
>>   include/ui/console.h  | 2 +-
>>   monitor/misc.c        | 1 -
>>   ui/input.c            | 5 +----
>>   ui/ui-hmp-cmds.c      | 8 ++++++++
>>   5 files changed, 11 insertions(+), 6 deletions(-)
>
>
>> diff --git a/include/ui/console.h b/include/ui/console.h
>> index e400ee9fa7..e7f375312c 100644
>> --- a/include/ui/console.h
>> +++ b/include/ui/console.h
>> @@ -65,7 +65,7 @@ void qemu_remove_led_event_handler(QEMUPutLEDEntry *entry);
>>   
>>   void kbd_put_ledstate(int ledstate);
>>   
>> -void hmp_mouse_set(Monitor *mon, const QDict *qdict);
>> +void qemu_mouse_set(int index, Error **err);
>
> Can we return a boolean to indicate eventual failure?

The only caller isn't interested.  But we could do it anyway to conform
to the guidance in qapi/error.h.

>> @@ -594,10 +592,9 @@ MouseInfoList *qmp_query_mice(Error **errp)
>>       return mice_list;
>>   }
>>   
>> -void hmp_mouse_set(Monitor *mon, const QDict *qdict)
>> +void qemu_mouse_set(int index, Error **err)
>>   {
>>       QemuInputHandlerState *s;
>> -    int index = qdict_get_int(qdict, "index");
>>       int found = 0;
>>   
>>       QTAILQ_FOREACH(s, &handlers, node) {
>
> However no failure is reported, errp isn't used... Do we really
> want to pass it along?

Daniel spotted this, too.  See my reply there.

Thanks!