Re: [Qemu-devel] [PATCH v11 5/6] qapi: add a QmpInputVisitor that does string conversion

[Markus Armbruster <armbru@redhat.com>]

"Daniel P. Berrange" <berrange@redhat.com> writes:

> On Tue, Sep 13, 2016 at 11:05:08AM +0200, Markus Armbruster wrote:
>> "Daniel P. Berrange" <berrange@redhat.com> writes:
>> 
>> > Currently the QmpInputVisitor assumes that all scalar
>> > values are directly represented as their final types.
>> > ie it assumes an 'int' is using QInt, and a 'bool' is
>> > using QBool.
>> >
>> > This adds an alternative constructor for QmpInputVisitor
>> > that will set it up such that it expects a QString for
>> > all scalar types instead.
>> >
>> > This makes it possible to use QmpInputVisitor with a
>> > QDict produced from QemuOpts, where everything is in
>> > string format.
>> 
>> Can you explain how this is related to the Options visitor?
>
> The intention is that this can replace the existing OptsVisitor,
> for cases that don't rely on the magic "List of scalars" semantics
> of OptsVisitor - eg where  'foo=3,foo=5,foo=533' gets turned into
> a QList.

There's also the even more magical foo=3-5,foo=533.

> When using QemuOpts w/ qdict_crumple + QmpInputVisitor, you would
> do list of scalars in different manner  'foo.1=3,foo.2=5,foo.3=533'
> since this syntax is extendable to deal with arbitrary nesting of
> dicts + lists, where as the OptsVisitor syntax cannot be extended
> in a back-compatible manner.
>
> This series converts -object to use QmpInputVisitor and this is
> safe todo right now, since no currently created QOM object has
> a property which is a list of scalars and this the changed syntax
> is not going to affect any existing usage.

Peeking at the patch... aha!  Instead of having the options visitor
visit the QemuOpts, you convert the QemuOpts to a QDict, which you then
visit with your new visitor.  Less efficient, because you have to build
and destroy the intermediate QDict.  Not an issue when processing
configuration or even monitor commands, of course.

I guess you convert to QDict so that the work of going from a
QemuOpts-style string using -drive conventions to a visit splits into
manageable chunks more easily, preferably into chunks that already
exist: parse string into QemuOpts, convert to QDict, crumple, visit,
destroy QDict.

Still, I take the conversion as a signal that our data structures are
wrong.  Wild idea: should QemuOpts use a QDict rather than a QTAILQ of
QemuOpt to store options?

The pipeline then becomes parse string into QemuOpts, clone its QDict,
crumple, visit, destroy QDict.  Next step would be crumpling in place,
i.e. parse string into nested QemuOpts, get its QDict, visit.  Mind, I'm
not demanding you to do that now, I'm just trying to figure out whether
this series is shoveling into or out of the QemuOpts hole :)

> The -drive arg can be converted to QmpInputVisitor too, since
> the qdict_crumple/QmpInputVisitor combination was explicitly
> designed to be 100% compatible with -drive syntax for blockdevs
> nested options.

You abstaining from inventing yet another option syntax dialect is
appreciated.

> Other args would have to be considered on a case-by-case basis
> to see if they rely on the magic list of scalars syntax used
> by OptsVisitor. Probably only a few of them need this.

Obvious maintainer question: what would it take to get rid of the
options visitor entirely?  Because this maintainer looks on additions to
his fiefdom much more kindly when they're balanced by subtractions.

I guess all the uses that don't rely on the "list of scalars" magic are
easy prey: replace by conversion to intermediate QDict + your new
visitor.

What about the ones that do rely on it?  Which ones do?  Any ideas on
how to change them so that they don't require a complete options
visitor?