From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35346) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WUGyo-0001to-Eq for qemu-devel@nongnu.org; Sun, 30 Mar 2014 10:44:59 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WUGyf-0008UM-CA for qemu-devel@nongnu.org; Sun, 30 Mar 2014 10:44:58 -0400 Received: from mail-pb0-x231.google.com ([2607:f8b0:400e:c01::231]:53783) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WUGyf-0008UG-5h for qemu-devel@nongnu.org; Sun, 30 Mar 2014 10:44:49 -0400 Received: by mail-pb0-f49.google.com with SMTP id jt11so7057045pbb.36 for ; Sun, 30 Mar 2014 07:44:48 -0700 (PDT) Message-ID: <53382DDD.8050001@gmail.com> Date: Sun, 30 Mar 2014 22:44:45 +0800 From: Chen Gang MIME-Version: 1.0 References: <53337BEB.90305@gmail.com> <87wqfgrp83.fsf@blackfin.pond.sub.org> <5333F6E2.9080202@gmail.com> In-Reply-To: <5333F6E2.9080202@gmail.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator() List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: QEMU Developers , aliguori@amazon.com Hello Maintainers: If it is necessary to send patch v2 by me, please let me know, I will/should send. Thanks. On 03/27/2014 06:01 PM, Chen Gang wrote: > > > On 03/27/2014 04:59 PM, Markus Armbruster wrote: >> Chen Gang writes: >> >>> At present, each 'opt_name' of 'accel_list' is uniq with each other, so >>> 'buf' can only match one 'opt_name'. >>> >>> When drop into the matching code block, can 'break' outside related >>> 'for' looping after finish processing it (just like the other 'break' >>> within the matching block). >>> >>> After print "... not support for this target", it can avoid to print >>> "... accelerator does not exist". >>> >>> >>> Signed-off-by: Chen Gang >>> --- >>> vl.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/vl.c b/vl.c >>> index 842e897..b4f98fa 100644 >>> --- a/vl.c >>> +++ b/vl.c >>> @@ -2709,7 +2709,7 @@ static int configure_accelerator(QEMUMachine *machine) >>> if (!accel_list[i].available()) { >>> printf("%s not supported for this target\n", >>> accel_list[i].name); >>> - continue; >>> + break; >>> } >>> *(accel_list[i].allowed) = true; >>> ret = accel_list[i].init(machine); >> >> Works, because the opt_name in accel_list[] are distinct, as you >> explained in your commit message. >> >> Apropos commit message. You first explain what you do, and only then >> state the problem you're trying to solve. That's backwards. Start with >> the problem. Only then explain your solution, if it needs explaining. >> This one, I think, doesn't. >> >> Suggested commit message: >> >> vl: Report accelerator not supported for target more nicely >> >> When you ask for an accelerator not supported for your target, you >> get a bogus "accelerator does not exist" message: >> >> $ qemu-system-arm -machine none,accel=kvm >> KVM not supported for this target >> "kvm" accelerator does not exist. >> No accelerator found! >> >> Suppress it. >> > > Thank you for your details, that sounds reasonable to me. > > Next, I will/should notice start with the issue, and then explain the > solution. > > > Thanks. > -- Chen Gang Open, share and attitude like air, water and life which God blessed