[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu
From: |
Christian Borntraeger |
Subject: |
Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper() |
Date: |
Wed, 04 Feb 2015 15:18:29 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0 |
Am 02.02.2015 um 11:02 schrieb Marcel Apfelbaum:
> On 01/31/2015 11:23 AM, Jan Kiszka wrote:
>> 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 <address@hidden>
>>>> Cc: Tiejun Chen <address@hidden>
>>>> Signed-off-by: Stefan Hajnoczi <address@hidden>
>>>> ---
>>>> 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?
> No, is not forgotten. I waited to see that the approach to fix this issue
> is accepted by the reviewers.
>
> Thanks for the reminder, patches will be submitted soon.
> Marcel
Anything to look at a branch or something? Adding new machine bool options
is currently pretty hard - as long as it doesnt work ;-)
Christian