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

[Benoît Canet]

The Tuesday 03 Jun 2014 à 15:10:57 (+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 vdi block driver.
> 
> Signed-off-by: Kevin Wolf <address@hidden>
> Reviewed-by: Stefan Hajnoczi <address@hidden>
> ---
>  block/vdi.c | 24 +++++++++++++++++++-----
>  1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/block/vdi.c b/block/vdi.c
> index 27737af..1f76441 100644
> --- a/block/vdi.c
> +++ b/block/vdi.c
> @@ -293,7 +293,12 @@ static int vdi_check(BlockDriverState *bs, 
> BdrvCheckResult *res,
>          return -ENOTSUP;
>      }
>  
> -    bmap = g_malloc(s->header.blocks_in_image * sizeof(uint32_t));
> +    bmap = g_try_malloc(s->header.blocks_in_image * sizeof(uint32_t));
> +    if (s->header.blocks_in_image && bmap == NULL) {
> +        res->check_errors++;
> +        return -ENOMEM;
> +    }
> +
>      memset(bmap, 0xff, s->header.blocks_in_image * sizeof(uint32_t));
>  
>      /* Check block map and value of blocks_allocated. */
> @@ -472,7 +477,12 @@ static int vdi_open(BlockDriverState *bs, QDict 
> *options, int flags,
>  
>      bmap_size = header.blocks_in_image * sizeof(uint32_t);
>      bmap_size = (bmap_size + SECTOR_SIZE - 1) / SECTOR_SIZE;
> -    s->bmap = g_malloc(bmap_size * SECTOR_SIZE);
> +    s->bmap = qemu_try_blockalign(bs->file, bmap_size * SECTOR_SIZE);
> +    if (s->bmap == NULL) {
> +        ret = -ENOMEM;
> +        goto fail;
> +    }
> +
>      ret = bdrv_read(bs->file, s->bmap_sector, (uint8_t *)s->bmap, bmap_size);
>      if (ret < 0) {
>          goto fail_free_bmap;
> @@ -487,7 +497,7 @@ static int vdi_open(BlockDriverState *bs, QDict *options, 
> int flags,
>      return 0;
>  
>   fail_free_bmap:
> -    g_free(s->bmap);
> +    qemu_vfree(s->bmap);
>  
>   fail:
>      return ret;
> @@ -760,7 +770,11 @@ static int vdi_create(const char *filename, 
> QEMUOptionParameter *options,
>      }
>  
>      if (bmap_size > 0) {
> -        uint32_t *bmap = g_malloc0(bmap_size);
> +        uint32_t *bmap = g_try_malloc0(bmap_size);
> +        if (bmap == NULL) {
> +            result = -ENOMEM;
> +            goto close_and_exit;
> +        }
>          for (i = 0; i < blocks; i++) {
>              if (image_type == VDI_TYPE_STATIC) {
>                  bmap[i] = i;
> @@ -796,7 +810,7 @@ static void vdi_close(BlockDriverState *bs)
>  {
>      BDRVVdiState *s = bs->opaque;
>  
> -    g_free(s->bmap);
> +    qemu_vfree(s->bmap);
>  
>      migrate_del_blocker(s->migration_blocker);
>      error_free(s->migration_blocker);
> -- 
> 1.8.3.1
> 
> 
Reviewed-by: Benoit Canet <address@hidden>