From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from eggs.gnu.org ([2001:4830:134:3::10]:50480)
	by lists.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dslutz@verizon.com>) id 1UnEtB-00042f-1g
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 17:17:03 -0400
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <dslutz@verizon.com>) id 1UnEt8-0004XJ-3D
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 17:17:00 -0400
Received: from fldsmtpe03.verizon.com ([140.108.26.142]:24722)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <dslutz@verizon.com>) id 1UnEt7-0004Ul-VY
	for qemu-devel@nongnu.org; Thu, 13 Jun 2013 17:16:58 -0400
From: Don Slutz <dslutz@verizon.com>
Message-ID: <51BA3681.3020003@terremark.com>
Date: Thu, 13 Jun 2013 17:15:45 -0400
MIME-Version: 1.0
References: <1371020901-12782-1-git-send-email-evgeny.budilovsky@ravellosystems.com>
In-Reply-To: <1371020901-12782-1-git-send-email-evgeny.budilovsky@ravellosystems.com>
Content-Type: text/plain; charset="ISO-8859-1"; format=flowed
Content-Transfer-Encoding: 7bit
Subject: Re: [Qemu-devel] [PATCH] allow reading variable size vmdk
 descriptor files
List-Id: <qemu-devel.nongnu.org>
List-Unsubscribe: <https://lists.nongnu.org/mailman/options/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <https://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Evgeny Budilovsky <evgeny.budilovsky@ravellosystems.com>
Cc: qemu-devel@nongnu.org

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>
> ---
>   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