[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and dr
From: |
Kevin Wolf |
Subject: |
Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation |
Date: |
Mon, 15 Feb 2021 10:22:03 +0100 |
Am 13.02.2021 um 00:13 hat Eric Blake geschrieben:
> 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
The problem is this hunk:
diff --git a/block/qcow2.c b/block/qcow2.c
index 5d94f45be9..e8dd42d73b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1611,9 +1611,8 @@ static int coroutine_fn qcow2_do_open(BlockDriverState
*bs, QDict *options,
/* Open external data file */
s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
&child_of_bds, BDRV_CHILD_DATA,
- true, &local_err);
- if (local_err) {
- error_propagate(errp, local_err);
+ true, errp);
+ if (!s->data_file) {
ret = -EINVAL;
goto fail;
}
bdrv_open_child() can return NULL in non-error cases, when the child is
optional and it isn't given. The test doesn't use an external data file,
so this returns NULL without setting an error, which now gets turned
into -EINVAL instead.
This makes the most basic tests fail with qcow2 (iotests 001 is enough).
The other hunks in this patch don't suffer from the same problem because
they pass allow_none=false.
Kevin
- [PATCH v7 00/14] block: deal with errp: part I, Vladimir Sementsov-Ogievskiy, 2021/02/02
- [PATCH v7 01/14] block: return status from bdrv_append and friends, Vladimir Sementsov-Ogievskiy, 2021/02/02
- [PATCH v7 04/14] blockdev: fix drive_backup_prepare() missed error, Vladimir Sementsov-Ogievskiy, 2021/02/02
- [PATCH v7 05/14] block: drop extra error propagation for bdrv_set_backing_hd, Vladimir Sementsov-Ogievskiy, 2021/02/02
- [PATCH v7 02/14] block: use return status of bdrv_append(), Vladimir Sementsov-Ogievskiy, 2021/02/02
- [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 10/14] block/qcow2-bitmap: return status from qcow2_store_persistent_dirty_bitmaps, Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 07/14] blockjob: return status from block_job_set_speed(), Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 06/14] block/mirror: drop extra error propagation in commit_active_start(), Vladimir Sementsov-Ogievskiy, 2021/02/02
[PATCH v7 08/14] block/qcow2: qcow2_get_specific_info(): drop error propagation, Vladimir Sementsov-Ogievskiy, 2021/02/02