[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: |
Eric Blake |
Subject: |
Re: [PATCH v7 03/14] block: check return value of bdrv_open_child and drop error propagation |
Date: |
Mon, 15 Feb 2021 14:04:35 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.0 |
On 2/15/21 3:22 AM, Kevin Wolf wrote:
>> 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
>>
> 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.
Thanks; that's enough to figure out how to repair the patch:
diff --git i/block/qcow2.c w/block/qcow2.c
index e8dd42d73b4c..38137ca30eb0 100644
--- i/block/qcow2.c
+++ w/block/qcow2.c
@@ -1292,6 +1292,7 @@ static int
validate_compression_type(BDRVQcow2State *s, Error **errp)
static int coroutine_fn qcow2_do_open(BlockDriverState *bs, QDict *options,
int flags, Error **errp)
{
+ ERRP_GUARD();
BDRVQcow2State *s = bs->opaque;
unsigned int len, i;
int ret = 0;
@@ -1612,7 +1613,7 @@ static int coroutine_fn
qcow2_do_open(BlockDriverState *bs, QDict *options,
s->data_file = bdrv_open_child(NULL, options, "data-file", bs,
&child_of_bds, BDRV_CHILD_DATA,
true, errp);
- if (!s->data_file) {
+ if (*errp) {
ret = -EINVAL;
goto fail;
}
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3226
Virtualization: qemu.org | libvirt.org
- [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