Re: [Qemu-devel] [PATCH 4/5] monitor: add object-add (QMP) and object_add (HMP) command

[Michael Roth]

Quoting Markus Armbruster (2013-12-17 01:20:16)
> [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.

Is dumping static properties based on class name sufficient, or do we
need introspection for dynamic properties as well? (or are those not
exposed outside of qom-set?) We could maybe introduce a QAPI 'built-in'
such as 'ObjectProperties' that automatically does the query based on the
now-special 'type' param and handles all the type-checking up-front. This
would avoid an open-ended 'dict' type proliferating too much and provide
infrastructure for introspection.

> 
> 
> [*] Can we have a definition of QAPI schema semantics other than code?
> Pretty-please?