From: Markus Armbruster <armbru@redhat.com>
References: <53337BEB.90305@gmail.com>
Date: Thu, 27 Mar 2014 09:59:56 +0100
In-Reply-To: <53337BEB.90305@gmail.com> (Chen Gang's message of "Thu, 27 Mar
	2014 09:16:27 +0800")
Message-ID: <87wqfgrp83.fsf@blackfin.pond.sub.org>
MIME-Version: 1.0
Content-Type: text/plain
Subject: Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in
	configure_accelerator()
To: Chen Gang <gang.chen.5i5j@gmail.com>
Cc: QEMU Developers <qemu-devel@nongnu.org>, aliguori@amazon.com

Chen Gang <gang.chen.5i5j@gmail.com> 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 <gang.chen.5i5j@gmail.com>
> ---
>  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.