diff mbox

monitor: introduce query-config-schema

Message ID 1366365123-5412-1-git-send-email-akong@redhat.com
State New
Headers show

Commit Message

Amos Kong April 19, 2013, 9:52 a.m. UTC
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.

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(-)

Comments

Osier Yang April 19, 2013, 12:02 p.m. UTC | #1
On 19/04/13 17:52, Amos Kong wrote:
> Libvirt doesn't have a stable way to know option support

Actually no way now.

Libvirt swtiched to use qmp to collect the qemu capabilities
for qemu newer than 1.2.0,  thus there is no way to probe either
if a option or a property is supported or not by QEMU. Any
support for the new options/properties in libvirt are blocked.

Except we go back to probe using the help output, but it's not
right way to go.

Consider to change the sentence to:

Libvirt has no way to probe if an option or property is supported,

> detail. This patch introdued a new qmp command to query

s/introdued/introduces/

> configuration schema information. hmp command isn't added.

Not quite sure if QEMU wants the hmp command, though I think no,
per all the monitor commands are converted (or going to be converted)
to qmp. Anyway, libvirt won't want it.


> 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(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 751d3c2..f781372 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3505,3 +3505,29 @@
>       '*asl_compiler_rev':  'uint32',
>       '*file':              'str',
>       '*data':              'str' }}
> +
> +##
> +# @ConfigSchemaInfo:
> +#
> +# Configration schema information.
> +#
> +# @option: option name
> +#
> +# @config-schema: configuration schema string of one option

Since if no option name is  specified, all the config schema is 
returned, you
might want to change this comment a bit.

> +#
> +# Since 1.5
> +##
> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'config-schema': 'str'} }
> +
> +##
> +# @query-config-schema
> +#
> +# Query configuration schema information of options

And this.

> +#
> +# @option: #optional option name
> +#
> +# Returns: @ConfigSchemaInfo list
> +#
> +# Since 1.5
> +##
> +{'command': 'query-config-schema', 'data': {'*option': 'str'}, 'returns': ['ConfigSchemaInfo']}
> diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
> index 13bfea0..6449268 100644
> --- a/qemu-options-wrapper.h
> +++ b/qemu-options-wrapper.h
> @@ -18,6 +18,22 @@
>   
>   #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
>   
> +#elif defined(QEMU_OPTIONS_GENERATE_CONFIG)
> +
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
> +    opt_help,
> +
> +#define DEFHEADING(text)
> +#define ARCHHEADING(text, arch_mask)
> +
> +#elif defined(QEMU_OPTIONS_GENERATE_NAME)

How about QEMU_OPTIONS_GENERATE_OPTION_NAME?

> +
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
> +    option,
> +
> +#define DEFHEADING(text)
> +#define ARCHHEADING(text, arch_mask)
> +
>   #elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
>   
>   #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4d65422..1bb691e 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2414,7 +2414,39 @@ EQMP
>           .args_type  = "",
>           .mhandler.cmd_new = qmp_marshal_input_query_uuid,
>       },
> +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

And this.

> +
> +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",
> +      "option": "boot"
> +     }
> +    ]
> +}
> +
> +EQMP
>   
> +    {
> +        .name       = "query-config-schema",
> +        .args_type  = "option:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_query_config_schema,
> +    },

Need one blank line.

