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

[Eric Blake]

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