Re: [Qemu-devel] [PATCH 1/2] block: Add Error argument to bdrv_refresh_limits()

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2215 bytes --]

On 07/16/2014 09:48 AM, Kevin Wolf wrote:
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c                   | 33 +++++++++++++++++++++++----------
>  block/iscsi.c             |  3 +--
>  block/qcow2.c             |  4 +---
>  block/qed.c               |  4 +---
>  block/raw-posix.c         |  4 +---
>  block/raw_bsd.c           |  3 +--
>  block/stream.c            |  2 +-
>  block/vmdk.c              |  4 +---
>  include/block/block.h     |  2 +-
>  include/block/block_int.h |  2 +-
>  10 files changed, 32 insertions(+), 29 deletions(-)
> 

>      /* Take some limits from the children as a default */
>      if (bs->file) {
> -        bdrv_refresh_limits(bs->file);
> +        bdrv_refresh_limits(bs->file, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }

Nice that you are no longer ignoring failure from the child.


> @@ -1154,7 +1167,7 @@ void bdrv_set_backing_hd(BlockDriverState *bs, BlockDriverState *backing_hd)
>      bdrv_op_unblock(bs->backing_hd, BLOCK_OP_TYPE_COMMIT,
>                      bs->backing_blocker);
>  out:
> -    bdrv_refresh_limits(bs);
> +    bdrv_refresh_limits(bs, NULL);
>  }
>  
>  /*
> @@ -1778,7 +1791,7 @@ void bdrv_reopen_commit(BDRVReopenState *reopen_state)
>                                                BDRV_O_CACHE_WB);
>      reopen_state->bs->read_only = !(reopen_state->flags & BDRV_O_RDWR);
>  
> -    bdrv_refresh_limits(reopen_state->bs);
> +    bdrv_refresh_limits(reopen_state->bs, NULL);

> +++ b/block/stream.c
> @@ -76,7 +76,7 @@ static void close_unused_images(BlockDriverState *top, BlockDriverState *base,
>          bdrv_unref(unused);
>      }
>  
> -    bdrv_refresh_limits(top);
> +    bdrv_refresh_limits(top, NULL);
>  }
>  

Should these three callers be concerned about failure?  If so, would
&error_abort be better than NULL?  But as for this patch, you are
preserving existing semantics, so you could save it for a later patch.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]