* [PATCH 0/9] ui: Move and clean up monitor command code
@ 2022-12-01 6:13 Markus Armbruster
2022-12-01 6:13 ` [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
` (8 more replies)
0 siblings, 9 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
This is mainly about splitting off monitor-related code. There's also
a few UI fixes to HMP commands sendkey and change vnc.
PATCH 3 drops long-disabled code. We could bump the required version
of Spice instead. Opinions?
Markus Armbruster (9):
ui: Check numeric part of expire_password argument @time properly
ui: Fix silent truncation of numeric keys in HMP sendkey
ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
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: 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
include/monitor/hmp.h | 5 +
monitor/hmp-cmds.c | 370 +--------------------------------------
monitor/qmp-cmds.c | 163 ------------------
ui/ui-hmp-cmds.c | 390 ++++++++++++++++++++++++++++++++++++++++++
ui/ui-qmp-cmds.c | 194 +++++++++++++++++++++
ui/meson.build | 2 +
6 files changed, 592 insertions(+), 532 deletions(-)
create mode 100644 ui/ui-hmp-cmds.c
create mode 100644 ui/ui-qmp-cmds.c
--
2.37.3
^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:13 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
` (7 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
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>
---
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] 31+ messages in thread
* [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
2022-12-01 6:13 ` [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:14 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV Markus Armbruster
` (6 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
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>
---
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] 31+ messages in thread
* [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
2022-12-01 6:13 ` [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
2022-12-01 6:13 ` [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:49 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
` (5 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
HMP "info spice" has a bit of code to show channel type
SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
"hmp: info spice: take out webdav" (v2.3.0), because it compiles only
with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
Looks like nobody minded in more than seven years. Drop it, so that
checkpatch.pl won't complain when I move the code.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor/hmp-cmds.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a7c9ae2520..86dd961462 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -626,12 +626,6 @@ 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] 31+ messages in thread
* [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (2 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:50 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
` (4 subsequent siblings)
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
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>
---
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 86dd961462..29fcc730cb 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] 31+ messages in thread
* [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (3 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 6:38 ` Markus Armbruster
2022-12-01 8:51 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
` (3 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
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>
---
monitor/qmp-cmds.c | 177 -----------------------------------------
ui/ui-qmp-cmds.c | 194 +++++++++++++++++++++++++++++++++++++++++++++
ui/meson.build | 1 +
3 files changed, 195 insertions(+), 177 deletions(-)
create mode 100644 ui/ui-qmp-cmds.c
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index a7c95e8e39..85b2089873 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -16,14 +16,9 @@
#include "qemu/osdep.h"
#include "qemu/cutils.h"
#include "qemu/option.h"
-#include "monitor/monitor.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"
@@ -36,9 +31,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,144 +165,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)
-{
- Chardev *s;
- int fd;
-
- 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);
- return;
- }
- }
-}
-
-
MemoryDeviceInfoList *qmp_query_memory_devices(Error **errp)
{
return qmp_memory_device_list();
@@ -348,38 +203,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..5d145c26f7
--- /dev/null
+++ b/ui/ui-qmp-cmds.c
@@ -0,0 +1,194 @@
+/*
+ * 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 "chardev/char.h"
+#include "monitor/monitor.h"
+#include "qapi/qapi-commands-misc.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)
+{
+ 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)
+{
+ Chardev *s;
+ int fd;
+
+ 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);
+ return;
+ }
+ }
+}
+
+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] 31+ messages in thread
* [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (4 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 7:14 ` Philippe Mathieu-Daudé
2022-12-01 8:53 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 7/9] ui: Improve "change vnc" error reporting Markus Armbruster
` (2 subsequent siblings)
8 siblings, 2 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
This moves these commands from MAINTAINERS section "Human
Monitor (HMP)" to "Graphics".
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
monitor/hmp-cmds.c | 342 ------------------------------------------
ui/ui-hmp-cmds.c | 360 +++++++++++++++++++++++++++++++++++++++++++++
ui/meson.build | 1 +
3 files changed, 361 insertions(+), 342 deletions(-)
create mode 100644 ui/ui-hmp-cmds.c
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 29fcc730cb..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,169 +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;
- 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",
- };
-
- 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;
@@ -1375,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)
@@ -1637,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..af290da9e1
--- /dev/null
+++ b/ui/ui-hmp-cmds.c
@@ -0,0 +1,360 @@
+/*
+ * 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;
+ 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",
+ };
+
+ 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] 31+ messages in thread
* [PATCH 7/9] ui: Improve "change vnc" error reporting
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (5 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:56 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
2022-12-01 6:13 ` [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
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>
---
monitor/hmp-cmds.c | 8 ++++----
ui/ui-hmp-cmds.c | 22 ++++++++++++++++++++++
2 files changed, 26 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
diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
index af290da9e1..90a4f86f25 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -270,6 +270,28 @@ out:
hmp_handle_error(mon, err);
}
+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(mon, "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, &err);
+ }
+ } else {
+ monitor_printf(mon, "Expected 'password' after 'vnc'\n");
+ }
+}
+
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] 31+ messages in thread
* [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (6 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 7/9] ui: Improve "change vnc" error reporting Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 8:57 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
8 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
include/monitor/hmp.h | 5 +++++
monitor/hmp-cmds.c | 28 +---------------------------
ui/ui-hmp-cmds.c | 19 +++++++++++++++----
3 files changed, 21 insertions(+), 31 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 90a4f86f25..b1b18d5a5d 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"
@@ -270,12 +271,20 @@ 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(mon, "Parameter 'read-only-mode' is invalid for VNC");
+ error_setg(errp, "Parameter 'read-only-mode' is invalid for VNC");
return;
}
if (strcmp(target, "passwd") == 0 ||
@@ -285,12 +294,14 @@ void hmp_change_vnc(Monitor *mon, const char *device, const char *target,
monitor_read_password(hmp_mon, hmp_change_read_arg, NULL);
return;
} else {
- qmp_change_vnc_password(arg, &err);
+ qmp_change_vnc_password(arg, errp);
}
} else {
- monitor_printf(mon, "Expected 'password' after 'vnc'\n");
+ error_setg(errp, "Expected 'password' after 'vnc'");
+ return;
}
}
+#endif
void hmp_sendkey(Monitor *mon, const QDict *qdict)
{
--
2.37.3
^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
` (7 preceding siblings ...)
2022-12-01 6:13 ` [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-01 6:13 ` Markus Armbruster
2022-12-01 7:19 ` Philippe Mathieu-Daudé
2022-12-01 8:58 ` Daniel P. Berrangé
8 siblings, 2 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:13 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
Transform
if (good) {
do stuff
} else {
handle error
}
to
if (!good) {
handle error
return;
}
do stuff
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
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 b1b18d5a5d..31778122d3 100644
--- a/ui/ui-hmp-cmds.c
+++ b/ui/ui-hmp-cmds.c
@@ -287,19 +287,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] 31+ messages in thread
* Re: [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
2022-12-01 6:13 ` [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
@ 2022-12-01 6:38 ` Markus Armbruster
2022-12-11 16:48 ` Markus Armbruster
2022-12-01 8:51 ` Daniel P. Berrangé
1 sibling, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 6:38 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
Markus Armbruster <armbru@redhat.com> writes:
> 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.
Last sentence no verb. Trying again:
Aside: @protocol is either a display type or else a character device
ID. Bad design.
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
2022-12-01 6:13 ` [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-01 7:14 ` Philippe Mathieu-Daudé
2022-12-01 10:26 ` Markus Armbruster
2022-12-01 8:53 ` Daniel P. Berrangé
1 sibling, 1 reply; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-01 7:14 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert
On 1/12/22 07:13, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "Graphics".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 342 ------------------------------------------
> ui/ui-hmp-cmds.c | 360 +++++++++++++++++++++++++++++++++++++++++++++
> ui/meson.build | 1 +
> 3 files changed, 361 insertions(+), 342 deletions(-)
> create mode 100644 ui/ui-hmp-cmds.c
> +#ifdef CONFIG_SPICE
> +void hmp_info_spice(Monitor *mon, const QDict *qdict)
> +{
> + SpiceChannelList *chan;
> + SpiceInfo *info;
> + const char *channel_name;
> + const char * const channel_names[] = {
Can be static (pre-existing).
> + [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",
> + };
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly
2022-12-01 6:13 ` [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
@ 2022-12-01 7:19 ` Philippe Mathieu-Daudé
2022-12-01 8:58 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Philippe Mathieu-Daudé @ 2022-12-01 7:19 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: kraxel, dgilbert
On 1/12/22 07:13, Markus Armbruster wrote:
> Transform
>
> if (good) {
> do stuff
> } else {
> handle error
> }
>
> to
>
> if (!good) {
> handle error
> return;
> }
> do stuff
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> ui/ui-hmp-cmds.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 deletions(-)
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly
2022-12-01 6:13 ` [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
@ 2022-12-01 8:13 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:13 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:03AM +0100, Markus Armbruster wrote:
> When argument @time isn't 'now' or 'never', we parse it as an integer,
> optionally prefixed with '+'. If parsing fails, we silently assume
> zero. Report an error and fail instead.
>
> While there, use qemu_strtou64() instead of strtoull() so
> checkpatch.pl won't complain.
>
> Aside: encoding numbers in strings is bad QMP practice.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/qmp-cmds.c | 17 +++++++++++++++--
> 1 file changed, 15 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey
2022-12-01 6:13 ` [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
@ 2022-12-01 8:14 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:14 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:04AM +0100, Markus Armbruster wrote:
> Keys are int. HMP sendkey assigns them from the value strtoul(),
> silently truncating values greater than INT_MAX. Fix to reject them.
>
> While there, use qemu_strtoul() instead of strtoul() so checkpatch.pl
> won't complain.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 6:13 ` [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV Markus Armbruster
@ 2022-12-01 8:49 ` Daniel P. Berrangé
2022-12-01 10:25 ` Markus Armbruster
2022-12-01 12:39 ` Markus Armbruster
0 siblings, 2 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:49 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
> HMP "info spice" has a bit of code to show channel type
> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
Last version bump was 4 years ago
commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
Author: Marc-André Lureau <marcandre.lureau@redhat.com>
Date: Wed Nov 28 19:59:32 2018 +0400
configure: bump spice-server required version to 0.12.5
...snip....
According to repology, all the distros that are build target platforms
for QEMU include it:
RHEL-7: 0.14.0
Debian (Stretch): 0.12.8
Debian (Jessie): 0.12.5
FreeBSD (ports): 0.14.0
OpenSUSE Leap 15: 0.14.0
Ubuntu (Xenial): 0.12.6
We moved on from Debian and RHEL since then
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
IOW, we can bump to 0.14.0, and then revert the
webdav conditional commit.
>
> Looks like nobody minded in more than seven years. Drop it, so that
> checkpatch.pl won't complain when I move the code.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 6 ------
> 1 file changed, 6 deletions(-)
>
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index a7c9ae2520..86dd961462 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -626,12 +626,6 @@ 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
>
>
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] 31+ messages in thread
* Re: [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on
2022-12-01 6:13 ` [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
@ 2022-12-01 8:50 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:50 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:06AM +0100, Markus Armbruster wrote:
> Fix a few style violations so that checkpatch.pl won't complain when I
> move this code.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 7 ++++---
> monitor/qmp-cmds.c | 21 +++++++++++----------
> 2 files changed, 15 insertions(+), 13 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] 31+ messages in thread
* Re: [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
2022-12-01 6:13 ` [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
2022-12-01 6:38 ` Markus Armbruster
@ 2022-12-01 8:51 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:51 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:07AM +0100, 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>
> ---
> monitor/qmp-cmds.c | 177 -----------------------------------------
> ui/ui-qmp-cmds.c | 194 +++++++++++++++++++++++++++++++++++++++++++++
> ui/meson.build | 1 +
> 3 files changed, 195 insertions(+), 177 deletions(-)
> create mode 100644 ui/ui-qmp-cmds.c
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] 31+ messages in thread
* Re: [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
2022-12-01 6:13 ` [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
2022-12-01 7:14 ` Philippe Mathieu-Daudé
@ 2022-12-01 8:53 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:53 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:08AM +0100, Markus Armbruster wrote:
> This moves these commands from MAINTAINERS section "Human
> Monitor (HMP)" to "Graphics".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 342 ------------------------------------------
> ui/ui-hmp-cmds.c | 360 +++++++++++++++++++++++++++++++++++++++++++++
> ui/meson.build | 1 +
> 3 files changed, 361 insertions(+), 342 deletions(-)
> create mode 100644 ui/ui-hmp-cmds.c
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] 31+ messages in thread
* Re: [PATCH 7/9] ui: Improve "change vnc" error reporting
2022-12-01 6:13 ` [PATCH 7/9] ui: Improve "change vnc" error reporting Markus Armbruster
@ 2022-12-01 8:56 ` Daniel P. Berrangé
2022-12-01 10:27 ` Markus Armbruster
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:56 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:09AM +0100, Markus Armbruster wrote:
> Switch from monitor_printf() to error_setg() and hmp_handle_error().
> This makes "this is an error" more obvious both in the source and in
> the monitor, where hmp_handle_error() prefixes the message with
> "Error: ".
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> monitor/hmp-cmds.c | 8 ++++----
> ui/ui-hmp-cmds.c | 22 ++++++++++++++++++++++
> 2 files changed, 26 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
Upto this point
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
> diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
> index af290da9e1..90a4f86f25 100644
> --- a/ui/ui-hmp-cmds.c
> +++ b/ui/ui-hmp-cmds.c
> @@ -270,6 +270,28 @@ out:
> hmp_handle_error(mon, err);
> }
>
> +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(mon, "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, &err);
> + }
> + } else {
> + monitor_printf(mon, "Expected 'password' after 'vnc'\n");
> + }
> +}
> +
This chunk ought to be in the next patch IIUC
> void hmp_sendkey(Monitor *mon, const QDict *qdict)
> {
> const char *keys = qdict_get_str(qdict, "keys");
> --
> 2.37.3
>
>
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] 31+ messages in thread
* Re: [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c
2022-12-01 6:13 ` [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
@ 2022-12-01 8:57 ` Daniel P. Berrangé
0 siblings, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:10AM +0100, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> include/monitor/hmp.h | 5 +++++
> monitor/hmp-cmds.c | 28 +---------------------------
> ui/ui-hmp-cmds.c | 19 +++++++++++++++----
> 3 files changed, 21 insertions(+), 31 deletions(-)
If you pull in the hmp_change_vnc impl from the previous patch
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] 31+ messages in thread
* Re: [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly
2022-12-01 6:13 ` [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
2022-12-01 7:19 ` Philippe Mathieu-Daudé
@ 2022-12-01 8:58 ` Daniel P. Berrangé
1 sibling, 0 replies; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 8:58 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 07:13:11AM +0100, Markus Armbruster wrote:
> Transform
>
> if (good) {
> do stuff
> } else {
> handle error
> }
>
> to
>
> if (!good) {
> handle error
> return;
> }
> do stuff
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> ui/ui-hmp-cmds.c | 17 +++++++----------
> 1 file changed, 7 insertions(+), 10 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] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 8:49 ` Daniel P. Berrangé
@ 2022-12-01 10:25 ` Markus Armbruster
2022-12-01 12:39 ` Markus Armbruster
1 sibling, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 10:25 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, kraxel, dgilbert
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
>> HMP "info spice" has a bit of code to show channel type
>> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
>> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
>> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
>
> Last version bump was 4 years ago
>
> commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
> Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> Date: Wed Nov 28 19:59:32 2018 +0400
>
> configure: bump spice-server required version to 0.12.5
>
> ...snip....
>
> According to repology, all the distros that are build target platforms
> for QEMU include it:
>
> RHEL-7: 0.14.0
> Debian (Stretch): 0.12.8
> Debian (Jessie): 0.12.5
> FreeBSD (ports): 0.14.0
> OpenSUSE Leap 15: 0.14.0
> Ubuntu (Xenial): 0.12.6
>
> We moved on from Debian and RHEL since then
>
> 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
>
> IOW, we can bump to 0.14.0, and then revert the
> webdav conditional commit.
Will do in v2. Thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c
2022-12-01 7:14 ` Philippe Mathieu-Daudé
@ 2022-12-01 10:26 ` Markus Armbruster
0 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 10:26 UTC (permalink / raw)
To: Philippe Mathieu-Daudé; +Cc: qemu-devel, kraxel, dgilbert
Philippe Mathieu-Daudé <philmd@linaro.org> writes:
> On 1/12/22 07:13, Markus Armbruster wrote:
>> This moves these commands from MAINTAINERS section "Human
>> Monitor (HMP)" to "Graphics".
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> monitor/hmp-cmds.c | 342 ------------------------------------------
>> ui/ui-hmp-cmds.c | 360 +++++++++++++++++++++++++++++++++++++++++++++
>> ui/meson.build | 1 +
>> 3 files changed, 361 insertions(+), 342 deletions(-)
>> create mode 100644 ui/ui-hmp-cmds.c
>
>
>> +#ifdef CONFIG_SPICE
>> +void hmp_info_spice(Monitor *mon, const QDict *qdict)
>> +{
>> + SpiceChannelList *chan;
>> + SpiceInfo *info;
>> + const char *channel_name;
>> + const char * const channel_names[] = {
>
> Can be static (pre-existing).
Separate patch, to keep the code motion pure.
>> + [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",
>> + };
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 7/9] ui: Improve "change vnc" error reporting
2022-12-01 8:56 ` Daniel P. Berrangé
@ 2022-12-01 10:27 ` Markus Armbruster
0 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 10:27 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, kraxel, dgilbert
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Dec 01, 2022 at 07:13:09AM +0100, Markus Armbruster wrote:
>> Switch from monitor_printf() to error_setg() and hmp_handle_error().
>> This makes "this is an error" more obvious both in the source and in
>> the monitor, where hmp_handle_error() prefixes the message with
>> "Error: ".
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> monitor/hmp-cmds.c | 8 ++++----
>> ui/ui-hmp-cmds.c | 22 ++++++++++++++++++++++
>> 2 files changed, 26 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
>
> Upto this point
>
> Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
>
>
>> diff --git a/ui/ui-hmp-cmds.c b/ui/ui-hmp-cmds.c
>> index af290da9e1..90a4f86f25 100644
>> --- a/ui/ui-hmp-cmds.c
>> +++ b/ui/ui-hmp-cmds.c
>> @@ -270,6 +270,28 @@ out:
>> hmp_handle_error(mon, err);
>> }
>>
>> +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(mon, "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, &err);
>> + }
>> + } else {
>> + monitor_printf(mon, "Expected 'password' after 'vnc'\n");
>> + }
>> +}
>> +
>
> This chunk ought to be in the next patch IIUC
Oops! Thank you!
>> void hmp_sendkey(Monitor *mon, const QDict *qdict)
>> {
>> const char *keys = qdict_get_str(qdict, "keys");
>> --
>> 2.37.3
>>
>>
>
> With regards,
> Daniel
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 8:49 ` Daniel P. Berrangé
2022-12-01 10:25 ` Markus Armbruster
@ 2022-12-01 12:39 ` Markus Armbruster
2022-12-01 14:34 ` Daniel P. Berrangé
1 sibling, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 12:39 UTC (permalink / raw)
To: Daniel P. Berrangé; +Cc: qemu-devel, kraxel, dgilbert
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
>> HMP "info spice" has a bit of code to show channel type
>> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
>> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
>> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
>
> Last version bump was 4 years ago
>
> commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
> Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> Date: Wed Nov 28 19:59:32 2018 +0400
>
> configure: bump spice-server required version to 0.12.5
>
> ...snip....
>
> According to repology, all the distros that are build target platforms
> for QEMU include it:
>
> RHEL-7: 0.14.0
> Debian (Stretch): 0.12.8
> Debian (Jessie): 0.12.5
> FreeBSD (ports): 0.14.0
> OpenSUSE Leap 15: 0.14.0
> Ubuntu (Xenial): 0.12.6
>
> We moved on from Debian and RHEL since then
>
> 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
>
> IOW, we can bump to 0.14.0, and then revert the
> webdav conditional commit.
We need to bump spice-protocol, actually.
Would you like me to bump spice-server as well? To which version?
[...]
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 12:39 ` Markus Armbruster
@ 2022-12-01 14:34 ` Daniel P. Berrangé
2022-12-01 15:49 ` Markus Armbruster
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 14:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert
On Thu, Dec 01, 2022 at 01:39:13PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
> >> HMP "info spice" has a bit of code to show channel type
> >> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
> >> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
> >> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
> >
> > Last version bump was 4 years ago
> >
> > commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
> > Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Date: Wed Nov 28 19:59:32 2018 +0400
> >
> > configure: bump spice-server required version to 0.12.5
> >
> > ...snip....
> >
> > According to repology, all the distros that are build target platforms
> > for QEMU include it:
> >
> > RHEL-7: 0.14.0
> > Debian (Stretch): 0.12.8
> > Debian (Jessie): 0.12.5
> > FreeBSD (ports): 0.14.0
> > OpenSUSE Leap 15: 0.14.0
> > Ubuntu (Xenial): 0.12.6
> >
> > We moved on from Debian and RHEL since then
> >
> > 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
> >
> > IOW, we can bump to 0.14.0, and then revert the
> > webdav conditional commit.
>
> We need to bump spice-protocol, actually.
Opps, I'm getting mixed up. The commit I mentioned was spice-server,
but the new versions I've listed just were indeed spice-protocol
> Would you like me to bump spice-server as well? To which version?
Yes, might as well, the spice-server versions are slightly different:
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
I think we might as well pick 0.14.0 for both protocol and server.
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] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 14:34 ` Daniel P. Berrangé
@ 2022-12-01 15:49 ` Markus Armbruster
2022-12-01 18:34 ` Daniel P. Berrangé
0 siblings, 1 reply; 31+ messages in thread
From: Markus Armbruster @ 2022-12-01 15:49 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, qemu-devel, kraxel, dgilbert, Marc-André Lureau
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Dec 01, 2022 at 01:39:13PM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
>> >> HMP "info spice" has a bit of code to show channel type
>> >> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
>> >> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
>> >> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
>> >
>> > Last version bump was 4 years ago
>> >
>> > commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
>> > Author: Marc-André Lureau <marcandre.lureau@redhat.com>
>> > Date: Wed Nov 28 19:59:32 2018 +0400
>> >
>> > configure: bump spice-server required version to 0.12.5
>> >
>> > ...snip....
>> >
>> > According to repology, all the distros that are build target platforms
>> > for QEMU include it:
>> >
>> > RHEL-7: 0.14.0
>> > Debian (Stretch): 0.12.8
>> > Debian (Jessie): 0.12.5
>> > FreeBSD (ports): 0.14.0
>> > OpenSUSE Leap 15: 0.14.0
>> > Ubuntu (Xenial): 0.12.6
>> >
>> > We moved on from Debian and RHEL since then
>> >
>> > 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
>> >
>> > IOW, we can bump to 0.14.0, and then revert the
>> > webdav conditional commit.
>>
>> We need to bump spice-protocol, actually.
>
> Opps, I'm getting mixed up. The commit I mentioned was spice-server,
> but the new versions I've listed just were indeed spice-protocol
>
>> Would you like me to bump spice-server as well? To which version?
>
> Yes, might as well, the spice-server versions are slightly different:
>
> 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
>
> I think we might as well pick 0.14.0 for both protocol and server.
Makes sense, but it leads to another question.
I found obvious version checks for spice-protocol, and dropped the
outmoded ones, namely
#if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 0)
For spice-server, I see a bunch of SPICE_INTERFACE_FOO_{MAJOR,MINOR} we
check, and which ones become outmoded is not obvious to me. Help?
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 15:49 ` Markus Armbruster
@ 2022-12-01 18:34 ` Daniel P. Berrangé
2022-12-02 6:52 ` Markus Armbruster
0 siblings, 1 reply; 31+ messages in thread
From: Daniel P. Berrangé @ 2022-12-01 18:34 UTC (permalink / raw)
To: Markus Armbruster; +Cc: qemu-devel, kraxel, dgilbert, Marc-André Lureau
On Thu, Dec 01, 2022 at 04:49:25PM +0100, Markus Armbruster wrote:
> Daniel P. Berrangé <berrange@redhat.com> writes:
>
> > On Thu, Dec 01, 2022 at 01:39:13PM +0100, Markus Armbruster wrote:
> >> Daniel P. Berrangé <berrange@redhat.com> writes:
> >>
> >> > On Thu, Dec 01, 2022 at 07:13:05AM +0100, Markus Armbruster wrote:
> >> >> HMP "info spice" has a bit of code to show channel type
> >> >> SPICE_CHANNEL_WEBDAV as "webdav", disabled since commit 7c6044a94e
> >> >> "hmp: info spice: take out webdav" (v2.3.0), because it compiles only
> >> >> with Spice versions 0.12.7 and later. Our minimum version is 0.12.5.
> >> >
> >> > Last version bump was 4 years ago
> >> >
> >> > commit 1b63665c2c0e0d52735e0dd5217f109fe0dd2322
> >> > Author: Marc-André Lureau <marcandre.lureau@redhat.com>
> >> > Date: Wed Nov 28 19:59:32 2018 +0400
> >> >
> >> > configure: bump spice-server required version to 0.12.5
> >> >
> >> > ...snip....
> >> >
> >> > According to repology, all the distros that are build target platforms
> >> > for QEMU include it:
> >> >
> >> > RHEL-7: 0.14.0
> >> > Debian (Stretch): 0.12.8
> >> > Debian (Jessie): 0.12.5
> >> > FreeBSD (ports): 0.14.0
> >> > OpenSUSE Leap 15: 0.14.0
> >> > Ubuntu (Xenial): 0.12.6
> >> >
> >> > We moved on from Debian and RHEL since then
> >> >
> >> > 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
> >> >
> >> > IOW, we can bump to 0.14.0, and then revert the
> >> > webdav conditional commit.
> >>
> >> We need to bump spice-protocol, actually.
> >
> > Opps, I'm getting mixed up. The commit I mentioned was spice-server,
> > but the new versions I've listed just were indeed spice-protocol
> >
> >> Would you like me to bump spice-server as well? To which version?
> >
> > Yes, might as well, the spice-server versions are slightly different:
> >
> > 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
> >
> > I think we might as well pick 0.14.0 for both protocol and server.
>
> Makes sense, but it leads to another question.
>
> I found obvious version checks for spice-protocol, and dropped the
> outmoded ones, namely
>
> #if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 0)
>
> For spice-server, I see a bunch of SPICE_INTERFACE_FOO_{MAJOR,MINOR} we
> check, and which ones become outmoded is not obvious to me. Help?
Ignore all the interface ones. For the server, the check to look
for is against SPICE_SERVER_VERSION
chardev/spice.c:#if SPICE_SERVER_VERSION >= 0x000c06
chardev/spice.c:#if SPICE_SERVER_VERSION < 0x000e02
hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
hw/display/qxl.h:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
include/ui/qemu-spice.h:#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
include/ui/qemu-spice.h:#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
include/ui/spice-display.h:# if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */
ui/spice-display.c:#if SPICE_SERVER_VERSION >= 0x000e04 /* release 0.14.4 */
ui/spice-display.c:#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
A fair few of those will be obsolete
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] 31+ messages in thread
* Re: [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV
2022-12-01 18:34 ` Daniel P. Berrangé
@ 2022-12-02 6:52 ` Markus Armbruster
0 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-02 6:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Markus Armbruster, qemu-devel, kraxel, dgilbert, Marc-André Lureau
Daniel P. Berrangé <berrange@redhat.com> writes:
> On Thu, Dec 01, 2022 at 04:49:25PM +0100, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>>
>> > On Thu, Dec 01, 2022 at 01:39:13PM +0100, Markus Armbruster wrote:
[...]
>> >> Would you like me to bump spice-server as well? To which version?
>> >
>> > Yes, might as well, the spice-server versions are slightly different:
>> >
>> > 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
>> >
>> > I think we might as well pick 0.14.0 for both protocol and server.
>>
>> Makes sense, but it leads to another question.
>>
>> I found obvious version checks for spice-protocol, and dropped the
>> outmoded ones, namely
>>
>> #if CHECK_SPICE_PROTOCOL_VERSION(0, 14, 0)
>>
>> For spice-server, I see a bunch of SPICE_INTERFACE_FOO_{MAJOR,MINOR} we
>> check, and which ones become outmoded is not obvious to me. Help?
>
> Ignore all the interface ones. For the server, the check to look
> for is against SPICE_SERVER_VERSION
>
> chardev/spice.c:#if SPICE_SERVER_VERSION >= 0x000c06
> chardev/spice.c:#if SPICE_SERVER_VERSION < 0x000e02
> hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 && SPICE_SERVER_VERSION < 0x000e02
> hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
> hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
> hw/display/qxl.c:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
> hw/display/qxl.h:#if SPICE_SERVER_VERSION >= 0x000c06 /* release 0.12.6 */
> include/ui/qemu-spice.h:#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
> include/ui/qemu-spice.h:#if defined(SPICE_SERVER_VERSION) && (SPICE_SERVER_VERSION >= 0x000f00)
> include/ui/spice-display.h:# if SPICE_SERVER_VERSION >= 0x000d01 /* release 0.13.1 */
> ui/spice-display.c:#if SPICE_SERVER_VERSION >= 0x000e04 /* release 0.14.4 */
> ui/spice-display.c:#if SPICE_SERVER_VERSION >= 0x000e02 /* release 0.14.2 */
>
> A fair few of those will be obsolete
Got it, thanks!
^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c
2022-12-01 6:38 ` Markus Armbruster
@ 2022-12-11 16:48 ` Markus Armbruster
0 siblings, 0 replies; 31+ messages in thread
From: Markus Armbruster @ 2022-12-11 16:48 UTC (permalink / raw)
To: qemu-devel; +Cc: kraxel, dgilbert
Markus Armbruster <armbru@redhat.com> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> This moves these commands from MAINTAINERS section "Human
>> Monitor (HMP)" to "Graphics".
Make that 'section "QMP"', of course.
>> 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.
>
> Last sentence no verb. Trying again:
>
> Aside: @protocol is either a display type or else a character device
> ID. Bad design.
>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2022-12-11 16:49 UTC | newest]
Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 6:13 [PATCH 0/9] ui: Move and clean up monitor command code Markus Armbruster
2022-12-01 6:13 ` [PATCH 1/9] ui: Check numeric part of expire_password argument @time properly Markus Armbruster
2022-12-01 8:13 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 2/9] ui: Fix silent truncation of numeric keys in HMP sendkey Markus Armbruster
2022-12-01 8:14 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 3/9] ui: Drop disabled code for SPICE_CHANNEL_WEBDAV Markus Armbruster
2022-12-01 8:49 ` Daniel P. Berrangé
2022-12-01 10:25 ` Markus Armbruster
2022-12-01 12:39 ` Markus Armbruster
2022-12-01 14:34 ` Daniel P. Berrangé
2022-12-01 15:49 ` Markus Armbruster
2022-12-01 18:34 ` Daniel P. Berrangé
2022-12-02 6:52 ` Markus Armbruster
2022-12-01 6:13 ` [PATCH 4/9] ui: Clean up a few things checkpatch.pl would flag later on Markus Armbruster
2022-12-01 8:50 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 5/9] ui: Move QMP commands from monitor to new ui/ui-qmp-cmds.c Markus Armbruster
2022-12-01 6:38 ` Markus Armbruster
2022-12-11 16:48 ` Markus Armbruster
2022-12-01 8:51 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 6/9] ui: Move HMP commands from monitor to new ui/ui-hmp-cmds.c Markus Armbruster
2022-12-01 7:14 ` Philippe Mathieu-Daudé
2022-12-01 10:26 ` Markus Armbruster
2022-12-01 8:53 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 7/9] ui: Improve "change vnc" error reporting Markus Armbruster
2022-12-01 8:56 ` Daniel P. Berrangé
2022-12-01 10:27 ` Markus Armbruster
2022-12-01 6:13 ` [PATCH 8/9] ui: Factor out hmp_change_vnc(), and move to ui/ui-hmp-cmds.c Markus Armbruster
2022-12-01 8:57 ` Daniel P. Berrangé
2022-12-01 6:13 ` [PATCH 9/9] ui: Reduce nesting in hmp_change_vnc() slightly Markus Armbruster
2022-12-01 7:19 ` Philippe Mathieu-Daudé
2022-12-01 8:58 ` Daniel P. Berrangé
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.