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

[Osier Yang <jyang@redhat.com>]

On 19/04/13 20:39, Eric Blake wrote:
> On 04/19/2013 03:52 AM, Amos Kong wrote:
>> Libvirt doesn't have a stable way to know option support
>> detail. This patch introdued a new qmp command to query
>> configuration schema information. hmp command isn't added.
> Agreed; HMP is not needed: by the time you can connect to a human
> monitor, you've already invoked the command line, and humans can read
> -help.  It's just libvirt (and other management apps) that need a
> machine-parseable alternative to -help.
>
>> Signed-off-by: Amos Kong <akong@redhat.com>
>> CC: Osier Yang <jyang@redhat.com>
>> CC: Anthony Liguori <aliguori@us.ibm.com>
>> ---
>> Reposted this patch: https://www.redhat.com/archives/libvir-list/2013-January/msg01656.html
>> ---
>>   qapi-schema.json       |   26 ++++++++++++++++++++++++++
>>   qemu-options-wrapper.h |   16 ++++++++++++++++
>>   qmp-commands.hx        |   32 ++++++++++++++++++++++++++++++++
>>   qmp.c                  |   36 ++++++++++++++++++++++++++++++++++++
>>   4 files changed, 110 insertions(+), 0 deletions(-)
> In addition to Osier's review,
>
>> +##
>> +# @ConfigSchemaInfo:
>> +#
>> +# Configration schema information.
>> +#
>> +# @option: option name
>> +#
>> +# @config-schema: configuration schema string of one option
> Any more details about the typical contents of this string?  Is it
> supposed to be machine-parseable, or is it merely a human-readable
> replay of -help text where the only machine-parseable aspect is whether
> a particular @option name exists?
>
>> +#
>> +# Since 1.5
>> +##
>> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'config-schema': 'str'} }
>> +
>> +##
>> +# @query-config-schema
>> +#
>> +# Query configuration schema information of options
>> +#
>> +# @option: #optional option name
> I don't know that may existing query-* commands support optional
> arguments for filtering; but it seems like a nice enough thing to provide.
>
>> +#
>> +# Returns: @ConfigSchemaInfo list
>> +#
>> +# Since 1.5
>> +##
>> +{'command': 'query-config-schema', 'data': {'*option': 'str'}, 'returns': ['ConfigSchemaInfo']}
> Long line; it might be worth wrapping this similar to what other
> commands have done.
>
>> +SQMP
>> +query-config-schema
>> +------------
>> +
>> +Show configuration schema.
>> +
>> +Return configuration schema of one option, if no option is assigned,
>> +will return configuration schema of all options.
>> +
>> +- "option": option name
>> +- "config-schema": configuration schema string of one option
>> +
>> +Example:
>> +-> {"execute": "query-config-schema", "arguments" : {"option": "boot"}}
>> +<- {"return": [
>> +     {"config-schema": "-boot [order=drives][,once=drives][,menu=on|off]\n
>> +        [,splash=sp_name][,splash-time=sp_time][,reboot-timeout=rb_time][,strict=on|off]\n
>> +	'drives': floppy (a), hard disk (c), CD-ROM (d), network (n)\n
>> +	'sp_name': the file's name that would be passed to bios as logo picture, if menu=on\n
>> +	'sp_time': the period that splash picture last if menu=on, unit is ms\n
>> +	'rb_timeout': the timeout before guest reboot when boot failed, unit is ms\n",
> Ah, the example makes it clear - this is human-readable text from -help
> output, and not something that is further machine-parseable.  Which
> means that if we have an existing command line version in one qemu
> release, and the next qemu release adds more features to that option, we
> have no reliable way (other than string-scraping) to detect that new
> feature of the existing option.  Can we do any better?
>
> I was expecting more of a JSON representation of the
> QEMUOptionParameter[] used to parse a given option - THAT would be
> machine-parseable, and would also let us know when an option has learned
> more arguments in a newer qemu version.

Agreed. Parsing with the string is not a happy work. Though the existing
code in libvirt can be reused.

> But it is also something that
> can be added later (we've already decided that query-* commands can add
> output fields to their dictionary as needed).

I'm wondering if can have a JSON dictionary from the beginning.

>
> The name 'config-schema' sounds like it is machine-parseable; can we use
> a more descriptive name such as 'help-text' that makes it obvious what
> you are really providing?
>
>> +++ b/qmp.c
>> @@ -24,6 +24,9 @@
>>   #include "hw/qdev.h"
>>   #include "sysemu/blockdev.h"
>>   #include "qom/qom-qobject.h"
>> +#include "qemu-options.h"
>> +#include "net/net.h"
> What does net/net.h have to do with anything?

It's required for building, see the two #includes  of qemu-options-wrapper.h

Osier