Re: [Qemu-devel] [PATCH v3 09/10] vmdk: use heap allocation for whole_grain

[Jeff Cody]

On Tue, Aug 06, 2013 at 09:40:42AM +0800, Fam Zheng wrote:
> We should never grow the stack beyond 1 MB, otherwise we'll fall off the
> end.  Thread stacks and coroutine stacks (1 MB) do not grow.
> get_cluster_offset() allocates a big stack offset, it will fail for big
> cluster images, change to heap allocated buffer.
> 
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>  block/vmdk.c | 19 +++++++++++++------
>  1 file changed, 13 insertions(+), 6 deletions(-)
> 
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 3bcab26..21610e3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -842,16 +842,19 @@ static int get_whole_cluster(BlockDriverState *bs,
>                  uint64_t offset,
>                  bool allocate)
>  {
> -    /* 128 sectors * 512 bytes each = grain size 64KB */
> -    uint8_t  whole_grain[extent->cluster_sectors * 512];
> +    int ret = VMDK_OK;
> +    uint8_t *whole_grain = NULL;
>  
>      /* we will be here if it's first write on non-exist grain(cluster).
>       * try to read from parent image, if exist */
>      if (bs->backing_hd) {
>          int ret;
            ^^^^^^
This above line should be removed; as it is this function will always return
VMDK_OK with this here, due to the scoping of this 'ret' inside the
if() block.

>  
> +        whole_grain =
> +            qemu_blockalign(bs, extent->cluster_sectors << BDRV_SECTOR_BITS);
>          if (!vmdk_is_cid_valid(bs)) {
> -            return VMDK_ERROR;
> +            ret = VMDK_ERROR;
> +            goto exit;
>          }
>  
>          /* floor offset to cluster */
> @@ -859,17 +862,21 @@ static int get_whole_cluster(BlockDriverState *bs,
>          ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
>                  extent->cluster_sectors);
>          if (ret < 0) {
> -            return VMDK_ERROR;
> +            ret = VMDK_ERROR;
> +            goto exit;
>          }
>  
>          /* Write grain only into the active image */
>          ret = bdrv_write(extent->file, cluster_offset, whole_grain,
>                  extent->cluster_sectors);
>          if (ret < 0) {
> -            return VMDK_ERROR;
> +            ret = VMDK_ERROR;
> +            goto exit;
>          }
>      }
> -    return VMDK_OK;
> +exit:
> +    qemu_vfree(whole_grain);
> +    return ret;
>  }
>  
>  static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
> -- 
> 1.8.3.4
>