[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-stable] [PATCH] vmdk: Fix creating big description file
From: |
Stefan Hajnoczi |
Subject: |
Re: [Qemu-stable] [PATCH] vmdk: Fix creating big description file |
Date: |
Mon, 2 Dec 2013 14:26:17 +0100 |
User-agent: |
Mutt/1.5.21 (2010-09-15) |
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 <address@hidden>
> ---
> 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);