Re: [Qemu-devel] [PATCH] vl.c: use 'break' instead of 'continue' in configure_accelerator()

[Chen Gang <gang.chen.5i5j@gmail.com>]

On 03/27/2014 04:59 PM, Markus Armbruster wrote:
> 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.
> 

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