Amos Kong <akong@xxxxxxxxxx> writes:
> All the options are defined in qemu-options.hx. If we can't find a
> matched option definition by group name of option table, then the
> group name doesn't match with defined option name, it's not allowed
> from 2.0
>
> Signed-off-by: Amos Kong <akong@xxxxxxxxxx>
> ---
> qemu-options.h | 12 ++++++++++++
> util/qemu-config.c | 28 ++++++++++++++++++++++++++++
> vl.c | 19 ++-----------------
> 3 files changed, 42 insertions(+), 17 deletions(-)
>
> 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
Instead of exposing struct QEMUOption and qemu_options[] here, why not
simply put opt_is_defined() in vl.c, and declare it in qemu-options.h?
> diff --git a/util/qemu-config.c b/util/qemu-config.c
> index f610101..eba5428 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];
> @@ -184,6 +192,20 @@ void qemu_add_drive_opts(QemuOptsList *list)
> abort();
> }
>
> +/* check if the option is defined in qemu-options.hx */
> +static bool opt_is_defined(const char *name)
> +{
> + int i;
> +
> + for (i = 0; qemu_options[i].name; i++) {
> + if (!strcmp(qemu_options[i].name, name)) {
> + return true;
> + }
> + }
> +
> + return false;
> +}
> +
The same loop is hidden in lookup_opt(). We could avoid the duplication
by putting it in a function returning the option index. That could
easily be a proper enumeration type, like this, in qemu-options.h:
typedef enum QEMUOption {
#define QEMU_OPTIONS_GENERATE_ENUM
#include "qemu-options-wrapper.h"
} QEMUOption;
QEMUOption qemu_option_lookup(const char *name);
plus a rename of vl.c's QEMUOption to QEMUOptionDesc or something.
Can be done on top.
> void qemu_add_opts(QemuOptsList *list)
> {
> int entries, i;
> @@ -193,6 +215,12 @@ void qemu_add_opts(QemuOptsList *list)
> for (i = 0; i < entries; i++) {
> if (vm_config_groups[i] == NULL) {
> vm_config_groups[i] = list;
> + if (!opt_is_defined(list->name)) {
> + error_report("Didn't find a matched option definition, "
> + "group name (%s) of option table must match with "
> + "defined option name (Since 2.0)", list->name);
> + abort();
> + }
> return;
> }
> }
Simple! Wish it was my idea ;)
Why not simply assert(opt_is_defined(list->name))?
> diff --git a/vl.c b/vl.c
> index 0464494..bd44c52 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"
> @@ -2082,22 +2081,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");
> @@ -2842,6 +2825,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);
Undefining HAS_ARG here, where it hasn't done any harm, while letting it
pollute every other compilation unit including qemu-options.h makes no
sense.
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list