Re: [PATCH v2 37/44] error: Reduce unnecessary error propagation

2020-07-03 Thread Markus Armbruster 
Eric Blake  writes:

> On 7/2/20 10:49 AM, Markus Armbruster wrote:
>> When all we do with an Error we receive into a local variable is
>> propagating to somewhere else, we can just as well receive it there
>> right away, even when we need to keep error_propagate() for other
>> error paths.
>>
>> Signed-off-by: Markus Armbruster 
>> ---
>
>> +++ b/block/replication.c
>> @@ -85,7 +85,6 @@ static int replication_open(BlockDriverState *bs, QDict 
>> *options,
>>   {
>>   int ret;
>>   BDRVReplicationState *s = bs->opaque;
>> -Error *local_err = NULL;
>>   QemuOpts *opts = NULL;
>>   const char *mode;
>>   const char *top_id;
>> @@ -99,7 +98,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;
>>   }
>
> Does this one belong in 36/44, given that removal of 'local_err' is
> evidence that no other error path needed it?
>
> Either way, it belongs in the series, and the result of the two
> patches together is fine.
>
> Reviewed-by: Eric Blake 

Actually, this hunk needs to go before PATCH 33 to keep it correct.
I'll find out how to best reshuffle hunks.  The end result will be the
same.

Thanks!

Re: [PATCH v2 37/44] error: Reduce unnecessary error propagation

2020-07-02 Thread Eric Blake 

On 7/2/20 10:49 AM, Markus Armbruster wrote:

When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away, even when we need to keep error_propagate() for other
error paths.

Signed-off-by: Markus Armbruster 
---



+++ b/block/replication.c
@@ -85,7 +85,6 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
  {
  int ret;
  BDRVReplicationState *s = bs->opaque;
-Error *local_err = NULL;
  QemuOpts *opts = NULL;
  const char *mode;
  const char *top_id;
@@ -99,7 +98,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;
  }


Does this one belong in 36/44, given that removal of 'local_err' is 
evidence that no other error path needed it?


Either way, it belongs in the series, and the result of the two patches 
together is fine.


Reviewed-by: Eric Blake 

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

[PATCH v2 37/44] error: Reduce unnecessary error propagation

2020-07-02 Thread Markus Armbruster 
When all we do with an Error we receive into a local variable is
propagating to somewhere else, we can just as well receive it there
right away, even when we need to keep error_propagate() for other
error paths.

Signed-off-by: Markus Armbruster 
---
 block.c | 2 +-
 block/gluster.c | 8 
 block/parallels.c   | 2 +-
 block/quorum.c  | 2 +-
 block/replication.c | 3 +--
 block/vxhs.c| 4 ++--
 hw/core/qdev.c  | 2 +-
 hw/net/virtio-net.c | 4 ++--
 8 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/block.c b/block.c
index 60d2945c2c..2dcf9afd61 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 324085ccea..c259799111 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -827,7 +827,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 5d52e605db..6df9449fc2 100644
--- a/block/quorum.c
+++ b/block/quorum.c
@@ -921,7 +921,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 dcd430624e..0c70215784 100644
--- a/block/replication.c
+++ b/block/replication.c
@@ -85,7 +85,6 @@ static int replication_open(BlockDriverState *bs, QDict 
*options,
 {
 int ret;
 BDRVReplicationState *s = bs->opaque;
-Error *local_err = NULL;
 QemuOpts *opts = NULL;
 const char *mode;
 const char *top_id;
@@ -99,7 +98,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 fecaeb82c9..dc0e254730 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 th