Re: [PATCH v2 19/44] block/parallels: Simplify parallels_open() after previous commit

[Vladimir Sementsov-Ogievskiy]

02.07.2020 18:49, Markus Armbruster wrote:
Signed-off-by: Markus Armbruster <arm...@redhat.com>
Reviewed-by: Eric Blake <ebl...@redhat.com>
---
  block/parallels.c | 7 ++-----
  1 file changed, 2 insertions(+), 5 deletions(-)
diff --git a/block/parallels.c b/block/parallels.c
index 32d0ecd398..e0ec819550 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -843,6 +843,7 @@ static int parallels_open(BlockDriverState *bs, QDict 
*options, int flags,
                                         &local_err);
      g_free(buf);
      if (local_err != NULL) {
+        error_propagate(errp, local_err);
          goto fail_options;
      }
  
@@ -873,15 +874,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
  
  fail_format:
      error_setg(errp, "Image not in Parallels format");
+fail_options:
      ret = -EINVAL;
  fail:
      qemu_vfree(s->header);
      return ret;
-
-fail_options:
-    error_propagate(errp, local_err);
-    ret = -EINVAL;
-    goto fail;
  }
  
  


You leak local_err in one case. With at least:

diff --git a/block/parallels.c b/block/parallels.c
index e0ec819550..5c1940ee02 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -829,7 +829,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;
     }


squashed-in:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>


Still, if we have a special patch for the this function, we can get rid of one 
more propagation:

diff --git a/block/parallels.c b/block/parallels.c
index e0ec819550..d4ad83ac19 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -829,7 +829,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;
     }
 
@@ -863,9 +863,8 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     error_setg(&s->migration_blocker, "The Parallels format used by node '%s' "
                "does not support live migration",
                bdrv_get_device_or_node_name(bs));
-    ret = migrate_add_blocker(s->migration_blocker, &local_err);
-    if (local_err) {
-        error_propagate(errp, local_err);
+    ret = migrate_add_blocker(s->migration_blocker, errp);
+    if (ret < 0) {
         error_free(s->migration_blocker);
         goto fail;
     }


with it, as well:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>

--
Best regards,
Vladimir