On 03/28/2014 08:55 AM, Markus Armbruster wrote:
> 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
>>
>> @@ -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
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list