Re: [PATCH 18/46] qemu-option: Smooth error checking manually

[Markus Armbruster]

Eric Blake <eblake@redhat.com> writes:

> On 6/24/20 11:43 AM, Markus Armbruster wrote:
>> When foo(..., &err) is followed by error_propagate(errp, err), we can
>> often just as well do foo(..., errp).  The previous commit did that
>> for simple cases with Coccinelle.  Do it for a few more manually.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>>   block.c             | 2 +-
>>   block/gluster.c     | 8 ++++----
>>   block/parallels.c   | 2 +-
>>   block/quorum.c      | 2 +-
>>   block/replication.c | 2 +-
>>   block/vxhs.c        | 4 ++--
>>   hw/net/virtio-net.c | 4 ++--
>>   7 files changed, 12 insertions(+), 12 deletions(-)
>>
>
>> +++ b/block/gluster.c
>> @@ -523,7 +523,7 @@ static int 
>> qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
>>         /* create opts info from runtime_json_opts list */
>>       opts = qemu_opts_create(&runtime_json_opts, NULL, 0, &error_abort);
>> -    if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
>> +    if (!qemu_opts_absorb_qdict(opts, options, errp)) {
>>           goto out;
>>       }
>
> This function also has a few error_setg(&local_err) that could be
> cleaned up to error_setg(errp);

More elsewhere.  PATCH 08 transforms them only where it permits deleting
the error_propagate().  I left the remainder for another day.  Not all
of them can be transformed more or less mechanically, e.g. this one in
mirror.c:

        if (bdrv_recurse_can_replace(src, to_replace)) {
            bdrv_replace_node(to_replace, target_bs, &local_err);
        } else {
            error_setg(&local_err, "Can no longer replace '%s' by '%s', "
                       "because it can no longer be guaranteed that doing so "
                       "would not lead to an abrupt change of visible data",
                       to_replace->node_name, target_bs->node_name);
        }
        bdrv_drained_end(target_bs);
        if (local_err) {
            error_report_err(local_err);
            ret = -EPERM;
        }

>                                 but the ones that use
> error_append_hint() immediately after (and thus the
> error_propagate(errp, local_err) in the out: label) still have to
> remain, until we have Vladimir's macro in place.

Correct.

> Reviewed-by: Eric Blake <eblake@redhat.com>

Thanks!