Re: [PATCH 10/46] qemu-option: Check return value instead of @err where convenient

[Vladimir Sementsov-Ogievskiy]

24.06.2020 19:43, Markus Armbruster wrote:
> Convert uses like
>
>      opts = qemu_opts_create(..., &err);
>      if (err) {
>          ...
>      }
>
> to
>
>      opts = qemu_opts_create(..., &err);
>      if (!opts) {
>          ...
>      }
>
> Eliminate error_propagate() that are now unnecessary.  Delete @err
> that are now unused.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   block/parallels.c  |  4 ++--
>   blockdev.c         |  5 ++---
>   qdev-monitor.c     |  6 ++----
>   util/qemu-config.c | 10 ++++------
>   util/qemu-option.c | 12 ++++--------
>   5 files changed, 14 insertions(+), 23 deletions(-)
>
> diff --git a/block/parallels.c b/block/parallels.c
> index 860dbb80a2..b15c9ac28d 100644
> --- a/block/parallels.c
> +++ b/block/parallels.c
> @@ -823,8 +823,8 @@ static int parallels_open(BlockDriverState *bs, QDict 
> *options, int flags,
>           }
>       }
>   
> -    opts = qemu_opts_create(&parallels_runtime_opts, NULL, 0, &local_err);
> -    if (local_err != NULL) {
> +    opts = qemu_opts_create(&parallels_runtime_opts, NULL, 0, errp);
> +    if (!opts) {
>           goto fail_options;
>       }

Honestly, I don't like this hunk. as already complicated code (crossing gotos) 
becomes more
complicated (add one more pattern to fail_options path: no-op error_propagate).

At least, we'll need a follow-up patch, refactoring parallels_open() to drop 
"fail_options"
label completely.

Still, it should work and the rest is fine, so:
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>

--
Best regards,
Vladimir