Re: [PATCH v3 00/30] qapi/qom: QAPIfy --object and object-add

[Markus Armbruster <armbru@redhat.com>]

Kevin Wolf <kwolf@redhat.com> writes:

> Am 11.03.2021 um 12:24 hat Peter Krempa geschrieben:
>> On Thu, Mar 11, 2021 at 09:37:11 +0100, Kevin Wolf wrote:
>> > Am 11.03.2021 um 08:47 hat Peter Krempa geschrieben:
>> > > On Wed, Mar 10, 2021 at 18:30:44 +0100, Kevin Wolf wrote:
>> > > > Am 10.03.2021 um 15:31 hat Paolo Bonzini geschrieben:
>> > > > > On 10/03/21 15:22, Peter Krempa wrote:
>> 
>> [...]
>> 
>> > > -object memory-backend-ram,id=ram-node2,size=24578621440,host-nodes=1-2,host-nodes=5,host-nodes=7,policy=bind
>> > 
>> > Oh, we have ranges, too... That makes the compatibility code even
>> > nastier to write. I doubt that we can implement this in the keyval
>> > parser alone without touching the visitor. :-/
>> > 
>> > > If any of the above is to be deprecated we'll need to adjust our
>> > > JSON->commandline generator accordignly.
>> > > 
>> > > Luckily the 'host-nodes' is storeable as a bitmap and the generator is
>> > > actually modular to allow plugging an array interpretor which actually
>> > > does the above conversion from a JSON array.
>> > > 
>> > > So, what is the preferred syntax here? Additionally we might need a
>> > > witness property to detect when to use the new syntax if basing it on
>> > > the object-add qapification will not be enough.
>> > 
>> > The list syntax supported by the keyval parser is the one you know from
>> > -blockdev: host-nodes.0=3,host-nodes.1=7, ...
>> 
>> I think that should be easy enough to convert to.
>
> We could also support JSON syntax in QEMU. That would probably be even
> more convenient for libvirt?

Cleanly QAPIfied options like -blockdev do

    if (optarg[0] == '{') {
        parse @optarg as JSON with qobject_from_json()
        convert to C type with qobject_input_visitor_new()
    } else {
        parse @optarg with keyval_parse()
        convert to C type with qobject_input_visitor_new_keyval()
    }

Options where compatibility problems defeat keyval_parse() can do

    if (optarg[0] == '{') {
        parse @optarg as JSON with qobject_from_json()
        convert to C type with qobject_input_visitor_new()
    } else {
        parse the old way
        convert to C type somehow
    }

Precedence: -display.  There, the old way is an ad hoc parser, and the
conversion to C type DisplayOptions is manual.  For -object, the old way
would be QemuOpts, and the conversion would use the opts visitor.

>> Is it safe to do right away (with the old parser?). Otherwise we need
>> to agree on something which will let us detect when it's safe to
>> change.
>
> Neither keyval nor JSON syntax work with the old QemuOpts parser.
>
> What is the usual way to do this for command line options? If we don't
> have a good way there, we can always tie it to something in the QAPI
> schema. If we still get this solved in time for 6.0, we could use the
> existence of ObjectOptions. Or all else failing, we can use a feature
> flag.

You should not look for types in output of query-qmp-schema, only for
commands and events.  To discourage looking for types, query-qmp-schema
masks the names of user-defined types.

A feature flag is fine as last resort.  That's what they were designed
for.