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

[Don Slutz <dslutz@verizon.com>]

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

On 06/14/13 02:41, Evgeny Budilovsky wrote:
>
>
>
> On Fri, Jun 14, 2013 at 12:15 AM, Don Slutz <dslutz@verizon.com 
> <mailto:dslutz@verizon.com>> wrote:
>
>     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
>         <evgeny.budilovsky@ravellosystems.com
>         <mailto:evgeny.budilovsky@ravellosystems.com>>
>         ---
>           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).
>
> With the "monolithicFlat" vmdk the descriptor file is a small textual 
> file. So this code should work. In the second version of this patch 
> I've added some constraint to the allocation size just in case the 
> file is corrupted or we have misinterpreted the format.
>
Opps, I did the wrong one.  Both createType="streamOptimized" and 
createType="monolithicSparse" are only 1 file.
> size = MIN(size, 1 << 20);  /* avoid unbounded allocation */
This will "fix" the issue.
    -Don Slutz
> buf = g_malloc0(size + 1);
>
>         +    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
>
>
>
>
> -- 
> Best Regards,
> Evgeny


[-- Attachment #2: Type: text/html, Size: 8711 bytes --]