Re: [Qemu-devel] [PATCH 2/2] monitor: introduce query-config-schema command

[Anthony Liguori <aliguori@us.ibm.com>]

Eric Blake <eblake@redhat.com> writes:

> On 04/24/2013 06:47 AM, Amos Kong wrote:
>> Libvirt has no way to probe if an option or property is supported,
>> This patch introdues a new qmp command to query configuration schema
>> information. hmp command isn't added because it's not needed.
>
> Agreed that no HMP counterpart is needed.  However, I don't think we
> have quite the right interface yet.
>
>> 
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> CC: Osier Yang <jyang@redhat.com>
>> CC: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>>  qapi-schema.json   |   29 +++++++++++++++++++++++++++++
>>  qmp-commands.hx    |   40 ++++++++++++++++++++++++++++++++++++++++
>>  util/qemu-config.c |   40 ++++++++++++++++++++++++++++++++++++++++
>>  3 files changed, 109 insertions(+), 0 deletions(-)
>> 
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 751d3c2..aeab057 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3505,3 +3505,32 @@
>>      '*asl_compiler_rev':  'uint32',
>>      '*file':              'str',
>>      '*data':              'str' }}
>> +
>> +##
>> +# @ConfigSchemaInfo:
>> +#
>> +# Configration schema information.
>> +#
>> +# @option: option name
>> +#
>> +# @params: parameters strList of one option
>
> Why just a strList?  That only tells me option names.  But we already
> know so much more than that - we know the type and the help string.
>
>> +#
>> +# Since 1.5
>> +##
>> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'params': ['str']} }
>
> I'd rather see an array of structs, more closely mirroring what
> include/qemu/option.h gives us:
>
> # JSON representation of values of QEMUOptionParType, may grow in future
> { 'enum': 'ConfigParamType',
>   'data': [ 'flag', 'number', 'size', 'string' ] }
>
> # JSON representation of QEMUOptionParameter, may grow in future
> # @help is optional if no text was present
> { 'type': 'ConfigParamInfo',
>   'data': { 'name': 'str', 'type': 'ConfigParamType', '*help':'str' } }
>
> # Each command line option, and its list of parameters
> { 'type': 'ConfigSchemaInfo',
>   'data': { 'option':'str', 'params': ['ConfigParamInfo'] } }
>
> And that means we no longer have ['str'], which bypasses the need for
> your patch 1/2.

Strong Ack on this schema.

Regards,

Anthony Liguori