On 04/24/2013 06:47 AM, Amos Kong wrote: > Libvirt has no way to probe if an option or property is supported, > This patch introdues a new qmp command to query configuration schema > information. hmp command isn't added because it's not needed.
Agreed that no HMP counterpart is needed. However, I don't think we
have quite the right interface yet.
>
> Signed-off-by: Amos Kong <ak...@redhat.com>
> CC: Osier Yang <jy...@redhat.com>
> CC: Anthony Liguori <aligu...@us.ibm.com>
> ---
> qapi-schema.json | 29 +++++++++++++++++++++++++++++
> qmp-commands.hx | 40 ++++++++++++++++++++++++++++++++++++++++
> util/qemu-config.c | 40 ++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 109 insertions(+), 0 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 751d3c2..aeab057 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3505,3 +3505,32 @@
> '*asl_compiler_rev': 'uint32',
> '*file': 'str',
> '*data': 'str' }}
> +
> +##
> +# @ConfigSchemaInfo:
> +#
> +# Configration schema information.
> +#
> +# @option: option name
> +#
> +# @params: parameters strList of one option
Why just a strList? That only tells me option names. But we already
know so much more than that - we know the type and the help string.
> +#
> +# Since 1.5
> +##
> +{ 'type': 'ConfigSchemaInfo', 'data': {'option': 'str', 'params': ['str']} }
I'd rather see an array of structs, more closely mirroring what
include/qemu/option.h gives us:
# JSON representation of values of QEMUOptionParType, may grow in future
{ 'enum': 'ConfigParamType',
'data': [ 'flag', 'number', 'size', 'string' ] }
# JSON representation of QEMUOptionParameter, may grow in future
# @help is optional if no text was present
{ 'type': 'ConfigParamInfo',
'data': { 'name': 'str', 'type': 'ConfigParamType', '*help':'str' } }
# Each command line option, and its list of parameters
{ 'type': 'ConfigSchemaInfo',
'data': { 'option':'str', 'params': ['ConfigParamInfo'] } }
And that means we no longer have ['str'], which bypasses the need for
your patch 1/2.
> +
> +##
> +# @query-config-schema
> +#
> +# Query configuration schema information of options
> +#
> +# @option: #optional option name
> +#
> +# Returns: returns @ConfigSchemaInfo if option is assigned, returns
> +# @ConfigSchemaInfo list if no option is assigned, returns an error
> +# QERR_INVALID_OPTION_GROUP if assigned option doesn't exist.
That didn't read well. Also, QERR_INVALID_OPTION_GROUP is a generic
error; we don't mention any other QERR_* names in qapi-schema.json. How
about:
Returns: list of @ConfigSchemaInfo for all options (or for the given
@option). Returns an error if a given @option doesn't exist.
> +#
> +# Since 1.5
> +##
> +{'command': 'query-config-schema', 'data': {'*option': 'str'},
> + 'returns': ['ConfigSchemaInfo']}
This part looks good.
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4d65422..c6399be 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -2414,6 +2414,46 @@ 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 option is assigned, return
> +configuration schema list of all options if no option is assigned. return
> +an error QERR_INVALID_OPTION_GROUP if assigned option doesn't exist.
Again, QERR_INVALID_OPTION_GROUP is not a defined error (it is shorthand
for a specific message for a GenericError class).
> +
> +- "option": option name
> +- "params": parameters string list of one option
> +
> +Example:
> +
> +-> {"execute": "query-config-schema", "arguments" : {"option": "boot-opts"}}
> +<- {
> + "return": [
> + {
> + "params": [
> + "strict",
> + "reboot-timeout",
> + "splash-time",
> + "splash",
> + "menu",
> + "once",
> + "order"
> + ],
> + "option": "boot-opts"
> + }
> + ]
> + }
Back to my desire for more structure, I'd like to see:
-> {"execute": "query-config-schema", "arguments" : {"option": "boot-opts"}}
<- {
"return": [
{
"params": [
{
"name": "strict",
"type": "string"
},
{
"name": "reboot-timeout",
"type": "string"
},
...
],
"option": "boot-opts"
}
]
}
Another more interesting example, showing why the "type" and optional
"help" fields are useful, assuming
https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04810.html goes in:
-> {"execute": "query-config-schema", "arguments" : {"option": "drive"}}
<- {
"return": [
{
"params": [
{
"name": "bus",
"type": "number",
"help": "bus number"
},
{
"name": "if",
"type": "string",
"help": "interface (ide, scsi, sd, mtd, floppy,
pflash, virtio)"
},
...
],
"option": "drive"
}
]
}
>
> +ConfigSchemaInfoList *qmp_query_config_schema(bool has_option,
> + const char *option, Error
> **errp)
> +{
> + ConfigSchemaInfoList *conf_list = NULL, *entry;
> + ConfigSchemaInfo *info;
> + strList *str_list = NULL, *str_entry;
> + int entries, i, j;
> +
> + entries = ARRAY_SIZE(vm_config_groups);
> +
> + for (i = 0; i < entries; i++) {
> + if (vm_config_groups[i] != NULL &&
> + (!has_option || !strcmp(option, vm_config_groups[i]->name))) {
> + info = g_malloc0(sizeof(*info));
This part of the iteration looks fine.
> + info->option = g_strdup(vm_config_groups[i]->name);
> + str_list = NULL;
> +
> + for (j = 0; vm_config_groups[i]->desc[j].name != NULL; j++) {
> + str_entry = g_malloc0(sizeof(*str_entry));
> + str_entry->value =
> g_strdup(vm_config_groups[i]->desc[j].name);
> + str_entry->next = str_list;
> + str_list = str_entry;
> + }
Here, you'd need to allocate a bit more structure, to match the JSON I
proposed above.
> +
> + info->params = str_list;
> + entry = g_malloc0(sizeof(*entry));
> + entry->value = info;
> + entry->next = conf_list;
> + conf_list = entry;
> + }
> + }
> +
> + if (conf_list == NULL) {
> + error_set(errp, QERR_INVALID_OPTION_GROUP, option);
> + }
> +
> + return conf_list;
> +}
> +
> QemuOptsList *qemu_find_opts_err(const char *group, Error **errp)
> {
> return find_list(vm_config_groups, group, errp);
>
Looking forward to v2.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
