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

[Amos Kong <akong@redhat.com>]

On Mon, Jan 28, 2013 at 11:19:34AM +0800, Amos Kong wrote:
> 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 <eblake@redhat.com> 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.

Anthony, ping :)

[1] http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg01259.html
[2] http://lists.nongnu.org/archive/html/qemu-devel/2013-01/msg05167.html
 
> 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
> > +@item 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;