| Message ID | 1394172567-19634-3-git-send-email-akong@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 03/06/2014 11:09 PM, Amos Kong wrote: > vm_config_groups[] only contains part of the options which have > parameters, and all options which have no parameter aren't added > to vm_config_groups[]. Current query-command-line-options only > checks options from vm_config_groups[], so some options will > be lost. > > We have macro in qemu-options.hx to generate a table that > contains all the options. This patch tries to query options > from the table. > > Then we won't lose the legacy options that weren't added to > vm_config_groups[] (eg: -vnc, -smbios). The options that have > no parameter will also be returned (eg: -enable-fips) > > Some options that have parameters have a NULL desc list, some > options don't have parameters, and "parameters" is mandatory > in the past. So we add a new field "unspecified-parameters" to > present if the option takes unspecified parameters. > > This patch also fixes options to match their actual command-line > spelling rather than an alternate name associated with the > option table in use by the command. > > Signed-off-by: Amos Kong <akong@redhat.com> > --- > qapi-schema.json | 9 +++++++-- > qemu-options.h | 12 ++++++++++++ > util/qemu-config.c | 43 ++++++++++++++++++++++++++++++++++++------- > vl.c | 19 ++----------------- > 4 files changed, 57 insertions(+), 26 deletions(-) Based on the thread on v4, it sounds like this design is still not finalized, and won't make 2.0. It sounds like once we start exposing all options, it would also be nice to show which options are sugar for other spellings. > > diff --git a/qapi-schema.json b/qapi-schema.json > index 193e7e4..fb7ca1b 100644 > --- a/qapi-schema.json > +++ b/qapi-schema.json > @@ -4070,12 +4070,17 @@ > # > # @option: option name > # > -# @parameters: an array of @CommandLineParameterInfo > +# @parameters: array of @CommandLineParameterInfo, possibly empty > +# @unspecified-parameters: @optional present if the @parameters array is empty. Blank lines between the two parameters, for consistency. > +# If true, then the option takes unspecified > +# parameters, if false, then the option takes no > +# parameter (since 2.0) So this will need to be changed to '(since 2.1)'.
On Tue, Mar 11, 2014 at 06:46:10PM -0600, Eric Blake wrote: > On 03/06/2014 11:09 PM, Amos Kong wrote: > > vm_config_groups[] only contains part of the options which have > > parameters, and all options which have no parameter aren't added > > to vm_config_groups[]. Current query-command-line-options only > > checks options from vm_config_groups[], so some options will > > be lost. > > > > We have macro in qemu-options.hx to generate a table that > > contains all the options. This patch tries to query options > > from the table. > > > > Then we won't lose the legacy options that weren't added to > > vm_config_groups[] (eg: -vnc, -smbios). The options that have > > no parameter will also be returned (eg: -enable-fips) > > > > Some options that have parameters have a NULL desc list, some > > options don't have parameters, and "parameters" is mandatory > > in the past. So we add a new field "unspecified-parameters" to > > present if the option takes unspecified parameters. > > > > This patch also fixes options to match their actual command-line > > spelling rather than an alternate name associated with the > > option table in use by the command. > > > > Signed-off-by: Amos Kong <akong@redhat.com> > > --- > > qapi-schema.json | 9 +++++++-- > > qemu-options.h | 12 ++++++++++++ > > util/qemu-config.c | 43 ++++++++++++++++++++++++++++++++++++------- > > vl.c | 19 ++----------------- > > 4 files changed, 57 insertions(+), 26 deletions(-) > Based on the thread on v4, it sounds like this design is still not > finalized, and won't make 2.0. It sounds like once we start exposing > all options, it would also be nice to show which options are sugar for > other spellings. It's not good to check if one option is sugar for other spellings from help message. Does libvirt really need this information? As I mentioned in V4 thread, we can add a tri-state: +# @ArgumentStateType: +# +# Possible types for argument state. +# +# @no-argument: the option takes no argument +# +# @argument-with-parameters: the option takes argument with +# unspecified parameters +# +# @argument-without-parameters: the option takes argument without +# unspecified parameters +# +# Since 2.1 +## +{ 'enum': 'ArgumentStateType', + 'data': ['no-argument', 'argument-with-parameters', + 'argument-without-parameters'] +} I only implement the code by checking help message -option_name xxx,[xx=xx]\n However, I will send a V6 later, let discuss based on it. Thanks, Amos > > diff --git a/qapi-schema.json b/qapi-schema.json > > index 193e7e4..fb7ca1b 100644 > > --- a/qapi-schema.json > > +++ b/qapi-schema.json > > @@ -4070,12 +4070,17 @@ > > # > > # @option: option name > > # > > -# @parameters: an array of @CommandLineParameterInfo > > +# @parameters: array of @CommandLineParameterInfo, possibly empty > > +# @unspecified-parameters: @optional present if the @parameters array is empty. > > Blank lines between the two parameters, for consistency. > > > +# If true, then the option takes unspecified > > +# parameters, if false, then the option takes no > > +# parameter (since 2.0) > > So this will need to be changed to '(since 2.1)'. >
diff --git a/qapi-schema.json b/qapi-schema.json index 193e7e4..fb7ca1b 100644 --- a/qapi-schema.json +++ b/qapi-schema.json @@ -4070,12 +4070,17 @@ # # @option: option name # -# @parameters: an array of @CommandLineParameterInfo +# @parameters: array of @CommandLineParameterInfo, possibly empty +# @unspecified-parameters: @optional present if the @parameters array is empty. +# If true, then the option takes unspecified +# parameters, if false, then the option takes no +# parameter (since 2.0) # # Since 1.5 ## { 'type': 'CommandLineOptionInfo', - 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } } + 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'], + '*unspecified-parameters': 'bool' } } ## # @query-command-line-options: diff --git a/qemu-options.h b/qemu-options.h index 89a009e..4024487 100644 --- a/qemu-options.h +++ b/qemu-options.h @@ -28,9 +28,21 @@ #ifndef _QEMU_OPTIONS_H_ #define _QEMU_OPTIONS_H_ +#include "sysemu/arch_init.h" + enum { #define QEMU_OPTIONS_GENERATE_ENUM #include "qemu-options-wrapper.h" }; +#define HAS_ARG 0x0001 + +typedef struct QEMUOption { + const char *name; + int flags; + int index; + uint32_t arch_mask; +} QEMUOption; + +extern const QEMUOption qemu_options[]; #endif diff --git a/util/qemu-config.c b/util/qemu-config.c index d2facfd..ea8a419 100644 --- a/util/qemu-config.c +++ b/util/qemu-config.c @@ -6,6 +6,14 @@ #include "hw/qdev.h" #include "qapi/error.h" #include "qmp-commands.h" +#include "qemu-options.h" + +const QEMUOption qemu_options[] = { + { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL }, +#define QEMU_OPTIONS_GENERATE_OPTIONS +#include "qemu-options-wrapper.h" + { NULL }, +}; static QemuOptsList *vm_config_groups[32]; static QemuOptsList *drive_config_groups[4]; @@ -78,6 +86,17 @@ static CommandLineParameterInfoList *get_param_infolist(const QemuOptDesc *desc) return param_list; } +static int get_group_index(const char *name) +{ + int i; + + for (i = 0; vm_config_groups[i] != NULL; i++) { + if (!strcmp(vm_config_groups[i]->name, name)) { + return i; + } + } + return -1; +} /* remove repeated entry from the info list */ static void cleanup_infolist(CommandLineParameterInfoList *head) { @@ -137,17 +156,25 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, { CommandLineOptionInfoList *conf_list = NULL, *entry; CommandLineOptionInfo *info; - int i; + int i, idx; - for (i = 0; vm_config_groups[i] != NULL; i++) { - if (!has_option || !strcmp(option, vm_config_groups[i]->name)) { + for (i = 0; qemu_options[i].name; i++) { + if (!has_option || !strcmp(option, qemu_options[i].name)) { info = g_malloc0(sizeof(*info)); - info->option = g_strdup(vm_config_groups[i]->name); - if (!strcmp("drive", vm_config_groups[i]->name)) { + info->option = g_strdup(qemu_options[i].name); + + idx = get_group_index(qemu_options[i].name); + + if (!strcmp("drive", qemu_options[i].name)) { info->parameters = get_drive_infolist(); - } else { + } else if (idx >= 0) { info->parameters = - get_param_infolist(vm_config_groups[i]->desc); + get_param_infolist(vm_config_groups[idx]->desc); + } + + if (!info->parameters) { + info->has_unspecified_parameters = true; + info->unspecified_parameters = qemu_options[i].flags & HAS_ARG; } entry = g_malloc0(sizeof(*entry)); entry->value = info; @@ -163,6 +190,8 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option, return conf_list; } +#undef HAS_ARG + QemuOptsList *qemu_find_opts_err(const char *group, Error **errp) { return find_list(vm_config_groups, group, errp); diff --git a/vl.c b/vl.c index 685a7a9..a967b17 100644 --- a/vl.c +++ b/vl.c @@ -111,7 +111,6 @@ int main(int argc, char **argv) #include "trace/control.h" #include "qemu/queue.h" #include "sysemu/cpus.h" -#include "sysemu/arch_init.h" #include "qemu/osdep.h" #include "ui/qemu-spice.h" @@ -1990,22 +1989,6 @@ static void help(int exitcode) exit(exitcode); } -#define HAS_ARG 0x0001 - -typedef struct QEMUOption { - const char *name; - int flags; - int index; - uint32_t arch_mask; -} QEMUOption; - -static const QEMUOption qemu_options[] = { - { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL }, -#define QEMU_OPTIONS_GENERATE_OPTIONS -#include "qemu-options-wrapper.h" - { NULL }, -}; - static bool vga_available(void) { return object_class_by_name("VGA") || object_class_by_name("isa-vga"); @@ -2721,6 +2704,8 @@ static const QEMUOption *lookup_opt(int argc, char **argv, return popt; } +#undef HAS_ARG + static gpointer malloc_and_trace(gsize n_bytes) { void *ptr = malloc(n_bytes);
vm_config_groups[] only contains part of the options which have parameters, and all options which have no parameter aren't added to vm_config_groups[]. Current query-command-line-options only checks options from vm_config_groups[], so some options will be lost. We have macro in qemu-options.hx to generate a table that contains all the options. This patch tries to query options from the table. Then we won't lose the legacy options that weren't added to vm_config_groups[] (eg: -vnc, -smbios). The options that have no parameter will also be returned (eg: -enable-fips) Some options that have parameters have a NULL desc list, some options don't have parameters, and "parameters" is mandatory in the past. So we add a new field "unspecified-parameters" to present if the option takes unspecified parameters. This patch also fixes options to match their actual command-line spelling rather than an alternate name associated with the option table in use by the command. Signed-off-by: Amos Kong <akong@redhat.com> --- qapi-schema.json | 9 +++++++-- qemu-options.h | 12 ++++++++++++ util/qemu-config.c | 43 ++++++++++++++++++++++++++++++++++++------- vl.c | 19 ++----------------- 4 files changed, 57 insertions(+), 26 deletions(-)