>   SQMP
>   query-migrate
>   -------------
> diff --git a/qmp.c b/qmp.c
> index ed6c7ef..a3f7cc9 100644
> --- a/qmp.c
> +++ 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"
> +#include "exec/gdbstub.h"
>   
>   NameInfo *qmp_query_name(Error **errp)
>   {
> @@ -78,6 +81,39 @@ UuidInfo *qmp_query_uuid(Error **errp)
>       return info;
>   }
>   
> +ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
> +                              const char *option, Error **errp)
> +{
> +    ConfigSchemaInfoList *conf_list = NULL, *entry;
> +    ConfigSchemaInfo *info;
> +
> +    char const *optionstr[] = {

s/optionstr/options/, ?

> +#define QEMU_OPTIONS_GENERATE_NAME
> +#include "qemu-options-wrapper.h"
> +    };
> +
> +    char const *configstr[] = {

s/configstr/configs/, ?

> +#define QEMU_OPTIONS_GENERATE_CONFIG
> +#include "qemu-options-wrapper.h"
> +    };
> +
> +    int i;
> +    for (i = 0; i < sizeof(optionstr) / sizeof(char *); i++) {
> +        if (!has_option || !strcmp(option, optionstr[i])) {

How about an invalid option name is passed? E.g.

{"execute": "query-config-schema", "arguments" : {"option": "foo"}}

Should we have an QMP error instead of sliently ignoring it with nothing
returned?

How about changing the logic to:

     bool found = false;

     if (has_option) {
         for (i = 0; i < sizeof(options) / sizeof(char *); i++) {
             if (!strcmp(option, option[i])) {
                 found = true;
                 break;
             }
         }

         if (found) {
             /* Return the found option schema */
         } else {
             /* QMP error */
         }
     } else {
         /* Return the schema of all options */
     }

> +            info = g_malloc0(sizeof(*info));
> +            info->option = g_strdup(option);
> +            info->config_schema = g_strdup(configstr[i]);
> +
> +            entry = g_malloc0(sizeof(*entry));
> +            entry->value = info;
> +            entry->next = conf_list;
> +            conf_list = entry;

No macros or helpers to manage the list?

Osier
Eric Blake April 19, 2013, 12:22 p.m. UTC | #2
On 04/19/2013 06:02 AM, Osier Yang wrote:
> On 19/04/13 17:52, Amos Kong wrote:
>> Libvirt doesn't have a stable way to know option support
> 
> Actually no way now.
> 

>> +
>> +##
>> +# @ConfigSchemaInfo:
>> +#
>> +# Configration schema information.
>> +#
>> +# @option: option name
>> +#
>> +# @config-schema: configuration schema string of one option
> 
> Since if no option name is  specified, all the config schema is
> returned, you
> might want to change this comment a bit.

No, this part is correct.  This is an intermediate type for one option,
then the QMP command returns an array of these types.

> 
>> +#
>> +# Since 1.5
>> +##
>> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str',
>> 'config-schema': 'str'} }
>> +
>> +##
>> +# @query-config-schema
>> +#
>> +# Query configuration schema information of options
> 
> And this.

But here, it might make sense to call out that the default is all
options, but specifying a name limits to one option.
Eric Blake April 19, 2013, 12:39 p.m. UTC | #3
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.  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).

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?

Overall, looks quite useful, and it is worth getting into 1.5.
Osier Yang April 19, 2013, 1:44 p.m. UTC | #4
On 19/04/13 20:02, Osier Yang wrote:
> On 19/04/13 17:52, Amos Kong wrote:
>> Libvirt doesn't have a stable way to know option support
>
> Actually no way now.
>
> Libvirt swtiched to use qmp to collect the qemu capabilities
> for qemu newer than 1.2.0,  thus there is no way to probe either
> if a option or a property is supported or not by QEMU. Any
> support for the new options/properties in libvirt are blocked.
>
> Except we go back to probe using the help output, but it's not
> right way to go.
>
> Consider to change the sentence to:
>
> Libvirt has no way to probe if an option or property is supported,
>
>> detail. This patch introdued a new qmp command to query
>
> s/introdued/introduces/
>
>> configuration schema information. hmp command isn't added.
>
> Not quite sure if QEMU wants the hmp command, though I think no,
> per all the monitor commands are converted (or going to be converted)
> to qmp. Anyway, libvirt won't want it.
>
>
>> 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(-)
>>
>> diff --git a/qapi-schema.json b/qapi-schema.json
>> index 751d3c2..f781372 100644
>> --- a/qapi-schema.json
>> +++ b/qapi-schema.json
>> @@ -3505,3 +3505,29 @@
>>       '*asl_compiler_rev':  'uint32',
>>       '*file':              'str',
>>       '*data':              'str' }}
>> +
>> +##
>> +# @ConfigSchemaInfo:
>> +#
>> +# Configration schema information.
>> +#
>> +# @option: option name
>> +#
>> +# @config-schema: configuration schema string of one option
>
> Since if no option name is  specified, all the config schema is 
> returned, you
> might want to change this comment a bit.
>
>> +#
>> +# Since 1.5
>> +##
>> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 
>> 'config-schema': 'str'} }
>> +
>> +##
>> +# @query-config-schema
>> +#
>> +# Query configuration schema information of options
>
> And this.
>
>> +#
>> +# @option: #optional option name
>> +#
>> +# Returns: @ConfigSchemaInfo list
>> +#
>> +# Since 1.5
>> +##
>> +{'command': 'query-config-schema', 'data': {'*option': 'str'}, 
>> 'returns': ['ConfigSchemaInfo']}
>> diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
>> index 13bfea0..6449268 100644
>> --- a/qemu-options-wrapper.h
>> +++ b/qemu-options-wrapper.h
>> @@ -18,6 +18,22 @@
>>     #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
>>   +#elif defined(QEMU_OPTIONS_GENERATE_CONFIG)
>> +
>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask) \
>> +    opt_help,
>> +
>> +#define DEFHEADING(text)
>> +#define ARCHHEADING(text, arch_mask)
>> +
>> +#elif defined(QEMU_OPTIONS_GENERATE_NAME)
>
> How about QEMU_OPTIONS_GENERATE_OPTION_NAME?
>
>> +
>> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask) \
>> +    option,
>> +
>> +#define DEFHEADING(text)
>> +#define ARCHHEADING(text, arch_mask)
>> +
>>   #elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
>>     #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
>> diff --git a/qmp-commands.hx b/qmp-commands.hx
>> index 4d65422..1bb691e 100644
>> --- a/qmp-commands.hx
>> +++ b/qmp-commands.hx
>> @@ -2414,7 +2414,39 @@ EQMP
>>           .args_type  = "",
>>           .mhandler.cmd_new = qmp_marshal_input_query_uuid,
>>       },
>> +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
>
> And this.
>
>> +
>> +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",
>> +      "option": "boot"
>> +     }
>> +    ]
>> +}
>> +
>> +EQMP
>>   +    {
>> +        .name       = "query-config-schema",
>> +        .args_type  = "option:s?",
>> +        .mhandler.cmd_new = qmp_marshal_input_query_config_schema,
>> +    },
>
> Need one blank line.
>
>>   SQMP
>>   query-migrate
>>   -------------
>> diff --git a/qmp.c b/qmp.c
>> index ed6c7ef..a3f7cc9 100644
>> --- a/qmp.c
>> +++ 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"
>> +#include "exec/gdbstub.h"
>>     NameInfo *qmp_query_name(Error **errp)
>>   {
>> @@ -78,6 +81,39 @@ UuidInfo *qmp_query_uuid(Error **errp)
>>       return info;
>>   }
>>   +ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
>> +                              const char *option, Error **errp)

