Re: [Qemu-devel] [PATCH v14 00/19] QAPI/QOM work for non-scalar object properties

[Daniel P. Berrange]

On Thu, Oct 20, 2016 at 05:06:33PM +0200, Markus Armbruster wrote:
> I've warmed quite a bit to this series while reviewing it.  In
> particular, I've come around to like structuring the command line ->
> QAPI pipeline exactly like the JSON -> QAPI pipeline, namely 1. parse
> into QObject, 2. convert to QAPI with the QObject input visitor.
> QObject serves as abstract syntax here.  The differences between JSON
> and command line result in different abstract syntax, which in turn
> necessitates two cases in the input visitor.  The series adds more than
> two, to cater for option visitor funnies.  Perhaps we can do without
> some of them.
> 
> The other way to skin this cat would be an improved options visitor.
> Has its advantages and disadvantages, but the big one is that you
> already did the work for QObject input visitor solution.
> 
> The one major issue I have with the series is that it adds to the
> growing body of QemuOpts hacks/workarounds.
> 
> We've pushed QemuOpts beyond well its design limits.  What started as a
> simple store for scalar configuration parameters structured as key=value
> lists, plus command line and configuration file syntax, has grown three
> ways to specify lists (repeated keys, basically an implementation
> accident that got pressed into service; special integer list syntax in
> the options visitor, later adopted by the string input visitor,
> hopefully compatibly; and the block layer's dotted key convention) and a
> way to specify arbitrary complex values (block layer's dotted key
> convention again).  Of these, only "repeated keys" is in QemuOpts
> proper, all the others are bolted on and used only when needed.  How
> they interact is not obvious.
> 
> This series marries all the bolted-on stuff and puts it in the QObject
> visitor.  That's actually an improvement of sorts; at least it's in just
> two places now.  But it's still a smorgasbord of syntactical/semantic
> options.
> 
> I feel it's time to stop working around the design limits of QemuOpts
> and start replacing them by something that serves our current needs.  We
> basically need the expressive power of JSON on the command line.  Syntax
> is debatable, but it should be *one* concrete command-line syntax,
> parsed by *one* parser into *one* kind of abstract syntax tree, where
> the only optional variations are the ones forced upon us by backward
> compatibility.
> 
> We can't do this for 2.8, obviously.  We can try for 2.9.  I have pretty
> specific ideas on how it should be done, so I guess it's only fair I
> give it a try myself.
> 
> I know the block layer wants to use bits of this series to save some
> coding work for certain features targeting 2.8.  I have no objections as
> long as it doesn't create new ABI.  Exception: I'm okay with applying
> dotted key convention to more of the same, e.g. new block drivers.
> 
> Sounds sane?

It is all fine with me.

Of the patch series proposed, I think patches 1->6 ought to be safe to
take for 2.8, without exposing new ABI:

 1. option: make parse_option_bool/number non-static
 2. qdict: implement a qdict_crumple method for un-flattening a dict
 3. qapi: add trace events for visitor
 4. qapi: rename QmpInputVisitor to QObjectInputVisitor
 5. qapi: rename QmpOutputVisitor to QObjectOutputVisitor
 6. qapi: don't pass two copies of TestInputVisitorData to tests

The rest are clearly not an option for 2.8 since freeze is barely
1 week away, and I'm travelling all next week

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|