Re: [Qemu-devel] [PATCH 7/9] qmp: expose a command to query capabilities of config parser

[Anthony Liguori <anthony@codemonkey.ws>]

On 03/19/2012 03:10 PM, Eric Blake wrote:
> On 03/19/2012 09:09 AM, Anthony Liguori wrote:
>> Signed-off-by: Anthony Liguori<aliguori@us.ibm.com>
>> ---
>>   qapi-schema.json |   66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>>   qemu-config.c    |   51 +++++++++++++++++++++++++++++++++++++++++
>>   qmp-commands.hx  |    5 ++++
>>   3 files changed, 122 insertions(+), 0 deletions(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 04fa84f..2a80ef6 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -1663,3 +1663,69 @@
>>   { 'command': 'qom-list-types',
>>     'data': { '*implements': 'str', '*abstract': 'bool' },
>>     'returns': [ 'ObjectTypeInfo' ] }
>> +
>> +##
>> +# @ConfigOptionType
>> +#
>> +# An enumeration describing the type of a configuration file entry.
>> +#
>> +# @string: a UTF-8 string
>> +#
>> +# @bool: either 'on' or 'off'
>> +#
>> +# @number: an integer
>> +#
>> +# @size: an integer followed by either 'K', 'M', 'G', or 'T'.
>
> Not quite - I found three different parsers, but none of them match this
> description.

Heh, I was going by the documentation in the code:

     QEMU_OPT_SIZE,        /* size, accepts (K)ilo, (M)ega, (G)iga, (T)era 
postfix */


> That is, qemu-option.c:parse_option_size() supports 'k'
> and 'b', as well as omitting a suffix, but does not support 'm'.

TBH, I'd prefer to not document 'k' and 'b' and explicitly document that the 
suffix is optional and bytes is implied (which I thought was implied in my 
documentation, but I can see that it may not be).

I'd rather we support officially support one way of doing things and if we 
support more, do it under the mantra of being liberal in what we accept.

> Then
> there's cmd.c:cvtnum(), which is case-insensitive, and adds 'p' and 'e',
> but omits 'b'.  Then there's the 'o' type in monitor.c that defers to
> cutils.c:strtosz(), which defaults to bytes but is case-insensitive and
> supports 'b' but not 'p'.  Why are we using three different parsers, anyway?

Yes, I am aware of that, and yes, I'm going to fix it.  One bit at a time though.

> Please be sure that your documentation exactly matches the code used by
> the particular parser used for 'size', since it is is already confusing
> as-is.

Let's be careful about documenting what is the officially supported interface 
and what is possible with the given state of the code.


>> +#
>> +# @help: an optional help description.  This should not be parsed or relied upon
>> +#        in any way.  Its content's may change in future versions of QEMU.
>
> s/content's/contents/

Ack.

Regards,

Anthony Liguori