Indention problem.

>> +{
>> +    ConfigSchemaInfoList *conf_list = NULL, *entry;
>> +    ConfigSchemaInfo *info;
>> +
>> +    char const *optionstr[] = {
>
> s/optionstr/options/, ?
>
>> +#define QEMU_OPTIONS_GENERATE_NAME
>> +#include "qemu-options-wrapper.h"
>> +    };
>> +
>> +    char const *configstr[] = {
>
> s/configstr/configs/, ?
>
>> +#define QEMU_OPTIONS_GENERATE_CONFIG
>> +#include "qemu-options-wrapper.h"
>> +    };
>> +
>> +    int i;
>> +    for (i = 0; i < sizeof(optionstr) / sizeof(char *); i++) {
>> +        if (!has_option || !strcmp(option, optionstr[i])) {
>
> How about an invalid option name is passed? E.g.
>
> {"execute": "query-config-schema", "arguments" : {"option": "foo"}}
>
> Should we have an QMP error instead of sliently ignoring it with nothing
> returned?
>
> How about changing the logic to:
>
>     bool found = false;
>
>     if (has_option) {
>         for (i = 0; i < sizeof(options) / sizeof(char *); i++) {
>             if (!strcmp(option, option[i])) {
>                 found = true;
>                 break;
>             }
>         }
>
>         if (found) {
>             /* Return the found option schema */
>         } else {
>             /* QMP error */
>         }
>     } else {
>         /* Return the schema of all options */
>     }
>
>> +            info = g_malloc0(sizeof(*info));
>> +            info->option = g_strdup(option);
>> +            info->config_schema = g_strdup(configstr[i]);
>> +
>> +            entry = g_malloc0(sizeof(*entry));
>> +            entry->value = info;
>> +            entry->next = conf_list;
>> +            conf_list = entry;
>
> No macros or helpers to manage the list?
>
> Osier
>
Osier Yang April 19, 2013, 1:50 p.m. UTC | #5
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
Paolo Bonzini April 19, 2013, 3:21 p.m. UTC | #6
Il 19/04/2013 11:52, Amos Kong ha scritto:
> 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.

Can you introspect QemuOpts instead?  All new options are added there.

Paolo

> 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(-)
> 
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 751d3c2..f781372 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3505,3 +3505,29 @@
>      '*asl_compiler_rev':  'uint32',
>      '*file':              'str',
>      '*data':              'str' }}
> +
> +##
> +# @ConfigSchemaInfo:
> +#
> +# Configration schema information.
> +#
> +# @option: option name
> +#
> +# @config-schema: configuration schema string of one option
> +#
> +# Since 1.5
> +##
> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'config-schema': 'str'} }
> +
> +##
> +# @query-config-schema
> +#
> +# Query configuration schema information of options
> +#
> +# @option: #optional option name
> +#
> +# Returns: @ConfigSchemaInfo list
> +#
> +# Since 1.5
> +##
> +{'command': 'query-config-schema', 'data': {'*option': 'str'}, 'returns': ['ConfigSchemaInfo']}
> diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
> index 13bfea0..6449268 100644
> --- a/qemu-options-wrapper.h
> +++ b/qemu-options-wrapper.h
> @@ -18,6 +18,22 @@
>  
>  #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
>  
> +#elif defined(QEMU_OPTIONS_GENERATE_CONFIG)
> +
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
> +    opt_help,
> +
> +#define DEFHEADING(text)
> +#define ARCHHEADING(text, arch_mask)
> +
> +#elif defined(QEMU_OPTIONS_GENERATE_NAME)
> +
> +#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
> +    option,
> +
> +#define DEFHEADING(text)
> +#define ARCHHEADING(text, arch_mask)
> +
>  #elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
>  
>  #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4d65422..1bb691e 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2414,7 +2414,39 @@ EQMP
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_input_query_uuid,
>      },
> +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",
> +      "option": "boot"
> +     }
> +    ]
> +}
> +
> +EQMP
>  
> +    {
> +        .name       = "query-config-schema",
> +        .args_type  = "option:s?",
> +        .mhandler.cmd_new = qmp_marshal_input_query_config_schema,
> +    },
>  SQMP
>  query-migrate
>  -------------
> diff --git a/qmp.c b/qmp.c
> index ed6c7ef..a3f7cc9 100644
> --- a/qmp.c
> +++ 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"
> +#include "exec/gdbstub.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -78,6 +81,39 @@ UuidInfo *qmp_query_uuid(Error **errp)
>      return info;
>  }
>  
> +ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
> +                              const char *option, Error **errp)
> +{
> +    ConfigSchemaInfoList *conf_list = NULL, *entry;
> +    ConfigSchemaInfo *info;
> +
> +    char const *optionstr[] = {
> +#define QEMU_OPTIONS_GENERATE_NAME
> +#include "qemu-options-wrapper.h"
> +    };
> +
> +    char const *configstr[] = {
> +#define QEMU_OPTIONS_GENERATE_CONFIG
> +#include "qemu-options-wrapper.h"
> +    };
> +
> +    int i;
> +    for (i = 0; i < sizeof(optionstr) / sizeof(char *); i++) {
> +        if (!has_option || !strcmp(option, optionstr[i])) {
> +            info = g_malloc0(sizeof(*info));
> +            info->option = g_strdup(option);
> +            info->config_schema = g_strdup(configstr[i]);
> +
> +            entry = g_malloc0(sizeof(*entry));
> +            entry->value = info;
> +            entry->next = conf_list;
> +            conf_list = entry;
> +        }
> +    }
> +
> +    return conf_list;
> +}
> +
>  void qmp_quit(Error **err)
>  {
>      no_shutdown = 0;
>
Amos Kong April 22, 2013, 11:48 a.m. UTC | #7
On Fri, Apr 19, 2013 at 05:21:37PM +0200, Paolo Bonzini wrote:
> Il 19/04/2013 11:52, Amos Kong ha scritto:
> > 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.
> 
> Can you introspect QemuOpts instead?  All new options are added there.


It would be exact to use QemuOpts. I tried to output the vm_config_groups[]
in qemu-config.c, but it seems not enough. (desc list of -netdev, -drive,
-device are all empty)

Is there a better way to go through _all_ the QemuOpts?


                Amos.

name: drive
name: chardev
 \ desc->name: backend
 \ desc->name: path
 \ desc->name: host
 \ desc->name: port
 \ desc->name: localaddr
 \ desc->name: localport
 \ desc->name: to
 \ desc->name: ipv4
 \ desc->name: ipv6
 \ desc->name: wait
 \ desc->name: server
 \ desc->name: delay
 \ desc->name: telnet
 \ desc->name: width
 \ desc->name: height
 \ desc->name: cols
 \ desc->name: rows
 \ desc->name: mux
 \ desc->name: signal
 \ desc->name: name
 \ desc->name: debug
 \ desc->name: size
name: device
name: netdev
name: net
name: rtc
 \ desc->name: base
 \ desc->name: clock
 \ desc->name: driftfix
name: global
 \ desc->name: driver
 \ desc->name: property
 \ desc->name: value
name: mon
 \ desc->name: mode
 \ desc->name: chardev
 \ desc->name: default
 \ desc->name: pretty
name: trace
 \ desc->name: events
 \ desc->name: file
name: option-rom
 \ desc->name: bootindex
 \ desc->name: romfile
name: machine
 \ desc->name: type
 \ desc->name: accel
 \ desc->name: kernel_irqchip
 \ desc->name: kvm_shadow_mem
 \ desc->name: kernel
 \ desc->name: initrd
 \ desc->name: append
 \ desc->name: dtb
 \ desc->name: dumpdtb
 \ desc->name: phandle_start
 \ desc->name: dt_compatible
 \ desc->name: dump-guest-core
 \ desc->name: mem-merge
 \ desc->name: usb
name: boot-opts
 \ desc->name: order
 \ desc->name: once
 \ desc->name: menu
 \ desc->name: splash
 \ desc->name: splash-time
 \ desc->name: reboot-timeout
 \ desc->name: strict
name: sandbox
 \ desc->name: enable
name: add-fd
 \ desc->name: fd
 \ desc->name: set
 \ desc->name: opaque
name: object
name: tpmdev
 \ desc->name: type
 \ desc->name: cancel-path
 \ desc->name: path
name: acpi
name: fsdev
 \ desc->name: fsdriver
 \ desc->name: path
 \ desc->name: security_model
 \ desc->name: writeout
 \ desc->name: readonly
 \ desc->name: socket
 \ desc->name: sock_fd
name: virtfs
 \ desc->name: fsdriver
 \ desc->name: path
 \ desc->name: mount_tag
 \ desc->name: security_model
 \ desc->name: writeout
 \ desc->name: readonly
 \ desc->name: socket
 \ desc->name: sock_fd
Paolo Bonzini April 22, 2013, 12:07 p.m. UTC | #8
Il 22/04/2013 13:48, Amos Kong ha scritto:
>>> > > 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.
>> > 
>> > Can you introspect QemuOpts instead?  All new options are added there.
> 
> It would be exact to use QemuOpts. I tried to output the vm_config_groups[]
> in qemu-config.c, but it seems not enough. (desc list of -netdev, -drive,
> -device are all empty)

That's expected because they are parsed otherwise, depending on the
backend type.  -chardev is currently working but it's an implementation
detail.

Paolo
Eric Blake April 22, 2013, 3 p.m. UTC | #9
On 04/22/2013 06:07 AM, Paolo Bonzini wrote:
> Il 22/04/2013 13:48, Amos Kong ha scritto:
>>>>>> 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.
>>>>
>>>> Can you introspect QemuOpts instead?  All new options are added there.
>>
>> It would be exact to use QemuOpts. I tried to output the vm_config_groups[]
>> in qemu-config.c, but it seems not enough. (desc list of -netdev, -drive,
>> -device are all empty)
> 
> That's expected because they are parsed otherwise, depending on the
> backend type.  -chardev is currently working but it's an implementation
> detail.

Libvirt cares most about newly added options, which should use qemuOpts
all the way.  We can understand that legacy options like -netdev might
not yet use qemuOpts, but they are also legacy options, and therefore
libvirt can already assume they exist since at least qemu 1.3 (when
libvirt switched over to QMP probing).  If we later add a new feature to
-netdev, we should also convert -netdev to qemuOpts at that time, so
that libvirt would know whether the new feature is available.

At any rate, we really DO want introspection, and having it in 1.5 is a
worthwhile goal.  Even if the introspection turns up empty on legacy
options, having it for the sake of new options is worth the effort.
Paolo Bonzini April 22, 2013, 3:16 p.m. UTC | #10
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Il 22/04/2013 17:00, Eric Blake ha scritto:
> On 04/22/2013 06:07 AM, Paolo Bonzini wrote:
>> Il 22/04/2013 13:48, Amos Kong ha scritto:
>>>>>>> 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.
>>>>> 
>>>>> Can you introspect QemuOpts instead?  All new options are
>>>>> added there.
>>> 
>>> It would be exact to use QemuOpts. I tried to output the
>>> vm_config_groups[] in qemu-config.c, but it seems not enough.
>>> (desc list of -netdev, -drive, -device are all empty)
>> 
>> That's expected because they are parsed otherwise, depending on
>> the backend type.  -chardev is currently working but it's an
>> implementation detail.
> 
> Libvirt cares most about newly added options, which should use
> qemuOpts all the way.  We can understand that legacy options like
> -netdev might not yet use qemuOpts, but they are also legacy
> options, and therefore libvirt can already assume they exist since
> at least qemu 1.3 (when libvirt switched over to QMP probing).  If
> we later add a new feature to -netdev, we should also convert
> -netdev to qemuOpts at that time, so that libvirt would know
> whether the new feature is available.

- -netdev is not a legacy option.  -netdev/-drive/-device do use
QemuOpts, but not for validation.  They create an object, and let the
object parse the option.

They are more complex than the other option, and need a different kind
of introspection (on the properties of a class, or something like that).

Paolo

> At any rate, we really DO want introspection, and having it in 1.5
> is a worthwhile goal.  Even if the introspection turns up empty on
> legacy options, having it for the sake of new options is worth the
> effort.
> 

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.19 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iQIcBAEBAgAGBQJRdVQyAAoJEBvWZb6bTYbyopcP/jE5TX/NziMeXmXmSS+GFn4G
4SH+4XckjZZPe2ZnpshvkWi1OrbsK0CKw9nk4xcZRFmUFnIZ4T1J2VBXodnATvKZ
+r6yqvOCuoleWQBlnN8OJm/I5kil5UkztiUSsDbgYyP2D4pr3Z7+uGX7ju/4oGCK
qEASGYRsGFItvKkjfUDL2UjBv3dnDerWSPj9IsD/sFajGqcBrvfbGK8YeOG7YvQF
Tinv5dhHg9dpnTJ/fwmw0xr3BmgzLf4fT16I73ErG1INOBjWSUPkQ0h8UeydJEJq
nvpj3/zlqqJdSNxZXU1FRP+stQN2hHDZsTXhKuY+2kbDFRqpwB8WG94TEbOdx6gr
fyNHfueWIByylmQNgbvBCyR2hVI+RipgGHfQ6slTcMMu2pZpZ1m9vWfZF8bvAS+p
NL4+Rf+Ic4uMKZnS6GvD15px0ugGPIcDdwX7YgVqjNMIRZhKFiOf9HYnUfJstQpN
WrEpcnyE4p0JzkO27Otezoz+RTpJ8HaKHvnsbM49BDlWDMme7jKveymWnCyeJxib
g7Hz7V7M76LK0Mlcn66PYctF6JVZP25hC3p3poYbm2F9Duvwg78+b53O6k1XN0sP
/kuZfYWrWRr6sx+/oN6HWMP5q60jRVYKUOYcNOriWS+6Yi8xohFvqQVu4qmycXOG
3HBqamagi+JNMiiO9cx/
=4p7B
-----END PGP SIGNATURE-----
Amos Kong April 23, 2013, 2:40 a.m. UTC | #11
On Mon, Apr 22, 2013 at 05:16:02PM +0200, Paolo Bonzini wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Il 22/04/2013 17:00, Eric Blake ha scritto:
> > On 04/22/2013 06:07 AM, Paolo Bonzini wrote:
> >> Il 22/04/2013 13:48, Amos Kong ha scritto:
> >>>>>>> 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.
> >>>>> 
> >>>>> Can you introspect QemuOpts instead?  All new options are
> >>>>> added there.
> >>> 
> >>> It would be exact to use QemuOpts. I tried to output the
> >>> vm_config_groups[] in qemu-config.c, but it seems not enough.
> >>> (desc list of -netdev, -drive, -device are all empty)
> >> 
> >> That's expected because they are parsed otherwise, depending on
> >> the backend type.  -chardev is currently working but it's an
> >> implementation detail.
> > 
> > Libvirt cares most about newly added options, which should use
> > qemuOpts all the way.  We can understand that legacy options like
> > -netdev might not yet use qemuOpts, but they are also legacy
> > options, and therefore libvirt can already assume they exist since
> > at least qemu 1.3 (when libvirt switched over to QMP probing).  If
> > we later add a new feature to -netdev, we should also convert
> > -netdev to qemuOpts at that time, so that libvirt would know
> > whether the new feature is available.
> 
> - -netdev is not a legacy option.  -netdev/-drive/-device do use
> QemuOpts, but not for validation.  They create an object, and let the
> object parse the option.
> 
> They are more complex than the other option, and need a different kind
> of introspection (on the properties of a class, or something like that).

'-netdev fds=...' was added after supported multiqueue, but we can't
check it from the output of vm_config_groups[]

Do we need to process all non-legacy options for validation first?
then add the query command.
 
> Paolo
> 
> > At any rate, we really DO want introspection, and having it in 1.5
> > is a worthwhile goal.  Even if the introspection turns up empty on
> > legacy options, having it for the sake of new options is worth the
> > effort.

Ok, I will work on another patch to output vm_config_groups[] with
clear JSON structure. Thanks
Luiz Capitulino April 23, 2013, 1:20 p.m. UTC | #12
On Mon, 22 Apr 2013 09:00:05 -0600
Eric Blake <eblake@redhat.com> wrote:

> At any rate, we really DO want introspection, and having it in 1.5 is a
> worthwhile goal.  Even if the introspection turns up empty on legacy
> options, having it for the sake of new options is worth the effort.

Agreed. But as you said in another email, we need a JSON representation
for this command. Dumping all options as strings is just like using -help.

Something else that has occurred to me is that, do we want this to be
a QMP command or do we want this to be a command-line option? If we do this
as a QMP command then libvirt would have to actually start QEMU just to
query supported options.

Another interesting point is about full schema introspection. This patch
only supports command-line options. It would be desirable to be able to
introspect all QAPI types and QMP commands, although I'm aware that this
might be a lot of work to do for 1.5.
Eric Blake April 23, 2013, 3:32 p.m. UTC | #13
On 04/23/2013 07:20 AM, Luiz Capitulino wrote:
> On Mon, 22 Apr 2013 09:00:05 -0600
> Eric Blake <eblake@redhat.com> wrote:
> 
>> At any rate, we really DO want introspection, and having it in 1.5 is a
>> worthwhile goal.  Even if the introspection turns up empty on legacy
>> options, having it for the sake of new options is worth the effort.
> 
> Agreed. But as you said in another email, we need a JSON representation
> for this command. Dumping all options as strings is just like using -help.
> 
> Something else that has occurred to me is that, do we want this to be
> a QMP command or do we want this to be a command-line option? If we do this
> as a QMP command then libvirt would have to actually start QEMU just to
> query supported options.

A command-line option might (or might not) be useful to humans, but a
QMP command is what libvirt wants.  Libvirt already starts a QMP session
for lots of other queries (not the least of which is the
'query-commands' query which would let us know whether to even expect
the new QMP command for command-line introspection to work).  Adding one
more QMP query to the mix called by libvirt is easy (no additional
processes, and the response already goes through the JSON parser that
handles all QMP responses); adding a new command line option is more
expensive (we would have to start qemu once to use the command line
option, then again for the QMP queries; also, it's more coding effort to
wire up libvirt to pass the stdout from the command line call into the
JSON parser that was normally expecting data from a Unix socket).

> 
> Another interesting point is about full schema introspection. This patch
> only supports command-line options. It would be desirable to be able to
> introspect all QAPI types and QMP commands, although I'm aware that this
> might be a lot of work to do for 1.5.

Based on the call today, getting introspection for command line
additions in time for 1.5, and saving full QMP introspection until 1.6,
would be reasonable; libvirt's immediate goal is to determine when it is
safe to use new command line options while doing its one-shot QMP
probing of a qemu binary.
Luiz Capitulino April 23, 2013, 4:55 p.m. UTC | #14
On Tue, 23 Apr 2013 09:32:31 -0600
Eric Blake <eblake@redhat.com> wrote:

> On 04/23/2013 07:20 AM, Luiz Capitulino wrote:
> > On Mon, 22 Apr 2013 09:00:05 -0600
> > Eric Blake <eblake@redhat.com> wrote:
> > 
> >> At any rate, we really DO want introspection, and having it in 1.5 is a
> >> worthwhile goal.  Even if the introspection turns up empty on legacy
> >> options, having it for the sake of new options is worth the effort.
> > 
> > Agreed. But as you said in another email, we need a JSON representation
> > for this command. Dumping all options as strings is just like using -help.
> > 
> > Something else that has occurred to me is that, do we want this to be
> > a QMP command or do we want this to be a command-line option? If we do this
> > as a QMP command then libvirt would have to actually start QEMU just to
> > query supported options.
> 
> A command-line option might (or might not) be useful to humans, but a
> QMP command is what libvirt wants.  Libvirt already starts a QMP session
> for lots of other queries (not the least of which is the
> 'query-commands' query which would let us know whether to even expect
> the new QMP command for command-line introspection to work).  Adding one
> more QMP query to the mix called by libvirt is easy (no additional
> processes, and the response already goes through the JSON parser that
> handles all QMP responses);

That's right.

> adding a new command line option is more
> expensive (we would have to start qemu once to use the command line
> option, then again for the QMP queries; also, it's more coding effort to
> wire up libvirt to pass the stdout from the command line call into the
> JSON parser that was normally expecting data from a Unix socket).
> 
> > 
> > Another interesting point is about full schema introspection. This patch
> > only supports command-line options. It would be desirable to be able to
> > introspect all QAPI types and QMP commands, although I'm aware that this
> > might be a lot of work to do for 1.5.
> 
> Based on the call today, getting introspection for command line
> additions in time for 1.5, and saving full QMP introspection until 1.6,
> would be reasonable; libvirt's immediate goal is to determine when it is
> safe to use new command line options while doing its one-shot QMP
> probing of a qemu binary.

Yes. All my points have been clarified in today's meeting.
diff mbox

Patch

diff --git a/qapi-schema.json b/qapi-schema.json
index 751d3c2..f781372 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3505,3 +3505,29 @@ 
     '*asl_compiler_rev':  'uint32',
     '*file':              'str',
     '*data':              'str' }}
