* [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support
@ 2011-07-27 9:27 Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
` (6 more replies)
0 siblings, 7 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Add subformats support for:
twoGbMaxExtentFlat
twoGbMaxExtentSparse
streamOptimized
Fam Zheng (6):
VMDK: enable twoGbMaxExtentFlat
VMDK: add twoGbMaxExtentSparse support
VMDK: separate vmdk_read_extent/vmdk_write_extent
VMDK: Opening compressed extent.
VMDK: read/write compressed extent
VMDK: creating streamOptimized subformat
block/vmdk.c | 316 +++++++++++++++++++++++++++++++++++++++++++++-------------
1 files changed, 248 insertions(+), 68 deletions(-)
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-07-29 1:42 ` Fam Zheng
2011-07-30 4:43 ` Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
` (5 subsequent siblings)
6 siblings, 2 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Enable the createType 'twoGbMaxExtentFlat'. The supporting code is
already in.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 37478d2..9e6c67a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -551,7 +551,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
return -EINVAL;
}
- if (strcmp(ct, "monolithicFlat")) {
+ if (strcmp(ct, "monolithicFlat") &&
+ strcmp(ct, "twoGbMaxExtentFlat")) {
fprintf(stderr,
"VMDK: Not supported image type \"%s\""".\n", ct);
return -ENOTSUP;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-08-02 10:32 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 3/6] VMDK: separate vmdk_read_extent/vmdk_write_extent Fam Zheng
` (4 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Add twoGbMaxExtentSparse support. Only opening code is changed.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 124 ++++++++++++++++++++++++++++++++++++----------------------
1 files changed, 77 insertions(+), 47 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 9e6c67a..9d1ae32 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -357,7 +357,9 @@ 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;
@@ -365,10 +367,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
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,17 +379,16 @@ 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 */
+ s->num_extents--;
+ qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
}
- 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;
@@ -397,39 +397,30 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
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 */
+ s->num_extents--;
+ qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
}
- return 0;
- fail:
- vmdk_free_extents(bs);
return ret;
}
@@ -460,6 +451,35 @@ 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)
+{
+ int ret;
+ uint32_t magic;
+
+ if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+ return -EIO;
+ }
+
+ magic = be32_to_cpu(magic);
+ if (magic == VMDK3_MAGIC) {
+ ret = vmdk_open_vmdk3(bs, file, flags);
+ if (ret) {
+ return ret;
+ }
+ } else if (magic == VMDK4_MAGIC) {
+ ret = vmdk_open_vmdk4(bs, file, flags);
+ if (ret) {
+ return ret;
+ }
+ } else {
+ return -EINVAL;
+ }
+ return 0;
+}
+
static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
const char *desc_file_path)
{
@@ -470,6 +490,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,22 +526,28 @@ 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,
@@ -552,6 +580,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
return -EINVAL;
}
if (strcmp(ct, "monolithicFlat") &&
+ strcmp(ct, "twoGbMaxExtentSparse") &&
strcmp(ct, "twoGbMaxExtentFlat")) {
fprintf(stderr,
"VMDK: Not supported image type \"%s\""".\n", ct);
@@ -574,17 +603,18 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
static int vmdk_open(BlockDriverState *bs, int flags)
{
- uint32_t magic;
-
- if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
- return -EIO;
- }
+ int ret;
+ BDRVVmdkState *s = bs->opaque;
- magic = be32_to_cpu(magic);
- if (magic == VMDK3_MAGIC) {
- return vmdk_open_vmdk3(bs, flags);
- } else if (magic == VMDK4_MAGIC) {
- return vmdk_open_vmdk4(bs, flags);
+ if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
+ s->desc_offset = 0x200;
+ /* try to open parent images, if exist */
+ ret = vmdk_parent_open(bs);
+ if (ret) {
+ return ret;
+ }
+ s->parent_cid = vmdk_read_cid(bs, 1);
+ return 0;
} else {
return vmdk_open_desc_file(bs, flags);
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 3/6] VMDK: separate vmdk_read_extent/vmdk_write_extent
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent Fam Zheng
` (3 subsequent siblings)
6 siblings, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Factor out read/write extent code, since there will be more things to
take care of once reading/writing compressed clusters is introduced.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
1 files changed, 45 insertions(+), 9 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 9d1ae32..0d989f6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -829,6 +829,43 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
return ret;
}
+static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
+ int64_t offset_in_cluster, const uint8_t *buf,
+ int nb_sectors, int64_t sector_num)
+{
+ int ret;
+ const uint8_t *write_buf = buf;
+ int write_len = nb_sectors * 512;
+
+ ret = bdrv_pwrite(extent->file,
+ cluster_offset + offset_in_cluster,
+ write_buf,
+ write_len);
+ if (ret != write_len) {
+ ret = ret < 0 ? ret : -EIO;
+ goto out;
+ }
+ ret = 0;
+ out:
+ return ret;
+}
+
+static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
+ int64_t offset_in_cluster, uint8_t *buf,
+ int nb_sectors)
+{
+ int ret;
+
+ ret = bdrv_pread(extent->file,
+ cluster_offset + offset_in_cluster,
+ buf, nb_sectors * 512);
+ if (ret == nb_sectors * 512) {
+ return 0;
+ } else {
+ return -EIO;
+ }
+}
+
static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
@@ -865,10 +902,10 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
memset(buf, 0, 512 * n);
}
} else {
- ret = bdrv_pread(extent->file,
- cluster_offset + index_in_cluster * 512,
- buf, n * 512);
- if (ret < 0) {
+ ret = vmdk_read_extent(extent,
+ cluster_offset, index_in_cluster * 512,
+ buf, n);
+ if (ret) {
return ret;
}
}
@@ -917,11 +954,10 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
n = nb_sectors;
}
- ret = bdrv_pwrite(extent->file,
- cluster_offset + index_in_cluster * 512,
- buf,
- n * 512);
- if (ret < 0) {
+ ret = vmdk_write_extent(extent,
+ cluster_offset, index_in_cluster * 512,
+ buf, n, sector_num);
+ if (ret) {
return ret;
}
if (m_data.valid) {
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent.
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
` (2 preceding siblings ...)
2011-07-27 9:27 ` [Qemu-devel] [PATCH 3/6] VMDK: separate vmdk_read_extent/vmdk_write_extent Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-08-02 10:59 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 5/6] VMDK: read/write " Fam Zheng
` (2 subsequent siblings)
6 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Added flags field for compressed/streamOptimized extents, open and save
image configuration.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 16 ++++++++++++++++
1 files changed, 16 insertions(+), 0 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 0d989f6..5f1638e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -26,9 +26,13 @@
#include "qemu-common.h"
#include "block_int.h"
#include "module.h"
+#include "zlib.h"
#define VMDK3_MAGIC (('C' << 24) | ('O' << 16) | ('W' << 8) | 'D')
#define VMDK4_MAGIC (('K' << 24) | ('D' << 16) | ('M' << 8) | 'V')
+#define VMDK4_COMPRESSION_DEFLATE 1
+#define VMDK4_FLAG_COMPRESS (1 << 16)
+#define VMDK4_FLAG_MARKER (1 << 17)
typedef struct {
uint32_t version;
@@ -56,6 +60,7 @@ typedef struct {
int64_t grain_offset;
char filler[1];
char check_bytes[4];
+ uint16_t compressAlgorithm;
} __attribute__((packed)) VMDK4Header;
#define L2_CACHE_SIZE 16
@@ -63,6 +68,8 @@ typedef struct {
typedef struct VmdkExtent {
BlockDriverState *file;
bool flat;
+ bool compressed;
+ bool has_marker;
int64_t sectors;
int64_t end_sector;
int64_t flat_start_offset;
@@ -98,6 +105,12 @@ typedef struct VmdkMetaData {
int valid;
} VmdkMetaData;
+typedef struct VmdkGrainMarker {
+ uint64_t lba;
+ uint32_t size;
+ uint8_t data[0];
+} VmdkGrainMarker;
+
static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
@@ -415,6 +428,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
l1_size,
le32_to_cpu(header.num_gtes_per_gte),
le64_to_cpu(header.granularity));
+ extent->compressed =
+ le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE;
+ extent->has_marker = header.flags & VMDK4_FLAG_MARKER;
ret = vmdk_init_tables(bs, extent);
if (ret) {
/* free extent allocated by vmdk_add_extent */
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 5/6] VMDK: read/write compressed extent
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
` (3 preceding siblings ...)
2011-07-27 9:27 ` [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-08-02 11:11 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 6/6] VMDK: creating streamOptimized subformat Fam Zheng
2011-08-06 4:10 ` [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
6 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Add support for reading/writing compressed extent.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 115 ++++++++++++++++++++++++++++++++++++++++++++++++++++------
1 files changed, 103 insertions(+), 12 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 5f1638e..4799aa5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -771,10 +771,12 @@ static int get_cluster_offset(BlockDriverState *bs,
/* Avoid the L2 tables update for the images that have snapshots. */
*cluster_offset = bdrv_getlength(extent->file);
- bdrv_truncate(
- extent->file,
- *cluster_offset + (extent->cluster_sectors << 9)
- );
+ if (!extent->compressed) {
+ bdrv_truncate(
+ extent->file,
+ *cluster_offset + (extent->cluster_sectors << 9)
+ );
+ }
*cluster_offset >>= 9;
tmp = cpu_to_le32(*cluster_offset);
@@ -850,9 +852,28 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
int nb_sectors, int64_t sector_num)
{
int ret;
+ VmdkGrainMarker *data = NULL;
+ uLongf buf_len;
const uint8_t *write_buf = buf;
int write_len = nb_sectors * 512;
+ if (extent->compressed) {
+ if (!extent->has_marker) {
+ ret = -EINVAL;
+ goto out;
+ }
+ buf_len = (extent->cluster_sectors << 9) * 2;
+ data = qemu_mallocz(buf_len + sizeof(VmdkGrainMarker));
+ if (compress(data->data, &buf_len, buf, nb_sectors << 9) != Z_OK ||
+ buf_len == 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+ data->lba = sector_num;
+ data->size = buf_len;
+ write_buf = (uint8_t *)data;
+ write_len = buf_len + sizeof(VmdkGrainMarker);
+ }
ret = bdrv_pwrite(extent->file,
cluster_offset + offset_in_cluster,
write_buf,
@@ -863,6 +884,9 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
}
ret = 0;
out:
+ if (data) {
+ qemu_free(data);
+ }
return ret;
}
@@ -871,15 +895,65 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
int nb_sectors)
{
int ret;
-
+ int cluster_bytes, buf_bytes;
+ uint8_t *cluster_buf, *compressed_data;
+ uint8_t *uncomp_buf;
+ uint32_t data_len;
+ VmdkGrainMarker *marker;
+ uLongf buf_len;
+
+
+ if (!extent->compressed) {
+ ret = bdrv_pread(extent->file,
+ cluster_offset + offset_in_cluster,
+ buf, nb_sectors * 512);
+ if (ret == nb_sectors * 512) {
+ return 0;
+ } else {
+ return -EIO;
+ }
+ }
+ cluster_bytes = extent->cluster_sectors * 512;
+ /* Read two clusters in case GrainMarker + compressed data > one cluster */
+ buf_bytes = cluster_bytes * 2;
+ cluster_buf = qemu_mallocz(buf_bytes);
+ uncomp_buf = qemu_mallocz(cluster_bytes);
ret = bdrv_pread(extent->file,
- cluster_offset + offset_in_cluster,
- buf, nb_sectors * 512);
- if (ret == nb_sectors * 512) {
- return 0;
- } else {
- return -EIO;
+ cluster_offset,
+ cluster_buf, buf_bytes);
+ if (ret < 0) {
+ goto out;
+ }
+ compressed_data = cluster_buf;
+ buf_len = cluster_bytes;
+ data_len = cluster_bytes;
+ if (extent->has_marker) {
+ marker = (VmdkGrainMarker *)cluster_buf;
+ compressed_data = marker->data;
+ data_len = marker->size;
+ }
+ if (!data_len || data_len > buf_bytes) {
+ ret = -EINVAL;
+ goto out;
+ }
+ ret = uncompress(uncomp_buf, &buf_len, compressed_data, data_len);
+ if (ret != Z_OK) {
+ ret = -EINVAL;
+ goto out;
+
+ }
+ if (offset_in_cluster < 0 ||
+ offset_in_cluster + nb_sectors * 512 > buf_len) {
+ ret = -EINVAL;
+ goto out;
}
+ memcpy(buf, uncomp_buf + offset_in_cluster, nb_sectors * 512);
+ ret = 0;
+
+ out:
+ qemu_free(uncomp_buf);
+ qemu_free(cluster_buf);
+ return ret;
}
static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
@@ -959,8 +1033,25 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
bs,
extent,
&m_data,
- sector_num << 9, 1,
+ sector_num << 9, !extent->compressed,
&cluster_offset);
+ if (extent->compressed) {
+ if (ret == 0) {
+ /* Refuse write to allocated cluster for streamOptimized */
+ fprintf(stderr,
+ "VMDK: can't write to allocated cluster"
+ " for streamOptimized\n");
+ return -EIO;
+ } else {
+ /* allocate */
+ ret = get_cluster_offset(
+ bs,
+ extent,
+ &m_data,
+ sector_num << 9, 1,
+ &cluster_offset);
+ }
+ }
if (ret) {
return -EINVAL;
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH 6/6] VMDK: creating streamOptimized subformat
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
` (4 preceding siblings ...)
2011-07-27 9:27 ` [Qemu-devel] [PATCH 5/6] VMDK: read/write " Fam Zheng
@ 2011-07-27 9:27 ` Fam Zheng
2011-08-06 4:10 ` [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
6 siblings, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-27 9:27 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Creating streamOptimized subformat. Added subformat option
'streamOptimized', to create a image with compression enabled and each
cluster with a GrainMarker.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 18 ++++++++++++------
1 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 4799aa5..52e518d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1088,7 +1088,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
}
-static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
+static int vmdk_create_extent(const char *filename, int64_t filesize,
+ bool flat, bool compress)
{
int ret, i;
int fd = 0;
@@ -1112,7 +1113,9 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
magic = cpu_to_be32(VMDK4_MAGIC);
memset(&header, 0, sizeof(header));
header.version = 1;
- header.flags = 3; /* ?? */
+ header.flags =
+ 3 | (compress ? VMDK4_FLAG_COMPRESS | VMDK4_FLAG_MARKER : 0);
+ header.compressAlgorithm = compress ? VMDK4_COMPRESSION_DEFLATE : 0;
header.capacity = filesize / 512;
header.granularity = 128;
header.num_gtes_per_gte = 512;
@@ -1142,6 +1145,7 @@ static int vmdk_create_extent(const char *filename, int64_t filesize, bool flat)
header.rgd_offset = cpu_to_le64(header.rgd_offset);
header.gd_offset = cpu_to_le64(header.gd_offset);
header.grain_offset = cpu_to_le64(header.grain_offset);
+ header.compressAlgorithm = cpu_to_le16(header.compressAlgorithm);
header.check_bytes[0] = 0xa;
header.check_bytes[1] = 0x20;
@@ -1283,7 +1287,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
const char *fmt = NULL;
int flags = 0;
int ret = 0;
- bool flat, split;
+ bool flat, split, compress;
char ext_desc_lines[BUF_SIZE] = "";
char path[PATH_MAX], prefix[PATH_MAX], postfix[PATH_MAX];
const int64_t split_size = 0x80000000; /* VMDK has constant split size */
@@ -1332,7 +1336,8 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
} else if (strcmp(fmt, "monolithicFlat") &&
strcmp(fmt, "monolithicSparse") &&
strcmp(fmt, "twoGbMaxExtentSparse") &&
- strcmp(fmt, "twoGbMaxExtentFlat")) {
+ strcmp(fmt, "twoGbMaxExtentFlat") &&
+ strcmp(fmt, "streamOptimized")) {
fprintf(stderr, "VMDK: Unknown subformat: %s\n", fmt);
return -EINVAL;
}
@@ -1340,6 +1345,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
strcmp(fmt, "twoGbMaxExtentSparse"));
flat = !(strcmp(fmt, "monolithicFlat") &&
strcmp(fmt, "twoGbMaxExtentFlat"));
+ compress = !strcmp(fmt, "streamOptimized");
if (flat) {
desc_extent_line = "RW %lld FLAT \"%s\" 0\n";
} else {
@@ -1394,7 +1400,7 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
snprintf(ext_filename, sizeof(ext_filename), "%s%s",
path, desc_filename);
- if (vmdk_create_extent(ext_filename, size, flat)) {
+ if (vmdk_create_extent(ext_filename, size, flat, compress)) {
return -EINVAL;
}
filesize -= size;
@@ -1508,7 +1514,7 @@ static QEMUOptionParameter vmdk_create_options[] = {
.type = OPT_STRING,
.help =
"VMDK flat extent format, can be one of "
- "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat} "
+ "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat | streamOptimized} "
},
{ NULL }
};
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
@ 2011-07-29 1:42 ` Fam Zheng
2011-07-30 4:43 ` Fam Zheng
1 sibling, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-29 1:42 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Sorry, I missed one line in this patch. Will fix it in V2.
On Wed, Jul 27, 2011 at 5:27 PM, Fam Zheng <famcool@gmail.com> wrote:
> Enable the createType 'twoGbMaxExtentFlat'. The supporting code is
> already in.
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 3 ++-
> 1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 37478d2..9e6c67a 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -551,7 +551,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
> if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
> return -EINVAL;
> }
> - if (strcmp(ct, "monolithicFlat")) {
> + if (strcmp(ct, "monolithicFlat") &&
> + strcmp(ct, "twoGbMaxExtentFlat")) {
> fprintf(stderr,
> "VMDK: Not supported image type \"%s\""".\n", ct);
> return -ENOTSUP;
>
--
Best regards!
Fam Zheng
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
2011-07-29 1:42 ` Fam Zheng
@ 2011-07-30 4:43 ` Fam Zheng
1 sibling, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2011-07-30 4:43 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Enable the createType 'twoGbMaxExtentFlat'. The supporting code is
already in.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 4 +++-
1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 37478d2..e22a893 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -551,7 +551,8 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
return -EINVAL;
}
- if (strcmp(ct, "monolithicFlat")) {
+ if (strcmp(ct, "monolithicFlat") &&
+ strcmp(ct, "twoGbMaxExtentFlat")) {
fprintf(stderr,
"VMDK: Not supported image type \"%s\""".\n", ct);
return -ENOTSUP;
@@ -672,6 +673,7 @@ static int get_cluster_offset(BlockDriverState *bs,
return 0;
}
+ offset -= (extent->end_sector - extent->sectors) * SECTOR_SIZE;
l1_index = (offset >> 9) / extent->l1_entry_sectors;
if (l1_index >= extent->l1_size) {
return -1;
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support
2011-07-27 9:27 ` [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
@ 2011-08-02 10:32 ` Stefan Hajnoczi
2011-08-04 2:35 ` Fam Zheng
0 siblings, 1 reply; 15+ messages in thread
From: Stefan Hajnoczi @ 2011-08-02 10:32 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Wed, Jul 27, 2011 at 10:27 AM, Fam Zheng <famcool@gmail.com> wrote:
> Add twoGbMaxExtentSparse support. Only opening code is changed.
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 124 ++++++++++++++++++++++++++++++++++++----------------------
> 1 files changed, 77 insertions(+), 47 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 9e6c67a..9d1ae32 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -357,7 +357,9 @@ 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;
> @@ -365,10 +367,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
> 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,17 +379,16 @@ 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 */
> + s->num_extents--;
> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
The return value of qemu_realloc() is the new address of s->extents[].
You are ignoring the new address and continue to use the old one.
Please add a vmdk_free_last_extent() function that encapsulates this.
The rest of the vmdk.c code should not manipulate
s->extents[]/s->num_extents. Use vmdk_add_extent(),
vmdk_free_last_extent(), vmdk_free_extents() instead of spreading the
extent management code in various places.
> }
> - 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;
> @@ -397,39 +397,30 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
> 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 */
> + s->num_extents--;
> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
Same bug.
> }
> - return 0;
> - fail:
> - vmdk_free_extents(bs);
> return ret;
> }
>
> @@ -460,6 +451,35 @@ 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)
> +{
> + int ret;
> + uint32_t magic;
> +
> + if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
> + return -EIO;
> + }
> +
> + magic = be32_to_cpu(magic);
> + if (magic == VMDK3_MAGIC) {
> + ret = vmdk_open_vmdk3(bs, file, flags);
> + if (ret) {
> + return ret;
> + }
> + } else if (magic == VMDK4_MAGIC) {
> + ret = vmdk_open_vmdk4(bs, file, flags);
> + if (ret) {
> + return ret;
> + }
> + } else {
> + return -EINVAL;
> + }
> + return 0;
Shorter way of writing this (ret is no longer necessary):
switch (magic) {
case VMDK3_MAGIC:
return vmdk_open_vmdk3(bs, file, flags);
case VMDK4_MAGIC:
return vmdk_open_vmdk4(bs, file, flags);
default:
return -EINVAL;
}
> +}
> +
> static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
> const char *desc_file_path)
> {
> @@ -470,6 +490,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,22 +526,28 @@ 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 */
This comment is outdated.
> fprintf(stderr,
> @@ -552,6 +580,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
> return -EINVAL;
> }
> if (strcmp(ct, "monolithicFlat") &&
> + strcmp(ct, "twoGbMaxExtentSparse") &&
> strcmp(ct, "twoGbMaxExtentFlat")) {
> fprintf(stderr,
> "VMDK: Not supported image type \"%s\""".\n", ct);
> @@ -574,17 +603,18 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>
> static int vmdk_open(BlockDriverState *bs, int flags)
> {
> - uint32_t magic;
> -
> - if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
> - return -EIO;
> - }
> + int ret;
> + BDRVVmdkState *s = bs->opaque;
>
> - magic = be32_to_cpu(magic);
> - if (magic == VMDK3_MAGIC) {
> - return vmdk_open_vmdk3(bs, flags);
> - } else if (magic == VMDK4_MAGIC) {
> - return vmdk_open_vmdk4(bs, flags);
> + if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
> + s->desc_offset = 0x200;
> + /* try to open parent images, if exist */
> + ret = vmdk_parent_open(bs);
> + if (ret) {
> + return ret;
What needs to be freed here?
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent.
2011-07-27 9:27 ` [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent Fam Zheng
@ 2011-08-02 10:59 ` Stefan Hajnoczi
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2011-08-02 10:59 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Wed, Jul 27, 2011 at 10:27 AM, Fam Zheng <famcool@gmail.com> wrote:
> @@ -415,6 +428,9 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
> l1_size,
> le32_to_cpu(header.num_gtes_per_gte),
> le64_to_cpu(header.granularity));
> + extent->compressed =
> + le16_to_cpu(header.compressAlgorithm) == VMDK4_COMPRESSION_DEFLATE;
> + extent->has_marker = header.flags & VMDK4_FLAG_MARKER;
le32_to_cpu(header.flags)?
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 5/6] VMDK: read/write compressed extent
2011-07-27 9:27 ` [Qemu-devel] [PATCH 5/6] VMDK: read/write " Fam Zheng
@ 2011-08-02 11:11 ` Stefan Hajnoczi
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2011-08-02 11:11 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Wed, Jul 27, 2011 at 10:27 AM, Fam Zheng <famcool@gmail.com> wrote:
> @@ -863,6 +884,9 @@ static int vmdk_write_extent(VmdkExtent *extent, int64_t cluster_offset,
> }
> ret = 0;
> out:
> + if (data) {
> + qemu_free(data);
> + }
qemu_free(NULL) is a nop, you don't need the if statement. Just call
qemu_free(data).
> return ret;
> }
>
> @@ -871,15 +895,65 @@ static int vmdk_read_extent(VmdkExtent *extent, int64_t cluster_offset,
> int nb_sectors)
> {
> int ret;
> -
> + int cluster_bytes, buf_bytes;
> + uint8_t *cluster_buf, *compressed_data;
> + uint8_t *uncomp_buf;
> + uint32_t data_len;
> + VmdkGrainMarker *marker;
> + uLongf buf_len;
> +
> +
> + if (!extent->compressed) {
> + ret = bdrv_pread(extent->file,
> + cluster_offset + offset_in_cluster,
> + buf, nb_sectors * 512);
> + if (ret == nb_sectors * 512) {
> + return 0;
> + } else {
> + return -EIO;
> + }
> + }
> + cluster_bytes = extent->cluster_sectors * 512;
> + /* Read two clusters in case GrainMarker + compressed data > one cluster */
> + buf_bytes = cluster_bytes * 2;
> + cluster_buf = qemu_mallocz(buf_bytes);
> + uncomp_buf = qemu_mallocz(cluster_bytes);
Do these really need to be zeroed?
> ret = bdrv_pread(extent->file,
> - cluster_offset + offset_in_cluster,
> - buf, nb_sectors * 512);
> - if (ret == nb_sectors * 512) {
> - return 0;
> - } else {
> - return -EIO;
> + cluster_offset,
> + cluster_buf, buf_bytes);
> + if (ret < 0) {
> + goto out;
> + }
> + compressed_data = cluster_buf;
> + buf_len = cluster_bytes;
> + data_len = cluster_bytes;
> + if (extent->has_marker) {
> + marker = (VmdkGrainMarker *)cluster_buf;
> + compressed_data = marker->data;
> + data_len = marker->size;
Endianness?
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support
2011-08-02 10:32 ` Stefan Hajnoczi
@ 2011-08-04 2:35 ` Fam Zheng
2011-08-04 10:36 ` Stefan Hajnoczi
0 siblings, 1 reply; 15+ messages in thread
From: Fam Zheng @ 2011-08-04 2:35 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, qemu-devel, hch
On Tue, Aug 2, 2011 at 6:32 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Wed, Jul 27, 2011 at 10:27 AM, Fam Zheng <famcool@gmail.com> wrote:
>> Add twoGbMaxExtentSparse support. Only opening code is changed.
>>
>> Signed-off-by: Fam Zheng <famcool@gmail.com>
>> ---
>> block/vmdk.c | 124 ++++++++++++++++++++++++++++++++++++----------------------
>> 1 files changed, 77 insertions(+), 47 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index 9e6c67a..9d1ae32 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -357,7 +357,9 @@ 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;
>> @@ -365,10 +367,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
>> 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,17 +379,16 @@ 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 */
>> + s->num_extents--;
>> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
>
> The return value of qemu_realloc() is the new address of s->extents[].
> You are ignoring the new address and continue to use the old one.
>
> Please add a vmdk_free_last_extent() function that encapsulates this.
> The rest of the vmdk.c code should not manipulate
> s->extents[]/s->num_extents. Use vmdk_add_extent(),
> vmdk_free_last_extent(), vmdk_free_extents() instead of spreading the
> extent management code in various places.
>
>> }
>> - 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;
>> @@ -397,39 +397,30 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
>> 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 */
>> + s->num_extents--;
>> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
>
> Same bug.
>
>> }
>> - return 0;
>> - fail:
>> - vmdk_free_extents(bs);
>> return ret;
>> }
>>
>> @@ -460,6 +451,35 @@ 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)
>> +{
>> + int ret;
>> + uint32_t magic;
>> +
>> + if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
>> + return -EIO;
>> + }
>> +
>> + magic = be32_to_cpu(magic);
>> + if (magic == VMDK3_MAGIC) {
>> + ret = vmdk_open_vmdk3(bs, file, flags);
>> + if (ret) {
>> + return ret;
>> + }
>> + } else if (magic == VMDK4_MAGIC) {
>> + ret = vmdk_open_vmdk4(bs, file, flags);
>> + if (ret) {
>> + return ret;
>> + }
>> + } else {
>> + return -EINVAL;
>> + }
>> + return 0;
>
> Shorter way of writing this (ret is no longer necessary):
>
> switch (magic) {
> case VMDK3_MAGIC:
> return vmdk_open_vmdk3(bs, file, flags);
> case VMDK4_MAGIC:
> return vmdk_open_vmdk4(bs, file, flags);
> default:
> return -EINVAL;
> }
>
>> +}
>> +
>> static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>> const char *desc_file_path)
>> {
>> @@ -470,6 +490,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,22 +526,28 @@ 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 */
>
> This comment is outdated.
>
>> fprintf(stderr,
>> @@ -552,6 +580,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>> return -EINVAL;
>> }
>> if (strcmp(ct, "monolithicFlat") &&
>> + strcmp(ct, "twoGbMaxExtentSparse") &&
>> strcmp(ct, "twoGbMaxExtentFlat")) {
>> fprintf(stderr,
>> "VMDK: Not supported image type \"%s\""".\n", ct);
>> @@ -574,17 +603,18 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>>
>> static int vmdk_open(BlockDriverState *bs, int flags)
>> {
>> - uint32_t magic;
>> -
>> - if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
>> - return -EIO;
>> - }
>> + int ret;
>> + BDRVVmdkState *s = bs->opaque;
>>
>> - magic = be32_to_cpu(magic);
>> - if (magic == VMDK3_MAGIC) {
>> - return vmdk_open_vmdk3(bs, flags);
>> - } else if (magic == VMDK4_MAGIC) {
>> - return vmdk_open_vmdk4(bs, flags);
>> + if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
>> + s->desc_offset = 0x200;
>> + /* try to open parent images, if exist */
>> + ret = vmdk_parent_open(bs);
>> + if (ret) {
>> + return ret;
>
> What needs to be freed here?
>
No resource allocation in vmdk_parent_open, so I think none.
--
Best regards!
Fam Zheng
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support
2011-08-04 2:35 ` Fam Zheng
@ 2011-08-04 10:36 ` Stefan Hajnoczi
0 siblings, 0 replies; 15+ messages in thread
From: Stefan Hajnoczi @ 2011-08-04 10:36 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Thu, Aug 4, 2011 at 3:35 AM, Fam Zheng <famcool@gmail.com> wrote:
> On Tue, Aug 2, 2011 at 6:32 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Wed, Jul 27, 2011 at 10:27 AM, Fam Zheng <famcool@gmail.com> wrote:
>>> Add twoGbMaxExtentSparse support. Only opening code is changed.
>>>
>>> Signed-off-by: Fam Zheng <famcool@gmail.com>
>>> ---
>>> block/vmdk.c | 124 ++++++++++++++++++++++++++++++++++++----------------------
>>> 1 files changed, 77 insertions(+), 47 deletions(-)
>>>
>>> diff --git a/block/vmdk.c b/block/vmdk.c
>>> index 9e6c67a..9d1ae32 100644
>>> --- a/block/vmdk.c
>>> +++ b/block/vmdk.c
>>> @@ -357,7 +357,9 @@ 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;
>>> @@ -365,10 +367,9 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
>>> 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,17 +379,16 @@ 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 */
>>> + s->num_extents--;
>>> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
>>
>> The return value of qemu_realloc() is the new address of s->extents[].
>> You are ignoring the new address and continue to use the old one.
>>
>> Please add a vmdk_free_last_extent() function that encapsulates this.
>> The rest of the vmdk.c code should not manipulate
>> s->extents[]/s->num_extents. Use vmdk_add_extent(),
>> vmdk_free_last_extent(), vmdk_free_extents() instead of spreading the
>> extent management code in various places.
>>
>>> }
>>> - 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;
>>> @@ -397,39 +397,30 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
>>> 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 */
>>> + s->num_extents--;
>>> + qemu_realloc(s->extents, s->num_extents * sizeof(VmdkExtent));
>>
>> Same bug.
>>
>>> }
>>> - return 0;
>>> - fail:
>>> - vmdk_free_extents(bs);
>>> return ret;
>>> }
>>>
>>> @@ -460,6 +451,35 @@ 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)
>>> +{
>>> + int ret;
>>> + uint32_t magic;
>>> +
>>> + if (bdrv_pread(file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
>>> + return -EIO;
>>> + }
>>> +
>>> + magic = be32_to_cpu(magic);
>>> + if (magic == VMDK3_MAGIC) {
>>> + ret = vmdk_open_vmdk3(bs, file, flags);
>>> + if (ret) {
>>> + return ret;
>>> + }
>>> + } else if (magic == VMDK4_MAGIC) {
>>> + ret = vmdk_open_vmdk4(bs, file, flags);
>>> + if (ret) {
>>> + return ret;
>>> + }
>>> + } else {
>>> + return -EINVAL;
>>> + }
>>> + return 0;
>>
>> Shorter way of writing this (ret is no longer necessary):
>>
>> switch (magic) {
>> case VMDK3_MAGIC:
>> return vmdk_open_vmdk3(bs, file, flags);
>> case VMDK4_MAGIC:
>> return vmdk_open_vmdk4(bs, file, flags);
>> default:
>> return -EINVAL;
>> }
>>
>>> +}
>>> +
>>> static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
>>> const char *desc_file_path)
>>> {
>>> @@ -470,6 +490,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,22 +526,28 @@ 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 */
>>
>> This comment is outdated.
>>
>>> fprintf(stderr,
>>> @@ -552,6 +580,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>>> return -EINVAL;
>>> }
>>> if (strcmp(ct, "monolithicFlat") &&
>>> + strcmp(ct, "twoGbMaxExtentSparse") &&
>>> strcmp(ct, "twoGbMaxExtentFlat")) {
>>> fprintf(stderr,
>>> "VMDK: Not supported image type \"%s\""".\n", ct);
>>> @@ -574,17 +603,18 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>>>
>>> static int vmdk_open(BlockDriverState *bs, int flags)
>>> {
>>> - uint32_t magic;
>>> -
>>> - if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
>>> - return -EIO;
>>> - }
>>> + int ret;
>>> + BDRVVmdkState *s = bs->opaque;
>>>
>>> - magic = be32_to_cpu(magic);
>>> - if (magic == VMDK3_MAGIC) {
>>> - return vmdk_open_vmdk3(bs, flags);
>>> - } else if (magic == VMDK4_MAGIC) {
>>> - return vmdk_open_vmdk4(bs, flags);
>>> + if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
>>> + s->desc_offset = 0x200;
>>> + /* try to open parent images, if exist */
>>> + ret = vmdk_parent_open(bs);
>>> + if (ret) {
>>> + return ret;
>>
>> What needs to be freed here?
>>
>
> No resource allocation in vmdk_parent_open, so I think none.
But what about vmdk_open_sparse(), which succeeded? This function is
a .bdrv_open(). If you return an error then bs->file will be deleted
and bs->opaque is freed - but you need to clean up all image
format-specific resources *before* returning error. bdrv_close() will
not be called if bdrv_open() returns error.
Stefan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
` (5 preceding siblings ...)
2011-07-27 9:27 ` [Qemu-devel] [PATCH 6/6] VMDK: creating streamOptimized subformat Fam Zheng
@ 2011-08-06 4:10 ` Fam Zheng
6 siblings, 0 replies; 15+ messages in thread
From: Fam Zheng @ 2011-08-06 4:10 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Add twoGbMaxExtentSparse support. Introduce vmdk_free_last_extent.
Signed-off-by: Fam Zheng <famcool@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 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;
@@ -552,6 +582,7 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
return -EINVAL;
}
if (strcmp(ct, "monolithicFlat") &&
+ strcmp(ct, "twoGbMaxExtentSparse") &&
strcmp(ct, "twoGbMaxExtentFlat")) {
fprintf(stderr,
"VMDK: Not supported image type \"%s\""".\n", ct);
@@ -574,17 +605,19 @@ static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
static int vmdk_open(BlockDriverState *bs, int flags)
{
- uint32_t magic;
-
- if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
- return -EIO;
- }
+ int ret;
+ BDRVVmdkState *s = bs->opaque;
- magic = be32_to_cpu(magic);
- if (magic == VMDK3_MAGIC) {
- return vmdk_open_vmdk3(bs, flags);
- } else if (magic == VMDK4_MAGIC) {
- return vmdk_open_vmdk4(bs, flags);
+ if (vmdk_open_sparse(bs, bs->file, flags) == 0) {
+ s->desc_offset = 0x200;
+ /* try to open parent images, if exist */
+ ret = vmdk_parent_open(bs);
+ if (ret) {
+ vmdk_free_extents(bs);
+ return ret;
+ }
+ s->parent_cid = vmdk_read_cid(bs, 1);
+ return 0;
} else {
return vmdk_open_desc_file(bs, flags);
}
^ permalink raw reply related [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-08-06 4:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-07-27 9:27 [Qemu-devel] [PATCH 0/6] Add various VMDK subformats support Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 1/6] VMDK: enable twoGbMaxExtentFlat Fam Zheng
2011-07-29 1:42 ` Fam Zheng
2011-07-30 4:43 ` Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
2011-08-02 10:32 ` Stefan Hajnoczi
2011-08-04 2:35 ` Fam Zheng
2011-08-04 10:36 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 3/6] VMDK: separate vmdk_read_extent/vmdk_write_extent Fam Zheng
2011-07-27 9:27 ` [Qemu-devel] [PATCH 4/6] VMDK: Opening compressed extent Fam Zheng
2011-08-02 10:59 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 5/6] VMDK: read/write " Fam Zheng
2011-08-02 11:11 ` Stefan Hajnoczi
2011-07-27 9:27 ` [Qemu-devel] [PATCH 6/6] VMDK: creating streamOptimized subformat Fam Zheng
2011-08-06 4:10 ` [Qemu-devel] [PATCH v2 2/6] VMDK: add twoGbMaxExtentSparse support Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).