Re: [Qemu-devel] [PATCH v3 17/21] vhdx: Handle failure for potentially large allocations

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:58 (+0200), Kevin Wolf wrote :
> Some code in the block layer makes potentially huge allocations. Failure
> is not completely unexpected there, so avoid aborting qemu and handle
> out-of-memory situations gracefully.
> 
> This patch addresses the allocations in the vhdx block driver.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
>  block/vhdx-log.c |  6 +++++-
>  block/vhdx.c     | 12 ++++++++++--
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/block/vhdx-log.c b/block/vhdx-log.c
> index a77c040..3eb7e68 100644
> --- a/block/vhdx-log.c
> +++ b/block/vhdx-log.c
> @@ -349,7 +349,11 @@ static int vhdx_log_read_desc(BlockDriverState *bs, 
> BDRVVHDXState *s,
>      }
>  
>      desc_sectors = vhdx_compute_desc_sectors(hdr.descriptor_count);
> -    desc_entries = qemu_blockalign(bs, desc_sectors * VHDX_LOG_SECTOR_SIZE);
> +    desc_entries = qemu_try_blockalign(bs, desc_sectors * 
> VHDX_LOG_SECTOR_SIZE);
> +    if (desc_entries == NULL) {
> +        ret = -ENOMEM;
> +        goto exit;
> +    }
>  
>      ret = vhdx_log_read_sectors(bs, log, &sectors_read, desc_entries,
>                                  desc_sectors, false);
> diff --git a/block/vhdx.c b/block/vhdx.c
> index 353c74d..0922f55 100644
> --- a/block/vhdx.c
> +++ b/block/vhdx.c
> @@ -950,7 +950,11 @@ static int vhdx_open(BlockDriverState *bs, QDict 
> *options, int flags,
>      }
>  
>      /* s->bat is freed in vhdx_close() */
> -    s->bat = qemu_blockalign(bs, s->bat_rt.length);
> +    s->bat = qemu_try_blockalign(bs, s->bat_rt.length);
> +    if (s->bat == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
>  
>      ret = bdrv_pread(bs->file, s->bat_offset, s->bat, s->bat_rt.length);
>      if (ret < 0) {
> @@ -1579,7 +1583,11 @@ static int vhdx_create_bat(BlockDriverState *bs, 
> BDRVVHDXState *s,
>                  use_zero_blocks ||
>                  bdrv_has_zero_init(bs) == 0) {
>          /* for a fixed file, the default BAT entry is not zero */
> -        s->bat = g_malloc0(rt_bat->length);
> +        s->bat = g_try_malloc0(rt_bat->length);
> +        if (rt_bat->length && s->bat != NULL) {
> +            ret = -ENOMEM;
> +            goto exit;
> +        }
>          block_state = type == VHDX_TYPE_FIXED ? PAYLOAD_BLOCK_FULLY_PRESENT :
>                                                  PAYLOAD_BLOCK_NOT_PRESENT;
>          block_state = use_zero_blocks ? PAYLOAD_BLOCK_ZERO : block_state;
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <address@hidden>