+
+##
+# @ConfigSchemaInfo:
+#
+# Configration schema information.
+#
+# @option: option name
+#
+# @config-schema: configuration schema string of one option
+#
+# Since 1.5
+##
+{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'config-schema': 'str'} }
+
+##
+# @query-config-schema
+#
+# Query configuration schema information of options
+#
+# @option: #optional option name
+#
+# Returns: @ConfigSchemaInfo list
+#
+# Since 1.5
+##
+{'command': 'query-config-schema', 'data': {'*option': 'str'}, 'returns': ['ConfigSchemaInfo']}
diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
index 13bfea0..6449268 100644
--- a/qemu-options-wrapper.h
+++ b/qemu-options-wrapper.h
@@ -18,6 +18,22 @@ 
 
 #define DEFHEADING(text) ARCHHEADING(text, QEMU_ARCH_ALL)
 
+#elif defined(QEMU_OPTIONS_GENERATE_CONFIG)
+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
+    opt_help,
+
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
+#elif defined(QEMU_OPTIONS_GENERATE_NAME)
+
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)    \
+    option,
+
+#define DEFHEADING(text)
+#define ARCHHEADING(text, arch_mask)
+
 #elif defined(QEMU_OPTIONS_GENERATE_OPTIONS)
 
 #define DEF(option, opt_arg, opt_enum, opt_help, arch_mask)     \
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4d65422..1bb691e 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -2414,7 +2414,39 @@  EQMP
         .args_type  = "",
         .mhandler.cmd_new = qmp_marshal_input_query_uuid,
     },
