Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> writes: > 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; > }
You're right, that's wrong. Missed when I reordered my patches. This PATCH needs to go after "[PATCH v2 37/44] error: Reduce unnecessary error propagation", which has this hunk. > squashed-in: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> I'll keep your R-by there, hope that's okay. > 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; > } This additional hunk is part of PATCH 41. > > > with it, as well: > Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> Thanks!