[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 4/5] monitor: add object-add (QMP) and object_ad
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 4/5] monitor: add object-add (QMP) and object_add (HMP) command |
Date: |
Tue, 17 Dec 2013 08:20:16 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) |
[Cc: Anthony, Mike for QAPI schema expertise]
Luiz Capitulino <address@hidden> writes:
> On Tue, 10 Dec 2013 19:15:05 +0100
> Paolo Bonzini <address@hidden> wrote:
>
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
>>
>> Il 10/12/2013 19:00, Eric Blake ha scritto:
>> >>> + 'data': {'qom-type': 'str', 'id': 'str', '*props': 'dict'},
>> >>> + 'gen': 'no' }
>> >
>> > This feels VERY open-coded. No where else in qapi-schema do we
>> > have 'dict' as a type
>>
>> Yes, in fact the "data" field is entirely skipped by the code
>> generator (that's 'gen':'no').
>>
>> > ; using it violates all sorts of type-safety (which, I guess, is
>> > the point), making it impossible to introspect what keys are valid
>> > for use in the "props":{...} dictionary. Do we really want to
>> > play this fast and loose with the type system, or should we try
>> > harder to make this a robust self-describing union of types?
>> >
>> > That is, why can't we have object-add use a discriminated union,
>> > where qom-type is the discriminator, and where props is an
>> > appropriate JSON struct type that corresponds to the branch of the
>> > union, so that we get full introspection on the set of valid keys
>> > to put in props for any given qom-type?
>>
>> The point of "props" is passing arbitrary data to a QOM object. We
>> should indeed have introspection for QOM objects, where each QOM class
>> name can be introspected separately. However, the union of all
>> possible QOM objects need not have a "C struct" representation.
>
> The "props" key was added to represent the "O" argument type of
> early QMP (which is used by commands like device_add), so that
> we could convert them to the QAPI. IIRC, we didn't plan for it
> to be used by new commands... But I don't have anything better
> to suggest, so I won't object to its usage here.
We created monitor argument type "O" to have name=val,... arguments in
the human monitor exactly like command line option arguments. Currently
used by device_add and netdev_add.
We shoehorned type "O" into QMP in a bout of QMP feature-completeness
desperation. This was before QAPI.
device_add still isn't in qapi-schema.json, but netdev_add is:
{ 'command': 'netdev_add',
'data': {'type': 'str', 'id': 'str', '*props': '**'},
'gen': 'no' }
Note the magic "'*props': '**'" (I'll be hanged if I know what that
means[*]), and "'gen': 'no'".
Yes, a proper schema for netdev_add and device_add is desirable. In
both cases (but especially for device_add), the arguments are the
obligatory id plus a union discriminated by the device type, contraining
that device's properties.
Unless we move device properties definition to qapi-schema.json (bad),
or duplicate them there (worse), we need to derive that part of the
schema dynamically from device information available in QOM.
[*] Can we have a definition of QAPI schema semantics other than code?
Pretty-please?
- [Qemu-devel] [PATCH 2/5] qom: fix leak for objects created with -object, (continued)
[Qemu-devel] [PATCH 5/5] monitor: add object-del (QMP) and object_del (HMP) command, Paolo Bonzini, 2013/12/10
Re: [Qemu-devel] [PATCH 0/5] Monitor commands for object-add/del, Igor Mammedov, 2013/12/12
Re: [Qemu-devel] [PATCH 0/5] Monitor commands for object-add/del, Luiz Capitulino, 2013/12/16