* [PULL 01/18] monitor: remove 'info ioapic' HMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 02/18] monitor: make hmp_handle_error return a boolean Daniel P. Berrangé
` (17 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster, Philippe Mathieu-Daudé
This command was turned into a no-op four years ago in
commit 0c8465440d50c18a7bb13d0a866748f0593e193a
Author: Peter Xu <peterx@redhat.com>
Date: Fri Dec 29 15:31:04 2017 +0800
hmp: obsolete "info ioapic"
Reviewed-by: Peter Xu <peterx@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 15 ---------------
include/monitor/hmp-target.h | 1 -
target/i386/monitor.c | 6 ------
3 files changed, 22 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 4c966e8a6b..24c478aead 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -127,21 +127,6 @@ SRST
Show local APIC state
ERST
-#if defined(TARGET_I386)
- {
- .name = "ioapic",
- .args_type = "",
- .params = "",
- .help = "show io apic state",
- .cmd = hmp_info_io_apic,
- },
-#endif
-
-SRST
- ``info ioapic``
- Show io APIC state
-ERST
-
{
.name = "cpus",
.args_type = "",
diff --git a/include/monitor/hmp-target.h b/include/monitor/hmp-target.h
index 96956d0fc4..ffdc15a34b 100644
--- a/include/monitor/hmp-target.h
+++ b/include/monitor/hmp-target.h
@@ -48,7 +48,6 @@ void hmp_info_mem(Monitor *mon, const QDict *qdict);
void hmp_info_tlb(Monitor *mon, const QDict *qdict);
void hmp_mce(Monitor *mon, const QDict *qdict);
void hmp_info_local_apic(Monitor *mon, const QDict *qdict);
-void hmp_info_io_apic(Monitor *mon, const QDict *qdict);
void hmp_info_sev(Monitor *mon, const QDict *qdict);
void hmp_info_sgx(Monitor *mon, const QDict *qdict);
diff --git a/target/i386/monitor.c b/target/i386/monitor.c
index 8166e17693..8e4b4d600c 100644
--- a/target/i386/monitor.c
+++ b/target/i386/monitor.c
@@ -667,9 +667,3 @@ void hmp_info_local_apic(Monitor *mon, const QDict *qdict)
}
x86_cpu_dump_local_apic_state(cs, CPU_DUMP_FPU);
}
-
-void hmp_info_io_apic(Monitor *mon, const QDict *qdict)
-{
- monitor_printf(mon, "This command is obsolete and will be "
- "removed soon. Please use 'info pic' instead.\n");
-}
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 02/18] monitor: make hmp_handle_error return a boolean
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 01/18] monitor: remove 'info ioapic' HMP command Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 03/18] docs/devel: rename file for writing monitor commands Daniel P. Berrangé
` (16 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This turns the pattern
if (err) {
hmp_handle_error(mon, err);
return;
}
into
if (hmp_handle_error(mon, err)) {
return;
}
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hw/core/machine-hmp-cmds.c | 3 +--
include/monitor/hmp.h | 2 +-
monitor/hmp-cmds.c | 28 +++++++++++-----------------
3 files changed, 13 insertions(+), 20 deletions(-)
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index 76b22b00d6..c356783ab9 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -53,8 +53,7 @@ void hmp_hotpluggable_cpus(Monitor *mon, const QDict *qdict)
HotpluggableCPUList *saved = l;
CpuInstanceProperties *c;
- if (err != NULL) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 6bc27639e0..a2cb002a3a 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -16,7 +16,7 @@
#include "qemu/readline.h"
-void hmp_handle_error(Monitor *mon, Error *err);
+bool hmp_handle_error(Monitor *mon, Error *err);
void hmp_info_name(Monitor *mon, const QDict *qdict);
void hmp_info_version(Monitor *mon, const QDict *qdict);
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index bcaa41350e..9031cea881 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -62,11 +62,13 @@
#include <spice/enums.h>
#endif
-void hmp_handle_error(Monitor *mon, Error *err)
+bool hmp_handle_error(Monitor *mon, Error *err)
{
if (err) {
error_reportf_err(err, "Error: ");
+ return true;
}
+ return false;
}
/*
@@ -577,8 +579,7 @@ void hmp_info_vnc(Monitor *mon, const QDict *qdict)
info2l = qmp_query_vnc_servers(&err);
info2l_head = info2l;
- if (err) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
if (!info2l) {
@@ -693,8 +694,7 @@ void hmp_info_balloon(Monitor *mon, const QDict *qdict)
Error *err = NULL;
info = qmp_query_balloon(&err);
- if (err) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -1065,8 +1065,7 @@ void hmp_ringbuf_read(Monitor *mon, const QDict *qdict)
int i;
data = qmp_ringbuf_read(chardev, size, false, 0, &err);
- if (err) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -1582,8 +1581,7 @@ void hmp_migrate(Monitor *mon, const QDict *qdict)
qmp_migrate(uri, !!blk, blk, !!inc, inc,
false, false, true, resume, &err);
- if (err) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -1917,8 +1915,7 @@ void hmp_rocker(Monitor *mon, const QDict *qdict)
Error *err = NULL;
rocker = qmp_query_rocker(name, &err);
- if (err != NULL) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -1936,8 +1933,7 @@ void hmp_rocker_ports(Monitor *mon, const QDict *qdict)
Error *err = NULL;
list = qmp_query_rocker_ports(name, &err);
- if (err != NULL) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -1965,8 +1961,7 @@ void hmp_rocker_of_dpa_flows(Monitor *mon, const QDict *qdict)
Error *err = NULL;
list = qmp_query_rocker_of_dpa_flows(name, tbl_id != -1, tbl_id, &err);
- if (err != NULL) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -2115,8 +2110,7 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
Error *err = NULL;
list = qmp_query_rocker_of_dpa_groups(name, type != 9, type, &err);
- if (err != NULL) {
- hmp_handle_error(mon, err);
+ if (hmp_handle_error(mon, err)) {
return;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 03/18] docs/devel: rename file for writing monitor commands
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 01/18] monitor: remove 'info ioapic' HMP command Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 02/18] monitor: make hmp_handle_error return a boolean Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 04/18] docs/devel: tweak headings in monitor command docs Daniel P. Berrangé
` (15 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
The file already covers writing HMP commands, in addition to
the QMP commands, so it deserves a more general name.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/index.rst | 2 +-
...riting-qmp-commands.rst => writing-monitor-commands.rst} | 6 +++---
2 files changed, 4 insertions(+), 4 deletions(-)
rename docs/devel/{writing-qmp-commands.rst => writing-monitor-commands.rst} (99%)
diff --git a/docs/devel/index.rst b/docs/devel/index.rst
index f95df10b3e..7c25177c5d 100644
--- a/docs/devel/index.rst
+++ b/docs/devel/index.rst
@@ -44,4 +44,4 @@ modifying QEMU's source code.
ebpf_rss
vfio-migration
qapi-code-gen
- writing-qmp-commands
+ writing-monitor-commands
diff --git a/docs/devel/writing-qmp-commands.rst b/docs/devel/writing-monitor-commands.rst
similarity index 99%
rename from docs/devel/writing-qmp-commands.rst
rename to docs/devel/writing-monitor-commands.rst
index 6a10a06c48..4a4c051624 100644
--- a/docs/devel/writing-qmp-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -1,8 +1,8 @@
-How to write QMP commands using the QAPI framework
-==================================================
+How to write monitor commands
+=============================
This document is a step-by-step guide on how to write new QMP commands using
-the QAPI framework. It also shows how to implement new style HMP commands.
+the QAPI framework and HMP commands.
This document doesn't discuss QMP protocol level details, nor does it dive
into the QAPI framework implementation.
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 04/18] docs/devel: tweak headings in monitor command docs
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (2 preceding siblings ...)
2021-11-02 17:56 ` [PULL 03/18] docs/devel: rename file for writing monitor commands Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 05/18] docs/devel: update error handling guidance for HMP commands Daniel P. Berrangé
` (14 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
The new headings reflect the intended structure of the document and will
better suit additions that follow.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/writing-monitor-commands.rst | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst
index 4a4c051624..a973c48f66 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -85,8 +85,8 @@ any data". Now you're ready to enter the QMP example commands as explained in
the following sections.
-Writing a command that doesn't return data
-------------------------------------------
+Writing a simple command: hello-world
+-------------------------------------
That's the most simple QMP command that can be written. Usually, this kind of
command carries some meaningful action in QEMU but here it will just print
@@ -340,8 +340,8 @@ Please, check the "-monitor" command-line option to know how to open a user
monitor.
-Writing a command that returns data
------------------------------------
+Writing more complex commands
+-----------------------------
A QMP command is capable of returning any data the QAPI supports like integers,
strings, booleans, enumerations and user defined types.
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 05/18] docs/devel: update error handling guidance for HMP commands
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (3 preceding siblings ...)
2021-11-02 17:56 ` [PULL 04/18] docs/devel: tweak headings in monitor command docs Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 06/18] monitor: introduce HumanReadableText and HMP support Daniel P. Berrangé
` (13 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
Best practice is to use the 'hmp_handle_error' function, not
'monitor_printf' or 'error_report_err'. This ensures that the
message always gets an 'Error: ' prefix, distinguishing it
from normal command output.
Reviewed-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/writing-monitor-commands.rst | 19 +++++++------------
1 file changed, 7 insertions(+), 12 deletions(-)
diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst
index a973c48f66..a381b52024 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -293,9 +293,7 @@ Here's the implementation of the "hello-world" HMP command::
Error *err = NULL;
qmp_hello_world(!!message, message, &err);
- if (err) {
- monitor_printf(mon, "%s\n", error_get_pretty(err));
- error_free(err);
+ if (hmp_handle_error(mon, err)) {
return;
}
}
@@ -307,9 +305,10 @@ There are three important points to be noticed:
1. The "mon" and "qdict" arguments are mandatory for all HMP functions. The
former is the monitor object. The latter is how the monitor passes
arguments entered by the user to the command implementation
-2. hmp_hello_world() performs error checking. In this example we just print
- the error description to the user, but we could do more, like taking
- different actions depending on the error qmp_hello_world() returns
+2. hmp_hello_world() performs error checking. In this example we just call
+ hmp_handle_error() which prints a message to the user, but we could do
+ more, like taking different actions depending on the error
+ qmp_hello_world() returns
3. The "err" variable must be initialized to NULL before performing the
QMP call
@@ -466,9 +465,7 @@ Here's the HMP counterpart of the query-alarm-clock command::
Error *err = NULL;
clock = qmp_query_alarm_clock(&err);
- if (err) {
- monitor_printf(mon, "Could not query alarm clock information\n");
- error_free(err);
+ if (hmp_handle_error(mon, err)) {
return;
}
@@ -607,9 +604,7 @@ has to traverse the list, it's shown below for reference::
Error *err = NULL;
method_list = qmp_query_alarm_methods(&err);
- if (err) {
- monitor_printf(mon, "Could not query alarm methods\n");
- error_free(err);
+ if (hmp_handle_error(mon, err)) {
return;
}
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 06/18] monitor: introduce HumanReadableText and HMP support
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (4 preceding siblings ...)
2021-11-02 17:56 ` [PULL 05/18] docs/devel: update error handling guidance for HMP commands Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 07/18] docs/devel: document expectations for QAPI data modelling for QMP Daniel P. Berrangé
` (12 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This provides a foundation on which to convert simple HMP commands to
use QMP. The QMP implementation will generate formatted text targeted
for human consumption, returning it in the HumanReadableText data type.
The HMP command handler will simply print out the formatted string
within the HumanReadableText data type. Since this will be an entirely
formulaic action in the case of HMP commands taking no arguments, a
custom command handler is provided.
Thus instead of registering a 'cmd' callback for the HMP command, a
'cmd_info_hrt' callback is provided, which will simply be a pointer
to the QMP implementation.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/monitor/hmp.h | 3 +++
include/monitor/monitor.h | 2 ++
include/qapi/type-helpers.h | 14 ++++++++++++++
monitor/hmp.c | 32 +++++++++++++++++++++++++++++---
monitor/misc.c | 18 +++++++++++++++++-
monitor/monitor-internal.h | 7 +++++++
qapi/common.json | 11 +++++++++++
qapi/meson.build | 3 +++
qapi/qapi-type-helpers.c | 23 +++++++++++++++++++++++
9 files changed, 109 insertions(+), 4 deletions(-)
create mode 100644 include/qapi/type-helpers.h
create mode 100644 qapi/qapi-type-helpers.c
diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index a2cb002a3a..96d014826a 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -15,6 +15,7 @@
#define HMP_H
#include "qemu/readline.h"
+#include "qapi/qapi-types-common.h"
bool hmp_handle_error(Monitor *mon, Error *err);
@@ -130,5 +131,7 @@ void hmp_replay_delete_break(Monitor *mon, const QDict *qdict);
void hmp_replay_seek(Monitor *mon, const QDict *qdict);
void hmp_info_dirty_rate(Monitor *mon, const QDict *qdict);
void hmp_calc_dirty_rate(Monitor *mon, const QDict *qdict);
+void hmp_human_readable_text_helper(Monitor *mon,
+ HumanReadableText *(*qmp_handler)(Error **));
#endif
diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h
index 1a8a369b50..12d395d62d 100644
--- a/include/monitor/monitor.h
+++ b/include/monitor/monitor.h
@@ -53,5 +53,7 @@ int64_t monitor_fdset_dup_fd_find(int dup_fd);
void monitor_register_hmp(const char *name, bool info,
void (*cmd)(Monitor *mon, const QDict *qdict));
+void monitor_register_hmp_info_hrt(const char *name,
+ HumanReadableText *(*handler)(Error **errp));
#endif /* MONITOR_H */
diff --git a/include/qapi/type-helpers.h b/include/qapi/type-helpers.h
new file mode 100644
index 0000000000..be1f181526
--- /dev/null
+++ b/include/qapi/type-helpers.h
@@ -0,0 +1,14 @@
+/*
+ * QAPI common helper functions
+ *
+ * This file provides helper functions related to types defined
+ * in the QAPI schema.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qapi/qapi-types-common.h"
+
+HumanReadableText *human_readable_text_from_str(GString *str);
diff --git a/monitor/hmp.c b/monitor/hmp.c
index d50c3124e1..b20737e63c 100644
--- a/monitor/hmp.c
+++ b/monitor/hmp.c
@@ -26,6 +26,7 @@
#include <dirent.h>
#include "hw/qdev-core.h"
#include "monitor-internal.h"
+#include "monitor/hmp.h"
#include "qapi/error.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qnum.h"
@@ -1061,6 +1062,31 @@ fail:
return NULL;
}
+static void hmp_info_human_readable_text(Monitor *mon,
+ HumanReadableText *(*handler)(Error **))
+{
+ Error *err = NULL;
+ g_autoptr(HumanReadableText) info = handler(&err);
+
+ if (hmp_handle_error(mon, err)) {
+ return;
+ }
+
+ monitor_printf(mon, "%s", info->human_readable_text);
+}
+
+static void handle_hmp_command_exec(Monitor *mon,
+ const HMPCommand *cmd,
+ QDict *qdict)
+{
+ if (cmd->cmd_info_hrt) {
+ hmp_info_human_readable_text(mon,
+ cmd->cmd_info_hrt);
+ } else {
+ cmd->cmd(mon, qdict);
+ }
+}
+
typedef struct HandleHmpCommandCo {
Monitor *mon;
const HMPCommand *cmd;
@@ -1071,7 +1097,7 @@ typedef struct HandleHmpCommandCo {
static void handle_hmp_command_co(void *opaque)
{
HandleHmpCommandCo *data = opaque;
- data->cmd->cmd(data->mon, data->qdict);
+ handle_hmp_command_exec(data->mon, data->cmd, data->qdict);
monitor_set_cur(qemu_coroutine_self(), NULL);
data->done = true;
}
@@ -1089,7 +1115,7 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
return;
}
- if (!cmd->cmd) {
+ if (!cmd->cmd && !cmd->cmd_info_hrt) {
/* FIXME: is it useful to try autoload modules here ??? */
monitor_printf(&mon->common, "Command \"%.*s\" is not available.\n",
(int)(cmdline - cmd_start), cmd_start);
@@ -1109,7 +1135,7 @@ void handle_hmp_command(MonitorHMP *mon, const char *cmdline)
if (!cmd->coroutine) {
/* old_mon is non-NULL when called from qmp_human_monitor_command() */
Monitor *old_mon = monitor_set_cur(qemu_coroutine_self(), &mon->common);
- cmd->cmd(&mon->common, qdict);
+ handle_hmp_command_exec(&mon->common, cmd, qdict);
monitor_set_cur(qemu_coroutine_self(), old_mon);
} else {
HandleHmpCommandCo data = {
diff --git a/monitor/misc.c b/monitor/misc.c
index c2d227a07c..0e124044d0 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -1964,7 +1964,7 @@ void monitor_register_hmp(const char *name, bool info,
while (table->name != NULL) {
if (strcmp(table->name, name) == 0) {
- g_assert(table->cmd == NULL);
+ g_assert(table->cmd == NULL && table->cmd_info_hrt == NULL);
table->cmd = cmd;
return;
}
@@ -1973,6 +1973,22 @@ void monitor_register_hmp(const char *name, bool info,
g_assert_not_reached();
}
+void monitor_register_hmp_info_hrt(const char *name,
+ HumanReadableText *(*handler)(Error **errp))
+{
+ HMPCommand *table = hmp_info_cmds;
+
+ while (table->name != NULL) {
+ if (strcmp(table->name, name) == 0) {
+ g_assert(table->cmd == NULL && table->cmd_info_hrt == NULL);
+ table->cmd_info_hrt = handler;
+ return;
+ }
+ table++;
+ }
+ g_assert_not_reached();
+}
+
void monitor_init_globals(void)
{
monitor_init_globals_core();
diff --git a/monitor/monitor-internal.h b/monitor/monitor-internal.h
index 9c3a09cb01..3da3f86c6a 100644
--- a/monitor/monitor-internal.h
+++ b/monitor/monitor-internal.h
@@ -74,6 +74,13 @@ typedef struct HMPCommand {
const char *help;
const char *flags; /* p=preconfig */
void (*cmd)(Monitor *mon, const QDict *qdict);
+ /*
+ * If implementing a command that takes no arguments and simply
+ * prints formatted data, then leave @cmd NULL, and then set
+ * @cmd_info_hrt to the corresponding QMP handler that returns
+ * the formatted text.
+ */
+ HumanReadableText *(*cmd_info_hrt)(Error **errp);
bool coroutine;
/*
* @sub_table is a list of 2nd level of commands. If it does not exist,
diff --git a/qapi/common.json b/qapi/common.json
index 7c976296f0..412cc4f5ae 100644
--- a/qapi/common.json
+++ b/qapi/common.json
@@ -197,3 +197,14 @@
{ 'enum': 'GrabToggleKeys',
'data': [ 'ctrl-ctrl', 'alt-alt', 'shift-shift','meta-meta', 'scrolllock',
'ctrl-scrolllock' ] }
+
+##
+# @HumanReadableText:
+#
+# @human-readable-text: Formatted output intended for humans.
+#
+# Since: 6.2
+#
+##
+{ 'struct': 'HumanReadableText',
+ 'data': { 'human-readable-text': 'str' } }
diff --git a/qapi/meson.build b/qapi/meson.build
index c356a385e3..c0c49c15e4 100644
--- a/qapi/meson.build
+++ b/qapi/meson.build
@@ -10,6 +10,9 @@ util_ss.add(files(
'string-input-visitor.c',
'string-output-visitor.c',
))
+if have_system
+ util_ss.add(files('qapi-type-helpers.c'))
+endif
if have_system or have_tools
util_ss.add(files(
'qmp-dispatch.c',
diff --git a/qapi/qapi-type-helpers.c b/qapi/qapi-type-helpers.c
new file mode 100644
index 0000000000..f76b34f647
--- /dev/null
+++ b/qapi/qapi-type-helpers.c
@@ -0,0 +1,23 @@
+/*
+ * QAPI common helper functions
+ *
+ * This file provides helper functions related to types defined
+ * in the QAPI schema.
+ *
+ * This work is licensed under the terms of the GNU LGPL, version 2.1 or later.
+ * See the COPYING.LIB file in the top-level directory.
+ *
+ */
+
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qapi/type-helpers.h"
+
+HumanReadableText *human_readable_text_from_str(GString *str)
+{
+ HumanReadableText *ret = g_new0(HumanReadableText, 1);
+
+ ret->human_readable_text = g_steal_pointer(&str->str);
+
+ return ret;
+}
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 07/18] docs/devel: document expectations for QAPI data modelling for QMP
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (5 preceding siblings ...)
2021-11-02 17:56 ` [PULL 06/18] monitor: introduce HumanReadableText and HMP support Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 08/18] docs/devel: add example of command returning unstructured text Daniel P. Berrangé
` (11 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
Traditionally we have required that newly added QMP commands will model
any returned data using fine grained QAPI types. This is good for
commands that are intended to be consumed by machines, where clear data
representation is very important. Commands that don't satisfy this have
generally been added to HMP only.
In effect the decision of whether to add a new command to QMP vs HMP has
been used as a proxy for the decision of whether the cost of designing a
fine grained QAPI type is justified by the potential benefits.
As a result the commands present in QMP and HMP are non-overlapping
sets, although HMP comamnds can be accessed indirectly via the QMP
command 'human-monitor-command'.
One of the downsides of 'human-monitor-command' is that the QEMU monitor
APIs remain tied into various internal parts of the QEMU code. For
example any exclusively HMP command will need to use 'monitor_printf'
to get data out. It would be desirable to be able to fully isolate the
monitor implementation from QEMU internals, however, this is only
possible if all commands are exclusively based on QAPI with direct
QMP exposure.
The way to achieve this desired end goal is to finese the requirements
for QMP command design. For cases where the output of a command is only
intended for human consumption, it is reasonable to want to simplify
the implementation by returning a plain string containing formatted
data instead of designing a fine grained QAPI data type. This can be
permitted if-and-only-if the command is exposed under the 'x-' name
prefix. This indicates that the command data format is liable to
future change and that it is not following QAPI design best practice.
The poster child example for this would be the 'info registers' HMP
command which returns printf formatted data representing CPU state.
This information varies enourmously across target architectures and
changes relatively frequently as new CPU features are implemented.
It is there as debugging data for human operators, and any machine
usage would treat it as an opaque blob. It is thus reasonable to
expose this in QMP as 'x-query-registers' returning a 'str' field.
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/writing-monitor-commands.rst | 27 +++++++++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst
index a381b52024..031e980bf5 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -349,6 +349,33 @@ In this section we will focus on user defined types. Please, check the QAPI
documentation for information about the other types.
+Modelling data in QAPI
+~~~~~~~~~~~~~~~~~~~~~~
+
+For a QMP command that to be considered stable and supported long term,
+there is a requirement returned data should be explicitly modelled
+using fine-grained QAPI types. As a general guide, a caller of the QMP
+command should never need to parse individual returned data fields. If
+a field appears to need parsing, then it should be split into separate
+fields corresponding to each distinct data item. This should be the
+common case for any new QMP command that is intended to be used by
+machines, as opposed to exclusively human operators.
+
+Some QMP commands, however, are only intended as ad hoc debugging aids
+for human operators. While they may return large amounts of formatted
+data, it is not expected that machines will need to parse the result.
+The overhead of defining a fine grained QAPI type for the data may not
+be justified by the potential benefit. In such cases, it is permitted
+to have a command return a simple string that contains formatted data,
+however, it is mandatory for the command to use the 'x-' name prefix.
+This indicates that the command is not guaranteed to be long term
+stable / liable to change in future and is not following QAPI design
+best practices. An example where this approach is taken is the QMP
+command "x-query-registers". This returns a formatted dump of the
+architecture specific CPU state. The way the data is formatted varies
+across QEMU targets, is liable to change over time, and is only
+intended to be consumed as an opaque string by machines.
+
User Defined Types
~~~~~~~~~~~~~~~~~~
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 08/18] docs/devel: add example of command returning unstructured text
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (6 preceding siblings ...)
2021-11-02 17:56 ` [PULL 07/18] docs/devel: document expectations for QAPI data modelling for QMP Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 09/18] docs/devel: document expectations for HMP commands in the future Daniel P. Berrangé
` (10 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This illustrates how to add a QMP command returning unstructured text,
following the guidelines added in the previous patch. The example uses
a simplified version of 'info roms'.
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/writing-monitor-commands.rst | 101 +++++++++++++++++++++++-
1 file changed, 100 insertions(+), 1 deletion(-)
diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst
index 031e980bf5..b87992df91 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -374,7 +374,9 @@ best practices. An example where this approach is taken is the QMP
command "x-query-registers". This returns a formatted dump of the
architecture specific CPU state. The way the data is formatted varies
across QEMU targets, is liable to change over time, and is only
-intended to be consumed as an opaque string by machines.
+intended to be consumed as an opaque string by machines. Refer to the
+`Writing a debugging aid returning unstructured text`_ section for
+an illustration.
User Defined Types
~~~~~~~~~~~~~~~~~~
@@ -642,3 +644,100 @@ has to traverse the list, it's shown below for reference::
qapi_free_TimerAlarmMethodList(method_list);
}
+
+Writing a debugging aid returning unstructured text
+---------------------------------------------------
+
+As discussed in section `Modelling data in QAPI`_, it is required that
+commands expecting machine usage be using fine-grained QAPI data types.
+The exception to this rule applies when the command is solely intended
+as a debugging aid and allows for returning unstructured text. This is
+commonly needed for query commands that report aspects of QEMU's
+internal state that are useful to human operators.
+
+In this example we will consider a simplified variant of the HMP
+command ``info roms``. Following the earlier rules, this command will
+need to live under the ``x-`` name prefix, so its QMP implementation
+will be called ``x-query-roms``. It will have no parameters and will
+return a single text string::
+
+ { 'struct': 'HumanReadableText',
+ 'data': { 'human-readable-text': 'str' } }
+
+ { 'command': 'x-query-roms',
+ 'returns': 'HumanReadableText' }
+
+The ``HumanReadableText`` struct is intended to be used for all
+commands, under the ``x-`` name prefix that are returning unstructured
+text targetted at humans. It should never be used for commands outside
+the ``x-`` name prefix, as those should be using structured QAPI types.
+
+Implementing the QMP command
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+The QMP implementation will typically involve creating a ``GString``
+object and printing formatted data into it::
+
+ HumanReadableText *qmp_x_query_roms(Error **errp)
+ {
+ g_autoptr(GString) buf = g_string_new("");
+ Rom *rom;
+
+ QTAILQ_FOREACH(rom, &roms, next) {
+ g_string_append_printf("%s size=0x%06zx name=\"%s\"\n",
+ memory_region_name(rom->mr),
+ rom->romsize,
+ rom->name);
+ }
+
+ return human_readable_text_from_str(buf);
+ }
+
+
+Implementing the HMP command
+~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Now that the QMP command is in place, we can also make it available in
+the human monitor (HMP) as shown in previous examples. The HMP
+implementations will all look fairly similar, as all they need do is
+invoke the QMP command and then print the resulting text or error
+message. Here's the implementation of the "info roms" HMP command::
+
+ void hmp_info_roms(Monitor *mon, const QDict *qdict)
+ {
+ Error err = NULL;
+ g_autoptr(HumanReadableText) info = qmp_x_query_roms(&err);
+
+ if (hmp_handle_error(mon, err)) {
+ return;
+ }
+ monitor_printf(mon, "%s", info->human_readable_text);
+ }
+
+Also, you have to add the function's prototype to the hmp.h file.
+
+There's one last step to actually make the command available to
+monitor users, we should add it to the hmp-commands-info.hx file::
+
+ {
+ .name = "roms",
+ .args_type = "",
+ .params = "",
+ .help = "show roms",
+ .cmd = hmp_info_roms,
+ },
+
+The case of writing a HMP info handler that calls a no-parameter QMP query
+command is quite common. To simplify the implementation there is a general
+purpose HMP info handler for this scenario. All that is required to expose
+a no-parameter QMP query command via HMP is to declare it using the
+'.cmd_info_hrt' field to point to the QMP handler, and leave the '.cmd'
+field NULL::
+
+ {
+ .name = "roms",
+ .args_type = "",
+ .params = "",
+ .help = "show roms",
+ .cmd_info_hrt = qmp_x_query_roms,
+ },
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 09/18] docs/devel: document expectations for HMP commands in the future
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (7 preceding siblings ...)
2021-11-02 17:56 ` [PULL 08/18] docs/devel: add example of command returning unstructured text Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 10/18] qapi: introduce x-query-roms QMP command Daniel P. Berrangé
` (9 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
We no longer wish to have commands implemented in HMP only. All commands
should start with a QMP implementation and the HMP merely be a shim
around this. To reduce the burden of implementing QMP commands where
there is low expectation of machine usage, requirements for QAPI
modelling are relaxed provided the command is under the "x-" name
prefix.
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
docs/devel/writing-monitor-commands.rst | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/docs/devel/writing-monitor-commands.rst b/docs/devel/writing-monitor-commands.rst
index b87992df91..b3e2c8481d 100644
--- a/docs/devel/writing-monitor-commands.rst
+++ b/docs/devel/writing-monitor-commands.rst
@@ -11,6 +11,14 @@ For an in-depth introduction to the QAPI framework, please refer to
docs/devel/qapi-code-gen.txt. For documentation about the QMP protocol,
start with docs/interop/qmp-intro.txt.
+New commands may be implemented in QMP only. New HMP commands should be
+implemented on top of QMP. The typical HMP command wraps around an
+equivalent QMP command, but HMP convenience commands built from QMP
+building blocks are also fine. The long term goal is to make all
+existing HMP commands conform to this, to fully isolate HMP from the
+internals of QEMU. Refer to the `Writing a debugging aid returning
+unstructured text`_ section for further guidance on commands that
+would have traditionally been HMP only.
Overview
--------
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 10/18] qapi: introduce x-query-roms QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (8 preceding siblings ...)
2021-11-02 17:56 ` [PULL 09/18] docs/devel: document expectations for HMP commands in the future Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 11/18] qapi: introduce x-query-profile " Daniel P. Berrangé
` (8 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info roms" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
hw/core/loader.c | 39 ++++++++++++++++++++++-----------------
monitor/misc.c | 1 +
qapi/machine.json | 12 ++++++++++++
4 files changed, 36 insertions(+), 18 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 24c478aead..b6325d36ed 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -594,7 +594,7 @@ ERST
.args_type = "",
.params = "",
.help = "show roms",
- .cmd = hmp_info_roms,
+ .cmd_info_hrt = qmp_x_query_roms,
},
SRST
diff --git a/hw/core/loader.c b/hw/core/loader.c
index c7f97fdce8..052a0fd719 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -46,6 +46,8 @@
#include "qemu-common.h"
#include "qemu/datadir.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/type-helpers.h"
#include "trace.h"
#include "hw/hw.h"
#include "disas/disas.h"
@@ -1474,32 +1476,35 @@ void *rom_ptr_for_as(AddressSpace *as, hwaddr addr, size_t size)
return cbdata.rom;
}
-void hmp_info_roms(Monitor *mon, const QDict *qdict)
+HumanReadableText *qmp_x_query_roms(Error **errp)
{
Rom *rom;
+ g_autoptr(GString) buf = g_string_new("");
QTAILQ_FOREACH(rom, &roms, next) {
if (rom->mr) {
- monitor_printf(mon, "%s"
- " size=0x%06zx name=\"%s\"\n",
- memory_region_name(rom->mr),
- rom->romsize,
- rom->name);
+ g_string_append_printf(buf, "%s"
+ " size=0x%06zx name=\"%s\"\n",
+ memory_region_name(rom->mr),
+ rom->romsize,
+ rom->name);
} else if (!rom->fw_file) {
- monitor_printf(mon, "addr=" TARGET_FMT_plx
- " size=0x%06zx mem=%s name=\"%s\"\n",
- rom->addr, rom->romsize,
- rom->isrom ? "rom" : "ram",
- rom->name);
+ g_string_append_printf(buf, "addr=" TARGET_FMT_plx
+ " size=0x%06zx mem=%s name=\"%s\"\n",
+ rom->addr, rom->romsize,
+ rom->isrom ? "rom" : "ram",
+ rom->name);
} else {
- monitor_printf(mon, "fw=%s/%s"
- " size=0x%06zx name=\"%s\"\n",
- rom->fw_dir,
- rom->fw_file,
- rom->romsize,
- rom->name);
+ g_string_append_printf(buf, "fw=%s/%s"
+ " size=0x%06zx name=\"%s\"\n",
+ rom->fw_dir,
+ rom->fw_file,
+ rom->romsize,
+ rom->name);
}
}
+
+ return human_readable_text_from_str(buf);
}
typedef enum HexRecord HexRecord;
diff --git a/monitor/misc.c b/monitor/misc.c
index 0e124044d0..c3efdf6336 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -71,6 +71,7 @@
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-qom.h"
#include "qapi/qapi-commands-trace.h"
+#include "qapi/qapi-commands-machine.h"
#include "qapi/qapi-init-commands.h"
#include "qapi/error.h"
#include "qapi/qmp-event.h"
diff --git a/qapi/machine.json b/qapi/machine.json
index 5db54df298..26d4ef8195 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1411,3 +1411,15 @@
'*cores': 'int',
'*threads': 'int',
'*maxcpus': 'int' } }
+
+##
+# @x-query-roms:
+#
+# Query information on the registered ROMS
+#
+# Returns: registered ROMs
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-roms',
+ 'returns': 'HumanReadableText' }
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 11/18] qapi: introduce x-query-profile QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (9 preceding siblings ...)
2021-11-02 17:56 ` [PULL 10/18] qapi: introduce x-query-roms QMP command Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 12/18] qapi: introduce x-query-numa " Daniel P. Berrangé
` (7 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info profile" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
monitor/misc.c | 27 ---------------------------
monitor/qmp-cmds.c | 32 ++++++++++++++++++++++++++++++++
qapi/machine.json | 12 ++++++++++++
tests/qtest/qmp-cmd-test.c | 3 +++
5 files changed, 48 insertions(+), 28 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index b6325d36ed..84c5e0da10 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -363,7 +363,7 @@ ERST
.args_type = "",
.params = "",
.help = "show profiling information",
- .cmd = hmp_info_profile,
+ .cmd_info_hrt = qmp_x_query_profile,
},
SRST
diff --git a/monitor/misc.c b/monitor/misc.c
index c3efdf6336..fabd25a241 100644
--- a/monitor/misc.c
+++ b/monitor/misc.c
@@ -931,33 +931,6 @@ static void hmp_info_mtree(Monitor *mon, const QDict *qdict)
mtree_info(flatview, dispatch_tree, owner, disabled);
}
-#ifdef CONFIG_PROFILER
-
-int64_t dev_time;
-
-static void hmp_info_profile(Monitor *mon, const QDict *qdict)
-{
- static int64_t last_cpu_exec_time;
- int64_t cpu_exec_time;
- int64_t delta;
-
- cpu_exec_time = tcg_cpu_exec_time();
- delta = cpu_exec_time - last_cpu_exec_time;
-
- monitor_printf(mon, "async time %" PRId64 " (%0.3f)\n",
- dev_time, dev_time / (double)NANOSECONDS_PER_SECOND);
- monitor_printf(mon, "qemu time %" PRId64 " (%0.3f)\n",
- delta, delta / (double)NANOSECONDS_PER_SECOND);
- last_cpu_exec_time = cpu_exec_time;
- dev_time = 0;
-}
-#else
-static void hmp_info_profile(Monitor *mon, const QDict *qdict)
-{
- monitor_printf(mon, "Internal profiler not compiled\n");
-}
-#endif
-
/* Capture support */
static QLIST_HEAD (capture_list_head, CaptureState) capture_head;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 5c0d5e116b..6122ad18b6 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -36,6 +36,7 @@
#include "qapi/qapi-commands-machine.h"
#include "qapi/qapi-commands-misc.h"
#include "qapi/qapi-commands-ui.h"
+#include "qapi/type-helpers.h"
#include "qapi/qmp/qerror.h"
#include "hw/mem/memory-device.h"
#include "hw/acpi/acpi_dev_interface.h"
@@ -350,3 +351,34 @@ void qmp_display_reload(DisplayReloadOptions *arg, Error **errp)
abort();
}
}
+
+#ifdef CONFIG_PROFILER
+
+int64_t dev_time;
+
+HumanReadableText *qmp_x_query_profile(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+ static int64_t last_cpu_exec_time;
+ int64_t cpu_exec_time;
+ int64_t delta;
+
+ cpu_exec_time = tcg_cpu_exec_time();
+ delta = cpu_exec_time - last_cpu_exec_time;
+
+ g_string_append_printf(buf, "async time %" PRId64 " (%0.3f)\n",
+ dev_time, dev_time / (double)NANOSECONDS_PER_SECOND);
+ g_string_append_printf(buf, "qemu time %" PRId64 " (%0.3f)\n",
+ delta, delta / (double)NANOSECONDS_PER_SECOND);
+ last_cpu_exec_time = cpu_exec_time;
+ dev_time = 0;
+
+ return human_readable_text_from_str(buf);
+}
+#else
+HumanReadableText *qmp_x_query_profile(Error **errp)
+{
+ error_setg(errp, "Internal profiler not compiled");
+ return NULL;
+}
+#endif
diff --git a/qapi/machine.json b/qapi/machine.json
index 26d4ef8195..73ff4bc168 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1412,6 +1412,18 @@
'*threads': 'int',
'*maxcpus': 'int' } }
+##
+# @x-query-profile:
+#
+# Query TCG profiling information
+#
+# Returns: profile information
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-profile',
+ 'returns': 'HumanReadableText' }
+
##
# @x-query-roms:
#
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 1af2f74c28..372c887eea 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -46,6 +46,9 @@ static int query_error_class(const char *cmd)
{ "query-balloon", ERROR_CLASS_DEVICE_NOT_ACTIVE },
{ "query-hotpluggable-cpus", ERROR_CLASS_GENERIC_ERROR },
{ "query-vm-generation-id", ERROR_CLASS_GENERIC_ERROR },
+#ifndef CONFIG_PROFILER
+ { "x-query-profile", ERROR_CLASS_GENERIC_ERROR },
+#endif
{ NULL, -1 }
};
int i;
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 12/18] qapi: introduce x-query-numa QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (10 preceding siblings ...)
2021-11-02 17:56 ` [PULL 11/18] qapi: introduce x-query-profile " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 13/18] qapi: introduce x-query-usb " Daniel P. Berrangé
` (6 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info numa" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
hw/core/machine-hmp-cmds.c | 35 ---------------------------------
hw/core/machine-qmp-cmds.c | 40 ++++++++++++++++++++++++++++++++++++++
qapi/machine.json | 12 ++++++++++++
4 files changed, 53 insertions(+), 36 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index 84c5e0da10..f66b1ca986 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -325,7 +325,7 @@ ERST
.args_type = "",
.params = "",
.help = "show NUMA information",
- .cmd = hmp_info_numa,
+ .cmd_info_hrt = qmp_x_query_numa,
},
SRST
diff --git a/hw/core/machine-hmp-cmds.c b/hw/core/machine-hmp-cmds.c
index c356783ab9..4e2f319aeb 100644
--- a/hw/core/machine-hmp-cmds.c
+++ b/hw/core/machine-hmp-cmds.c
@@ -130,38 +130,3 @@ void hmp_info_memdev(Monitor *mon, const QDict *qdict)
qapi_free_MemdevList(memdev_list);
hmp_handle_error(mon, err);
}
-
-void hmp_info_numa(Monitor *mon, const QDict *qdict)
-{
- int i, nb_numa_nodes;
- NumaNodeMem *node_mem;
- CpuInfoFastList *cpu_list, *cpu;
- MachineState *ms = MACHINE(qdev_get_machine());
-
- nb_numa_nodes = ms->numa_state ? ms->numa_state->num_nodes : 0;
- monitor_printf(mon, "%d nodes\n", nb_numa_nodes);
- if (!nb_numa_nodes) {
- return;
- }
-
- cpu_list = qmp_query_cpus_fast(&error_abort);
- node_mem = g_new0(NumaNodeMem, nb_numa_nodes);
-
- query_numa_node_mem(node_mem, ms);
- for (i = 0; i < nb_numa_nodes; i++) {
- monitor_printf(mon, "node %d cpus:", i);
- for (cpu = cpu_list; cpu; cpu = cpu->next) {
- if (cpu->value->has_props && cpu->value->props->has_node_id &&
- cpu->value->props->node_id == i) {
- monitor_printf(mon, " %" PRIi64, cpu->value->cpu_index);
- }
- }
- monitor_printf(mon, "\n");
- monitor_printf(mon, "node %d size: %" PRId64 " MB\n", i,
- node_mem[i].node_mem >> 20);
- monitor_printf(mon, "node %d plugged: %" PRId64 " MB\n", i,
- node_mem[i].node_plugged_mem >> 20);
- }
- qapi_free_CpuInfoFastList(cpu_list);
- g_free(node_mem);
-}
diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c
index 216fdfaf3a..4f4ab30f8c 100644
--- a/hw/core/machine-qmp-cmds.c
+++ b/hw/core/machine-qmp-cmds.c
@@ -15,6 +15,7 @@
#include "qapi/qmp/qerror.h"
#include "qapi/qmp/qobject.h"
#include "qapi/qobject-input-visitor.h"
+#include "qapi/type-helpers.h"
#include "qemu/main-loop.h"
#include "qom/qom-qobject.h"
#include "sysemu/hostmem.h"
@@ -204,3 +205,42 @@ MemdevList *qmp_query_memdev(Error **errp)
object_child_foreach(obj, query_memdev, &list);
return list;
}
+
+HumanReadableText *qmp_x_query_numa(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+ int i, nb_numa_nodes;
+ NumaNodeMem *node_mem;
+ CpuInfoFastList *cpu_list, *cpu;
+ MachineState *ms = MACHINE(qdev_get_machine());
+
+ nb_numa_nodes = ms->numa_state ? ms->numa_state->num_nodes : 0;
+ g_string_append_printf(buf, "%d nodes\n", nb_numa_nodes);
+ if (!nb_numa_nodes) {
+ goto done;
+ }
+
+ cpu_list = qmp_query_cpus_fast(&error_abort);
+ node_mem = g_new0(NumaNodeMem, nb_numa_nodes);
+
+ query_numa_node_mem(node_mem, ms);
+ for (i = 0; i < nb_numa_nodes; i++) {
+ g_string_append_printf(buf, "node %d cpus:", i);
+ for (cpu = cpu_list; cpu; cpu = cpu->next) {
+ if (cpu->value->has_props && cpu->value->props->has_node_id &&
+ cpu->value->props->node_id == i) {
+ g_string_append_printf(buf, " %" PRIi64, cpu->value->cpu_index);
+ }
+ }
+ g_string_append_printf(buf, "\n");
+ g_string_append_printf(buf, "node %d size: %" PRId64 " MB\n", i,
+ node_mem[i].node_mem >> 20);
+ g_string_append_printf(buf, "node %d plugged: %" PRId64 " MB\n", i,
+ node_mem[i].node_plugged_mem >> 20);
+ }
+ qapi_free_CpuInfoFastList(cpu_list);
+ g_free(node_mem);
+
+ done:
+ return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index 73ff4bc168..3732f80a82 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1412,6 +1412,18 @@
'*threads': 'int',
'*maxcpus': 'int' } }
+##
+# @x-query-numa:
+#
+# Query NUMA topology information
+#
+# Returns: topology information
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-numa',
+ 'returns': 'HumanReadableText' }
+
##
# @x-query-profile:
#
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 13/18] qapi: introduce x-query-usb QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (11 preceding siblings ...)
2021-11-02 17:56 ` [PULL 12/18] qapi: introduce x-query-numa " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 14/18] qapi: introduce x-query-rdma " Daniel P. Berrangé
` (5 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info usb" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
hw/usb/bus.c | 24 +++++++++++++++---------
qapi/machine.json | 12 ++++++++++++
stubs/usb-dev-stub.c | 8 ++++++++
tests/qtest/qmp-cmd-test.c | 2 ++
5 files changed, 38 insertions(+), 10 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index f66b1ca986..ef1bfe4f5a 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -338,7 +338,7 @@ ERST
.args_type = "",
.params = "",
.help = "show guest USB devices",
- .cmd = hmp_info_usb,
+ .cmd_info_hrt = qmp_x_query_usb,
},
SRST
diff --git a/hw/usb/bus.c b/hw/usb/bus.c
index 5d441a7065..92d6ed5626 100644
--- a/hw/usb/bus.c
+++ b/hw/usb/bus.c
@@ -2,6 +2,8 @@
#include "hw/qdev-properties.h"
#include "hw/usb.h"
#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/type-helpers.h"
#include "qemu/error-report.h"
#include "qemu/module.h"
#include "sysemu/sysemu.h"
@@ -631,15 +633,16 @@ static char *usb_get_fw_dev_path(DeviceState *qdev)
return fw_path;
}
-void hmp_info_usb(Monitor *mon, const QDict *qdict)
+HumanReadableText *qmp_x_query_usb(Error **errp)
{
+ g_autoptr(GString) buf = g_string_new("");
USBBus *bus;
USBDevice *dev;
USBPort *port;
if (QTAILQ_EMPTY(&busses)) {
- monitor_printf(mon, "USB support not enabled\n");
- return;
+ error_setg(errp, "USB support not enabled");
+ return NULL;
}
QTAILQ_FOREACH(bus, &busses, next) {
@@ -647,14 +650,17 @@ void hmp_info_usb(Monitor *mon, const QDict *qdict)
dev = port->dev;
if (!dev)
continue;
- monitor_printf(mon, " Device %d.%d, Port %s, Speed %s Mb/s, "
- "Product %s%s%s\n",
- bus->busnr, dev->addr, port->path,
- usb_speed(dev->speed), dev->product_desc,
- dev->qdev.id ? ", ID: " : "",
- dev->qdev.id ?: "");
+ g_string_append_printf(buf,
+ " Device %d.%d, Port %s, Speed %s Mb/s, "
+ "Product %s%s%s\n",
+ bus->busnr, dev->addr, port->path,
+ usb_speed(dev->speed), dev->product_desc,
+ dev->qdev.id ? ", ID: " : "",
+ dev->qdev.id ?: "");
}
}
+
+ return human_readable_text_from_str(buf);
}
/* handle legacy -usbdevice cmd line option */
diff --git a/qapi/machine.json b/qapi/machine.json
index 3732f80a82..15b6c98597 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1447,3 +1447,15 @@
##
{ 'command': 'x-query-roms',
'returns': 'HumanReadableText' }
+
+##
+# @x-query-usb:
+#
+# Query information on the USB devices
+#
+# Returns: USB device information
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-usb',
+ 'returns': 'HumanReadableText' }
diff --git a/stubs/usb-dev-stub.c b/stubs/usb-dev-stub.c
index b1adeeb454..aa557692b7 100644
--- a/stubs/usb-dev-stub.c
+++ b/stubs/usb-dev-stub.c
@@ -8,6 +8,8 @@
#include "qemu/osdep.h"
#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
#include "sysemu/sysemu.h"
#include "monitor/monitor.h"
#include "hw/usb.h"
@@ -19,6 +21,12 @@ USBDevice *usbdevice_create(const char *driver)
return NULL;
}
+HumanReadableText *qmp_x_query_usb(Error **errp)
+{
+ error_setg(errp, "Support for USB devices not built-in");
+ return NULL;
+}
+
void hmp_info_usb(Monitor *mon, const QDict *qdict)
{
monitor_printf(mon, "Support for USB devices not built-in\n");
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 372c887eea..0d52ea6c4b 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -49,6 +49,8 @@ static int query_error_class(const char *cmd)
#ifndef CONFIG_PROFILER
{ "x-query-profile", ERROR_CLASS_GENERIC_ERROR },
#endif
+ /* Only valid with a USB bus added */
+ { "x-query-usb", ERROR_CLASS_GENERIC_ERROR },
{ NULL, -1 }
};
int i;
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 14/18] qapi: introduce x-query-rdma QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (12 preceding siblings ...)
2021-11-02 17:56 ` [PULL 13/18] qapi: introduce x-query-usb " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 15/18] qapi: introduce x-query-ramblock " Daniel P. Berrangé
` (4 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info rdma" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
hw/rdma/rdma_rm.c | 104 +++++++++++++++++++-------------------
hw/rdma/rdma_rm.h | 2 +-
hw/rdma/vmw/pvrdma_main.c | 31 ++++++------
include/hw/rdma/rdma.h | 2 +-
monitor/hmp-cmds.c | 27 ----------
monitor/qmp-cmds.c | 32 ++++++++++++
qapi/machine.json | 12 +++++
8 files changed, 115 insertions(+), 97 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index ef1bfe4f5a..d9af216473 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -185,7 +185,7 @@ ERST
.args_type = "",
.params = "",
.help = "show RDMA state",
- .cmd = hmp_info_rdma,
+ .cmd_info_hrt = qmp_x_query_rdma,
},
SRST
diff --git a/hw/rdma/rdma_rm.c b/hw/rdma/rdma_rm.c
index 49141d4074..cfd85de3e6 100644
--- a/hw/rdma/rdma_rm.c
+++ b/hw/rdma/rdma_rm.c
@@ -27,58 +27,58 @@
#define PG_DIR_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
#define PG_TBL_SZ { TARGET_PAGE_SIZE / sizeof(__u64) }
-void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res)
-{
- monitor_printf(mon, "\ttx : %" PRId64 "\n",
- dev_res->stats.tx);
- monitor_printf(mon, "\ttx_len : %" PRId64 "\n",
- dev_res->stats.tx_len);
- monitor_printf(mon, "\ttx_err : %" PRId64 "\n",
- dev_res->stats.tx_err);
- monitor_printf(mon, "\trx_bufs : %" PRId64 "\n",
- dev_res->stats.rx_bufs);
- monitor_printf(mon, "\trx_srq : %" PRId64 "\n",
- dev_res->stats.rx_srq);
- monitor_printf(mon, "\trx_bufs_len : %" PRId64 "\n",
- dev_res->stats.rx_bufs_len);
- monitor_printf(mon, "\trx_bufs_err : %" PRId64 "\n",
- dev_res->stats.rx_bufs_err);
- monitor_printf(mon, "\tcomps : %" PRId64 "\n",
- dev_res->stats.completions);
- monitor_printf(mon, "\tmissing_comps : %" PRId32 "\n",
- dev_res->stats.missing_cqe);
- monitor_printf(mon, "\tpoll_cq (bk) : %" PRId64 "\n",
- dev_res->stats.poll_cq_from_bk);
- monitor_printf(mon, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
- dev_res->stats.poll_cq_ppoll_to);
- monitor_printf(mon, "\tpoll_cq (fe) : %" PRId64 "\n",
- dev_res->stats.poll_cq_from_guest);
- monitor_printf(mon, "\tpoll_cq_empty : %" PRId64 "\n",
- dev_res->stats.poll_cq_from_guest_empty);
- monitor_printf(mon, "\tmad_tx : %" PRId64 "\n",
- dev_res->stats.mad_tx);
- monitor_printf(mon, "\tmad_tx_err : %" PRId64 "\n",
- dev_res->stats.mad_tx_err);
- monitor_printf(mon, "\tmad_rx : %" PRId64 "\n",
- dev_res->stats.mad_rx);
- monitor_printf(mon, "\tmad_rx_err : %" PRId64 "\n",
- dev_res->stats.mad_rx_err);
- monitor_printf(mon, "\tmad_rx_bufs : %" PRId64 "\n",
- dev_res->stats.mad_rx_bufs);
- monitor_printf(mon, "\tmad_rx_bufs_err : %" PRId64 "\n",
- dev_res->stats.mad_rx_bufs_err);
- monitor_printf(mon, "\tPDs : %" PRId32 "\n",
- dev_res->pd_tbl.used);
- monitor_printf(mon, "\tMRs : %" PRId32 "\n",
- dev_res->mr_tbl.used);
- monitor_printf(mon, "\tUCs : %" PRId32 "\n",
- dev_res->uc_tbl.used);
- monitor_printf(mon, "\tQPs : %" PRId32 "\n",
- dev_res->qp_tbl.used);
- monitor_printf(mon, "\tCQs : %" PRId32 "\n",
- dev_res->cq_tbl.used);
- monitor_printf(mon, "\tCEQ_CTXs : %" PRId32 "\n",
- dev_res->cqe_ctx_tbl.used);
+void rdma_format_device_counters(RdmaDeviceResources *dev_res, GString *buf)
+{
+ g_string_append_printf(buf, "\ttx : %" PRId64 "\n",
+ dev_res->stats.tx);
+ g_string_append_printf(buf, "\ttx_len : %" PRId64 "\n",
+ dev_res->stats.tx_len);
+ g_string_append_printf(buf, "\ttx_err : %" PRId64 "\n",
+ dev_res->stats.tx_err);
+ g_string_append_printf(buf, "\trx_bufs : %" PRId64 "\n",
+ dev_res->stats.rx_bufs);
+ g_string_append_printf(buf, "\trx_srq : %" PRId64 "\n",
+ dev_res->stats.rx_srq);
+ g_string_append_printf(buf, "\trx_bufs_len : %" PRId64 "\n",
+ dev_res->stats.rx_bufs_len);
+ g_string_append_printf(buf, "\trx_bufs_err : %" PRId64 "\n",
+ dev_res->stats.rx_bufs_err);
+ g_string_append_printf(buf, "\tcomps : %" PRId64 "\n",
+ dev_res->stats.completions);
+ g_string_append_printf(buf, "\tmissing_comps : %" PRId32 "\n",
+ dev_res->stats.missing_cqe);
+ g_string_append_printf(buf, "\tpoll_cq (bk) : %" PRId64 "\n",
+ dev_res->stats.poll_cq_from_bk);
+ g_string_append_printf(buf, "\tpoll_cq_ppoll_to : %" PRId64 "\n",
+ dev_res->stats.poll_cq_ppoll_to);
+ g_string_append_printf(buf, "\tpoll_cq (fe) : %" PRId64 "\n",
+ dev_res->stats.poll_cq_from_guest);
+ g_string_append_printf(buf, "\tpoll_cq_empty : %" PRId64 "\n",
+ dev_res->stats.poll_cq_from_guest_empty);
+ g_string_append_printf(buf, "\tmad_tx : %" PRId64 "\n",
+ dev_res->stats.mad_tx);
+ g_string_append_printf(buf, "\tmad_tx_err : %" PRId64 "\n",
+ dev_res->stats.mad_tx_err);
+ g_string_append_printf(buf, "\tmad_rx : %" PRId64 "\n",
+ dev_res->stats.mad_rx);
+ g_string_append_printf(buf, "\tmad_rx_err : %" PRId64 "\n",
+ dev_res->stats.mad_rx_err);
+ g_string_append_printf(buf, "\tmad_rx_bufs : %" PRId64 "\n",
+ dev_res->stats.mad_rx_bufs);
+ g_string_append_printf(buf, "\tmad_rx_bufs_err : %" PRId64 "\n",
+ dev_res->stats.mad_rx_bufs_err);
+ g_string_append_printf(buf, "\tPDs : %" PRId32 "\n",
+ dev_res->pd_tbl.used);
+ g_string_append_printf(buf, "\tMRs : %" PRId32 "\n",
+ dev_res->mr_tbl.used);
+ g_string_append_printf(buf, "\tUCs : %" PRId32 "\n",
+ dev_res->uc_tbl.used);
+ g_string_append_printf(buf, "\tQPs : %" PRId32 "\n",
+ dev_res->qp_tbl.used);
+ g_string_append_printf(buf, "\tCQs : %" PRId32 "\n",
+ dev_res->cq_tbl.used);
+ g_string_append_printf(buf, "\tCEQ_CTXs : %" PRId32 "\n",
+ dev_res->cqe_ctx_tbl.used);
}
static inline void res_tbl_init(const char *name, RdmaRmResTbl *tbl,
diff --git a/hw/rdma/rdma_rm.h b/hw/rdma/rdma_rm.h
index e8639909cd..d69a917795 100644
--- a/hw/rdma/rdma_rm.h
+++ b/hw/rdma/rdma_rm.h
@@ -92,6 +92,6 @@ static inline union ibv_gid *rdma_rm_get_gid(RdmaDeviceResources *dev_res,
{
return &dev_res->port.gid_tbl[sgid_idx].gid;
}
-void rdma_dump_device_counters(Monitor *mon, RdmaDeviceResources *dev_res);
+void rdma_format_device_counters(RdmaDeviceResources *dev_res, GString *buf);
#endif
diff --git a/hw/rdma/vmw/pvrdma_main.c b/hw/rdma/vmw/pvrdma_main.c
index 7c0c3551a8..91206dbb8e 100644
--- a/hw/rdma/vmw/pvrdma_main.c
+++ b/hw/rdma/vmw/pvrdma_main.c
@@ -58,24 +58,25 @@ static Property pvrdma_dev_properties[] = {
DEFINE_PROP_END_OF_LIST(),
};
-static void pvrdma_print_statistics(Monitor *mon, RdmaProvider *obj)
+static void pvrdma_format_statistics(RdmaProvider *obj, GString *buf)
{
PVRDMADev *dev = PVRDMA_DEV(obj);
PCIDevice *pdev = PCI_DEVICE(dev);
- monitor_printf(mon, "%s, %x.%x\n", pdev->name, PCI_SLOT(pdev->devfn),
- PCI_FUNC(pdev->devfn));
- monitor_printf(mon, "\tcommands : %" PRId64 "\n",
- dev->stats.commands);
- monitor_printf(mon, "\tregs_reads : %" PRId64 "\n",
- dev->stats.regs_reads);
- monitor_printf(mon, "\tregs_writes : %" PRId64 "\n",
- dev->stats.regs_writes);
- monitor_printf(mon, "\tuar_writes : %" PRId64 "\n",
- dev->stats.uar_writes);
- monitor_printf(mon, "\tinterrupts : %" PRId64 "\n",
- dev->stats.interrupts);
- rdma_dump_device_counters(mon, &dev->rdma_dev_res);
+ g_string_append_printf(buf, "%s, %x.%x\n",
+ pdev->name, PCI_SLOT(pdev->devfn),
+ PCI_FUNC(pdev->devfn));
+ g_string_append_printf(buf, "\tcommands : %" PRId64 "\n",
+ dev->stats.commands);
+ g_string_append_printf(buf, "\tregs_reads : %" PRId64 "\n",
+ dev->stats.regs_reads);
+ g_string_append_printf(buf, "\tregs_writes : %" PRId64 "\n",
+ dev->stats.regs_writes);
+ g_string_append_printf(buf, "\tuar_writes : %" PRId64 "\n",
+ dev->stats.uar_writes);
+ g_string_append_printf(buf, "\tinterrupts : %" PRId64 "\n",
+ dev->stats.interrupts);
+ rdma_format_device_counters(&dev->rdma_dev_res, buf);
}
static void free_dev_ring(PCIDevice *pci_dev, PvrdmaRing *ring,
@@ -699,7 +700,7 @@ static void pvrdma_class_init(ObjectClass *klass, void *data)
device_class_set_props(dc, pvrdma_dev_properties);
set_bit(DEVICE_CATEGORY_NETWORK, dc->categories);
- ir->print_statistics = pvrdma_print_statistics;
+ ir->format_statistics = pvrdma_format_statistics;
}
static const TypeInfo pvrdma_info = {
diff --git a/include/hw/rdma/rdma.h b/include/hw/rdma/rdma.h
index e77e43a170..80b2e531c4 100644
--- a/include/hw/rdma/rdma.h
+++ b/include/hw/rdma/rdma.h
@@ -31,7 +31,7 @@ typedef struct RdmaProvider RdmaProvider;
struct RdmaProviderClass {
InterfaceClass parent;
- void (*print_statistics)(Monitor *mon, RdmaProvider *obj);
+ void (*format_statistics)(RdmaProvider *obj, GString *buf);
};
#endif
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9031cea881..9d221622d7 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -54,7 +54,6 @@
#include "qemu/error-report.h"
#include "exec/ramlist.h"
#include "hw/intc/intc.h"
-#include "hw/rdma/rdma.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
@@ -850,32 +849,6 @@ void hmp_info_pic(Monitor *mon, const QDict *qdict)
hmp_info_pic_foreach, mon);
}
-static int hmp_info_rdma_foreach(Object *obj, void *opaque)
-{
- RdmaProvider *rdma;
- RdmaProviderClass *k;
- Monitor *mon = opaque;
-
- if (object_dynamic_cast(obj, INTERFACE_RDMA_PROVIDER)) {
- rdma = RDMA_PROVIDER(obj);
- k = RDMA_PROVIDER_GET_CLASS(obj);
- if (k->print_statistics) {
- k->print_statistics(mon, rdma);
- } else {
- monitor_printf(mon, "RDMA statistics not available for %s.\n",
- object_get_typename(obj));
- }
- }
-
- return 0;
-}
-
-void hmp_info_rdma(Monitor *mon, const QDict *qdict)
-{
- object_child_foreach_recursive(object_get_root(),
- hmp_info_rdma_foreach, mon);
-}
-
void hmp_info_pci(Monitor *mon, const QDict *qdict)
{
PciInfoList *info_list, *info;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 6122ad18b6..0a9ba7595c 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -40,6 +40,7 @@
#include "qapi/qmp/qerror.h"
#include "hw/mem/memory-device.h"
#include "hw/acpi/acpi_dev_interface.h"
+#include "hw/rdma/rdma.h"
NameInfo *qmp_query_name(Error **errp)
{
@@ -382,3 +383,34 @@ HumanReadableText *qmp_x_query_profile(Error **errp)
return NULL;
}
#endif
+
+static int qmp_x_query_rdma_foreach(Object *obj, void *opaque)
+{
+ RdmaProvider *rdma;
+ RdmaProviderClass *k;
+ GString *buf = opaque;
+
+ if (object_dynamic_cast(obj, INTERFACE_RDMA_PROVIDER)) {
+ rdma = RDMA_PROVIDER(obj);
+ k = RDMA_PROVIDER_GET_CLASS(obj);
+ if (k->format_statistics) {
+ k->format_statistics(rdma, buf);
+ } else {
+ g_string_append_printf(buf,
+ "RDMA statistics not available for %s.\n",
+ object_get_typename(obj));
+ }
+ }
+
+ return 0;
+}
+
+HumanReadableText *qmp_x_query_rdma(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+
+ object_child_foreach_recursive(object_get_root(),
+ qmp_x_query_rdma_foreach, buf);
+
+ return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index 15b6c98597..1b2748c77a 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1436,6 +1436,18 @@
{ 'command': 'x-query-profile',
'returns': 'HumanReadableText' }
+##
+# @x-query-rdma:
+#
+# Query RDMA state
+#
+# Returns: RDMA state
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-rdma',
+ 'returns': 'HumanReadableText' }
+
##
# @x-query-roms:
#
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (13 preceding siblings ...)
2021-11-02 17:56 ` [PULL 14/18] qapi: introduce x-query-rdma " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2022-06-09 10:07 ` Claudio Fontana
2021-11-02 17:56 ` [PULL 16/18] qapi: introduce x-query-irq " Daniel P. Berrangé
` (3 subsequent siblings)
18 siblings, 1 reply; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info ramblock" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
include/exec/ramlist.h | 2 +-
monitor/hmp-cmds.c | 6 ------
monitor/qmp-cmds.c | 8 ++++++++
qapi/machine.json | 12 ++++++++++++
softmmu/physmem.c | 19 +++++++++++--------
6 files changed, 33 insertions(+), 16 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index d9af216473..c2d7275bf5 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -772,7 +772,7 @@ ERST
.args_type = "",
.params = "",
.help = "Display system ramblock information",
- .cmd = hmp_info_ramblock,
+ .cmd_info_hrt = qmp_x_query_ramblock,
},
SRST
diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
index ece6497ee2..2ad2a81acc 100644
--- a/include/exec/ramlist.h
+++ b/include/exec/ramlist.h
@@ -80,6 +80,6 @@ void ram_block_notify_add(void *host, size_t size, size_t max_size);
void ram_block_notify_remove(void *host, size_t size, size_t max_size);
void ram_block_notify_resize(void *host, size_t old_size, size_t new_size);
-void ram_block_dump(Monitor *mon);
+GString *ram_block_format(void);
#endif /* RAMLIST_H */
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 9d221622d7..90f9a64573 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -52,7 +52,6 @@
#include "ui/console.h"
#include "qemu/cutils.h"
#include "qemu/error-report.h"
-#include "exec/ramlist.h"
#include "hw/intc/intc.h"
#include "migration/snapshot.h"
#include "migration/misc.h"
@@ -2176,11 +2175,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
qapi_free_RockerOfDpaGroupList(list);
}
-void hmp_info_ramblock(Monitor *mon, const QDict *qdict)
-{
- ram_block_dump(mon);
-}
-
void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
{
Error *err = NULL;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index 0a9ba7595c..a9766fa38d 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -38,6 +38,7 @@
#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"
#include "hw/rdma/rdma.h"
@@ -414,3 +415,10 @@ HumanReadableText *qmp_x_query_rdma(Error **errp)
return human_readable_text_from_str(buf);
}
+
+HumanReadableText *qmp_x_query_ramblock(Error **errp)
+{
+ g_autoptr(GString) buf = ram_block_format();
+
+ return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index 1b2748c77a..be81170c2b 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1436,6 +1436,18 @@
{ 'command': 'x-query-profile',
'returns': 'HumanReadableText' }
+##
+# @x-query-ramblock:
+#
+# Query system ramblock information
+#
+# Returns: system ramblock information
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-ramblock',
+ 'returns': 'HumanReadableText' }
+
##
# @x-query-rdma:
#
diff --git a/softmmu/physmem.c b/softmmu/physmem.c
index b9a8c1d1f4..314f8b439c 100644
--- a/softmmu/physmem.c
+++ b/softmmu/physmem.c
@@ -1296,23 +1296,26 @@ void qemu_mutex_unlock_ramlist(void)
qemu_mutex_unlock(&ram_list.mutex);
}
-void ram_block_dump(Monitor *mon)
+GString *ram_block_format(void)
{
RAMBlock *block;
char *psize;
+ GString *buf = g_string_new("");
RCU_READ_LOCK_GUARD();
- monitor_printf(mon, "%24s %8s %18s %18s %18s\n",
- "Block Name", "PSize", "Offset", "Used", "Total");
+ g_string_append_printf(buf, "%24s %8s %18s %18s %18s\n",
+ "Block Name", "PSize", "Offset", "Used", "Total");
RAMBLOCK_FOREACH(block) {
psize = size_to_str(block->page_size);
- monitor_printf(mon, "%24s %8s 0x%016" PRIx64 " 0x%016" PRIx64
- " 0x%016" PRIx64 "\n", block->idstr, psize,
- (uint64_t)block->offset,
- (uint64_t)block->used_length,
- (uint64_t)block->max_length);
+ g_string_append_printf(buf, "%24s %8s 0x%016" PRIx64 " 0x%016" PRIx64
+ " 0x%016" PRIx64 "\n", block->idstr, psize,
+ (uint64_t)block->offset,
+ (uint64_t)block->used_length,
+ (uint64_t)block->max_length);
g_free(psize);
}
+
+ return buf;
}
#ifdef __linux__
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2021-11-02 17:56 ` [PULL 15/18] qapi: introduce x-query-ramblock " Daniel P. Berrangé
@ 2022-06-09 10:07 ` Claudio Fontana
2022-06-09 10:19 ` Daniel P. Berrangé
0 siblings, 1 reply; 27+ messages in thread
From: Claudio Fontana @ 2022-06-09 10:07 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, David Hildenbrand,
Michael Roth, Richard Henderson, Yuval Shaia, Peter Xu,
Dr. David Alan Gilbert, Gerd Hoffmann, Paolo Bonzini,
Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
Hello all,
it would be really good to be able to rely on this command or something similar,
to be able to know the approximate size of a migration before starting it.
in QEMU ram_bytes_total() returns what I would like to have,
but there is currently no QMP way to get it without starting a migration,
which when trying to optimize it/size it is just about too late.
Do you think x-query-ramblock could be promoted to non-experimental?
Should another one be made available instead, like :
query-ram-bytes-total ?
Thanks,
Claudio
On 11/2/21 18:56, Daniel P. Berrangé wrote:
> This is a counterpart to the HMP "info ramblock" command. It is being
> added with an "x-" prefix because this QMP command is intended as an
> adhoc debugging tool and will thus not be modelled in QAPI as fully
> structured data, nor will it have long term guaranteed stability.
> The existing HMP command is rewritten to call the QMP command.
>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
> hmp-commands-info.hx | 2 +-
> include/exec/ramlist.h | 2 +-
> monitor/hmp-cmds.c | 6 ------
> monitor/qmp-cmds.c | 8 ++++++++
> qapi/machine.json | 12 ++++++++++++
> softmmu/physmem.c | 19 +++++++++++--------
> 6 files changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
> index d9af216473..c2d7275bf5 100644
> --- a/hmp-commands-info.hx
> +++ b/hmp-commands-info.hx
> @@ -772,7 +772,7 @@ ERST
> .args_type = "",
> .params = "",
> .help = "Display system ramblock information",
> - .cmd = hmp_info_ramblock,
> + .cmd_info_hrt = qmp_x_query_ramblock,
> },
>
> SRST
> diff --git a/include/exec/ramlist.h b/include/exec/ramlist.h
> index ece6497ee2..2ad2a81acc 100644
> --- a/include/exec/ramlist.h
> +++ b/include/exec/ramlist.h
> @@ -80,6 +80,6 @@ void ram_block_notify_add(void *host, size_t size, size_t max_size);
> void ram_block_notify_remove(void *host, size_t size, size_t max_size);
> void ram_block_notify_resize(void *host, size_t old_size, size_t new_size);
>
> -void ram_block_dump(Monitor *mon);
> +GString *ram_block_format(void);
>
> #endif /* RAMLIST_H */
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index 9d221622d7..90f9a64573 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -52,7 +52,6 @@
> #include "ui/console.h"
> #include "qemu/cutils.h"
> #include "qemu/error-report.h"
> -#include "exec/ramlist.h"
> #include "hw/intc/intc.h"
> #include "migration/snapshot.h"
> #include "migration/misc.h"
> @@ -2176,11 +2175,6 @@ void hmp_rocker_of_dpa_groups(Monitor *mon, const QDict *qdict)
> qapi_free_RockerOfDpaGroupList(list);
> }
>
> -void hmp_info_ramblock(Monitor *mon, const QDict *qdict)
> -{
> - ram_block_dump(mon);
> -}
> -
> void hmp_info_vm_generation_id(Monitor *mon, const QDict *qdict)
> {
> Error *err = NULL;
> diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
> index 0a9ba7595c..a9766fa38d 100644
> --- a/monitor/qmp-cmds.c
> +++ b/monitor/qmp-cmds.c
> @@ -38,6 +38,7 @@
> #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"
> #include "hw/rdma/rdma.h"
> @@ -414,3 +415,10 @@ HumanReadableText *qmp_x_query_rdma(Error **errp)
>
> return human_readable_text_from_str(buf);
> }
> +
> +HumanReadableText *qmp_x_query_ramblock(Error **errp)
> +{
> + g_autoptr(GString) buf = ram_block_format();
> +
> + return human_readable_text_from_str(buf);
> +}
> diff --git a/qapi/machine.json b/qapi/machine.json
> index 1b2748c77a..be81170c2b 100644
> --- a/qapi/machine.json
> +++ b/qapi/machine.json
> @@ -1436,6 +1436,18 @@
> { 'command': 'x-query-profile',
> 'returns': 'HumanReadableText' }
>
> +##
> +# @x-query-ramblock:
> +#
> +# Query system ramblock information
> +#
> +# Returns: system ramblock information
> +#
> +# Since: 6.2
> +##
> +{ 'command': 'x-query-ramblock',
> + 'returns': 'HumanReadableText' }
> +
> ##
> # @x-query-rdma:
> #
> diff --git a/softmmu/physmem.c b/softmmu/physmem.c
> index b9a8c1d1f4..314f8b439c 100644
> --- a/softmmu/physmem.c
> +++ b/softmmu/physmem.c
> @@ -1296,23 +1296,26 @@ void qemu_mutex_unlock_ramlist(void)
> qemu_mutex_unlock(&ram_list.mutex);
> }
>
> -void ram_block_dump(Monitor *mon)
> +GString *ram_block_format(void)
> {
> RAMBlock *block;
> char *psize;
> + GString *buf = g_string_new("");
>
> RCU_READ_LOCK_GUARD();
> - monitor_printf(mon, "%24s %8s %18s %18s %18s\n",
> - "Block Name", "PSize", "Offset", "Used", "Total");
> + g_string_append_printf(buf, "%24s %8s %18s %18s %18s\n",
> + "Block Name", "PSize", "Offset", "Used", "Total");
> RAMBLOCK_FOREACH(block) {
> psize = size_to_str(block->page_size);
> - monitor_printf(mon, "%24s %8s 0x%016" PRIx64 " 0x%016" PRIx64
> - " 0x%016" PRIx64 "\n", block->idstr, psize,
> - (uint64_t)block->offset,
> - (uint64_t)block->used_length,
> - (uint64_t)block->max_length);
> + g_string_append_printf(buf, "%24s %8s 0x%016" PRIx64 " 0x%016" PRIx64
> + " 0x%016" PRIx64 "\n", block->idstr, psize,
> + (uint64_t)block->offset,
> + (uint64_t)block->used_length,
> + (uint64_t)block->max_length);
> g_free(psize);
> }
> +
> + return buf;
> }
>
> #ifdef __linux__
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-09 10:07 ` Claudio Fontana
@ 2022-06-09 10:19 ` Daniel P. Berrangé
2022-06-09 10:25 ` David Hildenbrand
2022-06-09 12:52 ` Dr. David Alan Gilbert
0 siblings, 2 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-06-09 10:19 UTC (permalink / raw)
To: Claudio Fontana
Cc: qemu-devel, Laurent Vivier, Thomas Huth, Eduardo Habkost,
David Hildenbrand, Michael Roth, Richard Henderson, Yuval Shaia,
Peter Xu, Dr. David Alan Gilbert, Gerd Hoffmann, Paolo Bonzini,
Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> Hello all,
>
> it would be really good to be able to rely on this command or something similar,
> to be able to know the approximate size of a migration before starting it.
>
> in QEMU ram_bytes_total() returns what I would like to have,
> but there is currently no QMP way to get it without starting a migration,
> which when trying to optimize it/size it is just about too late.
Aside from the main VM RAM, what other RAM blocks are likely to have
a size large enough to be of consequence to the live migration
data copy, and whose size is not already known to the mgmt app from
the guest config choices it made ? VGA RAM could be a few 100MB I
guess, but the mgmt app knows about that. I've always assumed everything
else is just noise in comparison to the main RAM region.
Still I wonder how useful this is as its just a static figure, and the
problems with migration transfer are the bulking up of data when the
VM is repeatedly dirtying stuff at a high rate.
> Do you think x-query-ramblock could be promoted to non-experimental?
It would have to be re-written, as this current impl is just emitting
a huge printf formatted string. To be considered supportable, the data
would have to be formally modelled in QAPI instead.
IOW, it would be a case of introducing a new command that emits formal
data, convertintg 'info ramblock' to use that, and then deprecating this
x-query-ramblock.
> Should another one be made available instead, like :
> query-ram-bytes-total ?
That would be simpler if you're just wanting it to give a single
figure.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-09 10:19 ` Daniel P. Berrangé
@ 2022-06-09 10:25 ` David Hildenbrand
2022-06-09 12:52 ` Dr. David Alan Gilbert
1 sibling, 0 replies; 27+ messages in thread
From: David Hildenbrand @ 2022-06-09 10:25 UTC (permalink / raw)
To: Daniel P. Berrangé, Claudio Fontana
Cc: qemu-devel, Laurent Vivier, Thomas Huth, Eduardo Habkost,
Michael Roth, Richard Henderson, Yuval Shaia, Peter Xu,
Dr. David Alan Gilbert, Gerd Hoffmann, Paolo Bonzini,
Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On 09.06.22 12:19, Daniel P. Berrangé wrote:
> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>> Hello all,
>>
>> it would be really good to be able to rely on this command or something similar,
>> to be able to know the approximate size of a migration before starting it.
>>
>> in QEMU ram_bytes_total() returns what I would like to have,
>> but there is currently no QMP way to get it without starting a migration,
>> which when trying to optimize it/size it is just about too late.
>
> Aside from the main VM RAM, what other RAM blocks are likely to have
> a size large enough to be of consequence to the live migration
> data copy, and whose size is not already known to the mgmt app from
> the guest config choices it made ? VGA RAM could be a few 100MB I
> guess, but the mgmt app knows about that. I've always assumed everything
> else is just noise in comparison to the main RAM region.
>
> Still I wonder how useful this is as its just a static figure, and the
> problems with migration transfer are the bulking up of data when the
> VM is repeatedly dirtying stuff at a high rate.
>
>> Do you think x-query-ramblock could be promoted to non-experimental?
>
> It would have to be re-written, as this current impl is just emitting
> a huge printf formatted string. To be considered supportable, the data
> would have to be formally modelled in QAPI instead.
>
> IOW, it would be a case of introducing a new command that emits formal
> data, convertintg 'info ramblock' to use that, and then deprecating this
> x-query-ramblock.
>
>> Should another one be made available instead, like :
>> query-ram-bytes-total ?
With virtio-balloon free page hinting and virtio-mem, that number does
not reflect reality (IOW, with sparse ramblocks the total size of
ramblocks is not expressive; it's rather a "worst case").
--
Thanks,
David / dhildenb
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-09 10:19 ` Daniel P. Berrangé
2022-06-09 10:25 ` David Hildenbrand
@ 2022-06-09 12:52 ` Dr. David Alan Gilbert
2022-06-30 10:14 ` Claudio Fontana
1 sibling, 1 reply; 27+ messages in thread
From: Dr. David Alan Gilbert @ 2022-06-09 12:52 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Claudio Fontana, qemu-devel, Laurent Vivier, Thomas Huth,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Gerd Hoffmann,
Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
* Daniel P. Berrangé (berrange@redhat.com) wrote:
> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> > Hello all,
> >
> > it would be really good to be able to rely on this command or something similar,
> > to be able to know the approximate size of a migration before starting it.
> >
> > in QEMU ram_bytes_total() returns what I would like to have,
> > but there is currently no QMP way to get it without starting a migration,
> > which when trying to optimize it/size it is just about too late.
>
> Aside from the main VM RAM, what other RAM blocks are likely to have
> a size large enough to be of consequence to the live migration
> data copy, and whose size is not already known to the mgmt app from
> the guest config choices it made ? VGA RAM could be a few 100MB I
> guess, but the mgmt app knows about that. I've always assumed everything
> else is just noise in comparison to the main RAM region.
>
> Still I wonder how useful this is as its just a static figure, and the
> problems with migration transfer are the bulking up of data when the
> VM is repeatedly dirtying stuff at a high rate.
>
> > Do you think x-query-ramblock could be promoted to non-experimental?
>
> It would have to be re-written, as this current impl is just emitting
> a huge printf formatted string. To be considered supportable, the data
> would have to be formally modelled in QAPI instead.
>
> IOW, it would be a case of introducing a new command that emits formal
> data, convertintg 'info ramblock' to use that, and then deprecating this
> x-query-ramblock.
>
> > Should another one be made available instead, like :
> > query-ram-bytes-total ?
>
> That would be simpler if you're just wanting it to give a single
> figure.
Is this what qmp_query_memory_size_summary does?
Dave
>
> 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 :|
>
--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-09 12:52 ` Dr. David Alan Gilbert
@ 2022-06-30 10:14 ` Claudio Fontana
2022-06-30 10:20 ` Daniel P. Berrangé
0 siblings, 1 reply; 27+ messages in thread
From: Claudio Fontana @ 2022-06-30 10:14 UTC (permalink / raw)
To: Dr. David Alan Gilbert, Daniel P. Berrangé
Cc: qemu-devel, Laurent Vivier, Thomas Huth, Eduardo Habkost,
David Hildenbrand, Michael Roth, Richard Henderson, Yuval Shaia,
Peter Xu, Gerd Hoffmann, Paolo Bonzini,
Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>>> Hello all,
>>>
>>> it would be really good to be able to rely on this command or something similar,
>>> to be able to know the approximate size of a migration before starting it.
>>>
>>> in QEMU ram_bytes_total() returns what I would like to have,
>>> but there is currently no QMP way to get it without starting a migration,
>>> which when trying to optimize it/size it is just about too late.
>>
>> Aside from the main VM RAM, what other RAM blocks are likely to have
>> a size large enough to be of consequence to the live migration
>> data copy, and whose size is not already known to the mgmt app from
>> the guest config choices it made ? VGA RAM could be a few 100MB I
>> guess, but the mgmt app knows about that. I've always assumed everything
>> else is just noise in comparison to the main RAM region.
>>
>> Still I wonder how useful this is as its just a static figure, and the
>> problems with migration transfer are the bulking up of data when the
>> VM is repeatedly dirtying stuff at a high rate.
>>
>>> Do you think x-query-ramblock could be promoted to non-experimental?
>>
>> It would have to be re-written, as this current impl is just emitting
>> a huge printf formatted string. To be considered supportable, the data
>> would have to be formally modelled in QAPI instead.
>>
>> IOW, it would be a case of introducing a new command that emits formal
>> data, convertintg 'info ramblock' to use that, and then deprecating this
>> x-query-ramblock.
>>
>>> Should another one be made available instead, like :
>>> query-ram-bytes-total ?
>>
>> That would be simpler if you're just wanting it to give a single
>> figure.
>
> Is this what qmp_query_memory_size_summary does?
No, I am not looking at something returning the machine->ram_size,
but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.
This would be the return value of something like migration/ram.c::ram_bytes_total().
The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.
If there was such a feature that just returns ram_bytes_total via QMP,
by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:
https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/
The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
but still incurs in a 4-5% performance penalty compared with the multiple files approach,
that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).
Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.
Feedback on the interleaved format I posted there is welcome as well,
Thanks,
Claudio
>
> Dave
>
>>
>> With regards,
>> Daniel
>> --
>> |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
>> |: https://libvirt.org -o- https://fstop138.berrange.com :|
>> |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
>>
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-30 10:14 ` Claudio Fontana
@ 2022-06-30 10:20 ` Daniel P. Berrangé
2022-06-30 12:55 ` Claudio Fontana
0 siblings, 1 reply; 27+ messages in thread
From: Daniel P. Berrangé @ 2022-06-30 10:20 UTC (permalink / raw)
To: Claudio Fontana
Cc: Dr. David Alan Gilbert, qemu-devel, Laurent Vivier, Thomas Huth,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Gerd Hoffmann,
Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On Thu, Jun 30, 2022 at 12:14:36PM +0200, Claudio Fontana wrote:
> On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
> > * Daniel P. Berrangé (berrange@redhat.com) wrote:
> >> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
> >>> Hello all,
> >>>
> >>> it would be really good to be able to rely on this command or something similar,
> >>> to be able to know the approximate size of a migration before starting it.
> >>>
> >>> in QEMU ram_bytes_total() returns what I would like to have,
> >>> but there is currently no QMP way to get it without starting a migration,
> >>> which when trying to optimize it/size it is just about too late.
> >>
> >> Aside from the main VM RAM, what other RAM blocks are likely to have
> >> a size large enough to be of consequence to the live migration
> >> data copy, and whose size is not already known to the mgmt app from
> >> the guest config choices it made ? VGA RAM could be a few 100MB I
> >> guess, but the mgmt app knows about that. I've always assumed everything
> >> else is just noise in comparison to the main RAM region.
> >>
> >> Still I wonder how useful this is as its just a static figure, and the
> >> problems with migration transfer are the bulking up of data when the
> >> VM is repeatedly dirtying stuff at a high rate.
> >>
> >>> Do you think x-query-ramblock could be promoted to non-experimental?
> >>
> >> It would have to be re-written, as this current impl is just emitting
> >> a huge printf formatted string. To be considered supportable, the data
> >> would have to be formally modelled in QAPI instead.
> >>
> >> IOW, it would be a case of introducing a new command that emits formal
> >> data, convertintg 'info ramblock' to use that, and then deprecating this
> >> x-query-ramblock.
> >>
> >>> Should another one be made available instead, like :
> >>> query-ram-bytes-total ?
> >>
> >> That would be simpler if you're just wanting it to give a single
> >> figure.
> >
> > Is this what qmp_query_memory_size_summary does?
>
> No, I am not looking at something returning the machine->ram_size,
> but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.
>
> This would be the return value of something like migration/ram.c::ram_bytes_total().
>
> The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.
>
> If there was such a feature that just returns ram_bytes_total via QMP,
> by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:
>
> https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/
>
> The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
> but still incurs in a 4-5% performance penalty compared with the multiple files approach,
> that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).
>
> Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
> but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.
>
> Feedback on the interleaved format I posted there is welcome as well,
I still believe that libvirt is the wrong place to be implementing any
of this logic. It all belongs in QEMU, because QEMU is the place which
holds all the information needed to do an optimal job, and libvirt does
not, as this request for extra QMP features shows.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PULL 15/18] qapi: introduce x-query-ramblock QMP command
2022-06-30 10:20 ` Daniel P. Berrangé
@ 2022-06-30 12:55 ` Claudio Fontana
0 siblings, 0 replies; 27+ messages in thread
From: Claudio Fontana @ 2022-06-30 12:55 UTC (permalink / raw)
To: Daniel P. Berrangé
Cc: Dr. David Alan Gilbert, qemu-devel, Laurent Vivier, Thomas Huth,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Gerd Hoffmann,
Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On 6/30/22 12:20, Daniel P. Berrangé wrote:
> On Thu, Jun 30, 2022 at 12:14:36PM +0200, Claudio Fontana wrote:
>> On 6/9/22 14:52, Dr. David Alan Gilbert wrote:
>>> * Daniel P. Berrangé (berrange@redhat.com) wrote:
>>>> On Thu, Jun 09, 2022 at 12:07:31PM +0200, Claudio Fontana wrote:
>>>>> Hello all,
>>>>>
>>>>> it would be really good to be able to rely on this command or something similar,
>>>>> to be able to know the approximate size of a migration before starting it.
>>>>>
>>>>> in QEMU ram_bytes_total() returns what I would like to have,
>>>>> but there is currently no QMP way to get it without starting a migration,
>>>>> which when trying to optimize it/size it is just about too late.
>>>>
>>>> Aside from the main VM RAM, what other RAM blocks are likely to have
>>>> a size large enough to be of consequence to the live migration
>>>> data copy, and whose size is not already known to the mgmt app from
>>>> the guest config choices it made ? VGA RAM could be a few 100MB I
>>>> guess, but the mgmt app knows about that. I've always assumed everything
>>>> else is just noise in comparison to the main RAM region.
>>>>
>>>> Still I wonder how useful this is as its just a static figure, and the
>>>> problems with migration transfer are the bulking up of data when the
>>>> VM is repeatedly dirtying stuff at a high rate.
>>>>
>>>>> Do you think x-query-ramblock could be promoted to non-experimental?
>>>>
>>>> It would have to be re-written, as this current impl is just emitting
>>>> a huge printf formatted string. To be considered supportable, the data
>>>> would have to be formally modelled in QAPI instead.
>>>>
>>>> IOW, it would be a case of introducing a new command that emits formal
>>>> data, convertintg 'info ramblock' to use that, and then deprecating this
>>>> x-query-ramblock.
>>>>
>>>>> Should another one be made available instead, like :
>>>>> query-ram-bytes-total ?
>>>>
>>>> That would be simpler if you're just wanting it to give a single
>>>> figure.
>>>
>>> Is this what qmp_query_memory_size_summary does?
>>
>> No, I am not looking at something returning the machine->ram_size,
>> but rather how many bytes are actually used in each RAMBlock, in order to estimate the transfer size of a guest to disk.
>>
>> This would be the return value of something like migration/ram.c::ram_bytes_total().
>>
>> The main guest RAM total size is in most cases an overestimation of the actual bytes required to be transferred.
>>
>> If there was such a feature that just returns ram_bytes_total via QMP,
>> by knowing the size in bytes before the transfer, we can prealloc the space on disk, which would improve the performance of this series:
>>
>> https://patchew.org/Libvirt/20220607091936.7948-1-cfontana@suse.de/
>>
>> The interleaved format I posted there works just fine to migrate a suspended VM to disk (virsh save) from multifd channels to a single file,
>> but still incurs in a 4-5% performance penalty compared with the multiple files approach,
>> that is apparently due to multiple threads competing on acquiring locks to adjust the file size (on XFS).
>>
>> Doing a fallocate() would likely remove this performance decrease compared with multifd to multiple files,
>> but requires knowing beforehand the approximate size of the transfer, and as mentioned mnachine->ram_size is just overkill in practice and risks erroring out if not enough space is available.
>>
>> Feedback on the interleaved format I posted there is welcome as well,
>
> I still believe that libvirt is the wrong place to be implementing any
> of this logic. It all belongs in QEMU, because QEMU is the place which
> holds all the information needed to do an optimal job, and libvirt does
> not, as this request for extra QMP features shows.
>
> With regards,
> Daniel
Hi Daniel,
I know your position about the implementation in libvirt vs a potential (non-existing for now) QEMU implementation.
The implementation in QEMU seems to me to require more investment due to the need of a new migration target protocol to be defined carefully (possibly "disk://")
and the need to alter and test all migrated devices participating in the creating the migration stream.
I don't think this request for an QMP feature shows anything really.
Knowing the _actual_ size of a migration stream before deciding to migrate is I think a pretty useful feature in itself I would think,
including for libvirt and higher level components in the stack. The lack of the feature just shows, well, the lack of this feature.
Regarding my prototype I pointed at that happens to use libvirt, I wonder if you or anyone have any feedback on the actual format of the VM saved in parallel to disk,
regardless of which component writes it, libvirt or QEMU, and I wonder also if there is any feedback on the O_DIRECT -friendly I/O API, which are both things we can make progress on also regardless of the libvirt vs QEMU implementation of the parallel migration to disk.
Thanks,
Claudio
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PULL 16/18] qapi: introduce x-query-irq QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (14 preceding siblings ...)
2021-11-02 17:56 ` [PULL 15/18] qapi: introduce x-query-ramblock " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:56 ` [PULL 17/18] qapi: introduce x-query-jit " Daniel P. Berrangé
` (2 subsequent siblings)
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info irq" command. It is being
added with an "x-" prefix because this QMP command is intended as an
adhoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
hmp-commands-info.hx | 2 +-
monitor/hmp-cmds.c | 38 --------------------------------------
monitor/qmp-cmds.c | 44 ++++++++++++++++++++++++++++++++++++++++++++
qapi/machine.json | 12 ++++++++++++
4 files changed, 57 insertions(+), 39 deletions(-)
diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx
index c2d7275bf5..407a1da800 100644
--- a/hmp-commands-info.hx
+++ b/hmp-commands-info.hx
@@ -159,7 +159,7 @@ ERST
.args_type = "",
.params = "",
.help = "show the interrupts statistics (if available)",
- .cmd = hmp_info_irq,
+ .cmd_info_hrt = qmp_x_query_irq,
},
SRST
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 90f9a64573..8ef605e29a 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -784,44 +784,6 @@ static void hmp_info_pci_device(Monitor *mon, const PciDeviceInfo *dev)
}
}
-static int hmp_info_irq_foreach(Object *obj, void *opaque)
-{
- InterruptStatsProvider *intc;
- InterruptStatsProviderClass *k;
- Monitor *mon = opaque;
-
- if (object_dynamic_cast(obj, TYPE_INTERRUPT_STATS_PROVIDER)) {
- intc = INTERRUPT_STATS_PROVIDER(obj);
- k = INTERRUPT_STATS_PROVIDER_GET_CLASS(obj);
- uint64_t *irq_counts;
- unsigned int nb_irqs, i;
- if (k->get_statistics &&
- k->get_statistics(intc, &irq_counts, &nb_irqs)) {
- if (nb_irqs > 0) {
- monitor_printf(mon, "IRQ statistics for %s:\n",
- object_get_typename(obj));
- for (i = 0; i < nb_irqs; i++) {
- if (irq_counts[i] > 0) {
- monitor_printf(mon, "%2d: %" PRId64 "\n", i,
- irq_counts[i]);
- }
- }
- }
- } else {
- monitor_printf(mon, "IRQ statistics not available for %s.\n",
- object_get_typename(obj));
- }
- }
-
- return 0;
-}
-
-void hmp_info_irq(Monitor *mon, const QDict *qdict)
-{
- object_child_foreach_recursive(object_get_root(),
- hmp_info_irq_foreach, mon);
-}
-
static int hmp_info_pic_foreach(Object *obj, void *opaque)
{
InterruptStatsProvider *intc;
diff --git a/monitor/qmp-cmds.c b/monitor/qmp-cmds.c
index a9766fa38d..343353e27a 100644
--- a/monitor/qmp-cmds.c
+++ b/monitor/qmp-cmds.c
@@ -41,6 +41,7 @@
#include "exec/ramlist.h"
#include "hw/mem/memory-device.h"
#include "hw/acpi/acpi_dev_interface.h"
+#include "hw/intc/intc.h"
#include "hw/rdma/rdma.h"
NameInfo *qmp_query_name(Error **errp)
@@ -422,3 +423,46 @@ HumanReadableText *qmp_x_query_ramblock(Error **errp)
return human_readable_text_from_str(buf);
}
+
+static int qmp_x_query_irq_foreach(Object *obj, void *opaque)
+{
+ InterruptStatsProvider *intc;
+ InterruptStatsProviderClass *k;
+ GString *buf = opaque;
+
+ if (object_dynamic_cast(obj, TYPE_INTERRUPT_STATS_PROVIDER)) {
+ intc = INTERRUPT_STATS_PROVIDER(obj);
+ k = INTERRUPT_STATS_PROVIDER_GET_CLASS(obj);
+ uint64_t *irq_counts;
+ unsigned int nb_irqs, i;
+ if (k->get_statistics &&
+ k->get_statistics(intc, &irq_counts, &nb_irqs)) {
+ if (nb_irqs > 0) {
+ g_string_append_printf(buf, "IRQ statistics for %s:\n",
+ object_get_typename(obj));
+ for (i = 0; i < nb_irqs; i++) {
+ if (irq_counts[i] > 0) {
+ g_string_append_printf(buf, "%2d: %" PRId64 "\n", i,
+ irq_counts[i]);
+ }
+ }
+ }
+ } else {
+ g_string_append_printf(buf,
+ "IRQ statistics not available for %s.\n",
+ object_get_typename(obj));
+ }
+ }
+
+ return 0;
+}
+
+HumanReadableText *qmp_x_query_irq(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+
+ object_child_foreach_recursive(object_get_root(),
+ qmp_x_query_irq_foreach, buf);
+
+ return human_readable_text_from_str(buf);
+}
diff --git a/qapi/machine.json b/qapi/machine.json
index be81170c2b..ca49358292 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1412,6 +1412,18 @@
'*threads': 'int',
'*maxcpus': 'int' } }
+##
+# @x-query-irq:
+#
+# Query interrupt statistics
+#
+# Returns: interrupt statistics
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-irq',
+ 'returns': 'HumanReadableText' }
+
##
# @x-query-numa:
#
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 17/18] qapi: introduce x-query-jit QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (15 preceding siblings ...)
2021-11-02 17:56 ` [PULL 16/18] qapi: introduce x-query-irq " Daniel P. Berrangé
@ 2021-11-02 17:56 ` Daniel P. Berrangé
2021-11-02 17:57 ` [PULL 18/18] qapi: introduce x-query-opcount " Daniel P. Berrangé
2021-11-03 13:30 ` [PULL 00/18] HMP-to-QMP info command patches Richard Henderson
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:56 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info jit" command. It is being
added with an "x-" prefix because this QMP command is intended as an
ad hoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
accel/tcg/cpu-exec.c | 37 ++++++++++++----
accel/tcg/hmp.c | 15 ++-----
accel/tcg/translate-all.c | 80 ++++++++++++++++++----------------
include/exec/cpu-all.h | 4 +-
include/tcg/tcg.h | 2 +-
qapi/machine.json | 13 ++++++
tcg/tcg.c | 88 ++++++++++++++++++++------------------
tests/qtest/qmp-cmd-test.c | 2 +
8 files changed, 140 insertions(+), 101 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index c9764c1325..4212645cb6 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -20,6 +20,9 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qemu/qemu-print.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
+#include "qapi/type-helpers.h"
#include "hw/core/tcg-cpu-ops.h"
#include "trace.h"
#include "disas/disas.h"
@@ -38,6 +41,7 @@
#include "exec/cpu-all.h"
#include "sysemu/cpu-timers.h"
#include "sysemu/replay.h"
+#include "sysemu/tcg.h"
#include "exec/helper-proto.h"
#include "tb-hash.h"
#include "tb-context.h"
@@ -1028,23 +1032,38 @@ void tcg_exec_unrealizefn(CPUState *cpu)
#ifndef CONFIG_USER_ONLY
-void dump_drift_info(void)
+void dump_drift_info(GString *buf)
{
if (!icount_enabled()) {
return;
}
- qemu_printf("Host - Guest clock %"PRIi64" ms\n",
- (cpu_get_clock() - icount_get()) / SCALE_MS);
+ g_string_append_printf(buf, "Host - Guest clock %"PRIi64" ms\n",
+ (cpu_get_clock() - icount_get()) / SCALE_MS);
if (icount_align_option) {
- qemu_printf("Max guest delay %"PRIi64" ms\n",
- -max_delay / SCALE_MS);
- qemu_printf("Max guest advance %"PRIi64" ms\n",
- max_advance / SCALE_MS);
+ g_string_append_printf(buf, "Max guest delay %"PRIi64" ms\n",
+ -max_delay / SCALE_MS);
+ g_string_append_printf(buf, "Max guest advance %"PRIi64" ms\n",
+ max_advance / SCALE_MS);
} else {
- qemu_printf("Max guest delay NA\n");
- qemu_printf("Max guest advance NA\n");
+ g_string_append_printf(buf, "Max guest delay NA\n");
+ g_string_append_printf(buf, "Max guest advance NA\n");
}
}
+HumanReadableText *qmp_x_query_jit(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+
+ if (!tcg_enabled()) {
+ error_setg(errp, "JIT information is only available with accel=tcg");
+ return NULL;
+ }
+
+ dump_exec_info(buf);
+ dump_drift_info(buf);
+
+ return human_readable_text_from_str(buf);
+}
+
#endif /* !CONFIG_USER_ONLY */
diff --git a/accel/tcg/hmp.c b/accel/tcg/hmp.c
index a6e72fdb3e..01c767a464 100644
--- a/accel/tcg/hmp.c
+++ b/accel/tcg/hmp.c
@@ -1,20 +1,11 @@
#include "qemu/osdep.h"
#include "qemu/error-report.h"
+#include "qapi/error.h"
+#include "qapi/qapi-commands-machine.h"
#include "exec/exec-all.h"
#include "monitor/monitor.h"
#include "sysemu/tcg.h"
-static void hmp_info_jit(Monitor *mon, const QDict *qdict)
-{
- if (!tcg_enabled()) {
- error_report("JIT information is only available with accel=tcg");
- return;
- }
-
- dump_exec_info();
- dump_drift_info();
-}
-
static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
{
dump_opcount_info();
@@ -22,7 +13,7 @@ static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
static void hmp_tcg_register(void)
{
- monitor_register_hmp("jit", true, hmp_info_jit);
+ monitor_register_hmp_info_hrt("jit", qmp_x_query_jit);
monitor_register_hmp("opcount", true, hmp_info_opcount);
}
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index fb9ebfad9e..8f17a91858 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1991,7 +1991,7 @@ void cpu_io_recompile(CPUState *cpu, uintptr_t retaddr)
cpu_loop_exit_noexc(cpu);
}
-static void print_qht_statistics(struct qht_stats hst)
+static void print_qht_statistics(struct qht_stats hst, GString *buf)
{
uint32_t hgram_opts;
size_t hgram_bins;
@@ -2000,9 +2000,11 @@ static void print_qht_statistics(struct qht_stats hst)
if (!hst.head_buckets) {
return;
}
- qemu_printf("TB hash buckets %zu/%zu (%0.2f%% head buckets used)\n",
- hst.used_head_buckets, hst.head_buckets,
- (double)hst.used_head_buckets / hst.head_buckets * 100);
+ g_string_append_printf(buf, "TB hash buckets %zu/%zu "
+ "(%0.2f%% head buckets used)\n",
+ hst.used_head_buckets, hst.head_buckets,
+ (double)hst.used_head_buckets /
+ hst.head_buckets * 100);
hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
hgram_opts |= QDIST_PR_100X | QDIST_PR_PERCENT;
@@ -2010,8 +2012,9 @@ static void print_qht_statistics(struct qht_stats hst)
hgram_opts |= QDIST_PR_NODECIMAL;
}
hgram = qdist_pr(&hst.occupancy, 10, hgram_opts);
- qemu_printf("TB hash occupancy %0.2f%% avg chain occ. Histogram: %s\n",
- qdist_avg(&hst.occupancy) * 100, hgram);
+ g_string_append_printf(buf, "TB hash occupancy %0.2f%% avg chain occ. "
+ "Histogram: %s\n",
+ qdist_avg(&hst.occupancy) * 100, hgram);
g_free(hgram);
hgram_opts = QDIST_PR_BORDER | QDIST_PR_LABELS;
@@ -2023,8 +2026,9 @@ static void print_qht_statistics(struct qht_stats hst)
hgram_opts |= QDIST_PR_NODECIMAL | QDIST_PR_NOBINRANGE;
}
hgram = qdist_pr(&hst.chain, hgram_bins, hgram_opts);
- qemu_printf("TB hash avg chain %0.3f buckets. Histogram: %s\n",
- qdist_avg(&hst.chain), hgram);
+ g_string_append_printf(buf, "TB hash avg chain %0.3f buckets. "
+ "Histogram: %s\n",
+ qdist_avg(&hst.chain), hgram);
g_free(hgram);
}
@@ -2061,7 +2065,7 @@ static gboolean tb_tree_stats_iter(gpointer key, gpointer value, gpointer data)
return false;
}
-void dump_exec_info(void)
+void dump_exec_info(GString *buf)
{
struct tb_tree_stats tst = {};
struct qht_stats hst;
@@ -2070,44 +2074,48 @@ void dump_exec_info(void)
tcg_tb_foreach(tb_tree_stats_iter, &tst);
nb_tbs = tst.nb_tbs;
/* XXX: avoid using doubles ? */
- qemu_printf("Translation buffer state:\n");
+ g_string_append_printf(buf, "Translation buffer state:\n");
/*
* Report total code size including the padding and TB structs;
* otherwise users might think "-accel tcg,tb-size" is not honoured.
* For avg host size we use the precise numbers from tb_tree_stats though.
*/
- qemu_printf("gen code size %zu/%zu\n",
- tcg_code_size(), tcg_code_capacity());
- qemu_printf("TB count %zu\n", nb_tbs);
- qemu_printf("TB avg target size %zu max=%zu bytes\n",
- nb_tbs ? tst.target_size / nb_tbs : 0,
- tst.max_target_size);
- qemu_printf("TB avg host size %zu bytes (expansion ratio: %0.1f)\n",
- nb_tbs ? tst.host_size / nb_tbs : 0,
- tst.target_size ? (double)tst.host_size / tst.target_size : 0);
- qemu_printf("cross page TB count %zu (%zu%%)\n", tst.cross_page,
- nb_tbs ? (tst.cross_page * 100) / nb_tbs : 0);
- qemu_printf("direct jump count %zu (%zu%%) (2 jumps=%zu %zu%%)\n",
- tst.direct_jmp_count,
- nb_tbs ? (tst.direct_jmp_count * 100) / nb_tbs : 0,
- tst.direct_jmp2_count,
- nb_tbs ? (tst.direct_jmp2_count * 100) / nb_tbs : 0);
+ g_string_append_printf(buf, "gen code size %zu/%zu\n",
+ tcg_code_size(), tcg_code_capacity());
+ g_string_append_printf(buf, "TB count %zu\n", nb_tbs);
+ g_string_append_printf(buf, "TB avg target size %zu max=%zu bytes\n",
+ nb_tbs ? tst.target_size / nb_tbs : 0,
+ tst.max_target_size);
+ g_string_append_printf(buf, "TB avg host size %zu bytes "
+ "(expansion ratio: %0.1f)\n",
+ nb_tbs ? tst.host_size / nb_tbs : 0,
+ tst.target_size ?
+ (double)tst.host_size / tst.target_size : 0);
+ g_string_append_printf(buf, "cross page TB count %zu (%zu%%)\n",
+ tst.cross_page,
+ nb_tbs ? (tst.cross_page * 100) / nb_tbs : 0);
+ g_string_append_printf(buf, "direct jump count %zu (%zu%%) "
+ "(2 jumps=%zu %zu%%)\n",
+ tst.direct_jmp_count,
+ nb_tbs ? (tst.direct_jmp_count * 100) / nb_tbs : 0,
+ tst.direct_jmp2_count,
+ nb_tbs ? (tst.direct_jmp2_count * 100) / nb_tbs : 0);
qht_statistics_init(&tb_ctx.htable, &hst);
- print_qht_statistics(hst);
+ print_qht_statistics(hst, buf);
qht_statistics_destroy(&hst);
- qemu_printf("\nStatistics:\n");
- qemu_printf("TB flush count %u\n",
- qatomic_read(&tb_ctx.tb_flush_count));
- qemu_printf("TB invalidate count %u\n",
- qatomic_read(&tb_ctx.tb_phys_invalidate_count));
+ g_string_append_printf(buf, "\nStatistics:\n");
+ g_string_append_printf(buf, "TB flush count %u\n",
+ qatomic_read(&tb_ctx.tb_flush_count));
+ g_string_append_printf(buf, "TB invalidate count %u\n",
+ qatomic_read(&tb_ctx.tb_phys_invalidate_count));
tlb_flush_counts(&flush_full, &flush_part, &flush_elide);
- qemu_printf("TLB full flushes %zu\n", flush_full);
- qemu_printf("TLB partial flushes %zu\n", flush_part);
- qemu_printf("TLB elided flushes %zu\n", flush_elide);
- tcg_dump_info();
+ g_string_append_printf(buf, "TLB full flushes %zu\n", flush_full);
+ g_string_append_printf(buf, "TLB partial flushes %zu\n", flush_part);
+ g_string_append_printf(buf, "TLB elided flushes %zu\n", flush_elide);
+ tcg_dump_info(buf);
}
void dump_opcount_info(void)
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index 32cfb634c6..d92f6fa7a9 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -429,9 +429,9 @@ static inline bool tlb_hit(target_ulong tlb_addr, target_ulong addr)
#ifdef CONFIG_TCG
/* accel/tcg/cpu-exec.c */
-void dump_drift_info(void);
+void dump_drift_info(GString *buf);
/* accel/tcg/translate-all.c */
-void dump_exec_info(void);
+void dump_exec_info(GString *buf);
void dump_opcount_info(void);
#endif /* CONFIG_TCG */
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 7069a401f1..7c8019a9b2 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -937,7 +937,7 @@ int tcg_check_temp_count(void);
#endif
int64_t tcg_cpu_exec_time(void);
-void tcg_dump_info(void);
+void tcg_dump_info(GString *buf);
void tcg_dump_op_count(void);
#define TCG_CT_CONST 1 /* any constant of register size */
diff --git a/qapi/machine.json b/qapi/machine.json
index ca49358292..422a44661f 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1424,6 +1424,19 @@
{ 'command': 'x-query-irq',
'returns': 'HumanReadableText' }
+##
+# @x-query-jit:
+#
+# Query TCG compiler statistics
+#
+# Returns: TCG compiler statistics
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-jit',
+ 'returns': 'HumanReadableText',
+ 'if': 'CONFIG_TCG' }
+
##
# @x-query-numa:
#
diff --git a/tcg/tcg.c b/tcg/tcg.c
index 6332cdceca..f9ede8e62e 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4383,7 +4383,7 @@ int tcg_gen_code(TCGContext *s, TranslationBlock *tb)
}
#ifdef CONFIG_PROFILER
-void tcg_dump_info(void)
+void tcg_dump_info(GString *buf)
{
TCGProfile prof = {};
const TCGProfile *s;
@@ -4397,53 +4397,59 @@ void tcg_dump_info(void)
tb_div_count = tb_count ? tb_count : 1;
tot = s->interm_time + s->code_time;
- qemu_printf("JIT cycles %" PRId64 " (%0.3f s at 2.4 GHz)\n",
- tot, tot / 2.4e9);
- qemu_printf("translated TBs %" PRId64 " (aborted=%" PRId64
- " %0.1f%%)\n",
- tb_count, s->tb_count1 - tb_count,
- (double)(s->tb_count1 - s->tb_count)
- / (s->tb_count1 ? s->tb_count1 : 1) * 100.0);
- qemu_printf("avg ops/TB %0.1f max=%d\n",
- (double)s->op_count / tb_div_count, s->op_count_max);
- qemu_printf("deleted ops/TB %0.2f\n",
- (double)s->del_op_count / tb_div_count);
- qemu_printf("avg temps/TB %0.2f max=%d\n",
- (double)s->temp_count / tb_div_count, s->temp_count_max);
- qemu_printf("avg host code/TB %0.1f\n",
- (double)s->code_out_len / tb_div_count);
- qemu_printf("avg search data/TB %0.1f\n",
- (double)s->search_out_len / tb_div_count);
+ g_string_append_printf(buf, "JIT cycles %" PRId64
+ " (%0.3f s at 2.4 GHz)\n",
+ tot, tot / 2.4e9);
+ g_string_append_printf(buf, "translated TBs %" PRId64
+ " (aborted=%" PRId64 " %0.1f%%)\n",
+ tb_count, s->tb_count1 - tb_count,
+ (double)(s->tb_count1 - s->tb_count)
+ / (s->tb_count1 ? s->tb_count1 : 1) * 100.0);
+ g_string_append_printf(buf, "avg ops/TB %0.1f max=%d\n",
+ (double)s->op_count / tb_div_count, s->op_count_max);
+ g_string_append_printf(buf, "deleted ops/TB %0.2f\n",
+ (double)s->del_op_count / tb_div_count);
+ g_string_append_printf(buf, "avg temps/TB %0.2f max=%d\n",
+ (double)s->temp_count / tb_div_count,
+ s->temp_count_max);
+ g_string_append_printf(buf, "avg host code/TB %0.1f\n",
+ (double)s->code_out_len / tb_div_count);
+ g_string_append_printf(buf, "avg search data/TB %0.1f\n",
+ (double)s->search_out_len / tb_div_count);
- qemu_printf("cycles/op %0.1f\n",
- s->op_count ? (double)tot / s->op_count : 0);
- qemu_printf("cycles/in byte %0.1f\n",
- s->code_in_len ? (double)tot / s->code_in_len : 0);
- qemu_printf("cycles/out byte %0.1f\n",
- s->code_out_len ? (double)tot / s->code_out_len : 0);
- qemu_printf("cycles/search byte %0.1f\n",
- s->search_out_len ? (double)tot / s->search_out_len : 0);
+ g_string_append_printf(buf, "cycles/op %0.1f\n",
+ s->op_count ? (double)tot / s->op_count : 0);
+ g_string_append_printf(buf, "cycles/in byte %0.1f\n",
+ s->code_in_len ? (double)tot / s->code_in_len : 0);
+ g_string_append_printf(buf, "cycles/out byte %0.1f\n",
+ s->code_out_len ? (double)tot / s->code_out_len : 0);
+ g_string_append_printf(buf, "cycles/search byte %0.1f\n",
+ s->search_out_len ?
+ (double)tot / s->search_out_len : 0);
if (tot == 0) {
tot = 1;
}
- qemu_printf(" gen_interm time %0.1f%%\n",
- (double)s->interm_time / tot * 100.0);
- qemu_printf(" gen_code time %0.1f%%\n",
- (double)s->code_time / tot * 100.0);
- qemu_printf("optim./code time %0.1f%%\n",
- (double)s->opt_time / (s->code_time ? s->code_time : 1)
- * 100.0);
- qemu_printf("liveness/code time %0.1f%%\n",
- (double)s->la_time / (s->code_time ? s->code_time : 1) * 100.0);
- qemu_printf("cpu_restore count %" PRId64 "\n",
- s->restore_count);
- qemu_printf(" avg cycles %0.1f\n",
- s->restore_count ? (double)s->restore_time / s->restore_count : 0);
+ g_string_append_printf(buf, " gen_interm time %0.1f%%\n",
+ (double)s->interm_time / tot * 100.0);
+ g_string_append_printf(buf, " gen_code time %0.1f%%\n",
+ (double)s->code_time / tot * 100.0);
+ g_string_append_printf(buf, "optim./code time %0.1f%%\n",
+ (double)s->opt_time / (s->code_time ?
+ s->code_time : 1)
+ * 100.0);
+ g_string_append_printf(buf, "liveness/code time %0.1f%%\n",
+ (double)s->la_time / (s->code_time ?
+ s->code_time : 1) * 100.0);
+ g_string_append_printf(buf, "cpu_restore count %" PRId64 "\n",
+ s->restore_count);
+ g_string_append_printf(buf, " avg cycles %0.1f\n",
+ s->restore_count ?
+ (double)s->restore_time / s->restore_count : 0);
}
#else
-void tcg_dump_info(void)
+void tcg_dump_info(GString *buf)
{
- qemu_printf("[TCG profiler not compiled]\n");
+ g_string_append_printf(buf, "[TCG profiler not compiled]\n");
}
#endif
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index 0d52ea6c4b..ea24fde1a3 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -51,6 +51,8 @@ static int query_error_class(const char *cmd)
#endif
/* Only valid with a USB bus added */
{ "x-query-usb", ERROR_CLASS_GENERIC_ERROR },
+ /* Only valid with accel=tcg */
+ { "x-query-jit", ERROR_CLASS_GENERIC_ERROR },
{ NULL, -1 }
};
int i;
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* [PULL 18/18] qapi: introduce x-query-opcount QMP command
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (16 preceding siblings ...)
2021-11-02 17:56 ` [PULL 17/18] qapi: introduce x-query-jit " Daniel P. Berrangé
@ 2021-11-02 17:57 ` Daniel P. Berrangé
2021-11-03 13:30 ` [PULL 00/18] HMP-to-QMP info command patches Richard Henderson
18 siblings, 0 replies; 27+ messages in thread
From: Daniel P. Berrangé @ 2021-11-02 17:57 UTC (permalink / raw)
To: qemu-devel
Cc: Laurent Vivier, Thomas Huth, Daniel P. Berrangé,
Eduardo Habkost, David Hildenbrand, Michael Roth,
Richard Henderson, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
This is a counterpart to the HMP "info opcount" command. It is being
added with an "x-" prefix because this QMP command is intended as an
ad hoc debugging tool and will thus not be modelled in QAPI as fully
structured data, nor will it have long term guaranteed stability.
The existing HMP command is rewritten to call the QMP command.
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
accel/tcg/cpu-exec.c | 14 ++++++++++++++
accel/tcg/hmp.c | 7 +------
accel/tcg/translate-all.c | 4 ++--
include/exec/cpu-all.h | 2 +-
include/tcg/tcg.h | 2 +-
qapi/machine.json | 13 +++++++++++++
tcg/tcg.c | 10 +++++-----
tests/qtest/qmp-cmd-test.c | 1 +
8 files changed, 38 insertions(+), 15 deletions(-)
diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
index 4212645cb6..c0620b4cc7 100644
--- a/accel/tcg/cpu-exec.c
+++ b/accel/tcg/cpu-exec.c
@@ -1066,4 +1066,18 @@ HumanReadableText *qmp_x_query_jit(Error **errp)
return human_readable_text_from_str(buf);
}
+HumanReadableText *qmp_x_query_opcount(Error **errp)
+{
+ g_autoptr(GString) buf = g_string_new("");
+
+ if (!tcg_enabled()) {
+ error_setg(errp, "Opcode count information is only available with accel=tcg");
+ return NULL;
+ }
+
+ dump_opcount_info(buf);
+
+ return human_readable_text_from_str(buf);
+}
+
#endif /* !CONFIG_USER_ONLY */
diff --git a/accel/tcg/hmp.c b/accel/tcg/hmp.c
index 01c767a464..d2ea352655 100644
--- a/accel/tcg/hmp.c
+++ b/accel/tcg/hmp.c
@@ -6,15 +6,10 @@
#include "monitor/monitor.h"
#include "sysemu/tcg.h"
-static void hmp_info_opcount(Monitor *mon, const QDict *qdict)
-{
- dump_opcount_info();
-}
-
static void hmp_tcg_register(void)
{
monitor_register_hmp_info_hrt("jit", qmp_x_query_jit);
- monitor_register_hmp("opcount", true, hmp_info_opcount);
+ monitor_register_hmp_info_hrt("opcount", qmp_x_query_opcount);
}
type_init(hmp_tcg_register);
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index 8f17a91858..bd0bb81d08 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -2118,9 +2118,9 @@ void dump_exec_info(GString *buf)
tcg_dump_info(buf);
}
-void dump_opcount_info(void)
+void dump_opcount_info(GString *buf)
{
- tcg_dump_op_count();
+ tcg_dump_op_count(buf);
}
#else /* CONFIG_USER_ONLY */
diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index d92f6fa7a9..3c8e24292b 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -432,7 +432,7 @@ static inline bool tlb_hit(target_ulong tlb_addr, target_ulong addr)
void dump_drift_info(GString *buf);
/* accel/tcg/translate-all.c */
void dump_exec_info(GString *buf);
-void dump_opcount_info(void);
+void dump_opcount_info(GString *buf);
#endif /* CONFIG_TCG */
#endif /* !CONFIG_USER_ONLY */
diff --git a/include/tcg/tcg.h b/include/tcg/tcg.h
index 7c8019a9b2..42f5b500ed 100644
--- a/include/tcg/tcg.h
+++ b/include/tcg/tcg.h
@@ -938,7 +938,7 @@ int tcg_check_temp_count(void);
int64_t tcg_cpu_exec_time(void);
void tcg_dump_info(GString *buf);
-void tcg_dump_op_count(void);
+void tcg_dump_op_count(GString *buf);
#define TCG_CT_CONST 1 /* any constant of register size */
diff --git a/qapi/machine.json b/qapi/machine.json
index 422a44661f..17794ef681 100644
--- a/qapi/machine.json
+++ b/qapi/machine.json
@@ -1449,6 +1449,19 @@
{ 'command': 'x-query-numa',
'returns': 'HumanReadableText' }
+##
+# @x-query-opcount:
+#
+# Query TCG opcode counters
+#
+# Returns: TCG opcode counters
+#
+# Since: 6.2
+##
+{ 'command': 'x-query-opcount',
+ 'returns': 'HumanReadableText',
+ 'if': 'CONFIG_TCG' }
+
##
# @x-query-profile:
#
diff --git a/tcg/tcg.c b/tcg/tcg.c
index f9ede8e62e..57f17a4649 100644
--- a/tcg/tcg.c
+++ b/tcg/tcg.c
@@ -4116,15 +4116,15 @@ static void tcg_profile_snapshot_table(TCGProfile *prof)
tcg_profile_snapshot(prof, false, true);
}
-void tcg_dump_op_count(void)
+void tcg_dump_op_count(GString *buf)
{
TCGProfile prof = {};
int i;
tcg_profile_snapshot_table(&prof);
for (i = 0; i < NB_OPS; i++) {
- qemu_printf("%s %" PRId64 "\n", tcg_op_defs[i].name,
- prof.table_op_count[i]);
+ g_string_append_printf(buf, "%s %" PRId64 "\n", tcg_op_defs[i].name,
+ prof.table_op_count[i]);
}
}
@@ -4143,9 +4143,9 @@ int64_t tcg_cpu_exec_time(void)
return ret;
}
#else
-void tcg_dump_op_count(void)
+void tcg_dump_op_count(GString *buf)
{
- qemu_printf("[TCG profiler not compiled]\n");
+ g_string_append_printf(buf, "[TCG profiler not compiled]\n");
}
int64_t tcg_cpu_exec_time(void)
diff --git a/tests/qtest/qmp-cmd-test.c b/tests/qtest/qmp-cmd-test.c
index ea24fde1a3..7f103ea3fd 100644
--- a/tests/qtest/qmp-cmd-test.c
+++ b/tests/qtest/qmp-cmd-test.c
@@ -53,6 +53,7 @@ static int query_error_class(const char *cmd)
{ "x-query-usb", ERROR_CLASS_GENERIC_ERROR },
/* Only valid with accel=tcg */
{ "x-query-jit", ERROR_CLASS_GENERIC_ERROR },
+ { "x-query-opcount", ERROR_CLASS_GENERIC_ERROR },
{ NULL, -1 }
};
int i;
--
2.31.1
^ permalink raw reply related [flat|nested] 27+ messages in thread* Re: [PULL 00/18] HMP-to-QMP info command patches
2021-11-02 17:56 [PULL 00/18] HMP-to-QMP info command patches Daniel P. Berrangé
` (17 preceding siblings ...)
2021-11-02 17:57 ` [PULL 18/18] qapi: introduce x-query-opcount " Daniel P. Berrangé
@ 2021-11-03 13:30 ` Richard Henderson
18 siblings, 0 replies; 27+ messages in thread
From: Richard Henderson @ 2021-11-03 13:30 UTC (permalink / raw)
To: Daniel P. Berrangé, qemu-devel
Cc: Laurent Vivier, Thomas Huth, Eduardo Habkost, David Hildenbrand,
Michael Roth, Yuval Shaia, Peter Xu, Dr. David Alan Gilbert,
Gerd Hoffmann, Paolo Bonzini, Philippe Mathieu-Daudé,
Eric Blake, Markus Armbruster
On 11/2/21 1:56 PM, Daniel P. Berrangé wrote:
> The following changes since commit 91e8394415f9bc9cd81c02bfafe02012855d4f98:
>
> Merge remote-tracking branch 'remotes/juanquintela/tags/migration-20211031-pull-request' into staging (2021-11-02 10:07:27 -0400)
>
> are available in the Git repository at:
>
> https://gitlab.com/berrange/qemu tags/hmp-x-qmp-620-pull-request
>
> for you to fetch changes up to b6a7f3e0d28248861cf46f59521129b179e8748d:
>
> qapi: introduce x-query-opcount QMP command (2021-11-02 15:57:20 +0000)
>
> ----------------------------------------------------------------
> Initial conversion of HMP debugging commands to QMP
>
> This introduces a new policy that all HMP commands will be converted to
> have QMP equivalents, marked unstable if no formal QAPI modelling is
> intended to be done.
>
> New unstable commands are added as follows:
>
> - HMP "info roms" => QMP "x-query-roms"
> - HMP "info profile" => QMP "x-query-profile"
> - HMP "info numa" => QMP "x-query-numa"
> - HMP "info usb" => QMP "x-query-usb"
> - HMP "info rdma" => QMP "x-query-rdma"
> - HMP "info ramblock" => QMP "x-query-ramblock"
> - HMP "info irq" => QMP "x-query-irq"
> - HMP "info jit" => QMP "x-query-jit"
> - HMP "info opcount" => QMP "x-query-opcount"
>
> ----------------------------------------------------------------
>
> Daniel P. Berrangé (18):
> monitor: remove 'info ioapic' HMP command
> monitor: make hmp_handle_error return a boolean
> docs/devel: rename file for writing monitor commands
> docs/devel: tweak headings in monitor command docs
> docs/devel: update error handling guidance for HMP commands
> monitor: introduce HumanReadableText and HMP support
> docs/devel: document expectations for QAPI data modelling for QMP
> docs/devel: add example of command returning unstructured text
> docs/devel: document expectations for HMP commands in the future
> qapi: introduce x-query-roms QMP command
> qapi: introduce x-query-profile QMP command
> qapi: introduce x-query-numa QMP command
> qapi: introduce x-query-usb QMP command
> qapi: introduce x-query-rdma QMP command
> qapi: introduce x-query-ramblock QMP command
> qapi: introduce x-query-irq QMP command
> qapi: introduce x-query-jit QMP command
> qapi: introduce x-query-opcount QMP command
>
> accel/tcg/cpu-exec.c | 51 +++++-
> accel/tcg/hmp.c | 22 +--
> accel/tcg/translate-all.c | 84 +++++----
> docs/devel/index.rst | 2 +-
> ...mands.rst => writing-monitor-commands.rst} | 167 ++++++++++++++++--
> hmp-commands-info.hx | 29 +--
> hw/core/loader.c | 39 ++--
> hw/core/machine-hmp-cmds.c | 38 +---
> hw/core/machine-qmp-cmds.c | 40 +++++
> hw/rdma/rdma_rm.c | 104 +++++------
> hw/rdma/rdma_rm.h | 2 +-
> hw/rdma/vmw/pvrdma_main.c | 31 ++--
> hw/usb/bus.c | 24 ++-
> include/exec/cpu-all.h | 6 +-
> include/exec/ramlist.h | 2 +-
> include/hw/rdma/rdma.h | 2 +-
> include/monitor/hmp-target.h | 1 -
> include/monitor/hmp.h | 5 +-
> include/monitor/monitor.h | 2 +
> include/qapi/type-helpers.h | 14 ++
> include/tcg/tcg.h | 4 +-
> monitor/hmp-cmds.c | 99 ++---------
> monitor/hmp.c | 32 +++-
> monitor/misc.c | 46 ++---
> monitor/monitor-internal.h | 7 +
> monitor/qmp-cmds.c | 116 ++++++++++++
> qapi/common.json | 11 ++
> qapi/machine.json | 110 ++++++++++++
> qapi/meson.build | 3 +
> qapi/qapi-type-helpers.c | 23 +++
> softmmu/physmem.c | 19 +-
> stubs/usb-dev-stub.c | 8 +
> target/i386/monitor.c | 6 -
> tcg/tcg.c | 98 +++++-----
> tests/qtest/qmp-cmd-test.c | 8 +
> 35 files changed, 829 insertions(+), 426 deletions(-)
> rename docs/devel/{writing-qmp-commands.rst => writing-monitor-commands.rst} (75%)
> create mode 100644 include/qapi/type-helpers.h
> create mode 100644 qapi/qapi-type-helpers.c
Applied, thanks.
r~
^ permalink raw reply [flat|nested] 27+ messages in thread