Re: [Qemu-devel] [PATCH 4/9] QemuOpts: Drop qemu_opts_foreach() parameter abort_on_failure

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> On 05/28/2015 06:21 AM, Markus Armbruster wrote:
>> When the argument is non-zero, qemus_opt_foreach() stops on callback
>
> s/qemus_opt/qemu_opts/

Wonder what got my fingers into *that* tangle.  Fixing...

>> returning non-zero, and returns that value.
>> 
>> When the argument is zero, it doesn't stop, and returns the bit-wise
>> inclusive or of all the return values.  Funky :)
>> 
>> The callers that pass zero could just as well pass one, because their
>> callbacks can't return anything but zero:
>> 
>> * qemu_add_globals()'s callback qdev_add_one_global()
>> 
>> * qemu_config_write()'s callback config_write_opts()
>> 
>> * main()'s callbacks default_driver_check(), drive_enable_snapshot(),
>>   vnc_init_func()
>> 
>> Drop the parameter, and always stop.
>
> I had a hunch you might be getting to this point, just from reading the
> earlier patches :)

I take that as a good sign :)

>> 
>> Signed-off-by: Markus Armbruster <address@hidden>
>> ---
>>  block/blkdebug.c                 |  4 ++--
>>  hw/core/qdev-properties-system.c |  2 +-
>>  include/qemu/option.h            |  4 ++--
>>  net/net.c                        |  5 +++--
>>  net/vhost-user.c                 |  2 +-
>>  numa.c                           |  3 +--
>>  tpm.c                            |  3 +--
>>  util/qemu-config.c               |  2 +-
>>  util/qemu-option.c               | 21 ++++++++++++++-------
>>  vl.c                             | 35 ++++++++++++++++++-----------------
>>  10 files changed, 44 insertions(+), 37 deletions(-)
>> 
>
>> +++ b/net/vhost-user.c
>> @@ -240,7 +240,7 @@ int net_init_vhost_user(const NetClientOptions *opts, 
>> const char *name,
>>  
>>      /* verify net frontend */
>>      if (qemu_opts_foreach(qemu_find_opts("device"), net_vhost_check_net,
>> -                          (char *)name, true) == -1) {
>> +                          (char *)name)) {
>
> Also evidence that we weren't clear on 'int' vs. 'bool' on the
> parameter's usage, so getting rid of it is indeed an improvement.
>
> Reviewed-by: Eric Blake <address@hidden>

Thanks!