[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v5 for 2.0 3/3] abort QEMU if group name in opti
From: |
Leandro Dorileo |
Subject: |
Re: [Qemu-devel] [PATCH v5 for 2.0 3/3] abort QEMU if group name in option table doesn't match with defined option name |
Date: |
Fri, 28 Mar 2014 09:53:14 -0300 |
User-agent: |
Mutt/1.5.23 (2014-03-12) |
Hi Amos,
On Thu, Mar 27, 2014 at 09:04:31PM +0800, Amos Kong wrote:
> 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 <address@hidden>
> ---
> 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
> 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;
> +}
> +
> 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);
I'm not a native English speaker but I don't fell comfortable with this
message output, what about "option table's group name (%s) must match with
predefined option name (Since 2.0)"?
The patch looks good, I tested it and it works properly, so
Reviewed-by: Leandro Dorileo <address@hidden>
> + abort();
> + }
> return;
> }
> }
> 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);
> --
> 1.8.5.3
>
>
--
Leandro Dorileo