From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35527) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8XL0-0005Mg-MZ for qemu-devel@nongnu.org; Tue, 06 Jan 2015 11:50:38 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8XKw-0000CE-Ad for qemu-devel@nongnu.org; Tue, 06 Jan 2015 11:50:34 -0500 Received: from mx1.redhat.com ([209.132.183.28]:41632) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8XKw-0000Bl-22 for qemu-devel@nongnu.org; Tue, 06 Jan 2015 11:50:30 -0500 Message-ID: <54AC1248.2020603@redhat.com> Date: Tue, 06 Jan 2015 18:50:16 +0200 From: Marcel Apfelbaum MIME-Version: 1.0 References: <1420436129-8467-1-git-send-email-tiejun.chen@intel.com> <54AAC62A.2030906@redhat.com> <54AB387F.4010208@intel.com> <54AB4AD1.5060802@intel.com> <20150106145659.GM29775@stefanha-thinkpad.redhat.com> In-Reply-To: <20150106145659.GM29775@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [RFC][PATCH] qemu_opt_get_bool_helper: back finding desc by name just if !opt->desc List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , "Chen, Tiejun" Cc: peter.maydell@linaro.org, marcel.a@redhat.com, qemu-devel@nongnu.org, agraf@suse.de, greg.bellows@linaro.org On 01/06/2015 04:56 PM, Stefan Hajnoczi wrote: > On Tue, Jan 06, 2015 at 10:39:13AM +0800, Chen, Tiejun wrote: >> On 2015/1/6 9:21, Chen, Tiejun wrote: >>> On 2015/1/6 1:13, Eric Blake wrote: >>>> On 01/04/2015 10:35 PM, Tiejun Chen wrote: >>>>> After one commit 49d2e648e808, "machine: remove qemu_machine_opts >>>>> global list", is introduced, QEMU doesn't keep a global list of >>>>> options but set desc lately. Then we can see the following, >>>>> >>>>> $ x86_64-softmmu/qemu-system-x86_64 -usb >>>>> qemu-system-x86_64: util/qemu-option.c:387: qemu_opt_get_bool_helper: \ >>>>> Assertion `opt->desc && opt->desc->type == QEMU_OPT_BOOL' failed. >>>>> Aborted (core dumped) >>>>> >>>>> So inside qemu_opt_get_bool_helper, we need to call find_desc_by_name() >>>>> to work parse_option_bool() out just in case of !opt->desc. >>>>> >>>>> Signed-off-by: Tiejun Chen >>>>> --- >>>>> util/qemu-option.c | 2 +- >>>>> 1 file changed, 1 insertion(+), 1 deletion(-) >>>>> >>>>> diff --git a/util/qemu-option.c b/util/qemu-option.c >>>>> index a708241..7cb3601 100644 >>>>> --- a/util/qemu-option.c >>>>> +++ b/util/qemu-option.c >>>>> @@ -377,7 +377,7 @@ static bool qemu_opt_get_bool_helper(QemuOpts >>>>> *opts, const char *name, >>>>> } >>>>> >>>>> opt = qemu_opt_find(opts, name); >>>>> - if (opt == NULL) { >>>>> + if ((opt == NULL) || !opt->desc) { >>>> >>>> Over-parenthesized, and looks like you also introduced a spurious space. >>>> Simpler to just have: >>>> >>>> if (!opt || !opt->desc) { >>> >>> Looks good. >>> >>>> >>>> Also, there are other threads about the same topic. >>>> https://lists.gnu.org/archive/html/qemu-devel/2015-01/msg00130.html >>> >>> Yeah, I already notice that. >>> >> >> Then I realize I need to extend this to all qemu_opt_get_*. > > Marcel is working on a fix. I just posted a fix on the mailing list. https://www.mail-archive.com/qemu-devel@nongnu.org/msg272607.html Thanks, Marcel > > I think hacking qemu_opt_get_* is not a clean solution. The intention > was to move away from QemuOpts to QOM properties. The patch that > removed -machine QemuOptsList descriptions was buggy, but the proper fix > is to make that approach work instead of hacking qemu_opt_get_*. > > Stefan >