qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/14] ui: Move and clean up monitor command code
@ 2022-12-02 10:04 Markus Armbruster
  2022-12-02 10:04 ` [PATCH v2 01/14] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
                   ` (13 more replies)
  0 siblings, 14 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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.

v2:
* PATCH 03,05-06: New [Daniel]
* PATCH 04: Enable instead of drop channel type "webdav" [Daniel]
* PATCH 07: New [Philippe]
* PATCH 09: Don't move add_client() to ui/
* PATCH 10: New, move ui parts of add_client() to ui/
* PATCH 12+13: Fix incorrect patch split [Daniel]

Markus Armbruster (14):
  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: 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

 meson.build                   |   4 +-
 hw/display/qxl.h              |   2 -
 include/monitor/hmp.h         |   5 +
 include/monitor/qmp-helpers.h |  26 +++
 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            | 370 +-------------------------------
 monitor/qmp-cmds.c            | 176 +++------------
 ui/spice-display.c            |  10 -
 ui/ui-hmp-cmds.c              | 391 ++++++++++++++++++++++++++++++++++
 ui/ui-qmp-cmds.c              | 177 +++++++++++++++
 ui/vdagent.c                  |   4 -
 hw/display/trace-events       |   1 -
 ui/meson.build                |   2 +
 16 files changed, 638 insertions(+), 568 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.37.3



^ permalink raw reply	[flat|nested] 27+ messages in thread

* [PATCH v2 01/14] ui: Check numeric part of expire_password argument @time properly
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
@ 2022-12-02 10:04 ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 02/14] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
                   ` (12 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:04 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 81c8fdadf8..054d7648b1 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -205,15 +205,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 02/14] ui: Fix silent truncation of numeric keys in HMP sendkey
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
  2022-12-02 10:04 ` [PATCH v2 01/14] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
                   ` (11 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 01b789a79e..a7c9ae2520 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1666,8 +1666,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
  2022-12-02 10:04 ` [PATCH v2 01/14] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 02/14] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:14   ` Philippe Mathieu-Daudé
  2022-12-05 11:09   ` Daniel P. Berrangé
  2022-12-02 10:05 ` [PATCH v2 04/14] Revert "hmp: info spice: take out webdav" Markus Armbruster
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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>
---
 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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 04/14] Revert "hmp: info spice: take out webdav"
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (2 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-05 11:09   ` Daniel P. Berrangé
  2022-12-02 10:05 ` [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
                   ` (9 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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>
---
 monitor/hmp-cmds.c | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a7c9ae2520..7360ed71a6 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -626,12 +626,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (3 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 04/14] Revert "hmp: info spice: take out webdav" Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:16   ` Philippe Mathieu-Daudé
  2022-12-05 11:11   ` Daniel P. Berrangé
  2022-12-02 10:05 ` [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
                   ` (8 subsequent siblings)
  13 siblings, 2 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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>
---
 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 e74de9579d..944e779615 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 5b10f697f1..5b3cd33066 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);
         }
