From patchwork Tue Jan 6 02:37:25 2015 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tiejun Chen X-Patchwork-Id: 425480 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.gnu.org (lists.gnu.org [IPv6:2001:4830:134:3::11]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 7FE481400A0 for ; Tue, 6 Jan 2015 13:37:55 +1100 (AEDT) Received: from localhost ([::1]:34708 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8K1p-0002x1-Jl for incoming@patchwork.ozlabs.org; Mon, 05 Jan 2015 21:37:53 -0500 Received: from eggs.gnu.org ([2001:4830:134:3::10]:33758) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8K1U-0002fb-UF for qemu-devel@nongnu.org; Mon, 05 Jan 2015 21:37:34 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Y8K1R-0003bR-L8 for qemu-devel@nongnu.org; Mon, 05 Jan 2015 21:37:32 -0500 Received: from mga11.intel.com ([192.55.52.93]:38994) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Y8K1R-0003bN-7n for qemu-devel@nongnu.org; Mon, 05 Jan 2015 21:37:29 -0500 Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 05 Jan 2015 18:37:27 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,704,1413270000"; d="scan'208";a="657295055" Received: from tiejunch-mobl.ccr.corp.intel.com (HELO [10.238.128.143]) ([10.238.128.143]) by fmsmga002.fm.intel.com with ESMTP; 05 Jan 2015 18:37:25 -0800 Message-ID: <54AB4A65.2020503@intel.com> Date: Tue, 06 Jan 2015 10:37:25 +0800 From: "Chen, Tiejun" User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Thunderbird/31.3.0 MIME-Version: 1.0 To: Marcel Apfelbaum , Stefan Hajnoczi , Jan Kiszka References: <1420456961-19631-1-git-send-email-stefanha@redhat.com> <54AA7791.9060705@siemens.com> <54AA801A.8050605@redhat.com> In-Reply-To: <54AA801A.8050605@redhat.com> X-detected-operating-system: by eggs.gnu.org: Genre and OS details not recognized. X-Received-From: 192.55.52.93 Cc: Peter Maydell , Marcel Apfelbaum , qemu-devel , plucinski.mariusz@gmail.com, Stefan Hajnoczi Subject: Re: [Qemu-devel] [PATCH] vl.c: fix -usb option assertion failure in qemu_opt_get_bool_helper() X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org Sender: qemu-devel-bounces+incoming=patchwork.ozlabs.org@nongnu.org On 2015/1/5 20:14, Marcel Apfelbaum wrote: > On 01/05/2015 01:50 PM, Stefan Hajnoczi wrote: >> On Mon, Jan 5, 2015 at 11:37 AM, 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. >> >> I think we should revert the original commit. >> >> qemu_option_get_*() callers need to be converted to query MachineState >> fields instead of using QemuOpts functions. Can this work out currently? --- util/qemu-option.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) if (desc && desc->def_value_str) { return desc->def_value_str; @@ -341,7 +341,7 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name) } opt = qemu_opt_find(opts, name); - if (!opt) { + if (!opt || !opt->desc) { desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { str = g_strdup(desc->def_value_str); @@ -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 || !opt->desc) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { parse_option_bool(name, desc->def_value_str, &ret, &error_abort); @@ -413,7 +413,7 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts, const char *name, } opt = qemu_opt_find(opts, name); - if (opt == NULL) { + if (!opt || !opt->desc) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { parse_option_number(name, desc->def_value_str, &ret, &error_abort); @@ -450,7 +450,7 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts, const char *name, } opt = qemu_opt_find(opts, name); - if (opt == NULL) { + if (!opt || !opt->desc) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { parse_option_size(name, desc->def_value_str, &ret, &error_abort); diff --git a/util/qemu-option.c b/util/qemu-option.c index a708241..933b885 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -317,7 +317,7 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) } opt = qemu_opt_find(opts, name); - if (!opt) { + if (!opt || !opt->desc) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name);