Amos Kong <akong@xxxxxxxxxx> writes:
> vm_config_groups[] contain the options which have parameter, but some
> legcy options haven't been added to vm_config_groups[].
>
> All the options can be found in qemu-options.hx, this patch used two
> new marcos to generate two tables, we can check if the option name is
> valid and if the option has arguments.
>
> This patch also try to visit all the options in option_names[], then
> we won't lost the legacy options that weren't added to vm_config_groups[].
> The options have no arguments will also be returned (eg: -enable-fips)
>
> Signed-off-by: Amos Kong <akong@xxxxxxxxxx>
> ---
> util/qemu-config.c | 41 +++++++++++++++++++++++++++++++++++------
> 1 file changed, 35 insertions(+), 6 deletions(-)
>
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index d624d92..de233d8 100644
> --- a/util/qemu-config.c
> +++ b/util/qemu-config.c
> @@ -78,6 +78,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,16 +150,34 @@ 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)) {
> + char const *option_names[] = {
> +#define QEMU_OPTIONS_GENERATE_NAME
> +#include "qemu-options-wrapper.h"
> + };
> +
> + char const *option_hasargs[] = {
> +#define QEMU_OPTIONS_GENERATE_HASARG
> +#include "qemu-options-wrapper.h"
> + };
Both tables are technically redundant. The same information is already
in vl.c's qemu_options[]. That one also includes -h, which the tables
here miss.
Duplicating tables can be okay, but I suspect using the existing one
would be simpler. Have you tried?
> +
> + for (i = 0; i < sizeof(option_names) / sizeof(char *); i++) {
> + if (!has_option || !strcmp(option, option_names[i])) {
> 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(option_names[i]);
> +
> + int idx = get_group_index(option_names[i]);
Variable declaration follows statement. Please declare int idx at the
beginning of a block. I'd declare it right at the beginning, together
with int i.
> +
> + if (!strcmp("HAS_ARG", option_hasargs[i])) {
> + info->has_parameters = true;
> + }
qemu/util/qemu-config.c:171:21: error: ‘CommandLineOptionInfo’ has no member named ‘has_parameters’
Did you forget to include a schema change?
Awfully roundabout way to test whether the option takes an argument.
Here's the other part, from PATCH 2/5:
+#define DEF(option, opt_arg, opt_enum, opt_help, arch_mask) \
+ stringify(opt_arg),
Please do it more like vl.c:
#define HAS_ARG true
bool option_has_arg[] = {
#define QEMU_OPTIONS_GENERATE_HASARG
#include "qemu-options-wrapper.h"
}
Then you can simply test option_has_arg[i].
Or reuse vl.c's table.
> +
> + if (!strcmp("drive", option_names[i])) {
> 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);
> }
> +
> entry = g_malloc0(sizeof(*entry));
> entry->value = info;
> entry->next = conf_list;
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list