| Message ID | 20210930132349.3601823-8-berrange@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | monitor: explicitly permit QMP commands to be added for all use cases | expand |
* Daniel P. Berrangé (berrange@redhat.com) 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 | 53 +++++++++++++++++++++++++------------ > hw/core/machine-qmp-cmds.c | 1 + > include/qapi/type-helpers.h | 14 ++++++++++ > qapi/common.json | 11 ++++++++ > qapi/machine.json | 12 +++++++++ > qapi/meson.build | 3 +++ > qapi/qapi-type-helpers.c | 23 ++++++++++++++++ > 7 files changed, 100 insertions(+), 17 deletions(-) > create mode 100644 include/qapi/type-helpers.h > create mode 100644 qapi/qapi-type-helpers.c > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index c623318b73..5ebdca3087 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" > @@ -1472,32 +1474,49 @@ 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); > +} > + > + > +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); This is getting copied in each one of these; it looks like you need either: a) A helper function like: void hmp_info_from_qmp(Monitor *mon, HumanReadableText *(void)func) { ... } b) Or teach the hmp parser to do the calls? Dave > } > > typedef enum HexRecord HexRecord; > diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c > index 216fdfaf3a..76f2b84d81 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" > 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/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/machine.json b/qapi/machine.json > index 32d47f4e35..4c18904521 100644 > --- a/qapi/machine.json > +++ b/qapi/machine.json > @@ -1346,3 +1346,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' } > 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 >
On Mon, Oct 04, 2021 at 01:32:14PM +0100, Dr. David Alan Gilbert wrote: > * Daniel P. Berrangé (berrange@redhat.com) 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 | 53 +++++++++++++++++++++++++------------ > > hw/core/machine-qmp-cmds.c | 1 + > > include/qapi/type-helpers.h | 14 ++++++++++ > > qapi/common.json | 11 ++++++++ > > qapi/machine.json | 12 +++++++++ > > qapi/meson.build | 3 +++ > > qapi/qapi-type-helpers.c | 23 ++++++++++++++++ > > 7 files changed, 100 insertions(+), 17 deletions(-) > > create mode 100644 include/qapi/type-helpers.h > > create mode 100644 qapi/qapi-type-helpers.c > > > > diff --git a/hw/core/loader.c b/hw/core/loader.c > > index c623318b73..5ebdca3087 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" > > @@ -1472,32 +1474,49 @@ 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); > > +} > > + > > + > > +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); > > This is getting copied in each one of these; it looks like you need > either: > a) A helper function like: > void hmp_info_from_qmp(Monitor *mon, HumanReadableText *(void)func) > { > ... > } > > b) Or teach the hmp parser to do the calls? This pattern is repeated in many, but not all, or the handlers in this series, because I started with the easy cases of no-arg 'info' commands. There's a few exceptions such as commands which drive off the currently selected CPU state. I'm not convince it is worth adding specials to the hmp parser, since it will only be used for a subset of the commands lng term. A helper function though could do the job, since I've already introduced a helper for the QMP side converting GString to HumanReadableText Regards, Daniel
diff --git a/hw/core/loader.c b/hw/core/loader.c index c623318b73..5ebdca3087 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" @@ -1472,32 +1474,49 @@ 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); +} + + +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/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index 216fdfaf3a..76f2b84d81 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" 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/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/machine.json b/qapi/machine.json index 32d47f4e35..4c18904521 100644 --- a/qapi/machine.json +++ b/qapi/machine.json @@ -1346,3 +1346,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' } 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; +}
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 | 53 +++++++++++++++++++++++++------------ hw/core/machine-qmp-cmds.c | 1 + include/qapi/type-helpers.h | 14 ++++++++++ qapi/common.json | 11 ++++++++ qapi/machine.json | 12 +++++++++ qapi/meson.build | 3 +++ qapi/qapi-type-helpers.c | 23 ++++++++++++++++ 7 files changed, 100 insertions(+), 17 deletions(-) create mode 100644 include/qapi/type-helpers.h create mode 100644 qapi/qapi-type-helpers.c