From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:53204) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0szr-0007H3-M0 for qemu-devel@nongnu.org; Tue, 06 Sep 2011 06:35:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1R0szq-0007iz-50 for qemu-devel@nongnu.org; Tue, 06 Sep 2011 06:35:15 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63731) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1R0szp-0007iv-U2 for qemu-devel@nongnu.org; Tue, 06 Sep 2011 06:35:14 -0400 Message-ID: <4E65F810.9080701@redhat.com> Date: Tue, 06 Sep 2011 12:38:08 +0200 From: Kevin Wolf MIME-Version: 1.0 References: <1313162374-13147-1-git-send-email-famcool@gmail.com> <1313162374-13147-3-git-send-email-famcool@gmail.com> In-Reply-To: <1313162374-13147-3-git-send-email-famcool@gmail.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v3 2/8] VMDK: add twoGbMaxExtentSparse support List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Fam Zheng Cc: stefanha@gmail.com, qemu-devel@nongnu.org, hch@lst.de Am 12.08.2011 17:19, schrieb Fam Zheng: > Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent. > > Signed-off-by: Fam Zheng > --- > block/vmdk.c | 133 ++++++++++++++++++++++++++++++++++++---------------------- > 1 files changed, 83 insertions(+), 50 deletions(-) > > diff --git a/block/vmdk.c b/block/vmdk.c > index e22a893..ab15840 100644 > --- a/block/vmdk.c > +++ b/block/vmdk.c > @@ -174,6 +174,17 @@ static void vmdk_free_extents(BlockDriverState *bs) > qemu_free(s->extents); > } > > +static void vmdk_free_last_extent(BlockDriverState *bs) > +{ > + BDRVVmdkState *s = bs->opaque; > + > + if (s->num_extents == 0) { > + return; > + } > + s->num_extents--; > + s->extents = qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent)); > +} > + > static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent) > { > char desc[DESC_SIZE]; > @@ -357,18 +368,18 @@ static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent) > return ret; > } > > -static int vmdk_open_vmdk3(BlockDriverState *bs, int flags) > +static int vmdk_open_vmdk3(BlockDriverState *bs, > + BlockDriverState *file, > + int flags) > { > int ret; > uint32_t magic; > VMDK3Header header; > - BDRVVmdkState *s = bs->opaque; > VmdkExtent *extent; > > - s->desc_offset = 0x200; > - ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)); > + ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); > if (ret < 0) { > - goto fail; > + return ret; > } > extent = vmdk_add_extent(bs, > bs->file, false, > @@ -378,58 +389,45 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags) > le32_to_cpu(header.granularity)); > ret = vmdk_init_tables(bs, extent); > if (ret) { > - /* vmdk_init_tables cleans up on fail, so only free allocation of > - * vmdk_add_extent here. */ > - goto fail; > + /* free extent allocated by vmdk_add_extent */ > + vmdk_free_last_extent(bs); > } > - return 0; > - fail: > - vmdk_free_extents(bs); > return ret; > } > > -static int vmdk_open_vmdk4(BlockDriverState *bs, int flags) > +static int vmdk_open_vmdk4(BlockDriverState *bs, > + BlockDriverState *file, > + int flags) > { > int ret; > uint32_t magic; > uint32_t l1_size, l1_entry_sectors; > VMDK4Header header; > - BDRVVmdkState *s = bs->opaque; > VmdkExtent *extent; > > - s->desc_offset = 0x200; > - ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)); > + ret = bdrv_pread(file, sizeof(magic), &header, sizeof(header)); > if (ret < 0) { > - goto fail; > + return ret; > } > l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte) > * le64_to_cpu(header.granularity); > + if (l1_entry_sectors <= 0) { > + return -EINVAL; > + } > l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1) > / l1_entry_sectors; > - extent = vmdk_add_extent(bs, bs->file, false, > + extent = vmdk_add_extent(bs, file, false, > le64_to_cpu(header.capacity), > le64_to_cpu(header.gd_offset) << 9, > le64_to_cpu(header.rgd_offset) << 9, > l1_size, > le32_to_cpu(header.num_gtes_per_gte), > le64_to_cpu(header.granularity)); > - if (extent->l1_entry_sectors <= 0) { > - ret = -EINVAL; > - goto fail; > - } > - /* try to open parent images, if exist */ > - ret = vmdk_parent_open(bs); > - if (ret) { > - goto fail; > - } > - s->parent_cid = vmdk_read_cid(bs, 1); > ret = vmdk_init_tables(bs, extent); > if (ret) { > - goto fail; > + /* free extent allocated by vmdk_add_extent */ > + vmdk_free_last_extent(bs); > } > - return 0; > - fail: > - vmdk_free_extents(bs); > return ret; > } > > @@ -460,6 +458,31 @@ static int vmdk_parse_description(const char *desc, const char *opt_name, > return 0; > } > > +/* Open an extent file and append to bs array */ > +static int vmdk_open_sparse(BlockDriverState *bs, > + BlockDriverState *file, > + int flags) > +{ > + uint32_t magic; > + > + if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) { > + return -EIO; > + } > + > + magic = be32_to_cpu(magic); > + switch (magic) { > + case VMDK3_MAGIC: > + return vmdk_open_vmdk3(bs, file, flags); > + break; > + case VMDK4_MAGIC: > + return vmdk_open_vmdk4(bs, file, flags); > + break; > + default: > + return -EINVAL; > + break; > + } > +} > + > static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > const char *desc_file_path) > { > @@ -470,6 +493,8 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > const char *p = desc; > int64_t sectors = 0; > int64_t flat_offset; > + char extent_path[PATH_MAX]; > + BlockDriverState *extent_file; > > while (*p) { > /* parse extent line: > @@ -504,24 +529,29 @@ static int vmdk_parse_extents(const char *desc, BlockDriverState *bs, > goto next_line; > } > > + path_combine(extent_path, sizeof(extent_path), > + desc_file_path, fname); > + ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags); > + if (ret) { > + return ret; > + } > + > /* save to extents array */ > if (!strcmp(type, "FLAT")) { > /* FLAT extent */ > - char extent_path[PATH_MAX]; > - BlockDriverState *extent_file; > VmdkExtent *extent; > > - path_combine(extent_path, sizeof(extent_path), > - desc_file_path, fname); > - ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags); > - if (ret) { > - return ret; > - } > extent = vmdk_add_extent(bs, extent_file, true, sectors, > 0, 0, 0, 0, sectors); > extent->flat_start_offset = flat_offset; > + } else if (!strcmp(type, "SPARSE")) { > + /* SPARSE extent */ > + ret = vmdk_open_sparse(bs, extent_file, bs->open_flags); > + if (ret) { > + bdrv_delete(extent_file); > + return ret; > + } > } else { > - /* SPARSE extent, not supported for now */ > fprintf(stderr, > "VMDK: Not supported extent type \"%s\""".\n", type); > return -ENOTSUP; I think you'll leak extent_file in the error case. I'll merge the patch anyway, please fix on top. Kevin