Re: [RFC PATCH] qom: Implement qom-get HMP command

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* Markus Armbruster (armbru@redhat.com) wrote:
> "Dr. David Alan Gilbert" <dgilbert@redhat.com> writes:
> 
> > * Markus Armbruster (armbru@redhat.com) wrote:
> >> Cédric Le Goater <clg@kaod.org> writes:
> >> 
> >> > From: "Dr. David Alan Gilbert" <dgilbert@redhat.com>
> >> >
> >> > Reimplement it based on qmp_qom_get() to avoid converting QObjects back
> >> > to strings.
> >> >
> >> > Inspired-by: Paolo Bonzini <pbonzini@redhat.com>
> >> > Signed-off-by: Andreas Färber <afaerber@suse.de>
> >> >
> >> > Slight fix for bit-rot:
> >> > Signed-off-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
> >> > [clg: updates for QEMU 5.0 ]
> >> > Signed-off-by: Cédric Le Goater <clg@kaod.org>
> >> > ---
> >> >
> >> >  I would like to restart the discussion on qom-get command to understand
> >> >  what was the conclusion and see if things have changed since.
> >> 
> >> I've since learned more about QOM.  I believe we should do HMP qom-get,
> >> but not quite like this patch does.  Let me explain.
> >> 
> >> A QOM property has ->get() and ->set() methods to expose the property
> >> via the Visitor interface.
> >> 
> >> ->get() works with an output visitor.  With the QObject output visitor,
> >> you can get the property value as a QObject.  QMP qom-get uses this.
> >> With the string output visitor, you can get it as a string.  Your
> >> proposed HMP qom-get uses this.
> >> 
> >> ->set() works with an input visitor.  With the QObject input visitor,
> >> you can set the property value from a QObject.  QMP qom-set uses this.
> >> With the string input visitor, you can set it from a string.  HMP
> >> qom-set uses this.  With the options visitor, you can set it from a
> >> QemuOpts.  CLI -object uses this.
> >> 
> >> The Visitor interface supports arbitrary QAPI types.  The ->get() and
> >> ->set() methods can use them all.
> >> 
> >> Some visitors, howerver, carry restrictions:
> >> 
> >>  * The string output visitor does not implement support for visiting
> >>  * QAPI structs, alternates, null, or arbitrary QTypes.  It also
> >>  * requires a non-null list argument to visit_start_list().
> >> 
> >>  * The string input visitor does not implement support for visiting
> >>  * QAPI structs, alternates, null, or arbitrary QTypes. Only flat lists
> >>  * of integers (except type "size") are supported.
> >> 
> >>  * The Opts input visitor does not implement support for visiting QAPI
> >>  * alternates, numbers (other than integers), null, or arbitrary
> >>  * QTypes.  It also requires a non-null list argument to
> >>  * visit_start_list().
> >> 
> >> If you try to qom-set a property whose ->set() uses something the string
> >> input visitor doesn't support, QEMU crashes.  Same for -object, and your
> >> proposed qom-get.
> >
> > Crashing would be bad.
> >
> >> I'm not aware of such a ->set(), but this is a death trap all the same.
> >> 
> >> The obvious way to disarm it is removing the restrictions.
> >> 
> >> One small step we took towards that goal is visible in the comments
> >> above: support for flat lists of integers.  The code for that will make
> >> your eyes bleed.  It's been a thorn in my side ever since I inherited
> >> QAPI.  Perhaps it could be done better.  But there's a reason why it
> >> should not be done at all: it's language design.
> >> 
> >> The QObject visitors translate between QAPI types and our internal
> >> representation of JSON.  The JSON parser and formatter complete the
> >> translation to and from JSON.  Sensible enough.
> >> 
> >> The string visitors translate between QAPI and ...  well, a barely
> >> documented language of our own "design".  Tolerable when the language it
> >> limited to numbers, booleans and strings.  Foolish when it grows into
> >> something isomorphic to JSON.
> >> 
> >> This is a dead end.
> >> 
> >> Can we still implement a safe and tolerably useful HMP qom-get and
> >> qom-set?  I think we can.  Remember the basic rule of HMP command
> >> implementation: wrap around the QMP command.  So let's try that.
> >> 
> >> hmp_qom_get() calls qmp_qom_get() and formats the resulting QObject with
> >> qobject_to_json_pretty().
> >
> > Why don't we *just* do this?
> > OK, we wont fix the qom-set or the CLI, but if we just get hmp_qom_get
> > to call QMP, format it into json and then dump the json to the user,
> > then we get a usable, if not pretty, qom-get command, without having to
> > solve all these hard problems to move it forward?
> 
> Count me in favour.  I'd like to see the matching change to HMP qom-set,
> though.

It turns out I actually did do a JSON version, as the follow up to the
version Cédric reposted; but then that got lost in people not liking
JSON;   https://lists.gnu.org/archive/html/qemu-devel/2016-09/msg01041.html

Having just resurrected that code, the only difference from my patch
then and what I just wrote was that instead of doing the object
resolution and stuff, I just call the qmp function.
It's still JSON output and I don't think the arguments from 4 years ago
moved forward.  I'll post it soon.

Dave

--
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK