* [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version
@ 2011-06-04 0:41 Fam Zheng
2011-06-18 13:59 ` Stefan Hajnoczi
0 siblings, 1 reply; 2+ messages in thread
From: Fam Zheng @ 2011-06-04 0:41 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Christoph Hellwig
Separate vmdk_open by subformats to:
* vmdk_open_vmdk3
* vmdk_open_vmdk4
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 171 +++++++++++++++++++++++++++++++++++-----------------------
1 files changed, 104 insertions(+), 67 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index bf8d02a..e360156 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -372,69 +372,9 @@ static int vmdk_parent_open(BlockDriverState *bs)
return 0;
}
-static int vmdk_open(BlockDriverState *bs, int flags)
+static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
{
- BDRVVmdkState *s = bs->opaque;
- uint32_t magic;
int l1_size, i;
- VmdkExtent *extent = NULL;
-
- if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
- goto fail;
-
- magic = be32_to_cpu(magic);
- if (magic == VMDK3_MAGIC) {
- VMDK3Header header;
- s->extents = qemu_mallocz(sizeof(VmdkExtent));
- s->num_extents = 1;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header))
- goto fail;
- extent = s->extents;
- extent->flat = false;
- extent->file = bs->file;
- extent->cluster_sectors = le32_to_cpu(header.granularity);
- extent->l2_size = 1 << 9;
- extent->l1_size = 1 << 6;
- extent->sectors = le32_to_cpu(header.disk_sectors);
- extent->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
- extent->l1_backup_table_offset = 0;
- extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
- } else if (magic == VMDK4_MAGIC) {
- VMDK4Header header;
- s->extents = qemu_mallocz(sizeof(VmdkExtent));
- s->num_extents = 1;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header))
- goto fail;
- extent = s->extents;
- extent->file = bs->file;
- extent->sectors = le64_to_cpu(header.capacity);
- extent->cluster_sectors = le64_to_cpu(header.granularity);
- extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
- extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
- if (extent->l1_entry_sectors <= 0)
- goto fail;
- extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
- / extent->l1_entry_sectors;
- extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
- extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
-
- // try to open parent images, if exist
- if (vmdk_parent_open(bs) != 0)
- goto fail;
- // write the CID once after the image creation
- extent->parent_cid = vmdk_read_cid(bs,1);
- } else {
- goto fail;
- }
-
- /* sum up the total sectors */
- bs->total_sectors = 0;
- for (i = 0; i < s->num_extents; i++)
- {
- bs->total_sectors += s->extents[i].sectors;
- }
/* read the L1 table */
l1_size = extent->l1_size * sizeof(uint32_t);
@@ -443,8 +383,9 @@ static int vmdk_open(BlockDriverState *bs, int flags)
extent->l1_table_offset,
extent->l1_table,
l1_size)
- != l1_size)
+ != l1_size) {
goto fail;
+ }
for(i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_table[i]);
}
@@ -455,8 +396,9 @@ static int vmdk_open(BlockDriverState *bs, int flags)
extent->l1_backup_table_offset,
extent->l1_backup_table,
l1_size)
- != l1_size)
+ != l1_size) {
goto fail;
+ }
for(i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_backup_table[i]);
}
@@ -466,14 +408,109 @@ static int vmdk_open(BlockDriverState *bs, int flags)
qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
return 0;
fail:
- if(s->extents) {
- qemu_free(s->extents[0].l1_backup_table);
- qemu_free(s->extents[0].l1_table);
- qemu_free(s->extents[0].l2_cache);
+ qemu_free(extent->l1_backup_table);
+ qemu_free(extent->l1_table);
+ qemu_free(extent->l2_cache);
+ return -1;
+}
+
+static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+{
+ uint32_t magic;
+ BDRVVmdkState *s = bs->opaque;
+ VMDK3Header header;
+ VmdkExtent *extent;
+
+ s->extents = qemu_mallocz(sizeof(VmdkExtent));
+ if (!s->extents) {
+ goto fail;
+ }
+ s->num_extents = 1;
+ if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+ != sizeof(header)) {
+ goto fail;
+ }
+ extent = s->extents;
+ extent->flat = false;
+ extent->file = bs->file;
+ extent->cluster_sectors = le32_to_cpu(header.granularity);
+ extent->l2_size = 1 << 9;
+ extent->l1_size = 1 << 6;
+ extent->sectors = le32_to_cpu(header.disk_sectors);
+ extent->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
+ extent->l1_backup_table_offset = 0;
+ extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
+ bs->total_sectors = extent->sectors;
+ if (vmdk_init_tables(bs, extent)) {
+ goto fail;
+ }
+ return 0;
+ fail:
+ qemu_free(s->extents);
+ return -1;
+}
+
+static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
+{
+ uint32_t magic;
+ VMDK4Header header;
+ BDRVVmdkState *s = bs->opaque;
+ VmdkExtent *extent;
+
+ s->extents = qemu_mallocz(sizeof(VmdkExtent));
+ s->num_extents = 1;
+ if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+ != sizeof(header)) {
+ goto fail;
+ }
+ extent = s->extents;
+ extent->file = bs->file;
+ extent->sectors = le64_to_cpu(header.capacity);
+ extent->cluster_sectors = le64_to_cpu(header.granularity);
+ extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
+ extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
+ if (extent->l1_entry_sectors <= 0) {
+ goto fail;
+ }
+ extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
+ / extent->l1_entry_sectors;
+ extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
+ extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
+
+ // try to open parent images, if exist
+ if (vmdk_parent_open(bs) != 0) {
+ goto fail;
+ }
+ // write the CID once after the image creation
+ extent->parent_cid = vmdk_read_cid(bs,1);
+ bs->total_sectors = extent->sectors;
+ if (vmdk_init_tables(bs, extent)) {
+ goto fail;
}
+ return 0;
+ fail:
+ qemu_free(s->extents);
return -1;
}
+static int vmdk_open(BlockDriverState *bs, int flags)
+{
+ uint32_t magic;
+
+ if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+ return -1;
+ }
+
+ 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);
+ } else {
+ return -1;
+ }
+}
+
static uint64_t get_cluster_offset(BlockDriverState *bs,
VmdkExtent *extent,
VmdkMetaData *m_data,
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version
2011-06-04 0:41 [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version Fam Zheng
@ 2011-06-18 13:59 ` Stefan Hajnoczi
0 siblings, 0 replies; 2+ messages in thread
From: Stefan Hajnoczi @ 2011-06-18 13:59 UTC (permalink / raw)
To: Fam Zheng; +Cc: Kevin Wolf, qemu-devel, Christoph Hellwig
On Sat, Jun 4, 2011 at 1:41 AM, Fam Zheng <famcool@gmail.com> wrote:
> +static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
> +{
> + uint32_t magic;
> + VMDK4Header header;
> + BDRVVmdkState *s = bs->opaque;
> + VmdkExtent *extent;
> +
> + s->extents = qemu_mallocz(sizeof(VmdkExtent));
> + s->num_extents = 1;
> + if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> + != sizeof(header)) {
> + goto fail;
> + }
> + extent = s->extents;
> + extent->file = bs->file;
> + extent->sectors = le64_to_cpu(header.capacity);
> + extent->cluster_sectors = le64_to_cpu(header.granularity);
> + extent->l2_size = le32_to_cpu(header.num_gtes_per_gte);
> + extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
> + if (extent->l1_entry_sectors <= 0) {
> + goto fail;
> + }
> + extent->l1_size = (extent->sectors + extent->l1_entry_sectors - 1)
> + / extent->l1_entry_sectors;
> + extent->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
> + extent->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
> +
> + // try to open parent images, if exist
> + if (vmdk_parent_open(bs) != 0) {
> + goto fail;
> + }
> + // write the CID once after the image creation
This comment is from vmdk_create() and can be removed?
> + extent->parent_cid = vmdk_read_cid(bs,1);
> + bs->total_sectors = extent->sectors;
> + if (vmdk_init_tables(bs, extent)) {
> + goto fail;
> }
> + return 0;
> + fail:
> + qemu_free(s->extents);
> return -1;
> }
>
> +static int vmdk_open(BlockDriverState *bs, int flags)
> +{
> + uint32_t magic;
> +
> + if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
> + return -1;
> + }
> +
> + 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);
> + } else {
> + return -1;
> + }
This is a good opportunity to use -errno instead of -1. If you look
at qcow2 or qed they return appropriate error numbers and not just -1.
(Also remember -1 is -EPERM so callers that interpret the return
value as an error will print junk error messages if -1 is returned.)
Stefan
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-06-18 13:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-04 0:41 [Qemu-devel] [PATCH 04/12] VMDK: separate vmdk_open by format version Fam Zheng
2011-06-18 13:59 ` Stefan Hajnoczi
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).