Re: [libvirt] [Qemu-devel] [PATCH v5 for 2.0 3/3] abort QEMU if group name in option table doesn't match with defined option name
On 03/28/2014 08:55 AM, Markus Armbruster wrote:
Amos Kong <akong(a)redhat.com> 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
>
> @@ -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))?
Indeed, using assert() would also solve the problem of the error message
being awkward.
>
> -#define HAS_ARG 0x0001
> -
> -
> -static const QEMUOption qemu_options[] = {
> - { "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
> -#define QEMU_OPTIONS_GENERATE_OPTIONS
> -#include "qemu-options-wrapper.h"
> - { NULL },
> -};
>
> +#undef HAS_ARG
HAS_ARG is not very namespace clean. Prior to your patch, it was used
only in a single file (where we know it doesn't collide). After your
patch, it is now in a header used by multiple files.
> +
> 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.
Maybe a better approach would be to create an enum in qemu-options.h of
actual flag values:
typedef enum {
QEMU_OPT_HAS_ARG = 1,
} QEMUOptionFlags;
and use QEMU_OPT_HAS_ARG instead of HAS_ARG in vl.c. Additionally, you
either have to s/HAS_ARG/QEMU_OPT_HAS_ARG/ throughout the .hx file, or
you can take a shortcut in qemu-config.c:
#define HAS_ARG QEMU_OPT_HAS_ARG
const QEMUOption qemu_options[] = {
{ "h", 0, QEMU_OPTION_h, QEMU_ARCH_ALL },
#define QEMU_OPTIONS_GENERATE_OPTIONS
#include "qemu-options-wrapper.h"
{ NULL },
};
#undef HAS_ARG
since that is the only place that includes the .hx file at a point where
HAS_ARG has to be expanded to something useful.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 604 bytes)