Re: [Qemu-devel] [RFC] introduce a general query-config cmd

[Amos Kong]

On Wed, Jan 23, 2013 at 06:41:44PM +0800, Amos Kong wrote:
> On Tue, Jan 22, 2013 at 12:26:03PM -0600, Anthony Liguori wrote:
> > Eric Blake <address@hidden> writes:
> > > On 01/22/2013 08:52 AM, Amos Kong wrote:
> > >>>>
> > >>>> Libvirt will need to expose an attribute that lets the user control
> > >>>> whether to use this new option; how do we probe via QMP whether the
> > >>>> new
> > >>>> -boot strict=on command-line option is available?
> > >>>
> > >>> Hi all,
> > >>>
> > >>> How about add new info/query command?
> > >>>
> > >>> (hmp) info strict-boot
> > >>>       on
> > >>>
> > >>> (qmp) {"execute": "query-strict-boot"}
> > >>>       {"return": {"state": true}}
> > >> 
> > >> It might be not a good solution, I already updated qemu-options.hx,
> > >> we can check help message to know if this new option is added or not.
> > >
> > > Having libvirt probe the -help output is out of the question.  We
> > > already declared that for qemu 1.3 and beyond, ALL command line behavior
> > > must ALSO be probe-able via QMP.  I think Anthony had a trick for
> > > testing for existence of various command line options without needing to
> > > add a new query-strict-boot command, but I don't remember what that
> > > trick was.
> > 
> > We need a generic query-config-schema command.
> 
> 
> Hi all,
> 
> The config info is included in qemu-options.def, current
> help() defines QEMU_OPTIONS_GENERATE_HELP, and includes
> qemu-options-wrapper.h,  DEF macro will output the help 
> message of options to the stdio.
> 
> I tried to add two branches in qemu-options-wrapper.h, which
> are used to generate two string arraies (one for option name,
> one for help message)
> 
> Thy help message is too long, it's failed to output all message to
> hmp monitor, always got a segfault, the max limitation maybe reached.
> It's more clear to only output help message of one option,
> 
> eg: {"execute": "query-config", "arguments" : {"name": "boot"}}
> 
> {"return": {"config": "-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"}}
> 
> But info command of hmp doesn't support parameter
>   (eg: (hmp) info config boot)
 
Hi Anthony,

As we talked in IRC, you will send a patch to resolve the query issue.
Can you post patch out when you have time? I will review it.

Thanks, Amos
 
> Q1) Is my patch ok for resolve the issue in last email (libvirt can check
> the help message of each option)?
> 
> Q2) Can we only added a command for qmp monitor?
> 
> Q3) Is it ok to output all help message for hmp (info config)?
> 
> Q4) query-config or query-config-schema ?
> 
> 
> diff --git a/hmp-commands.hx b/hmp-commands.hx
> index 010b8c9..13d1840 100644
> --- a/hmp-commands.hx
> +++ b/hmp-commands.hx
> @@ -1552,6 +1552,8 @@ show the vnc server status
>  show the current VM name
>  @item info uuid
>  show the current VM UUID
> address@hidden info config
> +show config
>  @item info cpustats
>  show CPU statistics
>  @item info usernet
> diff --git a/hmp.c b/hmp.c
> index 9e9e624..c0d84d1 100644
> --- a/hmp.c
> +++ b/hmp.c
> @@ -98,6 +98,17 @@ void hmp_info_uuid(Monitor *mon)
>      qapi_free_UuidInfo(info);
>  }
>  
> +void hmp_info_config(Monitor *mon)
> +{
> +    ConfigInfo *info;
> +
> +    /* Fixed ME: hmp info command doesn't support parameter */
> +
> +    info = qmp_query_config("boot", NULL);
> +    monitor_printf(mon, "%s\n", info->config);
> +    qapi_free_ConfigInfo(info);
> +}
> +
>  void hmp_info_chardev(Monitor *mon)
>  {
>      ChardevInfoList *char_info, *info;
> diff --git a/hmp.h b/hmp.h
> index 21f3e05..f217a8c 100644
> --- a/hmp.h
> +++ b/hmp.h
> @@ -23,6 +23,7 @@ void hmp_info_version(Monitor *mon);
>  void hmp_info_kvm(Monitor *mon);
>  void hmp_info_status(Monitor *mon);
>  void hmp_info_uuid(Monitor *mon);
> +void hmp_info_config(Monitor *mon);
>  void hmp_info_chardev(Monitor *mon);
>  void hmp_info_mice(Monitor *mon);
>  void hmp_info_migrate(Monitor *mon);
> diff --git a/monitor.c b/monitor.c
> index 9cf419b..6f331fa 100644
> --- a/monitor.c
> +++ b/monitor.c
> @@ -2661,6 +2661,13 @@ static mon_cmd_t info_cmds[] = {
>          .help       = "show the current VM UUID",
>          .mhandler.info = hmp_info_uuid,
>      },
> +    {
> +        .name       = "config",
> +        .args_type  = "",
> +        .params     = "",
> +        .help       = "show the config",
> +        .mhandler.info = hmp_info_config,
> +    },
>  #if defined(TARGET_PPC)
>      {
>          .name       = "cpustats",
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 5dfa052..8c46d57 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3017,3 +3017,23 @@
>  # Since: 1.3.0
>  ##
>  { 'command': 'nbd-server-stop' }
> +
> +##
> +# @ConfigInfo:
> +#
> +# Commandline configration information.
> +#
> +##
> +{ 'type': 'ConfigInfo', 'data': {'config': 'str'} }
> +
> +##
> +# @query-config
> +#
> +# Query configuration information of one option
> +#
> +# @name: option name
> +#
> +# Returns: configuration information.
> +#
> +##
> +{'command': 'query-config', 'data': {'name': 'str'}, 'returns': 'ConfigInfo'}
> diff --git a/qemu-options-wrapper.h b/qemu-options-wrapper.h
> index 13bfea0..97b44fb 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 5c692d0..ed42525 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2339,7 +2339,30 @@ EQMP
>          .args_type  = "",
>          .mhandler.cmd_new = qmp_marshal_input_query_uuid,
>      },
> +SQMP
> +query-config
> +------------
> +
> +Show config.
> +
> +- "Config": config
> +
> +Example:
> +-> {"execute": "query-config", "arguments" : {"name": "boot"}}
> +<- {"return": {"config": "-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"}}
> +
> +EQMP
>  
> +    {
> +        .name       = "query-config",
> +        .args_type  = "name:s",
> +        .mhandler.cmd_new = qmp_marshal_input_query_config,
> +    },
>  SQMP
>  query-migrate
>  -------------
> diff --git a/qmp.c b/qmp.c
> index 55b056b..6a3a13a 100644
> --- a/qmp.c
> +++ b/qmp.c
> @@ -24,6 +24,7 @@
>  #include "hw/qdev.h"
>  #include "sysemu/blockdev.h"
>  #include "qom/qom-qobject.h"
> +#include "qemu-options.h"
>  
>  NameInfo *qmp_query_name(Error **errp)
>  {
> @@ -78,6 +79,31 @@ UuidInfo *qmp_query_uuid(Error **errp)
>      return info;
>  }
>  
> +ConfigInfo *qmp_query_config(const char *name, Error **errp)
> +{
> +    ConfigInfo *info = g_malloc0(sizeof(*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 (!strcmp(name, optionstr[i])) {
> +            info->config = g_strdup(configstr[i]);
> +            break;
> +        }
> +    }
> +
> +    return info;
> +}
> +
>  void qmp_quit(Error **err)
>  {
>      no_shutdown = 0;