+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",
+      "option": "boot"
+     }
+    ]
+}
+
+EQMP
 
+    {
+        .name       = "query-config-schema",
+        .args_type  = "option:s?",
+        .mhandler.cmd_new = qmp_marshal_input_query_config_schema,
+    },
 SQMP
 query-migrate
 -------------
diff --git a/qmp.c b/qmp.c
index ed6c7ef..a3f7cc9 100644
--- a/qmp.c
+++ 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"
+#include "exec/gdbstub.h"
 
 NameInfo *qmp_query_name(Error **errp)
 {
@@ -78,6 +81,39 @@  UuidInfo *qmp_query_uuid(Error **errp)
     return info;
 }
 
+ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
+                              const char *option, Error **errp)
+{
+    ConfigSchemaInfoList *conf_list = NULL, *entry;
+    ConfigSchemaInfo *info;
+
+    char const *optionstr[] = {
+#define QEMU_OPTIONS_GENERATE_NAME
+#include "qemu-options-wrapper.h"
+    };
+
+    char const *configstr[] = {
+#define QEMU_OPTIONS_GENERATE_CONFIG
+#include "qemu-options-wrapper.h"
+    };
+
+    int i;
+    for (i = 0; i < sizeof(optionstr) / sizeof(char *); i++) {
+        if (!has_option || !strcmp(option, optionstr[i])) {
+            info = g_malloc0(sizeof(*info));
+            info->option = g_strdup(option);
+            info->config_schema = g_strdup(configstr[i]);
+
+            entry = g_malloc0(sizeof(*entry));
+            entry->value = info;
+            entry->next = conf_list;
+            conf_list = entry;
+        }
+    }
+
+    return conf_list;
+}
+
 void qmp_quit(Error **err)
 {
     no_shutdown = 0;