| Message ID | 1366883435-4993-1-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 25 Apr 2013 17:50:35 +0800 Amos Kong <akong@redhat.com> wrote: > Libvirt has no way to probe if an option or property is supported, > This patch introduces a new qmp command to query command line > option information. hmp command isn't added because it's not needed. > > Signed-off-by: Amos Kong <akong@redhat.com> > CC: Luiz Capitulino <lcapitulino@redhat.com> > CC: Osier Yang <jyang@redhat.com> > CC: Anthony Liguori <aliguori@us.ibm.com> This version looks good to me, but it has two small problems. First, some options like -drive, -net, -netdev and -device have an empty QemuOptsList, meaning that option validation is not done upfront. The other problem is that we're returning some options which seem to be created at run-time and I honestly have never heard of them, like tmpdev and acpi. I think there are two possible fixes for both issues: 1. Do nothing, accept patch as is and fix things in 1.6 2. Don't return an option when 'desc' is NULL I'm willing to do 1, as those options don't return any options anyway and I don't believe any client will mess with them (worst case the client will pass an option qemu doesn't accept and will refuse to run). I have one more comment below, but case we accept to do 1 I can fix it myself. > --- > V3: fix json schema and comments (Eric) > V4: fix descriptions, rename command, check enum type, cleanup > (Luiz, Eric) > V5: fix typo, cleanup (Osier) > --- > qapi-schema.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 47 ++++++++++++++++++++++++++++++++++++++ > util/qemu-config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 180 insertions(+) > > diff --git a/qapi-schema.json b/qapi-schema.json > index 751d3c2..10affbd 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -3505,3 +3505,69 @@ > '*asl_compiler_rev': 'uint32', > '*file': 'str', > '*data': 'str' }} > + > +## > +# @CommandLineParameterType: > +# > +# Possible types for an option parameter. > +# > +# @string: accepts a character string > +# > +# @boolean: accepts "on" or "off" > +# > +# @number: accepts a number > +# > +# @size: accepts a number followed by an optional postfix (K)ilo, > +# (M)ega, (G)iga, (T)era > +# > +# Since 1.5 > +## > +{ 'enum': 'CommandLineParameterType', > + 'data': ['string', 'boolean', 'number', 'size'] } > + > +## > +# @CommandLineParameterInfo: > +# > +# Details about a single parameter of a command line option. > +# > +# @name: parameter name > +# > +# @type: parameter @CommandLineParameterType > +# > +# @help: #optional human readable text string, not suitable for parsing. > +# > +# Since 1.5 > +## > +{ 'type': 'CommandLineParameterInfo', > + 'data': { 'name': 'str', > + 'type': 'CommandLineParameterType', > + '*help': 'str' } } > + > +## > +# @CommandLineOptionInfo: > +# > +# Details about a command line option, including its list of parameters details > +# > +# @option: option name > +# > +# @parameters: an array of @CommandLineParameterInfo > +# > +# Since 1.5 > +## > +{ 'type': 'CommandLineOptionInfo', > + 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } } > + > +## > +# @query-command-line-options: > +# > +# Query command line option schema. > +# > +# @option: #optional option name > +# > +# Returns: list of @CommandLineOptionInfo for all options (or for the given > +# @option). Returns an error if the given @option doesn't exist. > +# > +# Since 1.5 > +## > +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, I'm not a huge fan of option being optional, I prefer dropping it and making the command simpler. But I won't refuse the patch because of that. > + 'returns': ['CommandLineOptionInfo'] } > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 4d65422..31bb360 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2416,6 +2416,53 @@ EQMP > }, > > SQMP > +query-command-line-options > +-------------------------- > + > +Show command line option schema. > + > +Return a json-array of command line option schema for all options (or for > +the given option), returning an error if the given option doesn't exist. > + > +Each array entry contains the following: > + > +- "option": option name (json-string) > +- "parameters": a json-array describes all parameters of the option: > + - "name": parameter name (json-string) > + - "type": parameter type (one of 'string', boolean', 'number', > + or 'size') > + - "help": human readable description of the parameter > + (json-string, optional) > + > +Example: > + > +-> { "execute": "query-command-line-options", "arguments": { "option": "option-rom" } } > +<- { "return": [ > + { > + "parameters": [ > + { > + "name": "romfile", > + "type": "string" > + }, > + { > + "name": "bootindex", > + "type": "number" > + } > + ], > + "option": "option-rom" > + } > + ] > + } > + > +EQMP > + > + { > + .name = "query-command-line-options", > + .args_type = "option:s?", > + .mhandler.cmd_new = qmp_marshal_input_query_command_line_options, > + }, > + > +SQMP > query-migrate > ------------- > > diff --git a/util/qemu-config.c b/util/qemu-config.c > index 01ca890..7d3874f 100644 > --- a/util/qemu-config.c > +++ b/util/qemu-config.c > @@ -5,6 +5,7 @@ > #include "qapi/qmp/qerror.h" > #include "hw/qdev.h" > #include "qapi/error.h" > +#include "qmp-commands.h" > > static QemuOptsList *vm_config_groups[32]; > > @@ -37,6 +38,72 @@ QemuOptsList *qemu_find_opts(const char *group) > return ret; > } > > +static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc) > +{ > + CommandLineParameterInfoList *param_list = NULL, *entry; > + CommandLineParameterInfo *info; > + int i; > + > + for (i = 0; desc[i].name != NULL; i++) { > + info = g_malloc0(sizeof(*info)); > + info->name = g_strdup(desc[i].name); > + > + switch (desc[i].type) { > + case QEMU_OPT_STRING: > + info->type = COMMAND_LINE_PARAMETER_TYPE_STRING; > + break; > + case QEMU_OPT_BOOL: > + info->type = COMMAND_LINE_PARAMETER_TYPE_BOOLEAN; > + break; > + case QEMU_OPT_NUMBER: > + info->type = COMMAND_LINE_PARAMETER_TYPE_NUMBER; > + break; > + case QEMU_OPT_SIZE: > + info->type = COMMAND_LINE_PARAMETER_TYPE_SIZE; > + break; > + } > + > + if (desc[i].help) { > + info->has_help = true; > + info->help = g_strdup(desc[i].help); > + } > + > + entry = g_malloc0(sizeof(*entry)); > + entry->value = info; > + entry->next = param_list; > + param_list = entry; > + } > + > + return param_list; > +} > + > +CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, > + const char *option, > + Error **errp) > +{ > + CommandLineOptionInfoList *conf_list = NULL, *entry; > + CommandLineOptionInfo *info; > + int i; > + > + for (i = 0; vm_config_groups[i] != NULL; i++) { > + if (!has_option || !strcmp(option, vm_config_groups[i]->name)) { > + info = g_malloc0(sizeof(*info)); > + info->option = g_strdup(vm_config_groups[i]->name); > + info->parameters = query_option_descs(vm_config_groups[i]->desc); > + 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); You should be using error_setg() like: error_setg(errp, "invalid option name: %s", option); But I can fix that myself in qmp branch. > + } > + > + return conf_list; > +} > + > QemuOptsList *qemu_find_opts_err(const char *group, Error **errp) > { > return find_list(vm_config_groups, group, errp);
On 04/25/2013 03:50 AM, Amos Kong wrote: > Libvirt has no way to probe if an option or property is supported, > This patch introduces a new qmp command to query command line > option information. hmp command isn't added because it's not needed. > > Signed-off-by: Amos Kong <akong@redhat.com> > CC: Luiz Capitulino <lcapitulino@redhat.com> > CC: Osier Yang <jyang@redhat.com> > CC: Anthony Liguori <aliguori@us.ibm.com> > --- > V3: fix json schema and comments (Eric) > V4: fix descriptions, rename command, check enum type, cleanup > (Luiz, Eric) > V5: fix typo, cleanup (Osier) > --- > qapi-schema.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > qmp-commands.hx | 47 ++++++++++++++++++++++++++++++++++++++ > util/qemu-config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 180 insertions(+) Only minor typo fixups below; I'm assuming Luiz can fix them up while putting on the QMP pull request, without needing to see a v6. Reviewed-by: Eric Blake <eblake@redhat.com> > +## > +# @CommandLineParameterType: > +# > +# Possible types for an option parameter. > +# > +# @string: accepts a character string > +# > +# @boolean: accepts "on" or "off" > +# > +# @number: accepts a number > +# > +# @size: accepts a number followed by an optional postfix (K)ilo, s/postfix/suffix/ > + > +## > +# @CommandLineOptionInfo: > +# > +# Details about a command line option, including its list of parameters details s/parameters/parameter/ > +## > +# @query-command-line-options: > +# > +# Query command line option schema. > +# > +# @option: #optional option name > +# > +# Returns: list of @CommandLineOptionInfo for all options (or for the given > +# @option). Returns an error if the given @option doesn't exist. > +# > +# Since 1.5 > +## > +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, > + 'returns': ['CommandLineOptionInfo'] } Libvirt probably won't use the optional 'option' argument. I don't care whether you leave it in or Luiz takes filtering out for the 1.5 release. > diff --git a/qmp-commands.hx b/qmp-commands.hx > index 4d65422..31bb360 100644 > --- a/qmp-commands.hx > +++ b/qmp-commands.hx > @@ -2416,6 +2416,53 @@ EQMP > }, > > SQMP > +query-command-line-options > +-------------------------- > + > +Show command line option schema. > + > +Return a json-array of command line option schema for all options (or for > +the given option), returning an error if the given option doesn't exist. > + > +Each array entry contains the following: > + > +- "option": option name (json-string) > +- "parameters": a json-array describes all parameters of the option: > + - "name": parameter name (json-string) > + - "type": parameter type (one of 'string', boolean', 'number', s/boolean'/'boolean'/
On 04/25/2013 07:20 AM, Luiz Capitulino wrote: > On Thu, 25 Apr 2013 17:50:35 +0800 > Amos Kong <akong@redhat.com> wrote: > >> Libvirt has no way to probe if an option or property is supported, >> This patch introduces a new qmp command to query command line >> option information. hmp command isn't added because it's not needed. >> >> Signed-off-by: Amos Kong <akong@redhat.com> >> CC: Luiz Capitulino <lcapitulino@redhat.com> >> CC: Osier Yang <jyang@redhat.com> >> CC: Anthony Liguori <aliguori@us.ibm.com> > > This version looks good to me, but it has two small problems. > > First, some options like -drive, -net, -netdev and -device have an empty > QemuOptsList, meaning that option validation is not done upfront. > > The other problem is that we're returning some options which seem to be > created at run-time and I honestly have never heard of them, like tmpdev > and acpi. > > I think there are two possible fixes for both issues: > > 1. Do nothing, accept patch as is and fix things in 1.6 I can live with this for 1.5. Initially, libvirt will probably only be querying whether a set of known option names exist, and not looking at the parameter definitions tied to the option nor trying to invoke command line options that libvirt doesn't already know. Put another way, the driving factor to get this into 1.5 is so that libvirt knows whether it can use -mem-merge: https://www.redhat.com/archives/libvir-list/2013-April/msg01263.html While I anticipate that future libvirt versions may get fancier and use even more details of option introspection, such as learning whether a parameter was added in a later release, that's not the driving factor right now. > > 2. Don't return an option when 'desc' is NULL That might backfire; it's better to output too much than too little. > > I'm willing to do 1, as those options don't return any options anyway and > I don't believe any client will mess with them (worst case the client will pass > an option qemu doesn't accept and will refuse to run). Yep, and libvirt certainly won't be calling 'qemu -tmpdev'. > > I have one more comment below, but case we accept to do 1 I can fix it myself. > >> +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, > > I'm not a huge fan of option being optional, I prefer dropping it and making > the command simpler. But I won't refuse the patch because of that. I'm leaving that up to you. I personally like the idea of filtering (I would definitely use it via 'virsh qemu-monitor-command' when doing development work on libvirt), but it's not essential. >> + if (conf_list == NULL) { >> + error_set(errp, QERR_INVALID_OPTION_GROUP, option); > > You should be using error_setg() like: > > error_setg(errp, "invalid option name: %s", option); > > But I can fix that myself in qmp branch. Based on include/qapi/qmp/qerror.h, the message you would be replacing is: #define QERR_INVALID_OPTION_GROUP \ ERROR_CLASS_GENERIC_ERROR, "There is no option group '%s'" Yeah, "invalid option name: foo" reads better than "There is no option group 'foo'".
On Thu, 25 Apr 2013 07:57:11 -0600 Eric Blake <eblake@redhat.com> wrote: > On 04/25/2013 03:50 AM, Amos Kong wrote: > > Libvirt has no way to probe if an option or property is supported, > > This patch introduces a new qmp command to query command line > > option information. hmp command isn't added because it's not needed. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > CC: Luiz Capitulino <lcapitulino@redhat.com> > > CC: Osier Yang <jyang@redhat.com> > > CC: Anthony Liguori <aliguori@us.ibm.com> > > --- > > V3: fix json schema and comments (Eric) > > V4: fix descriptions, rename command, check enum type, cleanup > > (Luiz, Eric) > > V5: fix typo, cleanup (Osier) > > --- > > qapi-schema.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ > > qmp-commands.hx | 47 ++++++++++++++++++++++++++++++++++++++ > > util/qemu-config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 180 insertions(+) > > Only minor typo fixups below; I'm assuming Luiz can fix them up while > putting on the QMP pull request, without needing to see a v6. Yes. Applied to the qmp branch, thanks. > > Reviewed-by: Eric Blake <eblake@redhat.com> > > > +## > > +# @CommandLineParameterType: > > +# > > +# Possible types for an option parameter. > > +# > > +# @string: accepts a character string > > +# > > +# @boolean: accepts "on" or "off" > > +# > > +# @number: accepts a number > > +# > > +# @size: accepts a number followed by an optional postfix (K)ilo, > > s/postfix/suffix/ > > > + > > +## > > +# @CommandLineOptionInfo: > > +# > > +# Details about a command line option, including its list of parameters details > > s/parameters/parameter/ > > > +## > > +# @query-command-line-options: > > +# > > +# Query command line option schema. > > +# > > +# @option: #optional option name > > +# > > +# Returns: list of @CommandLineOptionInfo for all options (or for the given > > +# @option). Returns an error if the given @option doesn't exist. > > +# > > +# Since 1.5 > > +## > > +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, > > + 'returns': ['CommandLineOptionInfo'] } > > Libvirt probably won't use the optional 'option' argument. I don't care > whether you leave it in or Luiz takes filtering out for the 1.5 release. > > > diff --git a/qmp-commands.hx b/qmp-commands.hx > > index 4d65422..31bb360 100644 > > --- a/qmp-commands.hx > > +++ b/qmp-commands.hx > > @@ -2416,6 +2416,53 @@ EQMP > > }, > > > > SQMP > > +query-command-line-options > > +-------------------------- > > + > > +Show command line option schema. > > + > > +Return a json-array of command line option schema for all options (or for > > +the given option), returning an error if the given option doesn't exist. > > + > > +Each array entry contains the following: > > + > > +- "option": option name (json-string) > > +- "parameters": a json-array describes all parameters of the option: > > + - "name": parameter name (json-string) > > + - "type": parameter type (one of 'string', boolean', 'number', > > s/boolean'/'boolean'/ >
On 25/04/13 21:20, Luiz Capitulino wrote: > On Thu, 25 Apr 2013 17:50:35 +0800 > Amos Kong <akong@redhat.com> wrote: > >> Libvirt has no way to probe if an option or property is supported, >> This patch introduces a new qmp command to query command line >> option information. hmp command isn't added because it's not needed. >> >> Signed-off-by: Amos Kong <akong@redhat.com> >> CC: Luiz Capitulino <lcapitulino@redhat.com> >> CC: Osier Yang <jyang@redhat.com> >> CC: Anthony Liguori <aliguori@us.ibm.com> > This version looks good to me, but it has two small problems. > > First, some options like -drive, -net, -netdev and -device have an empty > QemuOptsList, meaning that option validation is not done upfront. Not sure if there is work of libvirt which is blocked by -net, -netdev, and -device, but I have one which needs to probe the properties of -drive. https://bugzilla.redhat.com/show_bug.cgi?id=821045 > > The other problem is that we're returning some options which seem to be > created at run-time and I honestly have never heard of them, like tmpdev > and acpi. > > I think there are two possible fixes for both issues: > > 1. Do nothing, accept patch as is and fix things in 1.6 > > 2. Don't return an option when 'desc' is NULL > > I'm willing to do 1, as those options don't return any options anyway and > I don't believe any client will mess with them (worst case the client will pass > an option qemu doesn't accept and will refuse to run). 1 sounds better, it doesn't hurt for libvirt to recongnize the empty result, and it may even better because at least we known the option is supported, even the result is empty. For the "-drive" properties probing, not sure if we can wait till 1.6. > > I have one more comment below, but case we accept to do 1 I can fix it myself. > >> --- >> V3: fix json schema and comments (Eric) >> V4: fix descriptions, rename command, check enum type, cleanup >> (Luiz, Eric) >> V5: fix typo, cleanup (Osier) >> --- >> qapi-schema.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ >> qmp-commands.hx | 47 ++++++++++++++++++++++++++++++++++++++ >> util/qemu-config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 180 insertions(+) >> >> diff --git a/qapi-schema.json b/qapi-schema.json >> index 751d3c2..10affbd 100644 >> --- a/qapi-schema.json >> +++ b/qapi-schema.json >> @@ -3505,3 +3505,69 @@ >> '*asl_compiler_rev': 'uint32', >> '*file': 'str', >> '*data': 'str' }} >> + >> +## >> +# @CommandLineParameterType: >> +# >> +# Possible types for an option parameter. >> +# >> +# @string: accepts a character string >> +# >> +# @boolean: accepts "on" or "off" >> +# >> +# @number: accepts a number >> +# >> +# @size: accepts a number followed by an optional postfix (K)ilo, >> +# (M)ega, (G)iga, (T)era >> +# >> +# Since 1.5 >> +## >> +{ 'enum': 'CommandLineParameterType', >> + 'data': ['string', 'boolean', 'number', 'size'] } >> + >> +## >> +# @CommandLineParameterInfo: >> +# >> +# Details about a single parameter of a command line option. >> +# >> +# @name: parameter name >> +# >> +# @type: parameter @CommandLineParameterType >> +# >> +# @help: #optional human readable text string, not suitable for parsing. >> +# >> +# Since 1.5 >> +## >> +{ 'type': 'CommandLineParameterInfo', >> + 'data': { 'name': 'str', >> + 'type': 'CommandLineParameterType', >> + '*help': 'str' } } >> + >> +## >> +# @CommandLineOptionInfo: >> +# >> +# Details about a command line option, including its list of parameters details >> +# >> +# @option: option name >> +# >> +# @parameters: an array of @CommandLineParameterInfo >> +# >> +# Since 1.5 >> +## >> +{ 'type': 'CommandLineOptionInfo', >> + 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } } >> + >> +## >> +# @query-command-line-options: >> +# >> +# Query command line option schema. >> +# >> +# @option: #optional option name >> +# >> +# Returns: list of @CommandLineOptionInfo for all options (or for the given >> +# @option). Returns an error if the given @option doesn't exist. >> +# >> +# Since 1.5 >> +## >> +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, > I'm not a huge fan of option being optional, I prefer dropping it and making > the command simpler. But I won't refuse the patch because of that. > >> + 'returns': ['CommandLineOptionInfo'] } >> diff --git a/qmp-commands.hx b/qmp-commands.hx >> index 4d65422..31bb360 100644 >> --- a/qmp-commands.hx >> +++ b/qmp-commands.hx >> @@ -2416,6 +2416,53 @@ EQMP >> }, >> >> SQMP >> +query-command-line-options >> +-------------------------- >> + >> +Show command line option schema. >> + >> +Return a json-array of command line option schema for all options (or for >> +the given option), returning an error if the given option doesn't exist. >> + >> +Each array entry contains the following: >> + >> +- "option": option name (json-string) >> +- "parameters": a json-array describes all parameters of the option: >> + - "name": parameter name (json-string) >> + - "type": parameter type (one of 'string', boolean', 'number', >> + or 'size') >> + - "help": human readable description of the parameter >> + (json-string, optional) >> + >> +Example: >> + >> +-> { "execute": "query-command-line-options", "arguments": { "option": "option-rom" } } >> +<- { "return": [ >> + { >> + "parameters": [ >> + { >> + "name": "romfile", >> + "type": "string" >> + }, >> + { >> + "name": "bootindex", >> + "type": "number" >> + } >> + ], >> + "option": "option-rom" >> + } >> + ] >> + } >> + >> +EQMP >> + >> + { >> + .name = "query-command-line-options", >> + .args_type = "option:s?", >> + .mhandler.cmd_new = qmp_marshal_input_query_command_line_options, >> + }, >> + >> +SQMP >> query-migrate >> ------------- >> >> diff --git a/util/qemu-config.c b/util/qemu-config.c >> index 01ca890..7d3874f 100644 >> --- a/util/qemu-config.c >> +++ b/util/qemu-config.c >> @@ -5,6 +5,7 @@ >> #include "qapi/qmp/qerror.h" >> #include "hw/qdev.h" >> #include "qapi/error.h" >> +#include "qmp-commands.h" >> >> static QemuOptsList *vm_config_groups[32]; >> >> @@ -37,6 +38,72 @@ QemuOptsList *qemu_find_opts(const char *group) >> return ret; >> } >> >> +static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc) >> +{ >> + CommandLineParameterInfoList *param_list = NULL, *entry; >> + CommandLineParameterInfo *info; >> + int i; >> + >> + for (i = 0; desc[i].name != NULL; i++) { >> + info = g_malloc0(sizeof(*info)); >> + info->name = g_strdup(desc[i].name); >> + >> + switch (desc[i].type) { >> + case QEMU_OPT_STRING: >> + info->type = COMMAND_LINE_PARAMETER_TYPE_STRING; >> + break; >> + case QEMU_OPT_BOOL: >> + info->type = COMMAND_LINE_PARAMETER_TYPE_BOOLEAN; >> + break; >> + case QEMU_OPT_NUMBER: >> + info->type = COMMAND_LINE_PARAMETER_TYPE_NUMBER; >> + break; >> + case QEMU_OPT_SIZE: >> + info->type = COMMAND_LINE_PARAMETER_TYPE_SIZE; >> + break; >> + } >> + >> + if (desc[i].help) { >> + info->has_help = true; >> + info->help = g_strdup(desc[i].help); >> + } >> + >> + entry = g_malloc0(sizeof(*entry)); >> + entry->value = info; >> + entry->next = param_list; >> + param_list = entry; >> + } >> + >> + return param_list; >> +} >> + >> +CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, >> + const char *option, >> + Error **errp) >> +{ >> + CommandLineOptionInfoList *conf_list = NULL, *entry; >> + CommandLineOptionInfo *info; >> + int i; >> + >> + for (i = 0; vm_config_groups[i] != NULL; i++) { >> + if (!has_option || !strcmp(option, vm_config_groups[i]->name)) { >> + info = g_malloc0(sizeof(*info)); >> + info->option = g_strdup(vm_config_groups[i]->name); >> + info->parameters = query_option_descs(vm_config_groups[i]->desc); >> + 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); > You should be using error_setg() like: > > error_setg(errp, "invalid option name: %s", option); > > But I can fix that myself in qmp branch. > >> + } >> + >> + return conf_list; >> +} >> + >> QemuOptsList *qemu_find_opts_err(const char *group, Error **errp) >> { >> return find_list(vm_config_groups, group, errp);
On 04/25/2013 09:48 AM, Osier Yang wrote: >> First, some options like -drive, -net, -netdev and -device have an empty >> QemuOptsList, meaning that option validation is not done upfront. > > Not sure if there is work of libvirt which is blocked by -net, -netdev, and > -device, but I have one which needs to probe the properties of -drive. > > https://bugzilla.redhat.com/show_bug.cgi?id=821045 That BZ is not public; but the gist of it talked about whether a future RHEL should support Paolo's work on providing thin-provisioning options as a parameter to -drive, and as far as I can see, that didn't land in upstream 1.5. I'm fairly certain that since we plan adding even more -drive capabilities than just Paolo's provisioning options in the 1.6 timeframe (such as a QMP setting of backing file parameters, including fd passing of a backing file during hotplug/snapshot/drive-mirror...), that we are safe in suspecting that distros will either rebase to 1.6, or take on the burden of backporting the parts of 1.6 that make sense for the features they want to support on whatever earlier qemu version they branch from. That means distros will be smart about backporting enough to give libvirt enough to go on for using those features as part of the distro's value-added premise. (ie. if your BZ is about a feature that doesn't land in 1.5, then you can assume that even if RHEL decides to base on 1.5 instead of 1.6, there will be enough of 1.6 backported to make use of your feature easy to probe for. Of course, all bets are off when guessing on a public list what Red Hat will actually use as their baseline for a future RHEL release, or whether the BZ will even be accepted - but then again, that's a problem for downstream to worry about, and shouldn't affect what we put into 1.5 upstream). And I just realized: this patch is queued to go into 1.5, and if I read it correctly, then once applied, -drive will no longer be empty in query-command-line-options output: https://lists.gnu.org/archive/html/qemu-devel/2013-04/msg04810.html
diff --git a/qapi-schema.json b/qapi-schema.json index 751d3c2..10affbd 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -3505,3 +3505,69 @@ '*asl_compiler_rev': 'uint32', '*file': 'str', '*data': 'str' }} + +## +# @CommandLineParameterType: +# +# Possible types for an option parameter. +# +# @string: accepts a character string +# +# @boolean: accepts "on" or "off" +# +# @number: accepts a number +# +# @size: accepts a number followed by an optional postfix (K)ilo, +# (M)ega, (G)iga, (T)era +# +# Since 1.5 +## +{ 'enum': 'CommandLineParameterType', + 'data': ['string', 'boolean', 'number', 'size'] } + +## +# @CommandLineParameterInfo: +# +# Details about a single parameter of a command line option. +# +# @name: parameter name +# +# @type: parameter @CommandLineParameterType +# +# @help: #optional human readable text string, not suitable for parsing. +# +# Since 1.5 +## +{ 'type': 'CommandLineParameterInfo', + 'data': { 'name': 'str', + 'type': 'CommandLineParameterType', + '*help': 'str' } } + +## +# @CommandLineOptionInfo: +# +# Details about a command line option, including its list of parameters details +# +# @option: option name +# +# @parameters: an array of @CommandLineParameterInfo +# +# Since 1.5 +## +{ 'type': 'CommandLineOptionInfo', + 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } } + +## +# @query-command-line-options: +# +# Query command line option schema. +# +# @option: #optional option name +# +# Returns: list of @CommandLineOptionInfo for all options (or for the given +# @option). Returns an error if the given @option doesn't exist. +# +# Since 1.5 +## +{'command': 'query-command-line-options', 'data': { '*option': 'str' }, + 'returns': ['CommandLineOptionInfo'] } diff --git a/qmp-commands.hx b/qmp-commands.hx index 4d65422..31bb360 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -2416,6 +2416,53 @@ EQMP }, SQMP +query-command-line-options +-------------------------- + +Show command line option schema. + +Return a json-array of command line option schema for all options (or for +the given option), returning an error if the given option doesn't exist. + +Each array entry contains the following: + +- "option": option name (json-string) +- "parameters": a json-array describes all parameters of the option: + - "name": parameter name (json-string) + - "type": parameter type (one of 'string', boolean', 'number', + or 'size') + - "help": human readable description of the parameter + (json-string, optional) + +Example: + +-> { "execute": "query-command-line-options", "arguments": { "option": "option-rom" } } +<- { "return": [ + { + "parameters": [ + { + "name": "romfile", + "type": "string" + }, + { + "name": "bootindex", + "type": "number" + } + ], + "option": "option-rom" + } + ] + } + +EQMP + + { + .name = "query-command-line-options", + .args_type = "option:s?", + .mhandler.cmd_new = qmp_marshal_input_query_command_line_options, + }, + +SQMP query-migrate ------------- diff --git a/util/qemu-config.c b/util/qemu-config.c index 01ca890..7d3874f 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -5,6 +5,7 @@ #include "qapi/qmp/qerror.h" #include "hw/qdev.h" #include "qapi/error.h" +#include "qmp-commands.h" static QemuOptsList *vm_config_groups[32]; @@ -37,6 +38,72 @@ QemuOptsList *qemu_find_opts(const char *group) return ret; } +static CommandLineParameterInfoList *query_option_descs(const QemuOptDesc *desc) +{ + CommandLineParameterInfoList *param_list = NULL, *entry; + CommandLineParameterInfo *info; + int i; + + for (i = 0; desc[i].name != NULL; i++) { + info = g_malloc0(sizeof(*info)); + info->name = g_strdup(desc[i].name); + + switch (desc[i].type) { + case QEMU_OPT_STRING: + info->type = COMMAND_LINE_PARAMETER_TYPE_STRING; + break; + case QEMU_OPT_BOOL: + info->type = COMMAND_LINE_PARAMETER_TYPE_BOOLEAN; + break; + case QEMU_OPT_NUMBER: + info->type = COMMAND_LINE_PARAMETER_TYPE_NUMBER; + break; + case QEMU_OPT_SIZE: + info->type = COMMAND_LINE_PARAMETER_TYPE_SIZE; + break; + } + + if (desc[i].help) { + info->has_help = true; + info->help = g_strdup(desc[i].help); + } + + entry = g_malloc0(sizeof(*entry)); + entry->value = info; + entry->next = param_list; + param_list = entry; + } + + return param_list; +} + +CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, + const char *option, + Error **errp) +{ + CommandLineOptionInfoList *conf_list = NULL, *entry; + CommandLineOptionInfo *info; + int i; + + for (i = 0; vm_config_groups[i] != NULL; i++) { + if (!has_option || !strcmp(option, vm_config_groups[i]->name)) { + info = g_malloc0(sizeof(*info)); + info->option = g_strdup(vm_config_groups[i]->name); + info->parameters = query_option_descs(vm_config_groups[i]->desc); + 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);
Libvirt has no way to probe if an option or property is supported, This patch introduces a new qmp command to query command line option information. hmp command isn't added because it's not needed. Signed-off-by: Amos Kong <akong@redhat.com> CC: Luiz Capitulino <lcapitulino@redhat.com> CC: Osier Yang <jyang@redhat.com> CC: Anthony Liguori <aliguori@us.ibm.com> --- V3: fix json schema and comments (Eric) V4: fix descriptions, rename command, check enum type, cleanup (Luiz, Eric) V5: fix typo, cleanup (Osier) --- qapi-schema.json | 66 +++++++++++++++++++++++++++++++++++++++++++++++++++++ qmp-commands.hx | 47 ++++++++++++++++++++++++++++++++++++++ util/qemu-config.c | 67 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 180 insertions(+)