Amos Kong <akong@xxxxxxxxxx> writes:
> vm_config_groups[] only contains part of the options which have
> argument, and all options which have no argument 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 argument will also be returned (eg: -enable-fips)
>
> Some options that have argument have a NULL desc list, some
> options don't have argument, and "parameters" is mandatory
> in the past. So we add a new field "argument" to present
> if the option takes unspecified arguments.
>
> 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@xxxxxxxxxx>
> ---
> qapi-schema.json | 8 ++++++--
> qemu-options.h | 10 ++++++++++
> util/qemu-config.c | 44 ++++++++++++++++++++++++++++++++++++++------
> vl.c | 15 ---------------
> 4 files changed, 54 insertions(+), 23 deletions(-)
>
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 193e7e4..4ab0d16 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -4070,12 +4070,16 @@
> #
> # @option: option name
> #
> -# @parameters: an array of @CommandLineParameterInfo
> +# @parameters: array of @CommandLineParameterInfo, possibly empty
> +# @argument: @optional present if the @parameters array is empty. If
> +# true, then the option takes unspecified arguments, if
> +# false, then the option is merely a boolean flag (since 2.0)
Suggest "if false, then the option takes no argument".
I'd call it something like 'unspecified-parameters' to emphasize the
connection with parameters.
> #
> # Since 1.5
> ##
> { 'type': 'CommandLineOptionInfo',
> - 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'] } }
> + 'data': { 'option': 'str', 'parameters': ['CommandLineParameterInfo'],
> + '*argument': 'bool' } }
>
> ##
> # @query-command-line-options:
> diff --git a/qemu-options.h b/qemu-options.h
> index 89a009e..8a515e0 100644
> --- a/qemu-options.h
> +++ b/qemu-options.h
> @@ -25,6 +25,8 @@
> * THE SOFTWARE.
> */
>
> +#include "sysemu/arch_init.h"
> +
Why do you need this header?
If you need it, you should #include within the multiple-inclusion guard!
> #ifndef _QEMU_OPTIONS_H_
> #define _QEMU_OPTIONS_H_
>
> @@ -33,4 +35,12 @@ enum {
> #include "qemu-options-wrapper.h"
> };
>
> +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..2f89b92 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -6,6 +6,16 @@
> #include "hw/qdev.h"
> #include "qapi/error.h"
> #include "qmp-commands.h"
> +#include "qemu-options.h"
> +
> +#define HAS_ARG 0x0001
> +
> +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 +88,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)
> {
> @@ -139,15 +160,26 @@ CommandLineOptionInfoList *qmp_query_command_line_options(bool has_option,
> CommandLineOptionInfo *info;
> int i;
>
> - 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);
> +
> + int idx = get_group_index(qemu_options[i].name);
Style nit: declaration after statement. Suggest to declare it together
with i above, as "int i, idx;".
> +
> + if (qemu_options[i].flags) {
> + info->argument = true;
> + }
The existing code using flags tests flags & HAS_ARG, see lookup_opt()).
Suggest to stick to that for consistency.
Please move the assigment next to the assignment to info->has_argument,
as shown below.
> +
> + 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_argument = true;
info->argument = qemu_options[i].flags & HAS_ARG;
> }
> entry = g_malloc0(sizeof(*entry));
> entry->value = info;
> diff --git a/vl.c b/vl.c
> index 685a7a9..6269762 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"
> @@ -1992,20 +1991,6 @@ static void help(int 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");
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list