@@ -1086,12 +1085,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,
@@ -2463,9 +2460,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (4 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:17   ` Philippe Mathieu-Daudé
  2022-12-05 11:11   ` Daniel P. Berrangé
  2022-12-02 10:05 ` [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage Markus Armbruster
                   ` (7 subsequent siblings)
  13 siblings, 2 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

SPICE_NEEDS_SET_MM_TIME is now always off.  Bury the dead code.

Signed-off-by: Markus Armbruster <armbru@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 5b3cd33066..82eafdcc68 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -540,22 +540,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);
@@ -1142,9 +1126,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (5 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:13   ` Philippe Mathieu-Daudé
  2022-12-02 10:05 ` [PATCH v2 08/14] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
                   ` (6 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

Suggested-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 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 7360ed71a6..8eb9c19b3a 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -615,7 +615,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 08/14] ui: Clean up a few things checkpatch.pl would flag later on
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (6 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 8eb9c19b3a..86dde966ff 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -591,9 +591,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 054d7648b1..a7c95e8e39 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -190,8 +190,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);
     }
 
@@ -276,12 +278,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) {
@@ -293,19 +293,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (7 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 08/14] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:18   ` Philippe Mathieu-Daudé
  2022-12-02 10:05 ` [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c Markus Armbruster
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" 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>
---
 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 a7c95e8e39..1189f195ed 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"
@@ -172,89 +170,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)
@@ -309,7 +224,6 @@ void qmp_add_client(const char *protocol, const char *fdname,
     }
 }
 
-
 MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
 {
     return qmp_memory_device_list();
@@ -348,38 +262,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (8 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:57   ` Philippe Mathieu-Daudé
  2022-12-02 10:05 ` [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 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 1189f195ed..eb9d7b6ab6 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"
@@ -174,54 +172,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (9 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-12 10:08   ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 12/14] ui: Improve "change vnc" error reporting Markus Armbruster
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 | 343 ------------------------------------------
 ui/ui-hmp-cmds.c   | 361 +++++++++++++++++++++++++++++++++++++++++++++
 ui/meson.build     |   1 +
 3 files changed, 362 insertions(+), 343 deletions(-)
 create mode 100644 ui/ui-hmp-cmds.c

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 86dde966ff..f0f7b74fb3 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -52,7 +52,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"
@@ -60,10 +59,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) {
@@ -179,26 +174,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;
@@ -518,170 +493,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->has_x509_dname ?
-                       cinfo->x509_dname : "none");
-        monitor_printf(mon, "    sasl_username: %s\n",
-                       cinfo->has_sasl_username ?
-                       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->has_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;
@@ -1376,71 +1187,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.has_display = !!display;
-        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.has_display = !!display;
-        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)
@@ -1638,95 +1384,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 != NULL, 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..c26a1f00d0
--- /dev/null
+++ b/ui/ui-hmp-cmds.c
@@ -0,0 +1,361 @@
+/*
+ * Human Monitor Interface commands
+ *
+ * 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->has_x509_dname ?
+                       cinfo->x509_dname : "none");
+        monitor_printf(mon, "    sasl_username: %s\n",
+                       cinfo->has_sasl_username ?
+                       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->has_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.has_display = !!display;
+        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.has_display = !!display;
+        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 != NULL, 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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 12/14] ui: Improve "change vnc" error reporting
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (10 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 13/14] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 14/14] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 f0f7b74fb3..8542eee3d4 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1209,9 +1209,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) {
@@ -1223,7 +1222,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 13/14] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (11 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 12/14] ui: Improve "change vnc" error reporting Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  2022-12-02 10:05 ` [PATCH v2 14/14] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 include/monitor/hmp.h |  5 +++++
 monitor/hmp-cmds.c    | 28 +---------------------------
 ui/ui-hmp-cmds.c      | 35 ++++++++++++++++++++++++++++++++++-
 3 files changed, 40 insertions(+), 28 deletions(-)

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index dfbc0c9a2f..992d91f181 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 8542eee3d4..78bcdede85 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -42,7 +42,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"
@@ -1187,15 +1186,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");
@@ -1208,23 +1198,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/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index c26a1f00d0..e17f22c869 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"
@@ -271,6 +272,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* [PATCH v2 14/14] ui: Reduce nesting in hmp_change_vnc() slightly
  2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
                   ` (12 preceding siblings ...)
  2022-12-02 10:05 ` [PATCH v2 13/14] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-02 10:05 ` Markus Armbruster
  13 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 10:05 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

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 e17f22c869..54ad7db5db 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -288,19 +288,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.37.3



^ permalink raw reply related	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage
  2022-12-02 10:05 ` [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage Markus Armbruster
@ 2022-12-02 10:13   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:13 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

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

Thanks,

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0
  2022-12-02 10:05 ` [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
@ 2022-12-02 10:14   ` Philippe Mathieu-Daudé
  2022-12-05 11:09   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:14 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

On 2/12/22 11:05, Markus Armbruster wrote:
> 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>
> ---
>   meson.build  | 2 +-
>   ui/vdagent.c | 4 ----
>   2 files changed, 1 insertion(+), 5 deletions(-)

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0
  2022-12-02 10:05 ` [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
@ 2022-12-02 10:16   ` Philippe Mathieu-Daudé
  2022-12-05 11:11   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:16 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

On 2/12/22 11:05, Markus Armbruster wrote:
> 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>
> ---
>   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(-)

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop
  2022-12-02 10:05 ` [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
@ 2022-12-02 10:17   ` Philippe Mathieu-Daudé
  2022-12-05 11:11   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:17 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

On 2/12/22 11:05, Markus Armbruster wrote:
> SPICE_NEEDS_SET_MM_TIME is now always off.  Bury the dead code.
> 
> Signed-off-by: Markus Armbruster <armbru@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(-)

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
  2022-12-02 10:05 ` [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-02 10:18   ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:18 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

On 2/12/22 11:05, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" 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>
> ---
>   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

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c
  2022-12-02 10:05 ` [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-02 10:57   ` Philippe Mathieu-Daudé
  2022-12-02 15:09     ` Markus Armbruster
  0 siblings, 1 reply; 27+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-02 10:57 UTC (permalink / raw)
  To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert, berrange

On 2/12/22 11:05, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   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


> @@ -174,54 +172,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.

> +        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
> +    };

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



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c
  2022-12-02 10:57   ` Philippe Mathieu-Daudé
@ 2022-12-02 15:09     ` Markus Armbruster
  0 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-02 15:09 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé; +Cc: qemu-devel, kraxel, dgilbert, berrange

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

> On 2/12/22 11:05, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   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
>
>
>> @@ -174,54 +172,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.

Okay.

>> +        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
>> +    };
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Thanks!



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0
  2022-12-02 10:05 ` [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
  2022-12-02 10:14   ` Philippe Mathieu-Daudé
@ 2022-12-05 11:09   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-12-05 11:09 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert, philmd

On Fri, Dec 02, 2022 at 11:05:01AM +0100, Markus Armbruster wrote:
> 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>
> ---
>  meson.build  | 2 +-
>  ui/vdagent.c | 4 ----
>  2 files changed, 1 insertion(+), 5 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 :|



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 04/14] Revert "hmp: info spice: take out webdav"
  2022-12-02 10:05 ` [PATCH v2 04/14] Revert "hmp: info spice: take out webdav" Markus Armbruster
@ 2022-12-05 11:09   ` Daniel P. Berrangé
  0 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-12-05 11:09 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert, philmd

