From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:53457) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UTA1L-0000tV-Jq for qemu-devel@nongnu.org; Fri, 19 Apr 2013 08:02:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1UTA1J-0004Nf-V1 for qemu-devel@nongnu.org; Fri, 19 Apr 2013 08:02:27 -0400 Received: from mx1.redhat.com ([209.132.183.28]:43309) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1UTA1J-0004NY-MY for qemu-devel@nongnu.org; Fri, 19 Apr 2013 08:02:25 -0400 Message-ID: <51713241.70403@redhat.com> Date: Fri, 19 Apr 2013 20:02:09 +0800 From: Osier Yang MIME-Version: 1.0 References: <1366365123-5412-1-git-send-email-akong@redhat.com> In-Reply-To: <1366365123-5412-1-git-send-email-akong@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] monitor: introduce query-config-schema List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Amos Kong Cc: mdroth@linux.vnet.ibm.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, lcapitulino@redhat.com 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 > CC: Osier Yang > CC: Anthony Liguori > --- > 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