Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper()

[Jan Kiszka <jan.kiszka@web.de>]

[-- Attachment #1: Type: text/plain, Size: 2058 bytes --]

On 2015-01-05 12:37, Jan Kiszka wrote:
> On 2015-01-05 12:22, Stefan Hajnoczi wrote:
>> Commit 49d2e648e8087d154d8bf8b91f27c8e05e79d5a6 ("machine: remove
>> qemu_machine_opts global list") removed option descriptions from the
>> -machine QemuOptsList to avoid repeating MachineState's QOM properties.
>>
>> This change broke vl.c:usb_enabled() because qemu_opt_get_bool() cannot
>> be used on QemuOptsList without option descriptions since QemuOpts
>> doesn't know the type and therefore left an unparsed string value.
>>
>> This patch avoids calling qemu_opt_get_bool() to fix the assertion
>> failure:
>>
>>   $ qemu-system-x86_64 -usb
>>   qemu_opt_get_bool_helper: Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed.
>>
>> Test the presence of -usb using qemu_opt_find() but use the
>> MachineState->usb field instead of qemu_opt_get_bool().
>>
>> Cc: Marcel Apfelbaum <marcel.a@redhat.com>
>> Cc: Tiejun Chen <tiejun.chen@intel.com>
>> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
>> ---
>>  vl.c | 7 +++++--
>>  1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/vl.c b/vl.c
>> index bea9656..6e8889c 100644
>> --- a/vl.c
>> +++ b/vl.c
>> @@ -999,8 +999,11 @@ static int parse_name(QemuOpts *opts, void *opaque)
>>  
>>  bool usb_enabled(bool default_usb)
>>  {
>> -    return qemu_opt_get_bool(qemu_get_machine_opts(), "usb",
>> -                             has_defaults && default_usb);
>> +    if (qemu_opt_find(qemu_get_machine_opts(), "usb")) {
>> +        return current_machine->usb;
>> +    } else {
>> +        return has_defaults && default_usb;
>> +    }
>>  }
>>  
>>  #ifndef _WIN32
>>
> 
> That still leaves the other boolean machine options broken. A generic
> fix would be good. Or revert the original commit until we have one.

Just pulled current master, and at least the iommu machine option is
still triggering an abort.

What is the status of fixing these fallouts? Was anything else addressed
by now, just this one forgotten?

Jan



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]