On Fri, Dec 02, 2022 at 11:05:02AM +0100, Markus Armbruster wrote:
> 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>
> ---
>  monitor/hmp-cmds.c | 5 -----
>  1 file changed, 5 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 :|



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0
  2022-12-02 10:05 ` [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
  2022-12-02 10:16   ` Philippe Mathieu-Daudé
@ 2022-12-05 11:11   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-12-05 11:11 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert, philmd

On Fri, Dec 02, 2022 at 11:05:03AM +0100, Markus Armbruster wrote:
> 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>
> ---
>  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(-)

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 :|



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop
  2022-12-02 10:05 ` [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
  2022-12-02 10:17   ` Philippe Mathieu-Daudé
@ 2022-12-05 11:11   ` Daniel P. Berrangé
  1 sibling, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-12-05 11:11 UTC (permalink / raw)
  To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert, philmd

On Fri, Dec 02, 2022 at 11:05:04AM +0100, Markus Armbruster wrote:
> SPICE_NEEDS_SET_MM_TIME is now always off.  Bury the dead code.
> 
> Signed-off-by: Markus Armbruster <armbru@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(-)

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 :|



^ permalink raw reply	[flat|nested] 27+ messages in thread

* Re: [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
  2022-12-02 10:05 ` [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-12 10:08   ` Markus Armbruster
  0 siblings, 0 replies; 27+ messages in thread
From: Markus Armbruster @ 2022-12-12 10:08 UTC (permalink / raw)
  To: qemu-devel; +Cc: kraxel, dgilbert, berrange, philmd

Markus Armbruster <armbru@redhat.com> writes:

> 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>

[...]

> diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
> new file mode 100644
> index 0000000000..c26a1f00d0
> --- /dev/null
> +++ b/ui/ui-hmp-cmds.c
> @@ -0,0 +1,361 @@
> +/*
> + * Human Monitor Interface commands

Make this

    * 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.
> + */

[...]



^ permalink raw reply	[flat|nested] 27+ messages in thread

end of thread, other threads:[~2022-12-12 10:08 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-02 10:04 [PATCH v2 00/14] ui: Move and clean up monitor command code Markus Armbruster
2022-12-02 10:04 ` [PATCH v2 01/14] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 02/14] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 03/14] ui/spice: Require spice-protocol >= 0.14.0 Markus Armbruster
2022-12-02 10:14   ` Philippe Mathieu-Daudé
2022-12-05 11:09   ` Daniel P. Berrangé
2022-12-02 10:05 ` [PATCH v2 04/14] Revert "hmp: info spice: take out webdav" Markus Armbruster
2022-12-05 11:09   ` Daniel P. Berrangé
2022-12-02 10:05 ` [PATCH v2 05/14] ui/spice: Require spice-server >= 0.14.0 Markus Armbruster
2022-12-02 10:16   ` Philippe Mathieu-Daudé
2022-12-05 11:11   ` Daniel P. Berrangé
2022-12-02 10:05 ` [PATCH v2 06/14] ui/spice: QXLInterface method set_mm_time() is now dead, drop Markus Armbruster
2022-12-02 10:17   ` Philippe Mathieu-Daudé
2022-12-05 11:11   ` Daniel P. Berrangé
2022-12-02 10:05 ` [PATCH v2 07/14] ui/spice: Give hmp_info_spice()'s channel_names[] static linkage Markus Armbruster
2022-12-02 10:13   ` Philippe Mathieu-Daudé
2022-12-02 10:05 ` [PATCH v2 08/14] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 09/14] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
2022-12-02 10:18   ` Philippe Mathieu-Daudé
2022-12-02 10:05 ` [PATCH v2 10/14] ui: Factor out qmp_add_client() parts and move to ui/ui-qmp-cmds.c Markus Armbruster
2022-12-02 10:57   ` Philippe Mathieu-Daudé
2022-12-02 15:09     ` Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 11/14] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
2022-12-12 10:08   ` Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 12/14] ui: Improve "change vnc" error reporting Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 13/14] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
2022-12-02 10:05 ` [PATCH v2 14/14] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).