From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:48925) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1VnTW8-0006y2-QG for qemu-devel@nongnu.org; Mon, 02 Dec 2013 08:26:41 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1VnTW2-0006Ba-Oi for qemu-devel@nongnu.org; Mon, 02 Dec 2013 08:26:28 -0500 Date: Mon, 2 Dec 2013 14:26:17 +0100 From: Stefan Hajnoczi Message-ID: <20131202132617.GA28132@stefanha-thinkpad.redhat.com> References: <1385953280-7390-1-git-send-email-famz@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1385953280-7390-1-git-send-email-famz@redhat.com> Subject: Re: [Qemu-devel] [PATCH] vmdk: Fix creating big description file List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: kwolf@redhat.com, qemu-devel@nongnu.org, qemu-stable@nongnu.org On Mon, Dec 02, 2013 at 11:01:20AM +0800, Fam Zheng wrote: > The buffer for description file was 4096 which only covers a few > hundred of extents. This changes the buffer to dynamic allocated with > g_strdup_printf in order to support bigger cases. > > Signed-off-by: Fam Zheng > --- > block/vmdk.c | 65 +- > tests/qemu-iotests/059 | 5 + > tests/qemu-iotests/059.out | 2012 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 2058 insertions(+), 24 deletions(-) Does VMware have a hard limit? For example, does it open the 1000 GB twoGbMaxExtentFlat file from your test case? Minor comments below but feel free to keep the code as-is if you wish: > @@ -1625,7 +1625,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, > "ddb.adapterType = \"%s\"\n"; > > if (filename_decompose(filename, path, prefix, postfix, PATH_MAX, errp)) { > - return -EINVAL; > + ret = -EINVAL; > + goto exit; > } > /* Read out options */ > while (options && options->name) { > @@ -1651,7 +1652,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, > strcmp(adapter_type, "lsilogic") && > strcmp(adapter_type, "legacyESX")) { > error_setg(errp, "Unknown adapter type: '%s'", adapter_type); > - return -EINVAL; > + ret = -EINVAL; > + goto exit; > } > if (strcmp(adapter_type, "ide") != 0) { > /* that's the number of heads with which vmware operates when > @@ -1667,7 +1669,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, > strcmp(fmt, "twoGbMaxExtentFlat") && > strcmp(fmt, "streamOptimized")) { > error_setg(errp, "Unknown subformat: '%s'", fmt); > - return -EINVAL; > + ret = -EINVAL; > + goto exit; > } > split = !(strcmp(fmt, "twoGbMaxExtentFlat") && > strcmp(fmt, "twoGbMaxExtentSparse")); > @@ -1681,22 +1684,25 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, > } > if (flat && backing_file) { > error_setg(errp, "Flat image can't have backing file"); > - return -ENOTSUP; > + ret = -ENOTSUP; > + goto exit; > } > if (flat && zeroed_grain) { > error_setg(errp, "Flat image can't enable zeroed grain"); > - return -ENOTSUP; > + ret = -ENOTSUP; > + goto exit; > } > if (backing_file) { > BlockDriverState *bs = bdrv_new(""); > ret = bdrv_open(bs, backing_file, NULL, 0, NULL, errp); > if (ret != 0) { > bdrv_unref(bs); > - return ret; > + goto exit; > } > if (strcmp(bs->drv->format_name, "vmdk")) { > bdrv_unref(bs); > - return -EINVAL; > + ret = -EINVAL; > + goto exit; > } > parent_cid = vmdk_read_cid(bs, 0); > bdrv_unref(bs); Changing these to goto isn't really necessary. > @@ -1730,25 +1737,31 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options, > > if (vmdk_create_extent(ext_filename, size, > flat, compress, zeroed_grain)) { > - return -EINVAL; > + ret = -EINVAL; > + goto exit; > } > filesize -= size; > > /* Format description line */ > snprintf(desc_line, sizeof(desc_line), > desc_extent_line, size / 512, desc_filename); > - pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line); > + new_desc_lines = g_strdup_printf("%s%s", > + ext_desc_lines ? : "", > + desc_line); > + g_free(ext_desc_lines); > + ext_desc_lines = new_desc_lines; These lines can be eliminated if you use GString: /* Format description line */ g_string_append_printf(ext_desc_lines, desc_extent_line, size / 512, desc_filename);