Re: [Qemu-devel] [PATCH 6/6] migration: Pass Error ** argument to {save, load}_vmstate

[Eric Blake]

On 04/28/2017 09:47 AM, Dr. David Alan Gilbert wrote:
> * Juan Quintela (address@hidden) wrote:
>> This way we use the "normal" way of printing errors for hmp commands.
>>
>> --

>>      if (!bdrv_all_can_snapshot(&bs)) {
>> -        error_report("Device '%s' is writable but does not support 
>> snapshots",
>> -                     bdrv_get_device_name(bs));
>> +        error_setg(errp, "Device '%s' is writable but does not support "
>> +                   "snapshots", bdrv_get_device_name(bs));
>>          return ret;
>>      }
>>  
>>      /* Delete old snapshots of the same name */
>>      if (name) {
>> -        ret = bdrv_all_delete_snapshot(name, &bs1, &local_err);
>> +        ret = bdrv_all_delete_snapshot(name, &bs1, errp);
>>          if (ret < 0) {
>> -            error_reportf_err(local_err,
>> -                              "Error while deleting snapshot on device 
>> '%s': ",
>> -                              bdrv_get_device_name(bs1));
>> +            error_prepend(errp, "Error while deleting snapshot on device "
>> +                          "'%s': ", bdrv_get_device_name(bs1));
> 
> I thought the rule was that you had to use a local_err and use 
> error_propagate?
> (I hate that rule)

The rule is that if you want to make code conditional on whether an
error occurred, you can't do it by checking errp (because the caller may
have passed NULL), so you instead have to use local_err and
error_propagate(). But if there are other ways to tell if an error
occurred (such as a return value), then passing errp directly through is
fine (error_prepend does the right thing even if errp is NULL because
the caller doesn't care about an error).

include/qapi/error.h has some good examples, and if it is missing
something, we'd be glad to patch it further to serve as a good reference.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature