Re: [Qemu-devel] [PATCH] allow reading variable size vmdk descriptor files

[Don Slutz]

On 06/12/13 03:08, Evgeny Budilovsky wrote:
> The hard-coded 2k buffer on the stack won't allow reading big descriptor
> files which can be generated when storing big images (For example 500G
> vmdk splitted to 2G chunks).
>
> Signed-off-by: Evgeny Budilovsky <address@hidden>
> ---
>   block/vmdk.c |   28 +++++++++++++++++++++-------
>   1 file changed, 21 insertions(+), 7 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 608daaf..1bc944b 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -719,27 +719,41 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int 
> flags,
>                                  int64_t desc_offset)
>   {
>       int ret;
> -    char buf[2048];
> +    char *buf = NULL;
>       char ct[128];
>       BDRVVmdkState *s = bs->opaque;
> +    int64_t size;
>
> -    ret = bdrv_pread(bs->file, desc_offset, buf, sizeof(buf));
> +    size = bdrv_get_allocated_file_size(bs);
> +    if (size < 0) {
> +        return -EINVAL;
> +    }
> +
While this is right for vmdk splitted to 2G chunks, I think this will 
fail for a big enough "monolithicFlat" vmdk where there is only the 1 
file (g_malloc() will most likely fail for a 500GB file).
> +    buf = g_malloc0(size+1);
> +
> +    ret = bdrv_pread(bs->file, desc_offset, buf, size);
>       if (ret < 0) {
> -        return ret;
> +        goto exit;
>       }
> -    buf[2047] = '\0';
>       if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
> -        return -EMEDIUMTYPE;
> +        ret = -EMEDIUMTYPE;
> +        goto exit;
>       }
>       if (strcmp(ct, "monolithicFlat") &&
>           strcmp(ct, "twoGbMaxExtentSparse") &&
>           strcmp(ct, "twoGbMaxExtentFlat")) {
>           fprintf(stderr,
>                   "VMDK: Not supported image type \"%s\""".\n", ct);
> -        return -ENOTSUP;
> +        ret = -ENOTSUP;
> +        goto exit;
>       }
>       s->desc_offset = 0;
> -    return vmdk_parse_extents(buf, bs, bs->file->filename);
> +    ret = vmdk_parse_extents(buf, bs, bs->file->filename);
> +exit:
> +    if (buf) {
> +        g_free(buf);
> +    }
> +    return ret;
>   }
>
>   static int vmdk_open(BlockDriverState *bs, QDict *options, int flags)
> --
> 1.7.9.5
   -Don Slutz