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

Markus Armbruster posted 46 patches 5 years, 5 months ago
There is a newer version of this series
[PATCH 18/46] qemu-option: Smooth error checking manually
Posted by Markus Armbruster 5 years, 5 months ago
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(-)

diff --git a/block.c b/block.c
index 77e85f13db..93a5fbf60a 100644
--- a/block.c
+++ b/block.c
@@ -6073,7 +6073,7 @@ void bdrv_img_create(const char *filename, const char *fmt,
 
     /* Parse -o options */
     if (options) {
-        if (!qemu_opts_do_parse(opts, options, NULL, &local_err)) {
+        if (!qemu_opts_do_parse(opts, options, NULL, errp)) {
             goto out;
         }
     }
diff --git a/block/gluster.c b/block/gluster.c
index c620880f27..4f1448e2bc 100644
--- a/block/gluster.c
+++ 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;
     }
 
@@ -554,7 +554,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
 
         /* create opts info from runtime_type_opts list */
         opts = qemu_opts_create(&runtime_type_opts, NULL, 0, &error_abort);
-        if (!qemu_opts_absorb_qdict(opts, backing_options, &local_err)) {
+        if (!qemu_opts_absorb_qdict(opts, backing_options, errp)) {
             goto out;
         }
 
@@ -584,7 +584,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
         if (gsconf->type == SOCKET_ADDRESS_TYPE_INET) {
             /* create opts info from runtime_inet_opts list */
             opts = qemu_opts_create(&runtime_inet_opts, NULL, 0, &error_abort);
-            if (!qemu_opts_absorb_qdict(opts, backing_options, &local_err)) {
+            if (!qemu_opts_absorb_qdict(opts, backing_options, errp)) {
                 goto out;
             }
 
@@ -632,7 +632,7 @@ static int qemu_gluster_parse_json(BlockdevOptionsGluster *gconf,
         } else {
             /* create opts info from runtime_unix_opts list */
             opts = qemu_opts_create(&runtime_unix_opts, NULL, 0, &error_abort);
-            if (!qemu_opts_absorb_qdict(opts, backing_options, &local_err)) {
+            if (!qemu_opts_absorb_qdict(opts, backing_options, errp)) {
                 goto out;
             }
 
diff --git a/block/parallels.c b/block/parallels.c
index ef0d92d05c..0397f3894f 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -828,7 +828,7 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail_options;
     }
 
-    if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
+    if (!qemu_opts_absorb_qdict(opts, options, errp)) {
         goto fail_options;
     }
 
diff --git a/block/quorum.c b/block/quorum.c
index beb3b6dbcc..9ed20e1998 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -922,7 +922,7 @@ static int quorum_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     opts = qemu_opts_create(&quorum_runtime_opts, NULL, 0, &error_abort);
-    if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
+    if (!qemu_opts_absorb_qdict(opts, options, errp)) {
         ret = -EINVAL;
         goto exit;
     }
diff --git a/block/replication.c b/block/replication.c
index 00a50b095e..7f4ab357a4 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -99,7 +99,7 @@ static int replication_open(BlockDriverState *bs, QDict *options,
 
     ret = -EINVAL;
     opts = qemu_opts_create(&replication_runtime_opts, NULL, 0, &error_abort);
-    if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
+    if (!qemu_opts_absorb_qdict(opts, options, errp)) {
         goto fail;
     }
 
diff --git a/block/vxhs.c b/block/vxhs.c
index 237df4f185..20513a43f4 100644
--- a/block/vxhs.c
+++ b/block/vxhs.c
@@ -318,7 +318,7 @@ static int vxhs_open(BlockDriverState *bs, QDict *options,
     opts = qemu_opts_create(&runtime_opts, NULL, 0, &error_abort);
     tcp_opts = qemu_opts_create(&runtime_tcp_opts, NULL, 0, &error_abort);
 
-    if (!qemu_opts_absorb_qdict(opts, options, &local_err)) {
+    if (!qemu_opts_absorb_qdict(opts, options, errp)) {
         ret = -EINVAL;
         goto out;
     }
@@ -345,7 +345,7 @@ static int vxhs_open(BlockDriverState *bs, QDict *options,
     /* get the 'server.' arguments */
     qdict_extract_subqdict(options, &backing_options, VXHS_OPT_SERVER".");
 
-    if (!qemu_opts_absorb_qdict(tcp_opts, backing_options, &local_err)) {
+    if (!qemu_opts_absorb_qdict(tcp_opts, backing_options, errp)) {
         ret = -EINVAL;
         goto out;
     }
diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 04b012e487..7502ff0b1e 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -3119,8 +3119,8 @@ static bool failover_replug_primary(VirtIONet *n, Error **errp)
     qdev_set_parent_bus(n->primary_dev, n->primary_bus);
     n->primary_should_be_hidden = false;
     if (!qemu_opt_set_bool(n->primary_device_opts,
-                           "partially_hotplugged", true, &err)) {
-        goto out;
+                           "partially_hotplugged", true, errp)) {
+        return false;
     }
     hotplug_ctrl = qdev_get_hotplug_handler(n->primary_dev);
     if (hotplug_ctrl) {
-- 
2.26.2


Re: [PATCH 18/46] qemu-option: Smooth error checking manually
Posted by Eric Blake 5 years, 5 months ago
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); 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.

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

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


Re: [PATCH 18/46] qemu-option: Smooth error checking manually
Posted by Markus Armbruster 5 years, 5 months ago
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!