On Tue, Oct 12, 2021 at 09:12:23AM +0200, Thomas Huth wrote:
> On 30/09/2021 15.23, Daniel P. Berrangé wrote:
> > This is a counterpart to the HMP "info skeys" 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.
> >
> > Including 'common.json' into 'machine-target.json' created a little
> > problem because the static marshalling method for HumanReadableText
> > is generated unconditionally. It is only used, however, conditionally
> > on certain target architectures.
> >
> > To deal with this we change the QAPI code generator to simply mark
> > all static marshalling functions with G_GNUC_UNSED to hide the
> > compiler warning.
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
> > ---
> ...
> > +void hmp_info_skeys(Monitor *mon, const QDict *qdict)
> > +{
> > + Error *err = NULL;
> > + g_autoptr(HumanReadableText) info = NULL;
> > + uint64_t addr = qdict_get_int(qdict, "addr");
> > +
> > + info = qmp_x_query_skeys(addr, &err);
> > + if (err) {
> > + error_report_err(err);
>
> Shouldn't that rather be:
>
> monitor_printf(mon, "%s\n", error_get_pretty(err));
>
> or something similar?
error_report_err gets (eventually) hooked into monitor_printf() if
current monitor in the thread is non-NULL
>
> > return;
> > }
> > - monitor_printf(mon, " key: 0x%X\n", key);
> > + monitor_printf(mon, "%s", info->human_readable_text);
> > }
>
> Apart the question above, patch looks fine to me.
>
> Thomas
>
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 :|
