Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation

[Eric Blake]

On 2/2/21 6:49 AM, Vladimir Sementsov-Ogievskiy wrote:
> This patch is generated by cocci script:
> 
> @@
> symbol bdrv_open_child, errp, local_err;
> expression file;
> @@
> 
>   file = bdrv_open_child(...,
> -                        &local_err
> +                        errp
>                         );
> - if (local_err)
> + if (!file)
>   {
>       ...
> -     error_propagate(errp, local_err);
>       ...
>   }
> 
> with command
> 
> spatch --sp-file x.cocci --macro-file scripts/cocci-macro-file.h \
> --in-place --no-show-diff --max-width 80 --use-gitgrep block

With this patch applied, 'check unit-test' fails with:

Running test test-replication
Unexpected error in bdrv_open_driver() at ../block.c:1481:
Could not open '/tmp/p_local_disk.z1Ugyc': Invalid argument
ERROR test-replication - missing test plan

Directly reverting it has ripple effect on later patches in the series.

Running test-replication under gdb gives this backtrace:

Thread 1 "test-replicatio" received signal SIGABRT, Aborted.
0x00007ffff6f6f9d5 in raise () from /lib64/libc.so.6
(gdb) bt
#0  0x00007ffff6f6f9d5 in raise () from /lib64/libc.so.6
#1  0x00007ffff6f588a4 in abort () from /lib64/libc.so.6
#2  0x00005555556ad820 in error_handle_fatal (
    errp=0x555555790568 <error_abort>, err=0x555555859010)
    at ../util/error.c:40
#3  0x00005555556ae3cf in error_propagate (
    dst_errp=0x555555790568 <error_abort>, local_err=0x555555859010)
    at ../util/error.c:286
#4  0x000055555558cc9e in bdrv_img_create (
    filename=0x555555822500 "/tmp/p_local_disk.DVFoWt",
    fmt=0x5555556e809a "qcow2", base_filename=0x0, base_fmt=0x0,
options=0x0,
    img_size=67108864, flags=2, quiet=true, errp=0x555555790568
<error_abort>)
    at ../block.c:6312

> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Reviewed-by: Alberto Garcia <berto@igalia.com>
> ---
>  block/blkdebug.c     |  6 ++----
>  block/blklogwrites.c | 10 ++++------
>  block/blkreplay.c    |  6 ++----
>  block/blkverify.c    | 11 ++++-------
>  block/qcow2.c        |  5 ++---
>  block/quorum.c       |  6 ++----
>  6 files changed, 16 insertions(+), 28 deletions(-)

And this diffstat doesn't immediately tell me what ended up violating
the assumptions of error_abort.  As such, at this point I'm temporarily
dropping the remainder of the series from my bitmaps queue, and only
including patches 1 and 2 in my next pull request.  Looking forward to v8.

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