Re: [PATCH v2 30/53] qapi: introduce x-query-roms QMP command

[Daniel P . Berrangé]

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