[Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Fam Zheng]

Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent.

Signed-off-by: Fam Zheng famc...@gmail.com
---
 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 

Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Stefan Hajnoczi]

On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng famc...@gmail.com wrote:
 +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));

vmdk_free_extents() frees extent-l1_table, extent-l2_cache, and
extent-l1_backup_table.  Are they being leaked here?

Stefan

Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Fam Zheng]

On Thu, Aug 4, 2011 at 6:34 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng famc...@gmail.com wrote:
 +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));

 vmdk_free_extents() frees extent-l1_table, extent-l2_cache, and
 extent-l1_backup_table.  Are they being leaked here?

No, it's only called after vmdk_init_tables fails, where no table is
actually allocated to the extent.


-- 
Best regards!
Fam Zheng

Re: [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Stefan Hajnoczi]

On Thu, Aug 4, 2011 at 11:38 AM, Fam Zheng famc...@gmail.com wrote:
 On Thu, Aug 4, 2011 at 6:34 PM, Stefan Hajnoczi stefa...@gmail.com wrote:
 On Thu, Aug 4, 2011 at 4:09 AM, Fam Zheng famc...@gmail.com wrote:
 +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));

 vmdk_free_extents() frees extent-l1_table, extent-l2_cache, and
 extent-l1_backup_table.  Are they being leaked here?

 No, it's only called after vmdk_init_tables fails, where no table is
 actually allocated to the extent.

Okay, thanks for explaining.

Stefan

[Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support

[Fam Zheng]

Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent.

Signed-off-by: Fam Zheng famc...@gmail.com
---
 block/vmdk.c |  132 --
 1 files changed, 82 insertions(+), 50 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index e22a893..be3b860 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