From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:51770) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ0vN-0002yl-NJ for qemu-devel@nongnu.org; Wed, 04 Feb 2015 09:27:26 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YJ0vI-0000xM-8a for qemu-devel@nongnu.org; Wed, 04 Feb 2015 09:27:25 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41329) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YJ0vI-0000w3-0j for qemu-devel@nongnu.org; Wed, 04 Feb 2015 09:27:20 -0500 Message-ID: <54D22C39.3060400@redhat.com> Date: Wed, 04 Feb 2015 16:27:05 +0200 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1420456961-19631-1-git-send-email-stefanha@redhat.com> <54AA7791.9060705@siemens.com> <54CC9F01.60305@web.de> <54CF4B42.4050008@redhat.com> <54D22A35.4050403@de.ibm.com> In-Reply-To: <54D22A35.4050403@de.ibm.com> Content-Type: text/plain; charset=iso-8859-15; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Christian Borntraeger , Jan Kiszka , Stefan Hajnoczi , qemu-devel@nongnu.org, Marcel Apfelbaum Cc: Peter Maydell , plucinski.mariusz@gmail.com, Tiejun Chen On 02/04/2015 04:18 PM, Christian Borntraeger wrote: > 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 >>>>> Cc: Tiejun Chen >>>>> Signed-off-by: Stefan Hajnoczi >>>>> --- >>>>> 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 ;-) It will be ready today or tomorrow :) Thanks, Marcel > > Christian >