* Markus Armbruster (arm...@redhat.com) wrote: > "Dr. David Alan Gilbert" <dgilb...@redhat.com> writes: > > > * Markus Armbruster (arm...@redhat.com) wrote: > >> Cédric Le Goater <c...@kaod.org> writes: > >> > >> > From: "Dr. David Alan Gilbert" <dgilb...@redhat.com> > >> > > >> > Reimplement it based on qmp_qom_get() to avoid converting QObjects back > >> > to strings. > >> > > >> > Inspired-by: Paolo Bonzini <pbonz...@redhat.com> > >> > Signed-off-by: Andreas Färber <afaer...@suse.de> > >> > > >> > Slight fix for bit-rot: > >> > Signed-off-by: Dr. David Alan Gilbert <dgilb...@redhat.com> > >> > [clg: updates for QEMU 5.0 ] > >> > Signed-off-by: Cédric Le Goater <c...@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 / dgilb...@redhat.com / Manchester, UK
