| Message ID | 20210914142042.1655100-31-berrange@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | monitor: explicitly permit QMP commands to be added for all use cases | expand |
On 9/14/21 4:20 PM, Daniel P. Berrangé wrote: > 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. > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > --- > hw/core/loader.c | 55 ++++++++++++++++++++++++++++++++--------------- > qapi/machine.json | 12 +++++++++++ > 2 files changed, 50 insertions(+), 17 deletions(-) > -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(""); > + HumanReadableText *ret; > > 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); > } > } > + > + ret = g_new0(HumanReadableText, 1); > + ret->human_readable_text = g_steal_pointer(&buf->str); > + return ret; > +} Is it possible to have an helper in 'qapi/qmp/smth.h' such: HumanReadableText *qmp_human_readable_text_new(GString **pbuf) { HumanReadableText *ret = g_new0(HumanReadableText, 1); ret->human_readable_text = g_steal_pointer(pbuf); return ret; } ?
On Tue, Sep 14, 2021 at 06:04:48PM +0200, Philippe Mathieu-Daudé wrote: > On 9/14/21 4:20 PM, Daniel P. Berrangé wrote: > > 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. > > > > Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> > > --- > > hw/core/loader.c | 55 ++++++++++++++++++++++++++++++++--------------- > > qapi/machine.json | 12 +++++++++++ > > 2 files changed, 50 insertions(+), 17 deletions(-) > > > -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(""); > > + HumanReadableText *ret; > > > > 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); > > } > > } > > + > > + ret = g_new0(HumanReadableText, 1); > > + ret->human_readable_text = g_steal_pointer(&buf->str); > > + return ret; > > +} > > Is it possible to have an helper in 'qapi/qmp/smth.h' such: > > HumanReadableText *qmp_human_readable_text_new(GString **pbuf) > { > HumanReadableText *ret = g_new0(HumanReadableText, 1); > > ret->human_readable_text = g_steal_pointer(pbuf); NB, we're not stealing the GString, we're stealing the char * inside it. > > return ret; > } but yes, we could do a helper like this. Regards, Daniel
diff --git a/hw/core/loader.c b/hw/core/loader.c index c623318b73..014d131847 100644 --- a/hw/core/loader.c +++ b/hw/core/loader.c @@ -46,6 +46,7 @@ #include "qemu-common.h" #include "qemu/datadir.h" #include "qapi/error.h" +#include "qapi/qapi-commands-machine.h" #include "trace.h" #include "hw/hw.h" #include "disas/disas.h" @@ -1472,32 +1473,52 @@ 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(""); + HumanReadableText *ret; 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); } } + + ret = g_new0(HumanReadableText, 1); + ret->human_readable_text = g_steal_pointer(&buf->str); + return ret; +} + + +void hmp_info_roms(Monitor *mon, const QDict *qdict) +{ + Error *err = NULL; + g_autoptr(HumanReadableText) info = qmp_x_query_roms(&err); + + if (err) { + error_report_err(err); + return; + } + + monitor_printf(mon, "%s", info->human_readable_text); } typedef enum HexRecord HexRecord; diff --git a/qapi/machine.json b/qapi/machine.json index 8737efa865..a4a1727589 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1327,3 +1327,15 @@ { 'command': 'x-query-registers', 'data': {'*cpu': 'int' }, 'returns': 'HumanReadableText' } + +## +# @x-query-roms: +# +# Query information on the registered ROMS +# +# Returns: registered ROMs +# +# Since: 6.2 +## +{ 'command': 'x-query-roms', + 'returns': 'HumanReadableText' }
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. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- hw/core/loader.c | 55 ++++++++++++++++++++++++++++++++--------------- qapi/machine.json | 12 +++++++++++ 2 files changed, 50 insertions(+), 17 deletions(-)