* [Qemu-devel] [PATCH v6 01/12] VMDK: introduce VmdkExtent
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 12:28 ` Kevin Wolf
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
` (11 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Introduced VmdkExtent array into BDRVVmdkState, enable holding multiple
image extents for multiple file image support.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 355 +++++++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 252 insertions(+), 103 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 922b23d..f26137d 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -60,7 +60,11 @@ typedef struct {
#define L2_CACHE_SIZE 16
-typedef struct BDRVVmdkState {
+typedef struct VmdkExtent {
+ BlockDriverState *file;
+ bool flat;
+ int64_t sectors;
+ int64_t end_sector;
int64_t l1_table_offset;
int64_t l1_backup_table_offset;
uint32_t *l1_table;
@@ -74,7 +78,12 @@ typedef struct BDRVVmdkState {
uint32_t l2_cache_counts[L2_CACHE_SIZE];
unsigned int cluster_sectors;
+} VmdkExtent;
+
+typedef struct BDRVVmdkState {
+ int num_extents;
uint32_t parent_cid;
+ VmdkExtent *extents;
} BDRVVmdkState;
typedef struct VmdkMetaData {
@@ -105,6 +114,19 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each
#define HEADER_SIZE 512 // first sector of 512 bytes
+static void vmdk_free_extents(BlockDriverState *bs)
+{
+ int i;
+ BDRVVmdkState *s = bs->opaque;
+
+ for (i = 0; i < s->num_extents; i++) {
+ qemu_free(s->extents[i].l1_table);
+ qemu_free(s->extents[i].l2_cache);
+ qemu_free(s->extents[i].l1_backup_table);
+ }
+ qemu_free(s->extents);
+}
+
static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
{
char desc[DESC_SIZE];
@@ -156,8 +178,8 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
static int vmdk_is_cid_valid(BlockDriverState *bs)
{
-#ifdef CHECK_CID
BDRVVmdkState *s = bs->opaque;
+#ifdef CHECK_CID
BlockDriverState *p_bs = bs->backing_hd;
uint32_t cur_pcid;
@@ -358,11 +380,50 @@ static int vmdk_parent_open(BlockDriverState *bs)
return 0;
}
+/* Create and append extent to the extent array. Return the added VmdkExtent
+ * address. return NULL if allocation failed. */
+static VmdkExtent *vmdk_add_extent(BlockDriverState *bs,
+ BlockDriverState *file, bool flat, int64_t sectors,
+ int64_t l1_offset, int64_t l1_backup_offset,
+ uint32_t l1_size,
+ int l2_size, unsigned int cluster_sectors)
+{
+ VmdkExtent *extent;
+ BDRVVmdkState *s = bs->opaque;
+
+ s->extents = qemu_realloc(s->extents,
+ (s->num_extents + 1) * sizeof(VmdkExtent));
+ extent = &s->extents[s->num_extents];
+ s->num_extents++;
+
+ memset(extent, 0, sizeof(VmdkExtent));
+ extent->file = file;
+ extent->flat = flat;
+ extent->sectors = sectors;
+ extent->l1_table_offset = l1_offset;
+ extent->l1_backup_table_offset = l1_backup_offset;
+ extent->l1_size = l1_size;
+ extent->l1_entry_sectors = l2_size * cluster_sectors;
+ extent->l2_size = l2_size;
+ extent->cluster_sectors = cluster_sectors;
+
+ if (s->num_extents > 1) {
+ extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
+ } else {
+ extent->end_sector = extent->sectors;
+ }
+ bs->total_sectors = extent->end_sector;
+ return extent;
+}
+
+
static int vmdk_open(BlockDriverState *bs, int flags)
{
BDRVVmdkState *s = bs->opaque;
uint32_t magic;
- int l1_size, i;
+ int i;
+ uint32_t l1_size, l1_entry_sectors;
+ VmdkExtent *extent = NULL;
if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
goto fail;
@@ -370,32 +431,34 @@ static int vmdk_open(BlockDriverState *bs, int flags)
magic = be32_to_cpu(magic);
if (magic == VMDK3_MAGIC) {
VMDK3Header header;
-
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != sizeof(header))
+ if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+ != sizeof(header)) {
goto fail;
- s->cluster_sectors = le32_to_cpu(header.granularity);
- s->l2_size = 1 << 9;
- s->l1_size = 1 << 6;
- bs->total_sectors = le32_to_cpu(header.disk_sectors);
- s->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
- s->l1_backup_table_offset = 0;
- s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
+ }
+ extent = vmdk_add_extent(bs, bs->file, false,
+ le32_to_cpu(header.disk_sectors),
+ le32_to_cpu(header.l1dir_offset) << 9, 0,
+ 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
} else if (magic == VMDK4_MAGIC) {
VMDK4Header header;
-
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != sizeof(header))
+ if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
+ != sizeof(header)) {
goto fail;
- bs->total_sectors = le64_to_cpu(header.capacity);
- s->cluster_sectors = le64_to_cpu(header.granularity);
- s->l2_size = le32_to_cpu(header.num_gtes_per_gte);
- s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
- if (s->l1_entry_sectors <= 0)
+ }
+ l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
+ * le64_to_cpu(header.granularity);
+ l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
+ / l1_entry_sectors;
+ extent = vmdk_add_extent(bs, 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) {
goto fail;
- s->l1_size = (bs->total_sectors + s->l1_entry_sectors - 1)
- / s->l1_entry_sectors;
- s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
- s->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;
@@ -405,41 +468,56 @@ static int vmdk_open(BlockDriverState *bs, int flags)
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 = s->l1_size * sizeof(uint32_t);
- s->l1_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, l1_size) != l1_size)
+ l1_size = extent->l1_size * sizeof(uint32_t);
+ extent->l1_table = qemu_malloc(l1_size);
+ if (bdrv_pread(bs->file,
+ extent->l1_table_offset,
+ extent->l1_table,
+ l1_size)
+ != l1_size) {
goto fail;
- for(i = 0; i < s->l1_size; i++) {
- le32_to_cpus(&s->l1_table[i]);
+ }
+ for (i = 0; i < extent->l1_size; i++) {
+ le32_to_cpus(&extent->l1_table[i]);
}
- if (s->l1_backup_table_offset) {
- s->l1_backup_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file, s->l1_backup_table_offset, s->l1_backup_table, l1_size) != l1_size)
+ if (extent->l1_backup_table_offset) {
+ extent->l1_backup_table = qemu_malloc(l1_size);
+ if (bdrv_pread(bs->file,
+ extent->l1_backup_table_offset,
+ extent->l1_backup_table,
+ l1_size)
+ != l1_size) {
goto fail;
- for(i = 0; i < s->l1_size; i++) {
- le32_to_cpus(&s->l1_backup_table[i]);
+ }
+ for (i = 0; i < extent->l1_size; i++) {
+ le32_to_cpus(&extent->l1_backup_table[i]);
}
}
- s->l2_cache = qemu_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
+ extent->l2_cache =
+ qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
return 0;
fail:
- qemu_free(s->l1_backup_table);
- qemu_free(s->l1_table);
- qemu_free(s->l2_cache);
+ vmdk_free_extents(bs);
return -1;
}
-static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
- uint64_t offset, int allocate);
-
-static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
- uint64_t offset, int allocate)
+static int get_whole_cluster(BlockDriverState *bs,
+ VmdkExtent *extent,
+ uint64_t cluster_offset,
+ uint64_t offset,
+ bool allocate)
{
- BDRVVmdkState *s = bs->opaque;
- uint8_t whole_grain[s->cluster_sectors*512]; // 128 sectors * 512 bytes each = grain size 64KB
+ /* 128 sectors * 512 bytes each = grain size 64KB */
+ uint8_t whole_grain[extent->cluster_sectors * 512];
// we will be here if it's first write on non-exist grain(cluster).
// try to read from parent image, if exist
@@ -450,14 +528,14 @@ static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
return -1;
ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
- s->cluster_sectors);
+ extent->cluster_sectors);
if (ret < 0) {
return -1;
}
//Write grain only into the active image
- ret = bdrv_write(bs->file, cluster_offset, whole_grain,
- s->cluster_sectors);
+ ret = bdrv_write(extent->file, cluster_offset, whole_grain,
+ extent->cluster_sectors);
if (ret < 0) {
return -1;
}
@@ -465,29 +543,39 @@ static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
return 0;
}
-static int vmdk_L2update(BlockDriverState *bs, VmdkMetaData *m_data)
+static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
{
- BDRVVmdkState *s = bs->opaque;
-
/* update L2 table */
- if (bdrv_pwrite_sync(bs->file, ((int64_t)m_data->l2_offset * 512) + (m_data->l2_index * sizeof(m_data->offset)),
- &(m_data->offset), sizeof(m_data->offset)) < 0)
+ if (bdrv_pwrite_sync(
+ extent->file,
+ ((int64_t)m_data->l2_offset * 512)
+ + (m_data->l2_index * sizeof(m_data->offset)),
+ &(m_data->offset),
+ sizeof(m_data->offset)
+ ) < 0) {
return -1;
+ }
/* update backup L2 table */
- if (s->l1_backup_table_offset != 0) {
- m_data->l2_offset = s->l1_backup_table[m_data->l1_index];
- if (bdrv_pwrite_sync(bs->file, ((int64_t)m_data->l2_offset * 512) + (m_data->l2_index * sizeof(m_data->offset)),
- &(m_data->offset), sizeof(m_data->offset)) < 0)
+ if (extent->l1_backup_table_offset != 0) {
+ m_data->l2_offset = extent->l1_backup_table[m_data->l1_index];
+ if (bdrv_pwrite_sync(
+ extent->file,
+ ((int64_t)m_data->l2_offset * 512)
+ + (m_data->l2_index * sizeof(m_data->offset)),
+ &(m_data->offset), sizeof(m_data->offset)
+ ) < 0) {
return -1;
+ }
}
return 0;
}
-static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
- uint64_t offset, int allocate)
+static uint64_t get_cluster_offset(BlockDriverState *bs,
+ VmdkExtent *extent,
+ VmdkMetaData *m_data,
+ uint64_t offset, int allocate)
{
- BDRVVmdkState *s = bs->opaque;
unsigned int l1_index, l2_offset, l2_index;
int min_index, i, j;
uint32_t min_count, *l2_table, tmp = 0;
@@ -496,21 +584,23 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
if (m_data)
m_data->valid = 0;
- l1_index = (offset >> 9) / s->l1_entry_sectors;
- if (l1_index >= s->l1_size)
+ l1_index = (offset >> 9) / extent->l1_entry_sectors;
+ if (l1_index >= extent->l1_size) {
return 0;
- l2_offset = s->l1_table[l1_index];
- if (!l2_offset)
+ }
+ l2_offset = extent->l1_table[l1_index];
+ if (!l2_offset) {
return 0;
+ }
for(i = 0; i < L2_CACHE_SIZE; i++) {
- if (l2_offset == s->l2_cache_offsets[i]) {
+ if (l2_offset == extent->l2_cache_offsets[i]) {
/* increment the hit count */
- if (++s->l2_cache_counts[i] == 0xffffffff) {
+ if (++extent->l2_cache_counts[i] == 0xffffffff) {
for(j = 0; j < L2_CACHE_SIZE; j++) {
- s->l2_cache_counts[j] >>= 1;
+ extent->l2_cache_counts[j] >>= 1;
}
}
- l2_table = s->l2_cache + (i * s->l2_size);
+ l2_table = extent->l2_cache + (i * extent->l2_size);
goto found;
}
}
@@ -518,20 +608,25 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
min_index = 0;
min_count = 0xffffffff;
for(i = 0; i < L2_CACHE_SIZE; i++) {
- if (s->l2_cache_counts[i] < min_count) {
- min_count = s->l2_cache_counts[i];
+ if (extent->l2_cache_counts[i] < min_count) {
+ min_count = extent->l2_cache_counts[i];
min_index = i;
}
}
- l2_table = s->l2_cache + (min_index * s->l2_size);
- if (bdrv_pread(bs->file, (int64_t)l2_offset * 512, l2_table, s->l2_size * sizeof(uint32_t)) !=
- s->l2_size * sizeof(uint32_t))
+ l2_table = extent->l2_cache + (min_index * extent->l2_size);
+ if (bdrv_pread(
+ extent->file,
+ (int64_t)l2_offset * 512,
+ l2_table,
+ extent->l2_size * sizeof(uint32_t)
+ ) != extent->l2_size * sizeof(uint32_t)) {
return 0;
+ }
- s->l2_cache_offsets[min_index] = l2_offset;
- s->l2_cache_counts[min_index] = 1;
+ extent->l2_cache_offsets[min_index] = l2_offset;
+ extent->l2_cache_counts[min_index] = 1;
found:
- l2_index = ((offset >> 9) / s->cluster_sectors) % s->l2_size;
+ l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
cluster_offset = le32_to_cpu(l2_table[l2_index]);
if (!cluster_offset) {
@@ -539,8 +634,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
return 0;
// Avoid the L2 tables update for the images that have snapshots.
- cluster_offset = bdrv_getlength(bs->file);
- bdrv_truncate(bs->file, cluster_offset + (s->cluster_sectors << 9));
+ cluster_offset = bdrv_getlength(extent->file);
+ bdrv_truncate(
+ extent->file,
+ cluster_offset + (extent->cluster_sectors << 9)
+ );
cluster_offset >>= 9;
tmp = cpu_to_le32(cluster_offset);
@@ -551,7 +649,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
* This problem may occur because of insufficient space on host disk
* or inappropriate VM shutdown.
*/
- if (get_whole_cluster(bs, cluster_offset, offset, allocate) == -1)
+ if (get_whole_cluster(
+ bs, extent, cluster_offset, offset, allocate) == -1)
return 0;
if (m_data) {
@@ -566,33 +665,69 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
return cluster_offset;
}
+static VmdkExtent *find_extent(BDRVVmdkState *s,
+ int64_t sector_num, VmdkExtent *start_hint)
+{
+ VmdkExtent *extent = start_hint;
+
+ if (!extent) {
+ extent = &s->extents[0];
+ }
+ while (extent < &s->extents[s->num_extents]) {
+ if (sector_num < extent->end_sector) {
+ return extent;
+ }
+ extent++;
+ }
+ return NULL;
+}
+
static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, int *pnum)
{
BDRVVmdkState *s = bs->opaque;
- int index_in_cluster, n;
- uint64_t cluster_offset;
- cluster_offset = get_cluster_offset(bs, NULL, sector_num << 9, 0);
- index_in_cluster = sector_num % s->cluster_sectors;
- n = s->cluster_sectors - index_in_cluster;
+ int64_t index_in_cluster, n, ret;
+ uint64_t offset;
+ VmdkExtent *extent;
+
+ extent = find_extent(s, sector_num, NULL);
+ if (!extent) {
+ return 0;
+ }
+ if (extent->flat) {
+ n = extent->end_sector - sector_num;
+ ret = 1;
+ } else {
+ offset = get_cluster_offset(bs, extent, NULL, sector_num * 512, 0);
+ index_in_cluster = sector_num % extent->cluster_sectors;
+ n = extent->cluster_sectors - index_in_cluster;
+ ret = offset ? 1 : 0;
+ }
if (n > nb_sectors)
n = nb_sectors;
*pnum = n;
- return (cluster_offset != 0);
+ return ret;
}
static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
uint8_t *buf, int nb_sectors)
{
BDRVVmdkState *s = bs->opaque;
- int index_in_cluster, n, ret;
+ int ret;
+ uint64_t n, index_in_cluster;
+ VmdkExtent *extent = NULL;
uint64_t cluster_offset;
while (nb_sectors > 0) {
- cluster_offset = get_cluster_offset(bs, NULL, sector_num << 9, 0);
- index_in_cluster = sector_num % s->cluster_sectors;
- n = s->cluster_sectors - index_in_cluster;
+ extent = find_extent(s, sector_num, extent);
+ if (!extent) {
+ return -EIO;
+ }
+ cluster_offset = get_cluster_offset(
+ bs, extent, NULL, sector_num << 9, 0);
+ index_in_cluster = sector_num % extent->cluster_sectors;
+ n = extent->cluster_sectors - index_in_cluster;
if (n > nb_sectors)
n = nb_sectors;
if (!cluster_offset) {
@@ -621,10 +756,12 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors)
{
BDRVVmdkState *s = bs->opaque;
- VmdkMetaData m_data;
- int index_in_cluster, n;
+ VmdkExtent *extent = NULL;
+ int n;
+ int64_t index_in_cluster;
uint64_t cluster_offset;
static int cid_update = 0;
+ VmdkMetaData m_data;
if (sector_num > bs->total_sectors) {
fprintf(stderr,
@@ -635,20 +772,35 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
}
while (nb_sectors > 0) {
- index_in_cluster = sector_num & (s->cluster_sectors - 1);
- n = s->cluster_sectors - index_in_cluster;
- if (n > nb_sectors)
- n = nb_sectors;
- cluster_offset = get_cluster_offset(bs, &m_data, sector_num << 9, 1);
- if (!cluster_offset)
+ extent = find_extent(s, sector_num, extent);
+ if (!extent) {
+ return -EIO;
+ }
+ cluster_offset = get_cluster_offset(
+ bs,
+ extent,
+ &m_data,
+ sector_num << 9, 1);
+ if (!cluster_offset) {
return -1;
+ }
+ index_in_cluster = sector_num % extent->cluster_sectors;
+ n = extent->cluster_sectors - index_in_cluster;
+ if (n > nb_sectors) {
+ n = nb_sectors;
+ }
- if (bdrv_pwrite(bs->file, cluster_offset + index_in_cluster * 512, buf, n * 512) != n * 512)
+ if (bdrv_pwrite(bs->file,
+ cluster_offset + index_in_cluster * 512,
+ buf, n * 512)
+ != n * 512) {
return -1;
+ }
if (m_data.valid) {
/* update L2 tables */
- if (vmdk_L2update(bs, &m_data) == -1)
+ if (vmdk_L2update(extent, &m_data) == -1) {
return -1;
+ }
}
nb_sectors -= n;
sector_num += n;
@@ -822,10 +974,7 @@ exit:
static void vmdk_close(BlockDriverState *bs)
{
- BDRVVmdkState *s = bs->opaque;
-
- qemu_free(s->l1_table);
- qemu_free(s->l2_cache);
+ vmdk_free_extents(bs);
}
static int vmdk_flush(BlockDriverState *bs)
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 01/12] VMDK: introduce VmdkExtent
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 01/12] VMDK: introduce VmdkExtent Fam Zheng
@ 2011-07-01 12:28 ` Kevin Wolf
0 siblings, 0 replies; 21+ messages in thread
From: Kevin Wolf @ 2011-07-01 12:28 UTC (permalink / raw)
To: Fam Zheng; +Cc: stefanha, qemu-devel, hch
Am 01.07.2011 06:55, schrieb Fam Zheng:
> Introduced VmdkExtent array into BDRVVmdkState, enable holding multiple
> image extents for multiple file image support.
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 355 +++++++++++++++++++++++++++++++++++++++++-----------------
> 1 files changed, 252 insertions(+), 103 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 922b23d..f26137d 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -60,7 +60,11 @@ typedef struct {
>
> #define L2_CACHE_SIZE 16
>
> -typedef struct BDRVVmdkState {
> +typedef struct VmdkExtent {
> + BlockDriverState *file;
> + bool flat;
> + int64_t sectors;
> + int64_t end_sector;
> int64_t l1_table_offset;
> int64_t l1_backup_table_offset;
> uint32_t *l1_table;
> @@ -74,7 +78,12 @@ typedef struct BDRVVmdkState {
> uint32_t l2_cache_counts[L2_CACHE_SIZE];
>
> unsigned int cluster_sectors;
> +} VmdkExtent;
> +
> +typedef struct BDRVVmdkState {
> + int num_extents;
> uint32_t parent_cid;
> + VmdkExtent *extents;
> } BDRVVmdkState;
>
> typedef struct VmdkMetaData {
> @@ -105,6 +114,19 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
> #define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each
> #define HEADER_SIZE 512 // first sector of 512 bytes
>
> +static void vmdk_free_extents(BlockDriverState *bs)
> +{
> + int i;
> + BDRVVmdkState *s = bs->opaque;
> +
> + for (i = 0; i < s->num_extents; i++) {
> + qemu_free(s->extents[i].l1_table);
> + qemu_free(s->extents[i].l2_cache);
> + qemu_free(s->extents[i].l1_backup_table);
> + }
> + qemu_free(s->extents);
> +}
> +
> static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
> {
> char desc[DESC_SIZE];
> @@ -156,8 +178,8 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
>
> static int vmdk_is_cid_valid(BlockDriverState *bs)
> {
> -#ifdef CHECK_CID
> BDRVVmdkState *s = bs->opaque;
> +#ifdef CHECK_CID
> BlockDriverState *p_bs = bs->backing_hd;
> uint32_t cur_pcid;
Hm, intentional? The only thing this seems to change is that it causes
an 'unused variable' warning of CHECK_CID isn't defined.
> @@ -358,11 +380,50 @@ static int vmdk_parent_open(BlockDriverState *bs)
> return 0;
> }
>
> +/* Create and append extent to the extent array. Return the added VmdkExtent
> + * address. return NULL if allocation failed. */
> +static VmdkExtent *vmdk_add_extent(BlockDriverState *bs,
> + BlockDriverState *file, bool flat, int64_t sectors,
> + int64_t l1_offset, int64_t l1_backup_offset,
> + uint32_t l1_size,
> + int l2_size, unsigned int cluster_sectors)
> +{
> + VmdkExtent *extent;
> + BDRVVmdkState *s = bs->opaque;
> +
> + s->extents = qemu_realloc(s->extents,
> + (s->num_extents + 1) * sizeof(VmdkExtent));
> + extent = &s->extents[s->num_extents];
> + s->num_extents++;
> +
> + memset(extent, 0, sizeof(VmdkExtent));
> + extent->file = file;
> + extent->flat = flat;
> + extent->sectors = sectors;
> + extent->l1_table_offset = l1_offset;
> + extent->l1_backup_table_offset = l1_backup_offset;
> + extent->l1_size = l1_size;
> + extent->l1_entry_sectors = l2_size * cluster_sectors;
> + extent->l2_size = l2_size;
> + extent->cluster_sectors = cluster_sectors;
> +
> + if (s->num_extents > 1) {
> + extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
> + } else {
> + extent->end_sector = extent->sectors;
> + }
> + bs->total_sectors = extent->end_sector;
This means that extents must be added in ascending order, so that the
latest extent describes the highest sector numbers. Should be documented.
> + return extent;
> +}
> +
> +
> static int vmdk_open(BlockDriverState *bs, int flags)
> {
> BDRVVmdkState *s = bs->opaque;
> uint32_t magic;
> - int l1_size, i;
> + int i;
> + uint32_t l1_size, l1_entry_sectors;
> + VmdkExtent *extent = NULL;
>
> if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
> goto fail;
> @@ -370,32 +431,34 @@ static int vmdk_open(BlockDriverState *bs, int flags)
> magic = be32_to_cpu(magic);
> if (magic == VMDK3_MAGIC) {
> VMDK3Header header;
> -
> - if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != sizeof(header))
> + if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> + != sizeof(header)) {
> goto fail;
> - s->cluster_sectors = le32_to_cpu(header.granularity);
> - s->l2_size = 1 << 9;
> - s->l1_size = 1 << 6;
> - bs->total_sectors = le32_to_cpu(header.disk_sectors);
> - s->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
> - s->l1_backup_table_offset = 0;
> - s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
> + }
> + extent = vmdk_add_extent(bs, bs->file, false,
> + le32_to_cpu(header.disk_sectors),
> + le32_to_cpu(header.l1dir_offset) << 9, 0,
> + 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
> } else if (magic == VMDK4_MAGIC) {
> VMDK4Header header;
> -
> - if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header)) != sizeof(header))
> + if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> + != sizeof(header)) {
> goto fail;
> - bs->total_sectors = le64_to_cpu(header.capacity);
> - s->cluster_sectors = le64_to_cpu(header.granularity);
> - s->l2_size = le32_to_cpu(header.num_gtes_per_gte);
> - s->l1_entry_sectors = s->l2_size * s->cluster_sectors;
> - if (s->l1_entry_sectors <= 0)
> + }
> + l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
> + * le64_to_cpu(header.granularity);
> + l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
> + / l1_entry_sectors;
> + extent = vmdk_add_extent(bs, 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) {
> goto fail;
> - s->l1_size = (bs->total_sectors + s->l1_entry_sectors - 1)
> - / s->l1_entry_sectors;
> - s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
> - s->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;
> @@ -405,41 +468,56 @@ static int vmdk_open(BlockDriverState *bs, int flags)
> 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;
> + }
Is this required? Doesn't vmdk_add_extent already update bs->total_sectors?
> +
> /* read the L1 table */
> - l1_size = s->l1_size * sizeof(uint32_t);
> - s->l1_table = qemu_malloc(l1_size);
> - if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, l1_size) != l1_size)
> + l1_size = extent->l1_size * sizeof(uint32_t);
> + extent->l1_table = qemu_malloc(l1_size);
> + if (bdrv_pread(bs->file,
> + extent->l1_table_offset,
> + extent->l1_table,
> + l1_size)
> + != l1_size) {
> goto fail;
> - for(i = 0; i < s->l1_size; i++) {
> - le32_to_cpus(&s->l1_table[i]);
> + }
> + for (i = 0; i < extent->l1_size; i++) {
> + le32_to_cpus(&extent->l1_table[i]);
> }
>
> - if (s->l1_backup_table_offset) {
> - s->l1_backup_table = qemu_malloc(l1_size);
> - if (bdrv_pread(bs->file, s->l1_backup_table_offset, s->l1_backup_table, l1_size) != l1_size)
> + if (extent->l1_backup_table_offset) {
> + extent->l1_backup_table = qemu_malloc(l1_size);
> + if (bdrv_pread(bs->file,
> + extent->l1_backup_table_offset,
> + extent->l1_backup_table,
> + l1_size)
> + != l1_size) {
> goto fail;
> - for(i = 0; i < s->l1_size; i++) {
> - le32_to_cpus(&s->l1_backup_table[i]);
> + }
> + for (i = 0; i < extent->l1_size; i++) {
> + le32_to_cpus(&extent->l1_backup_table[i]);
> }
> }
>
> - s->l2_cache = qemu_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
> + extent->l2_cache =
> + qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
> return 0;
> fail:
> - qemu_free(s->l1_backup_table);
> - qemu_free(s->l1_table);
> - qemu_free(s->l2_cache);
> + vmdk_free_extents(bs);
> return -1;
> }
>
> -static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> - uint64_t offset, int allocate);
> -
> -static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
> - uint64_t offset, int allocate)
> +static int get_whole_cluster(BlockDriverState *bs,
> + VmdkExtent *extent,
> + uint64_t cluster_offset,
> + uint64_t offset,
> + bool allocate)
> {
> - BDRVVmdkState *s = bs->opaque;
> - uint8_t whole_grain[s->cluster_sectors*512]; // 128 sectors * 512 bytes each = grain size 64KB
> + /* 128 sectors * 512 bytes each = grain size 64KB */
> + uint8_t whole_grain[extent->cluster_sectors * 512];
>
> // we will be here if it's first write on non-exist grain(cluster).
> // try to read from parent image, if exist
> @@ -450,14 +528,14 @@ static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
> return -1;
>
> ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
> - s->cluster_sectors);
> + extent->cluster_sectors);
> if (ret < 0) {
> return -1;
> }
>
> //Write grain only into the active image
> - ret = bdrv_write(bs->file, cluster_offset, whole_grain,
> - s->cluster_sectors);
> + ret = bdrv_write(extent->file, cluster_offset, whole_grain,
> + extent->cluster_sectors);
> if (ret < 0) {
> return -1;
> }
> @@ -465,29 +543,39 @@ static int get_whole_cluster(BlockDriverState *bs, uint64_t cluster_offset,
> return 0;
> }
>
> -static int vmdk_L2update(BlockDriverState *bs, VmdkMetaData *m_data)
> +static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
> {
> - BDRVVmdkState *s = bs->opaque;
> -
> /* update L2 table */
> - if (bdrv_pwrite_sync(bs->file, ((int64_t)m_data->l2_offset * 512) + (m_data->l2_index * sizeof(m_data->offset)),
> - &(m_data->offset), sizeof(m_data->offset)) < 0)
> + if (bdrv_pwrite_sync(
> + extent->file,
> + ((int64_t)m_data->l2_offset * 512)
> + + (m_data->l2_index * sizeof(m_data->offset)),
> + &(m_data->offset),
> + sizeof(m_data->offset)
> + ) < 0) {
> return -1;
> + }
> /* update backup L2 table */
> - if (s->l1_backup_table_offset != 0) {
> - m_data->l2_offset = s->l1_backup_table[m_data->l1_index];
> - if (bdrv_pwrite_sync(bs->file, ((int64_t)m_data->l2_offset * 512) + (m_data->l2_index * sizeof(m_data->offset)),
> - &(m_data->offset), sizeof(m_data->offset)) < 0)
> + if (extent->l1_backup_table_offset != 0) {
> + m_data->l2_offset = extent->l1_backup_table[m_data->l1_index];
> + if (bdrv_pwrite_sync(
> + extent->file,
> + ((int64_t)m_data->l2_offset * 512)
> + + (m_data->l2_index * sizeof(m_data->offset)),
> + &(m_data->offset), sizeof(m_data->offset)
> + ) < 0) {
> return -1;
> + }
> }
>
> return 0;
> }
>
> -static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> - uint64_t offset, int allocate)
> +static uint64_t get_cluster_offset(BlockDriverState *bs,
> + VmdkExtent *extent,
> + VmdkMetaData *m_data,
> + uint64_t offset, int allocate)
> {
> - BDRVVmdkState *s = bs->opaque;
> unsigned int l1_index, l2_offset, l2_index;
> int min_index, i, j;
> uint32_t min_count, *l2_table, tmp = 0;
> @@ -496,21 +584,23 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> if (m_data)
> m_data->valid = 0;
>
> - l1_index = (offset >> 9) / s->l1_entry_sectors;
> - if (l1_index >= s->l1_size)
> + l1_index = (offset >> 9) / extent->l1_entry_sectors;
> + if (l1_index >= extent->l1_size) {
> return 0;
> - l2_offset = s->l1_table[l1_index];
> - if (!l2_offset)
> + }
> + l2_offset = extent->l1_table[l1_index];
> + if (!l2_offset) {
> return 0;
> + }
> for(i = 0; i < L2_CACHE_SIZE; i++) {
> - if (l2_offset == s->l2_cache_offsets[i]) {
> + if (l2_offset == extent->l2_cache_offsets[i]) {
> /* increment the hit count */
> - if (++s->l2_cache_counts[i] == 0xffffffff) {
> + if (++extent->l2_cache_counts[i] == 0xffffffff) {
> for(j = 0; j < L2_CACHE_SIZE; j++) {
> - s->l2_cache_counts[j] >>= 1;
> + extent->l2_cache_counts[j] >>= 1;
> }
> }
> - l2_table = s->l2_cache + (i * s->l2_size);
> + l2_table = extent->l2_cache + (i * extent->l2_size);
> goto found;
> }
> }
> @@ -518,20 +608,25 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> min_index = 0;
> min_count = 0xffffffff;
> for(i = 0; i < L2_CACHE_SIZE; i++) {
> - if (s->l2_cache_counts[i] < min_count) {
> - min_count = s->l2_cache_counts[i];
> + if (extent->l2_cache_counts[i] < min_count) {
> + min_count = extent->l2_cache_counts[i];
> min_index = i;
> }
> }
> - l2_table = s->l2_cache + (min_index * s->l2_size);
> - if (bdrv_pread(bs->file, (int64_t)l2_offset * 512, l2_table, s->l2_size * sizeof(uint32_t)) !=
> - s->l2_size * sizeof(uint32_t))
> + l2_table = extent->l2_cache + (min_index * extent->l2_size);
> + if (bdrv_pread(
> + extent->file,
> + (int64_t)l2_offset * 512,
> + l2_table,
> + extent->l2_size * sizeof(uint32_t)
> + ) != extent->l2_size * sizeof(uint32_t)) {
> return 0;
> + }
>
> - s->l2_cache_offsets[min_index] = l2_offset;
> - s->l2_cache_counts[min_index] = 1;
> + extent->l2_cache_offsets[min_index] = l2_offset;
> + extent->l2_cache_counts[min_index] = 1;
> found:
> - l2_index = ((offset >> 9) / s->cluster_sectors) % s->l2_size;
> + l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
> cluster_offset = le32_to_cpu(l2_table[l2_index]);
>
> if (!cluster_offset) {
> @@ -539,8 +634,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> return 0;
>
> // Avoid the L2 tables update for the images that have snapshots.
> - cluster_offset = bdrv_getlength(bs->file);
> - bdrv_truncate(bs->file, cluster_offset + (s->cluster_sectors << 9));
> + cluster_offset = bdrv_getlength(extent->file);
> + bdrv_truncate(
> + extent->file,
> + cluster_offset + (extent->cluster_sectors << 9)
> + );
>
> cluster_offset >>= 9;
> tmp = cpu_to_le32(cluster_offset);
> @@ -551,7 +649,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> * This problem may occur because of insufficient space on host disk
> * or inappropriate VM shutdown.
> */
> - if (get_whole_cluster(bs, cluster_offset, offset, allocate) == -1)
> + if (get_whole_cluster(
> + bs, extent, cluster_offset, offset, allocate) == -1)
> return 0;
>
> if (m_data) {
> @@ -566,33 +665,69 @@ static uint64_t get_cluster_offset(BlockDriverState *bs, VmdkMetaData *m_data,
> return cluster_offset;
> }
>
> +static VmdkExtent *find_extent(BDRVVmdkState *s,
> + int64_t sector_num, VmdkExtent *start_hint)
> +{
> + VmdkExtent *extent = start_hint;
> +
> + if (!extent) {
> + extent = &s->extents[0];
> + }
> + while (extent < &s->extents[s->num_extents]) {
> + if (sector_num < extent->end_sector) {
> + return extent;
> + }
> + extent++;
> + }
This relies on the right order in the array again. Worth a comment in
the declaration of the array in BDRVVmdkState.
The rest of the patch looks good. None of the issues are really
critical, so if your prefer to do the changes on top of your current
series, that works for me, too.
Kevin
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v6 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 01/12] VMDK: introduce VmdkExtent Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 03/12] VMDK: probe for monolithicFlat images Fam Zheng
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
In get_whole_cluster, the offset is not aligned to cluster when reading
from backing_hd. When the first write to child is not at the cluster
boundary, wrong address data from parent is copied to child.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 8 +++++---
1 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index f26137d..557e01e 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -519,21 +519,23 @@ static int get_whole_cluster(BlockDriverState *bs,
/* 128 sectors * 512 bytes each = grain size 64KB */
uint8_t whole_grain[extent->cluster_sectors * 512];
- // we will be here if it's first write on non-exist grain(cluster).
- // try to read from parent image, if exist
+ /* we will be here if it's first write on non-exist grain(cluster).
+ * try to read from parent image, if exist */
if (bs->backing_hd) {
int ret;
if (!vmdk_is_cid_valid(bs))
return -1;
+ /* floor offset to cluster */
+ offset -= offset % (extent->cluster_sectors * 512);
ret = bdrv_read(bs->backing_hd, offset >> 9, whole_grain,
extent->cluster_sectors);
if (ret < 0) {
return -1;
}
- //Write grain only into the active image
+ /* Write grain only into the active image */
ret = bdrv_write(extent->file, cluster_offset, whole_grain,
extent->cluster_sectors);
if (ret < 0) {
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 03/12] VMDK: probe for monolithicFlat images
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 01/12] VMDK: introduce VmdkExtent Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version Fam Zheng
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Probe as the same behavior as VMware does.
Recognize image as monolithicFlat descriptor file when the file is text
and the first effective line (not '#' leaded comment or space line) is
either 'version=1' or 'version=2'. No space or upper case charactors
accepted.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 557e01e..4684670 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -102,10 +102,50 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
magic = be32_to_cpu(*(uint32_t *)buf);
if (magic == VMDK3_MAGIC ||
- magic == VMDK4_MAGIC)
+ magic == VMDK4_MAGIC) {
return 100;
- else
+ } else {
+ const char *p = (const char *)buf;
+ const char *end = p + buf_size;
+ while (p < end) {
+ if (*p == '#') {
+ /* skip comment line */
+ while (p < end && *p != '\n') {
+ p++;
+ }
+ p++;
+ continue;
+ }
+ if (*p == ' ') {
+ while (p < end && *p == ' ') {
+ p++;
+ }
+ /* skip '\r' if windows line endings used. */
+ if (p < end && *p == '\r') {
+ p++;
+ }
+ /* only accept blank lines before 'version=' line */
+ if (p == end || *p != '\n') {
+ return 0;
+ }
+ p++;
+ continue;
+ }
+ if (end - p >= strlen("version=X\n")) {
+ if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
+ strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
+ return 100;
+ }
+ }
+ if (end - p >= strlen("version=X\r\n")) {
+ if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
+ strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
+ return 100;
+ }
+ }
+ }
return 0;
+ }
}
#define CHECK_CID 1
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (2 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 03/12] VMDK: probe for monolithicFlat images Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 13:02 ` Kevin Wolf
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
` (8 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Separate vmdk_open by subformats to:
* vmdk_open_vmdk3
* vmdk_open_vmdk4
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 197 ++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 131 insertions(+), 66 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 4684670..03248f3 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -456,74 +456,26 @@ static VmdkExtent *vmdk_add_extent(BlockDriverState *bs,
return extent;
}
-
-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 i;
- uint32_t l1_size, l1_entry_sectors;
- 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;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header)) {
- goto fail;
- }
- extent = vmdk_add_extent(bs, bs->file, false,
- le32_to_cpu(header.disk_sectors),
- le32_to_cpu(header.l1dir_offset) << 9, 0,
- 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
- } else if (magic == VMDK4_MAGIC) {
- VMDK4Header header;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header)) {
- goto fail;
- }
- l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
- * le64_to_cpu(header.granularity);
- l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
- / l1_entry_sectors;
- extent = vmdk_add_extent(bs, 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) {
- goto fail;
- }
- // try to open parent images, if exist
- if (vmdk_parent_open(bs) != 0)
- goto fail;
- // write the CID once after the image creation
- s->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;
- }
+ int ret;
+ int l1_size, i;
/* read the L1 table */
l1_size = extent->l1_size * sizeof(uint32_t);
extent->l1_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file,
- extent->l1_table_offset,
- extent->l1_table,
- l1_size)
- != l1_size) {
+ if (!extent->l1_table) {
+ ret = -ENOMEM;
goto fail;
}
+ ret = bdrv_pread(bs->file,
+ extent->l1_table_offset,
+ extent->l1_table,
+ l1_size);
+ if (ret != l1_size) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail_l1;
+ }
for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_table[i]);
}
@@ -538,16 +490,129 @@ static int vmdk_open(BlockDriverState *bs, int flags)
goto fail;
}
for (i = 0; i < extent->l1_size; i++) {
+ if (!extent->l1_backup_table) {
+ ret = -ENOMEM;
+ goto fail_l1;
+ }
+ }
+ ret = bdrv_pread(bs->file,
+ extent->l1_backup_table_offset,
+ extent->l1_backup_table,
+ l1_size);
+ if (ret != l1_size) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_backup_table[i]);
}
}
extent->l2_cache =
qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
+ if (!extent->l2_cache) {
+ ret = -ENOMEM;
+ goto fail_l1b;
+ }
return 0;
+ fail_l1b:
+ qemu_free(extent->l1_backup_table);
+ fail_l1:
+ qemu_free(extent->l1_table);
fail:
- vmdk_free_extents(bs);
- return -1;
+ return ret;
+}
+
+static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+{
+ int ret;
+ uint32_t magic;
+ VMDK3Header header;
+ VmdkExtent *extent;
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ if (ret != sizeof(header)) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ extent = vmdk_add_extent(bs,
+ bs->file, false,
+ le32_to_cpu(header.disk_sectors),
+ le32_to_cpu(header.l1dir_offset) << 9,
+ 0, 1 << 6, 1 << 9,
+ le32_to_cpu(header.granularity));
+ ret = vmdk_init_tables(bs, extent);
+ if (ret) {
+ return ret;
+ }
+ return 0;
+ fail:
+ qemu_free(s->extents);
+ return ret;
+}
+
+static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
+{
+ int ret;
+ uint32_t magic;
+ uint32_t l1_size, l1_entry_sectors;
+ VMDK4Header header;
+ BDRVVmdkState *s = bs->opaque;
+ VmdkExtent *extent;
+
+ ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ if (ret != sizeof(header)) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
+ * le64_to_cpu(header.granularity);
+ l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
+ / l1_entry_sectors;
+ extent = vmdk_add_extent(bs, 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;
+ }
+ return 0;
+ fail:
+ qemu_free(s->extents);
+ return ret;
+}
+
+static int vmdk_open(BlockDriverState *bs, int flags)
+{
+ uint32_t magic;
+
+ if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+ return -EIO;
+ }
+
+ 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 -EINVAL;
+ }
}
static int get_whole_cluster(BlockDriverState *bs,
@@ -634,11 +699,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
if (!l2_offset) {
return 0;
}
- for(i = 0; i < L2_CACHE_SIZE; i++) {
+ for (i = 0; i < L2_CACHE_SIZE; i++) {
if (l2_offset == extent->l2_cache_offsets[i]) {
/* increment the hit count */
if (++extent->l2_cache_counts[i] == 0xffffffff) {
- for(j = 0; j < L2_CACHE_SIZE; j++) {
+ for (j = 0; j < L2_CACHE_SIZE; j++) {
extent->l2_cache_counts[j] >>= 1;
}
}
@@ -649,7 +714,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
/* not found: load a new entry in the least used one */
min_index = 0;
min_count = 0xffffffff;
- for(i = 0; i < L2_CACHE_SIZE; i++) {
+ for (i = 0; i < L2_CACHE_SIZE; i++) {
if (extent->l2_cache_counts[i] < min_count) {
min_count = extent->l2_cache_counts[i];
min_index = i;
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version Fam Zheng
@ 2011-07-01 13:02 ` Kevin Wolf
2011-07-01 13:06 ` Fam Zheng
0 siblings, 1 reply; 21+ messages in thread
From: Kevin Wolf @ 2011-07-01 13:02 UTC (permalink / raw)
To: Fam Zheng; +Cc: stefanha, qemu-devel, hch
Am 01.07.2011 06:55, schrieb Fam Zheng:
> Separate vmdk_open by subformats to:
> * vmdk_open_vmdk3
> * vmdk_open_vmdk4
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 197 ++++++++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 131 insertions(+), 66 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 4684670..03248f3 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -456,74 +456,26 @@ static VmdkExtent *vmdk_add_extent(BlockDriverState *bs,
> return extent;
> }
>
> -
> -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 i;
> - uint32_t l1_size, l1_entry_sectors;
> - 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;
> - if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> - != sizeof(header)) {
> - goto fail;
> - }
> - extent = vmdk_add_extent(bs, bs->file, false,
> - le32_to_cpu(header.disk_sectors),
> - le32_to_cpu(header.l1dir_offset) << 9, 0,
> - 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
> - } else if (magic == VMDK4_MAGIC) {
> - VMDK4Header header;
> - if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
> - != sizeof(header)) {
> - goto fail;
> - }
> - l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
> - * le64_to_cpu(header.granularity);
> - l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
> - / l1_entry_sectors;
> - extent = vmdk_add_extent(bs, 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) {
> - goto fail;
> - }
> - // try to open parent images, if exist
> - if (vmdk_parent_open(bs) != 0)
> - goto fail;
> - // write the CID once after the image creation
> - s->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;
> - }
> + int ret;
> + int l1_size, i;
>
> /* read the L1 table */
> l1_size = extent->l1_size * sizeof(uint32_t);
> extent->l1_table = qemu_malloc(l1_size);
> - if (bdrv_pread(bs->file,
> - extent->l1_table_offset,
> - extent->l1_table,
> - l1_size)
> - != l1_size) {
> + if (!extent->l1_table) {
> + ret = -ENOMEM;
> goto fail;
> }
qemu_malloc() never fails, so you don't need this check.
> + ret = bdrv_pread(bs->file,
> + extent->l1_table_offset,
> + extent->l1_table,
> + l1_size);
> + if (ret != l1_size) {
> + ret = ret < 0 ? ret : -EIO;
> + goto fail_l1;
> + }
bdrv_pread only ever returns 0 for success or -errno for errors. So you
can simplify the code like this:
ret = bdrv_pread(...);
if (ret < 0) {
goto fail_l1;
}
You have the same pattern in other places, too.
> for (i = 0; i < extent->l1_size; i++) {
> le32_to_cpus(&extent->l1_table[i]);
> }
> @@ -538,16 +490,129 @@ static int vmdk_open(BlockDriverState *bs, int flags)
> goto fail;
> }
> for (i = 0; i < extent->l1_size; i++) {
> + if (!extent->l1_backup_table) {
> + ret = -ENOMEM;
> + goto fail_l1;
> + }
> + }
Hm, isn't this checking the same thing multiple times?
Anyway, qemu_malloc() still doesn't fail. ;-)
> + ret = bdrv_pread(bs->file,
> + extent->l1_backup_table_offset,
> + extent->l1_backup_table,
> + l1_size);
This duplicates a read from a few lines above. Mismerge?
> + if (ret != l1_size) {
> + ret = ret < 0 ? ret : -EIO;
> + goto fail;
> + }
> + for (i = 0; i < extent->l1_size; i++) {
> le32_to_cpus(&extent->l1_backup_table[i]);
> }
> }
>
> extent->l2_cache =
> qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
> + if (!extent->l2_cache) {
> + ret = -ENOMEM;
> + goto fail_l1b;
> + }
Unnecessary check.
> return 0;
> + fail_l1b:
> + qemu_free(extent->l1_backup_table);
> + fail_l1:
> + qemu_free(extent->l1_table);
> fail:
> - vmdk_free_extents(bs);
> - return -1;
> + return ret;
> +}
> +
> +static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
> +{
> + int ret;
> + uint32_t magic;
> + VMDK3Header header;
> + VmdkExtent *extent;
> + BDRVVmdkState *s = bs->opaque;
> +
> + ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
> + if (ret != sizeof(header)) {
> + ret = ret < 0 ? ret : -EIO;
> + goto fail;
> + }
> + extent = vmdk_add_extent(bs,
> + bs->file, false,
> + le32_to_cpu(header.disk_sectors),
> + le32_to_cpu(header.l1dir_offset) << 9,
> + 0, 1 << 6, 1 << 9,
> + le32_to_cpu(header.granularity));
> + ret = vmdk_init_tables(bs, extent);
> + if (ret) {
> + return ret;
Are the extents leaked here?
> + }
> + return 0;
> + fail:
> + qemu_free(s->extents);
Why not vmdk_free_extents? Does this leak the tables/caches?
> + return ret;
> +}
> +
> +static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
> +{
> + int ret;
> + uint32_t magic;
> + uint32_t l1_size, l1_entry_sectors;
> + VMDK4Header header;
> + BDRVVmdkState *s = bs->opaque;
> + VmdkExtent *extent;
> +
> + ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
> + if (ret != sizeof(header)) {
> + ret = ret < 0 ? ret : -EIO;
> + goto fail;
> + }
> + l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
> + * le64_to_cpu(header.granularity);
> + l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
> + / l1_entry_sectors;
> + extent = vmdk_add_extent(bs, 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;
> + }
> + return 0;
> + fail:
> + qemu_free(s->extents);
> + return ret;
> +}
The same comments as for the version 3 function apply.
> +
> +static int vmdk_open(BlockDriverState *bs, int flags)
> +{
> + uint32_t magic;
> +
> + if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
> + return -EIO;
> + }
> +
> + 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 -EINVAL;
> + }
> }
>
> static int get_whole_cluster(BlockDriverState *bs,
> @@ -634,11 +699,11 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> if (!l2_offset) {
> return 0;
> }
> - for(i = 0; i < L2_CACHE_SIZE; i++) {
> + for (i = 0; i < L2_CACHE_SIZE; i++) {
> if (l2_offset == extent->l2_cache_offsets[i]) {
> /* increment the hit count */
> if (++extent->l2_cache_counts[i] == 0xffffffff) {
> - for(j = 0; j < L2_CACHE_SIZE; j++) {
> + for (j = 0; j < L2_CACHE_SIZE; j++) {
> extent->l2_cache_counts[j] >>= 1;
> }
> }
> @@ -649,7 +714,7 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
> /* not found: load a new entry in the least used one */
> min_index = 0;
> min_count = 0xffffffff;
> - for(i = 0; i < L2_CACHE_SIZE; i++) {
> + for (i = 0; i < L2_CACHE_SIZE; i++) {
> if (extent->l2_cache_counts[i] < min_count) {
> min_count = extent->l2_cache_counts[i];
> min_index = i;
Kevin
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version
2011-07-01 13:02 ` Kevin Wolf
@ 2011-07-01 13:06 ` Fam Zheng
2011-07-01 13:13 ` Kevin Wolf
0 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 13:06 UTC (permalink / raw)
To: Kevin Wolf; +Cc: stefanha, qemu-devel, hch
>
> bdrv_pread only ever returns 0 for success or -errno for errors. So you
> can simplify the code like this:
>
> ret = bdrv_pread(...);
> if (ret < 0) {
> goto fail_l1;
> }
>
> You have the same pattern in other places, too.
>
I think bdrv_pead do return the read bytes, did you mean bdrv_read here? :)
--
Best regards!
Fam Zheng
^ permalink raw reply [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version
2011-07-01 13:06 ` Fam Zheng
@ 2011-07-01 13:13 ` Kevin Wolf
0 siblings, 0 replies; 21+ messages in thread
From: Kevin Wolf @ 2011-07-01 13:13 UTC (permalink / raw)
To: Fam Zheng; +Cc: stefanha, qemu-devel, hch
Am 01.07.2011 15:06, schrieb Fam Zheng:
>>
>> bdrv_pread only ever returns 0 for success or -errno for errors. So you
>> can simplify the code like this:
>>
>> ret = bdrv_pread(...);
>> if (ret < 0) {
>> goto fail_l1;
>> }
>>
>> You have the same pattern in other places, too.
>
> I think bdrv_pead do return the read bytes, did you mean bdrv_read here? :)
Yes, you're right, it returns the read bytes. But it's always -errno or
the full byte count, there are no short reads. So my explanation wasn't
quite right, but the suggestion stays the same. :-)
Kevin
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v6 05/12] VMDK: add field BDRVVmdkState.desc_offset
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (3 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 04/12] VMDK: separate vmdk_open by format version Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 13:08 ` Kevin Wolf
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 06/12] VMDK: flush multiple extents Fam Zheng
` (7 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
There are several occurrence of magic number 0x200 as the descriptor
offset within mono sparse image file. This is not the case for images
with separate descriptor file. So a field is added to BDRVVmdkState to
hold the correct value.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 03248f3..9393ff9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -81,6 +81,7 @@ typedef struct VmdkExtent {
} VmdkExtent;
typedef struct BDRVVmdkState {
+ int desc_offset;
int num_extents;
uint32_t parent_cid;
VmdkExtent *extents;
@@ -173,10 +174,11 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
uint32_t cid;
const char *p_name, *cid_str;
size_t cid_str_size;
+ BDRVVmdkState *s = bs->opaque;
- /* the descriptor offset = 0x200 */
- if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
+ if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
return 0;
+ }
if (parent) {
cid_str = "parentCID";
@@ -198,10 +200,11 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
{
char desc[DESC_SIZE], tmp_desc[DESC_SIZE];
char *p_name, *tmp_str;
+ BDRVVmdkState *s = bs->opaque;
- /* the descriptor offset = 0x200 */
- if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
- return -1;
+ if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
+ return -EIO;
+ }
tmp_str = strstr(desc,"parentCID");
pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str);
@@ -211,8 +214,9 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
pstrcat(desc, sizeof(desc), tmp_desc);
}
- if (bdrv_pwrite_sync(bs->file, 0x200, desc, DESC_SIZE) < 0)
- return -1;
+ if (bdrv_pwrite_sync(bs->file, s->desc_offset, desc, DESC_SIZE) < 0) {
+ return -EIO;
+ }
return 0;
}
@@ -296,7 +300,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
ret = -errno;
goto fail;
}
- /* the descriptor offset = 0x200 */
if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
ret = -errno;
goto fail;
@@ -400,10 +403,11 @@ static int vmdk_parent_open(BlockDriverState *bs)
{
char *p_name;
char desc[DESC_SIZE];
+ BDRVVmdkState *s = bs->opaque;
- /* the descriptor offset = 0x200 */
- if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
+ if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
return -1;
+ }
if ((p_name = strstr(desc,"parentFileNameHint")) != NULL) {
char *end_name;
@@ -531,6 +535,7 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
VmdkExtent *extent;
BDRVVmdkState *s = bs->opaque;
+ s->desc_offset = 0x200;
ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
if (ret != sizeof(header)) {
ret = ret < 0 ? ret : -EIO;
@@ -561,6 +566,7 @@ 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));
if (ret != sizeof(header)) {
ret = ret < 0 ? ret : -EIO;
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 05/12] VMDK: add field BDRVVmdkState.desc_offset
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
@ 2011-07-01 13:08 ` Kevin Wolf
0 siblings, 0 replies; 21+ messages in thread
From: Kevin Wolf @ 2011-07-01 13:08 UTC (permalink / raw)
To: Fam Zheng; +Cc: stefanha, qemu-devel, hch
Am 01.07.2011 06:55, schrieb Fam Zheng:
> There are several occurrence of magic number 0x200 as the descriptor
> offset within mono sparse image file. This is not the case for images
> with separate descriptor file. So a field is added to BDRVVmdkState to
> hold the correct value.
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 26 ++++++++++++++++----------
> 1 files changed, 16 insertions(+), 10 deletions(-)
>
> @@ -296,7 +300,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
> ret = -errno;
> goto fail;
> }
> - /* the descriptor offset = 0x200 */
> if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
> ret = -errno;
> goto fail;
Is this comment wrong or why do you remove it? I think for the newly
created image it is correct and with this patch we have an unexplained
magic 0x200 in the code.
Kevin
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v6 06/12] VMDK: flush multiple extents
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (4 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
` (6 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Flush all the file that referenced by the image.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 12 +++++++++++-
1 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 9393ff9..0a09890 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1092,7 +1092,17 @@ static void vmdk_close(BlockDriverState *bs)
static int vmdk_flush(BlockDriverState *bs)
{
- return bdrv_flush(bs->file);
+ int i, ret, err;
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_flush(bs->file);
+ for (i = 0; i < s->num_extents; i++) {
+ err = bdrv_flush(s->extents[i].file);
+ if (err < 0) {
+ ret = err;
+ }
+ }
+ return ret;
}
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 07/12] VMDK: move 'static' cid_update flag to bs field
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (5 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 06/12] VMDK: flush multiple extents Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 08/12] VMDK: change get_cluster_offset return type Fam Zheng
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Cid_update is the flag for updating CID on first write after opening the
image. This should be per image open rather than per program life cycle,
so change it from static var of vmdk_write to a field in BDRVVmdkState.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 0a09890..8783629 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -82,6 +82,7 @@ typedef struct VmdkExtent {
typedef struct BDRVVmdkState {
int desc_offset;
+ bool cid_updated;
int num_extents;
uint32_t parent_cid;
VmdkExtent *extents;
@@ -873,7 +874,6 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
int n;
int64_t index_in_cluster;
uint64_t cluster_offset;
- static int cid_update = 0;
VmdkMetaData m_data;
if (sector_num > bs->total_sectors) {
@@ -920,9 +920,9 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
buf += n * 512;
// update CID on the first write every time the virtual disk is opened
- if (!cid_update) {
+ if (!s->cid_updated) {
vmdk_write_cid(bs, time(NULL));
- cid_update++;
+ s->cid_updated = true;
}
}
return 0;
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 08/12] VMDK: change get_cluster_offset return type
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (6 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 16:40 ` Kevin Wolf
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
` (4 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
The return type of get_cluster_offset was an offset that use 0 to denote
'not allocated', this will be no longer true for flat extents, as we see
flat extent file as a single huge cluster whose offset is 0 and length
is the whole file length.
So now we use int return value, 0 means success and otherwise offset
invalid.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 73 +++++++++++++++++++++++++++++++---------------------------
1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 8783629..40e4464 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -685,18 +685,23 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
return 0;
}
-static uint64_t get_cluster_offset(BlockDriverState *bs,
+static int get_cluster_offset(BlockDriverState *bs,
VmdkExtent *extent,
VmdkMetaData *m_data,
- uint64_t offset, int allocate)
+ uint64_t offset,
+ int allocate,
+ uint64_t *cluster_offset)
{
unsigned int l1_index, l2_offset, l2_index;
int min_index, i, j;
uint32_t min_count, *l2_table, tmp = 0;
- uint64_t cluster_offset;
if (m_data)
m_data->valid = 0;
+ if (extent->flat) {
+ *cluster_offset = 0;
+ return 0;
+ }
l1_index = (offset >> 9) / extent->l1_entry_sectors;
if (l1_index >= extent->l1_size) {
@@ -741,21 +746,22 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
extent->l2_cache_counts[min_index] = 1;
found:
l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
- cluster_offset = le32_to_cpu(l2_table[l2_index]);
+ *cluster_offset = le32_to_cpu(l2_table[l2_index]);
- if (!cluster_offset) {
- if (!allocate)
- return 0;
+ if (!*cluster_offset) {
+ if (!allocate) {
+ return -1;
+ }
// Avoid the L2 tables update for the images that have snapshots.
- cluster_offset = bdrv_getlength(extent->file);
+ *cluster_offset = bdrv_getlength(extent->file);
bdrv_truncate(
extent->file,
- cluster_offset + (extent->cluster_sectors << 9)
+ *cluster_offset + (extent->cluster_sectors << 9)
);
- cluster_offset >>= 9;
- tmp = cpu_to_le32(cluster_offset);
+ *cluster_offset >>= 9;
+ tmp = cpu_to_le32(*cluster_offset);
l2_table[l2_index] = tmp;
/* First of all we write grain itself, to avoid race condition
@@ -764,8 +770,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
* or inappropriate VM shutdown.
*/
if (get_whole_cluster(
- bs, extent, cluster_offset, offset, allocate) == -1)
- return 0;
+ bs, extent, *cluster_offset, offset, allocate) == -1)
+ return -1;
if (m_data) {
m_data->offset = tmp;
@@ -775,8 +781,8 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
m_data->valid = 1;
}
}
- cluster_offset <<= 9;
- return cluster_offset;
+ *cluster_offset <<= 9;
+ return 0;
}
static VmdkExtent *find_extent(BDRVVmdkState *s,
@@ -800,7 +806,6 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
int nb_sectors, int *pnum)
{
BDRVVmdkState *s = bs->opaque;
-
int64_t index_in_cluster, n, ret;
uint64_t offset;
VmdkExtent *extent;
@@ -809,15 +814,13 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
if (!extent) {
return 0;
}
- if (extent->flat) {
- n = extent->end_sector - sector_num;
- ret = 1;
- } else {
- offset = get_cluster_offset(bs, extent, NULL, sector_num * 512, 0);
- index_in_cluster = sector_num % extent->cluster_sectors;
- n = extent->cluster_sectors - index_in_cluster;
- ret = offset ? 1 : 0;
- }
+ ret = get_cluster_offset(bs, extent, NULL,
+ sector_num * 512, 0, &offset);
+ /* get_cluster_offset returning 0 means success */
+ ret = !ret;
+
+ index_in_cluster = sector_num % extent->cluster_sectors;
+ n = extent->cluster_sectors - index_in_cluster;
if (n > nb_sectors)
n = nb_sectors;
*pnum = n;
@@ -838,14 +841,15 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
if (!extent) {
return -EIO;
}
- cluster_offset = get_cluster_offset(
- bs, extent, NULL, sector_num << 9, 0);
+ ret = get_cluster_offset(
+ bs, extent, NULL,
+ sector_num << 9, 0, &cluster_offset);
index_in_cluster = sector_num % extent->cluster_sectors;
n = extent->cluster_sectors - index_in_cluster;
if (n > nb_sectors)
n = nb_sectors;
- if (!cluster_offset) {
- // try to read from parent image, if exist
+ if (ret) {
+ /* if not allocated, try to read from parent image, if exist */
if (bs->backing_hd) {
if (!vmdk_is_cid_valid(bs))
return -1;
@@ -871,7 +875,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
{
BDRVVmdkState *s = bs->opaque;
VmdkExtent *extent = NULL;
- int n;
+ int n, ret;
int64_t index_in_cluster;
uint64_t cluster_offset;
VmdkMetaData m_data;
@@ -889,13 +893,14 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
if (!extent) {
return -EIO;
}
- cluster_offset = get_cluster_offset(
+ ret = get_cluster_offset(
bs,
extent,
&m_data,
- sector_num << 9, 1);
- if (!cluster_offset) {
- return -1;
+ sector_num << 9, 1,
+ &cluster_offset);
+ if (ret) {
+ return -EINVAL;
}
index_in_cluster = sector_num % extent->cluster_sectors;
n = extent->cluster_sectors - index_in_cluster;
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 08/12] VMDK: change get_cluster_offset return type
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 08/12] VMDK: change get_cluster_offset return type Fam Zheng
@ 2011-07-01 16:40 ` Kevin Wolf
0 siblings, 0 replies; 21+ messages in thread
From: Kevin Wolf @ 2011-07-01 16:40 UTC (permalink / raw)
To: Fam Zheng; +Cc: stefanha, qemu-devel, hch
Am 01.07.2011 06:55, schrieb Fam Zheng:
> The return type of get_cluster_offset was an offset that use 0 to denote
> 'not allocated', this will be no longer true for flat extents, as we see
> flat extent file as a single huge cluster whose offset is 0 and length
> is the whole file length.
> So now we use int return value, 0 means success and otherwise offset
> invalid.
>
> Signed-off-by: Fam Zheng <famcool@gmail.com>
> ---
> block/vmdk.c | 73 +++++++++++++++++++++++++++++++---------------------------
> 1 files changed, 39 insertions(+), 34 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index 8783629..40e4464 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -685,18 +685,23 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
> return 0;
> }
>
> -static uint64_t get_cluster_offset(BlockDriverState *bs,
> +static int get_cluster_offset(BlockDriverState *bs,
> VmdkExtent *extent,
> VmdkMetaData *m_data,
> - uint64_t offset, int allocate)
> + uint64_t offset,
> + int allocate,
> + uint64_t *cluster_offset)
> {
> unsigned int l1_index, l2_offset, l2_index;
> int min_index, i, j;
> uint32_t min_count, *l2_table, tmp = 0;
> - uint64_t cluster_offset;
>
> if (m_data)
> m_data->valid = 0;
> + if (extent->flat) {
> + *cluster_offset = 0;
> + return 0;
> + }
>
> l1_index = (offset >> 9) / extent->l1_entry_sectors;
> if (l1_index >= extent->l1_size) {
Let me complete what comes next:
l1_index = (offset >> 9) / extent->l1_entry_sectors;
if (l1_index >= extent->l1_size) {
return 0;
}
l2_offset = extent->l1_table[l1_index];
if (!l2_offset) {
return 0;
}
Shouldn't these returns be changed to -1? Also there is a return 0;
after a failed read, which doesn't seem to be changed.
Kevin
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v6 09/12] VMDK: open/read/write for monolithicFlat image
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (7 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 08/12] VMDK: change get_cluster_offset return type Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 5:23 ` Stefan Hajnoczi
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 10/12] VMDK: create different subformats Fam Zheng
` (3 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Parse vmdk decriptor file and open mono flat image.
Read/write the flat extent.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 184 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 170 insertions(+), 14 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 40e4464..940c63f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -65,6 +65,7 @@ typedef struct VmdkExtent {
bool flat;
int64_t sectors;
int64_t end_sector;
+ int64_t flat_start_offset;
int64_t l1_table_offset;
int64_t l1_backup_table_offset;
uint32_t *l1_table;
@@ -403,9 +404,10 @@ fail:
static int vmdk_parent_open(BlockDriverState *bs)
{
char *p_name;
- char desc[DESC_SIZE];
+ char desc[DESC_SIZE + 1];
BDRVVmdkState *s = bs->opaque;
+ desc[DESC_SIZE] = '\0';
if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
return -1;
}
@@ -600,10 +602,158 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
}
return 0;
fail:
- qemu_free(s->extents);
+ vmdk_free_extents(bs);
return ret;
}
+/* find an option value out of descriptor file */
+static int vmdk_parse_description(const char *desc, const char *opt_name,
+ char *buf, int buf_size)
+{
+ char *opt_pos, *opt_end;
+ const char *end = desc + strlen(desc);
+
+ opt_pos = strstr(desc, opt_name);
+ if (!opt_pos) {
+ return -1;
+ }
+ /* Skip "=\"" following opt_name */
+ opt_pos += strlen(opt_name) + 2;
+ if (opt_pos >= end) {
+ return -1;
+ }
+ opt_end = opt_pos;
+ while (opt_end < end && *opt_end != '"') {
+ opt_end++;
+ }
+ if (opt_end == end || buf_size < opt_end - opt_pos + 1) {
+ return -1;
+ }
+ pstrcpy(buf, opt_end - opt_pos + 1, opt_pos);
+ return 0;
+}
+
+static int vmdk_parse_extents(const char *desc, BlockDriverState *bs,
+ const char *desc_file_path)
+{
+ int ret;
+ char access[11];
+ char type[11];
+ char fname[512];
+ const char *p = desc;
+ int64_t sectors = 0;
+ int64_t flat_offset;
+
+ while (*p) {
+ /* parse extent line:
+ * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
+ * or
+ * RW [size in sectors] SPARSE "file-name.vmdk"
+ */
+ flat_offset = -1;
+ ret = sscanf(p, "%10s %lld %10s %512s",
+ access, §ors, type, fname);
+ if (ret != 4) {
+ goto next_line;
+ }
+ if (!strcmp(type, "FLAT")) {
+ ret = sscanf(p, "%10s %lld %10s %511s %lld",
+ access, §ors, type, fname, &flat_offset);
+ if (ret != 5 || flat_offset < 0) {
+ return -EINVAL;
+ }
+ }
+
+ /* trim the quotation marks around */
+ if (fname[0] == '"') {
+ memmove(fname, fname + 1, strlen(fname));
+ if (strlen(fname) <= 1 || fname[strlen(fname) - 1] != '"') {
+ return -EINVAL;
+ }
+ fname[strlen(fname) - 1] = '\0';
+ }
+ if (sectors <= 0 ||
+ (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) ||
+ (strcmp(access, "RW"))) {
+ goto next_line;
+ }
+
+ /* save to extents array */
+ if (!strcmp(type, "FLAT")) {
+ /* FLAT extent */
+ char extent_path[PATH_MAX];
+ BlockDriverState *extent_file;
+ BlockDriver *drv;
+ VmdkExtent *extent;
+
+ extent_file = bdrv_new("");
+ drv = bdrv_find_format("file");
+ if (!drv) {
+ bdrv_delete(extent_file);
+ return -EINVAL;
+ }
+ path_combine(extent_path, sizeof(extent_path),
+ desc_file_path, fname);
+ ret = bdrv_open(extent_file, extent_path,
+ bs->open_flags | BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
+ if (ret) {
+ bdrv_delete(extent_file);
+ return -EINVAL;
+ }
+ extent = vmdk_add_extent(bs, extent_file, true, sectors,
+ 0, 0, 0, 0, sectors);
+ extent->flat_start_offset = flat_offset;
+ } else {
+ /* SPARSE extent, not supported for now */
+ fprintf(stderr,
+ "VMDK: Not supported extent type \"%s\""".\n", type);
+ return -ENOTSUP;
+ }
+next_line:
+ /* move to next line */
+ while (*p && *p != '\n') {
+ p++;
+ }
+ p++;
+ }
+ return 0;
+}
+
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
+{
+ int ret;
+ char buf[2048];
+ char ct[128];
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+ buf[2047] = '\0';
+ if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
+ return -EINVAL;
+ }
+ if (strcmp(ct, "monolithicFlat")) {
+ fprintf(stderr,
+ "VMDK: Not supported image type \"%s\""".\n", ct);
+ return -ENOTSUP;
+ }
+ s->desc_offset = 0;
+ ret = vmdk_parse_extents(buf, bs, bs->file->filename);
+ if (ret) {
+ return ret;
+ }
+
+ /* try to open parent images, if exist */
+ if (vmdk_parent_open(bs)) {
+ qemu_free(s->extents);
+ return -EINVAL;
+ }
+ s->parent_cid = vmdk_read_cid(bs, 1);
+ return 0;
+}
+
static int vmdk_open(BlockDriverState *bs, int flags)
{
uint32_t magic;
@@ -618,7 +768,7 @@ static int vmdk_open(BlockDriverState *bs, int flags)
} else if (magic == VMDK4_MAGIC) {
return vmdk_open_vmdk4(bs, flags);
} else {
- return -EINVAL;
+ return vmdk_open_desc_file(bs, flags);
}
}
@@ -699,7 +849,7 @@ static int get_cluster_offset(BlockDriverState *bs,
if (m_data)
m_data->valid = 0;
if (extent->flat) {
- *cluster_offset = 0;
+ *cluster_offset = extent->flat_start_offset;
return 0;
}
@@ -852,16 +1002,20 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
/* if not allocated, try to read from parent image, if exist */
if (bs->backing_hd) {
if (!vmdk_is_cid_valid(bs))
- return -1;
+ return -EINVAL;
ret = bdrv_read(bs->backing_hd, sector_num, buf, n);
if (ret < 0)
- return -1;
+ return ret;
} else {
memset(buf, 0, 512 * n);
}
} else {
- if(bdrv_pread(bs->file, cluster_offset + index_in_cluster * 512, buf, n * 512) != n * 512)
- return -1;
+ ret = bdrv_pread(extent->file,
+ cluster_offset + index_in_cluster * 512,
+ buf, n * 512);
+ if (ret != n * 512) {
+ return ret;
+ }
}
nb_sectors -= n;
sector_num += n;
@@ -885,7 +1039,7 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
"(VMDK) Wrong offset: sector_num=0x%" PRIx64
" total_sectors=0x%" PRIx64 "\n",
sector_num, bs->total_sectors);
- return -1;
+ return -EIO;
}
while (nb_sectors > 0) {
@@ -908,16 +1062,18 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
n = nb_sectors;
}
- if (bdrv_pwrite(bs->file,
+ ret = bdrv_pwrite(extent->file,
cluster_offset + index_in_cluster * 512,
- buf, n * 512)
- != n * 512) {
- return -1;
+ buf,
+ n * 512);
+ if (ret != n * 512) {
+ ret = ret < 0 ? ret : -EIO;
+ return ret;
}
if (m_data.valid) {
/* update L2 tables */
if (vmdk_L2update(extent, &m_data) == -1) {
- return -1;
+ return -EIO;
}
}
nb_sectors -= n;
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 09/12] VMDK: open/read/write for monolithicFlat image
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-07-01 5:23 ` Stefan Hajnoczi
0 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2011-07-01 5:23 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Fri, Jul 1, 2011 at 5:55 AM, Fam Zheng <famcool@gmail.com> wrote:
> + /* save to extents array */
> + if (!strcmp(type, "FLAT")) {
> + /* FLAT extent */
> + char extent_path[PATH_MAX];
> + BlockDriverState *extent_file;
> + BlockDriver *drv;
> + VmdkExtent *extent;
> +
> + extent_file = bdrv_new("");
> + drv = bdrv_find_format("file");
> + if (!drv) {
> + bdrv_delete(extent_file);
> + return -EINVAL;
> + }
> + path_combine(extent_path, sizeof(extent_path),
> + desc_file_path, fname);
> + ret = bdrv_open(extent_file, extent_path,
> + bs->open_flags | BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
Why are you forcing BDRV_O_RDWR and BDRV_O_NO_BACKING? It should be
possible to open a vmdk file readonly.
By the way, you can shortcut the bdrv_find_format()/bdrv_open() using
bdrv_open_file(). That will simplify things a little.
BDRV_O_NO_BACKING is not needed with the "file" driver (which doesn't
support backing files).
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread
* [Qemu-devel] [PATCH v6 10/12] VMDK: create different subformats
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (8 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 11/12] VMDK: fix coding style Fam Zheng
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Add create option 'format', with enums:
monolithicSparse
monolithicFlat
twoGbMaxExtentSparse
twoGbMaxExtentFlat
Each creates a subformat image file. The default is monolithiSparse.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 559 ++++++++++++++++++++++++++++++++++------------------------
block_int.h | 1 +
2 files changed, 329 insertions(+), 231 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 940c63f..2ea9a7f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -154,8 +154,8 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
#define CHECK_CID 1
#define SECTOR_SIZE 512
-#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each
-#define HEADER_SIZE 512 // first sector of 512 bytes
+#define DESC_SIZE (20 * SECTOR_SIZE) /* 20 sectors of 512 bytes each */
+#define HEADER_SIZE 512 /* first sector of 512 bytes */
static void vmdk_free_extents(BlockDriverState *bs)
{
@@ -240,167 +240,6 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
return 1;
}
-static int vmdk_snapshot_create(const char *filename, const char *backing_file)
-{
- int snp_fd, p_fd;
- int ret;
- uint32_t p_cid;
- char *p_name, *gd_buf, *rgd_buf;
- const char *real_filename, *temp_str;
- VMDK4Header header;
- uint32_t gde_entries, gd_size;
- int64_t gd_offset, rgd_offset, capacity, gt_size;
- char p_desc[DESC_SIZE], s_desc[DESC_SIZE], hdr[HEADER_SIZE];
- static const char desc_template[] =
- "# Disk DescriptorFile\n"
- "version=1\n"
- "CID=%x\n"
- "parentCID=%x\n"
- "createType=\"monolithicSparse\"\n"
- "parentFileNameHint=\"%s\"\n"
- "\n"
- "# Extent description\n"
- "RW %u SPARSE \"%s\"\n"
- "\n"
- "# The Disk Data Base \n"
- "#DDB\n"
- "\n";
-
- snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644);
- if (snp_fd < 0)
- return -errno;
- p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE);
- if (p_fd < 0) {
- close(snp_fd);
- return -errno;
- }
-
- /* read the header */
- if (lseek(p_fd, 0x0, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) {
- ret = -errno;
- goto fail;
- }
-
- /* write the header */
- if (lseek(snp_fd, 0x0, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (write(snp_fd, hdr, HEADER_SIZE) == -1) {
- ret = -errno;
- goto fail;
- }
-
- memset(&header, 0, sizeof(header));
- memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
-
- if (ftruncate(snp_fd, header.grain_offset << 9)) {
- ret = -errno;
- goto fail;
- }
- if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) {
- ret = -errno;
- goto fail;
- }
-
- if ((p_name = strstr(p_desc,"CID")) != NULL) {
- p_name += sizeof("CID");
- sscanf(p_name,"%x",&p_cid);
- }
-
- real_filename = filename;
- if ((temp_str = strrchr(real_filename, '\\')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, '/')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, ':')) != NULL)
- real_filename = temp_str + 1;
-
- snprintf(s_desc, sizeof(s_desc), desc_template, p_cid, p_cid, backing_file,
- (uint32_t)header.capacity, real_filename);
-
- /* write the descriptor */
- if (lseek(snp_fd, 0x200, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (write(snp_fd, s_desc, strlen(s_desc)) == -1) {
- ret = -errno;
- goto fail;
- }
-
- gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table
- rgd_offset = header.rgd_offset * SECTOR_SIZE; // offset of RGD table
- capacity = header.capacity * SECTOR_SIZE; // Extent size
- /*
- * Each GDE span 32M disk, means:
- * 512 GTE per GT, each GTE points to grain
- */
- gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * SECTOR_SIZE;
- if (!gt_size) {
- ret = -EINVAL;
- goto fail;
- }
- gde_entries = (uint32_t)(capacity / gt_size); // number of gde/rgde
- gd_size = gde_entries * sizeof(uint32_t);
-
- /* write RGD */
- rgd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
- if (read(p_fd, rgd_buf, gd_size) != gd_size) {
- ret = -errno;
- goto fail_rgd;
- }
- if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
- if (write(snp_fd, rgd_buf, gd_size) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
-
- /* write GD */
- gd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, gd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- if (read(p_fd, gd_buf, gd_size) != gd_size) {
- ret = -errno;
- goto fail_gd;
- }
- if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- if (write(snp_fd, gd_buf, gd_size) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- ret = 0;
-
-fail_gd:
- qemu_free(gd_buf);
-fail_rgd:
- qemu_free(rgd_buf);
-fail:
- close(p_fd);
- close(snp_fd);
- return ret;
-}
-
static int vmdk_parent_open(BlockDriverState *bs)
{
char *p_name;
@@ -1089,68 +928,32 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
return 0;
}
-static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+
+static int vmdk_create_sparse(const char *filename, int64_t filesize)
{
- int fd, i;
+ int ret, i;
+ int fd = 0;
VMDK4Header header;
uint32_t tmp, magic, grains, gd_size, gt_size, gt_count;
- static const char desc_template[] =
- "# Disk DescriptorFile\n"
- "version=1\n"
- "CID=%x\n"
- "parentCID=ffffffff\n"
- "createType=\"monolithicSparse\"\n"
- "\n"
- "# Extent description\n"
- "RW %" PRId64 " SPARSE \"%s\"\n"
- "\n"
- "# The Disk Data Base \n"
- "#DDB\n"
- "\n"
- "ddb.virtualHWVersion = \"%d\"\n"
- "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
- "ddb.geometry.heads = \"16\"\n"
- "ddb.geometry.sectors = \"63\"\n"
- "ddb.adapterType = \"ide\"\n";
- char desc[1024];
- const char *real_filename, *temp_str;
- int64_t total_size = 0;
- const char *backing_file = NULL;
- int flags = 0;
- int ret;
- // Read out options
- while (options && options->name) {
- if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
- total_size = options->value.n / 512;
- } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
- backing_file = options->value.s;
- } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
- flags |= options->value.n ? BLOCK_FLAG_COMPAT6: 0;
- }
- options++;
- }
-
- /* XXX: add support for backing file */
- if (backing_file) {
- return vmdk_snapshot_create(filename, backing_file);
- }
-
- fd = open(filename, O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
- 0644);
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
if (fd < 0)
return -errno;
magic = cpu_to_be32(VMDK4_MAGIC);
memset(&header, 0, sizeof(header));
header.version = 1;
header.flags = 3; /* ?? */
- header.capacity = total_size;
+ header.capacity = filesize / 512;
header.granularity = 128;
header.num_gtes_per_gte = 512;
- grains = (total_size + header.granularity - 1) / header.granularity;
+ grains = (filesize / 512 + header.granularity - 1) / header.granularity;
gt_size = ((header.num_gtes_per_gte * sizeof(uint32_t)) + 511) >> 9;
- gt_count = (grains + header.num_gtes_per_gte - 1) / header.num_gtes_per_gte;
+ gt_count =
+ (grains + header.num_gtes_per_gte - 1) / header.num_gtes_per_gte;
gd_size = (gt_count * sizeof(uint32_t) + 511) >> 9;
header.desc_offset = 1;
@@ -1161,7 +964,6 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
((header.gd_offset + gd_size + (gt_size * gt_count) +
header.granularity - 1) / header.granularity) *
header.granularity;
-
/* swap endianness for all header fields */
header.version = cpu_to_le32(header.version);
header.flags = cpu_to_le32(header.flags);
@@ -1219,28 +1021,316 @@ static int vmdk_create(const char *filename, QEMUOptionParameter *options)
}
}
- /* compose the descriptor */
- real_filename = filename;
- if ((temp_str = strrchr(real_filename, '\\')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, '/')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, ':')) != NULL)
- real_filename = temp_str + 1;
- snprintf(desc, sizeof(desc), desc_template, (unsigned int)time(NULL),
- total_size, real_filename,
- (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
- total_size / (int64_t)(63 * 16));
-
- /* write the descriptor */
- lseek(fd, le64_to_cpu(header.desc_offset) << 9, SEEK_SET);
- ret = qemu_write_full(fd, desc, strlen(desc));
- if (ret != strlen(desc)) {
+ filesize -= filesize;
+ ret = 0;
+ exit:
+ close(fd);
+ return ret;
+}
+
+static int vmdk_create_flat(const char *filename, int64_t filesize)
+{
+ int fd, ret;
+
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
+ if (fd < 0) {
+ return -errno;
+ }
+ ret = ftruncate(fd, filesize);
+ if (ret) {
ret = -errno;
- goto exit;
+ close(fd);
+ return -errno;
}
+ close(fd);
+ return 0;
+}
- ret = 0;
+static int filename_decompose(const char *filename, char *path, char *prefix,
+ char *postfix, int buf_len)
+{
+ const char *p, *q;
+
+ if (filename == NULL || !strlen(filename)) {
+ fprintf(stderr, "Vmdk: wrong filename (%s)\n", filename);
+ return -1;
+ }
+ p = strrchr(filename, '/');
+ if (p == NULL) {
+ p = strrchr(filename, '\\');
+ }
+ if (p == NULL) {
+ p = strrchr(filename, ':');
+ }
+ if (p != NULL) {
+ p++;
+ if (p - filename >= buf_len) {
+ return -1;
+ }
+ strncpy(path, filename, p - filename);
+ path[p - filename] = 0;
+ } else {
+ p = filename;
+ path[0] = '\0';
+ }
+ q = strrchr(p, '.');
+ if (q == NULL) {
+ pstrcpy(prefix, buf_len, p);
+ postfix[0] = '\0';
+ } else {
+ strncpy(prefix, p, q - p);
+ prefix[q - p] = '\0';
+ pstrcpy(postfix, buf_len, q);
+ }
+ return 0;
+}
+
+static int relative_path(char *dest, int dest_size,
+ const char *base, const char *target)
+{
+ int i = 0;
+ int n = 0;
+ const char *p, *q;
+#ifdef _WIN32
+ const char *sep = "\\";
+#else
+ const char *sep = "/";
+#endif
+
+ if (!(dest && base && target)) {
+ return -1;
+ }
+ if (path_is_absolute(target)) {
+ dest[dest_size - 1] = '\0';
+ strncpy(dest, target, dest_size - 1);
+ return 0;
+ }
+ while (base[i] == target[i]) {
+ i++;
+ }
+ p = &base[i];
+ q = &target[i];
+ while (*p) {
+ if (*p == *sep) {
+ n++;
+ }
+ p++;
+ }
+ dest[0] = '\0';
+ for (; n; n--) {
+ pstrcat(dest, dest_size, "..");
+ pstrcat(dest, dest_size, sep);
+ }
+ pstrcat(dest, dest_size, q);
+ return 0;
+}
+
+static int vmdk_create(const char *filename, QEMUOptionParameter *options)
+{
+ int fd = -1;
+ char desc[4096];
+ int64_t total_size = 0;
+ const char *backing_file = NULL;
+ const char *fmt = NULL;
+ int flags = 0;
+ int ret = 0;
+ char ext_desc_lines[1024] = "";
+ char path[1024], prefix[1024], postfix[1024];
+ const int64_t split_size = 0x80000000; /* VMDK has constant split size */
+ const char desc_template[] =
+ "# Disk DescriptorFile\n"
+ "version=1\n"
+ "CID=%x\n"
+ "parentCID=%x\n"
+ "createType=\"%s\"\n"
+ "%s"
+ "\n"
+ "# Extent description\n"
+ "%s"
+ "\n"
+ "# The Disk Data Base\n"
+ "#DDB\n"
+ "\n"
+ "ddb.virtualHWVersion = \"%d\"\n"
+ "ddb.geometry.cylinders = \"%" PRId64 "\"\n"
+ "ddb.geometry.heads = \"16\"\n"
+ "ddb.geometry.sectors = \"63\"\n"
+ "ddb.adapterType = \"ide\"\n";
+
+ if (filename_decompose(filename, path, prefix, postfix, 1024)) {
+ return -EINVAL;
+ }
+ /* Read out options */
+ while (options && options->name) {
+ if (!strcmp(options->name, BLOCK_OPT_SIZE)) {
+ total_size = options->value.n;
+ } else if (!strcmp(options->name, BLOCK_OPT_BACKING_FILE)) {
+ backing_file = options->value.s;
+ } else if (!strcmp(options->name, BLOCK_OPT_COMPAT6)) {
+ flags |= options->value.n ? BLOCK_FLAG_COMPAT6 : 0;
+ } else if (!strcmp(options->name, BLOCK_OPT_FMT)) {
+ fmt = options->value.s;
+ }
+ options++;
+ }
+ if (!fmt) {
+ fmt = "monolithicSparse";
+ }
+ if (!strcmp(fmt, "monolithicFlat") || !strcmp(fmt, "twoGbMaxExtentFlat")) {
+ bool split = strcmp(fmt, "monolithicFlat");
+ const char desc_line_templ[] = "RW %lld FLAT \"%s\" 0\n";
+ int64_t filesize = total_size;
+ int idx = 1;
+
+ if (backing_file) {
+ /* not supporting backing file for flat image */
+ return -ENOTSUP;
+ }
+ while (filesize > 0) {
+ char desc_line[1024];
+ char ext_filename[1024];
+ char desc_filename[1024];
+ int64_t size = filesize;
+
+ if (split && size > split_size) {
+ size = split_size;
+ }
+ if (split) {
+ sprintf(desc_filename, "%s-f%03d%s",
+ prefix, idx++, postfix);
+ sprintf(ext_filename, "%s%s",
+ path, desc_filename);
+ } else {
+ sprintf(desc_filename, "%s-flat%s",
+ prefix, postfix);
+ sprintf(ext_filename, "%s%s",
+ path, desc_filename);
+ }
+ if (vmdk_create_flat(ext_filename, size)) {
+ return -EINVAL;
+ }
+ filesize -= size;
+
+ /* Format description line */
+ snprintf(desc_line, 1024,
+ desc_line_templ, size / 512, desc_filename);
+ pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line);
+ }
+
+ /* generate descriptor file */
+ snprintf(desc, sizeof(desc), desc_template,
+ (unsigned int)time(NULL),
+ 0xffffffff,
+ fmt,
+ "",
+ ext_desc_lines,
+ (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+ total_size / (int64_t)(63 * 16 * 512));
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
+ if (fd < 0) {
+ return -errno;
+ }
+ ret = qemu_write_full(fd, desc, strlen(desc));
+ if (ret != strlen(desc)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = 0;
+ } else if (!strcmp(fmt, "monolithicSparse")
+ || !strcmp(fmt, "twoGbMaxExtentSparse")) {
+ int ret;
+ int fd = 0;
+ int idx = 1;
+ int64_t filesize = total_size;
+ const char desc_line_templ[] = "RW %lld SPARSE \"%s\"\n";
+ char desc_line[1024];
+ char desc_filename[1024];
+ char ext_filename[1024];
+ bool split = strcmp(fmt, "monolithicSparse");
+ char parent_desc_line[1024] = "";
+ uint32_t parent_cid = 0xffffffff;
+
+ if (backing_file) {
+ char parent_filename[1024];
+ BlockDriverState *bs = bdrv_new("");
+ ret = bdrv_open(bs, backing_file, 0, NULL);
+ if (ret != 0) {
+ bdrv_delete(bs);
+ return ret;
+ }
+ filesize = bdrv_getlength(bs);
+ parent_cid = vmdk_read_cid(bs, 0);
+ bdrv_delete(bs);
+ relative_path(parent_filename, 1024, filename, backing_file);
+ snprintf(parent_desc_line, sizeof(parent_desc_line),
+ "parentFileNameHint=\"%s\"", parent_filename);
+ }
+ while (filesize > 0) {
+ int64_t size = filesize;
+ if (split && size > split_size) {
+ size = split_size;
+ }
+
+ if (split) {
+ snprintf(desc_filename, 1024,
+ "%s-s%03d%s", prefix, idx++, postfix);
+ snprintf(ext_filename, 1024, "%s%s", path, desc_filename);
+ } else {
+ snprintf(desc_filename, 1024, "%s%s", prefix, postfix);
+ snprintf(ext_filename, 1024, "%s%s", path, desc_filename);
+ }
+ ret = vmdk_create_sparse(ext_filename, size);
+ if (ret != 0) {
+ return ret;
+ }
+ filesize -= size;
+ snprintf(desc_line, 1024,
+ desc_line_templ, size / 512, desc_filename);
+ pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line);
+ }
+ /* generate descriptor file */
+ snprintf(desc, sizeof(desc), desc_template,
+ (unsigned int)time(NULL),
+ parent_cid,
+ fmt,
+ parent_desc_line,
+ ext_desc_lines,
+ (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+ total_size / (int64_t)(63 * 16 * 512));
+ if (split) {
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
+ } else {
+ fd = open(
+ filename,
+ O_WRONLY | O_BINARY | O_LARGEFILE,
+ 0644);
+ }
+ if (fd < 0) {
+ return -errno;
+ }
+ if (!split && 0x200 != lseek(fd, 0x200, SEEK_SET)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = qemu_write_full(fd, desc, strlen(desc));
+ if (ret != strlen(desc)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = 0;
+ } else {
+ fprintf(stderr, "Vmdk: unsupported format (%s)\n", fmt);
+ return -ENOTSUP;
+ }
exit:
close(fd);
return ret;
@@ -1283,6 +1373,13 @@ static QEMUOptionParameter vmdk_create_options[] = {
.type = OPT_FLAG,
.help = "VMDK version 6 image"
},
+ {
+ .name = BLOCK_OPT_FMT,
+ .type = OPT_STRING,
+ .help =
+ "VMDK flat extent format, can be one of "
+ "{monolithicSparse (default) | monolithicFlat | twoGbMaxExtentSparse | twoGbMaxExtentFlat} "
+ },
{ NULL }
};
diff --git a/block_int.h b/block_int.h
index 1e265d2..e870c33 100644
--- a/block_int.h
+++ b/block_int.h
@@ -39,6 +39,7 @@
#define BLOCK_OPT_CLUSTER_SIZE "cluster_size"
#define BLOCK_OPT_TABLE_SIZE "table_size"
#define BLOCK_OPT_PREALLOC "preallocation"
+#define BLOCK_OPT_FMT "format"
typedef struct AIOPool {
void (*cancel)(BlockDriverAIOCB *acb);
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 11/12] VMDK: fix coding style
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (9 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 10/12] VMDK: create different subformats Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 12/12] block: add bdrv_get_allocated_file_size() operation Fam Zheng
2011-07-01 5:25 ` [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Stefan Hajnoczi
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Conform coding style in vmdk.c to pass scripts/checkpatch.pl checks.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 79 +++++++++++++++++++++++++++++++++++----------------------
1 files changed, 48 insertions(+), 31 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 2ea9a7f..6bf242c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -101,8 +101,9 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
- if (buf_size < 4)
+ if (buf_size < 4) {
return 0;
+ }
magic = be32_to_cpu(*(uint32_t *)buf);
if (magic == VMDK3_MAGIC ||
magic == VMDK4_MAGIC) {
@@ -190,9 +191,10 @@ static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
cid_str_size = sizeof("CID");
}
- if ((p_name = strstr(desc,cid_str)) != NULL) {
+ p_name = strstr(desc, cid_str);
+ if (p_name != NULL) {
p_name += cid_str_size;
- sscanf(p_name,"%x",&cid);
+ sscanf(p_name, "%x", &cid);
}
return cid;
@@ -208,9 +210,10 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
return -EIO;
}
- tmp_str = strstr(desc,"parentCID");
+ tmp_str = strstr(desc, "parentCID");
pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str);
- if ((p_name = strstr(desc,"CID")) != NULL) {
+ p_name = strstr(desc, "CID");
+ if (p_name != NULL) {
p_name += sizeof("CID");
snprintf(p_name, sizeof(desc) - (p_name - desc), "%x\n", cid);
pstrcat(desc, sizeof(desc), tmp_desc);
@@ -230,13 +233,14 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
uint32_t cur_pcid;
if (p_bs) {
- cur_pcid = vmdk_read_cid(p_bs,0);
- if (s->parent_cid != cur_pcid)
- // CID not valid
+ cur_pcid = vmdk_read_cid(p_bs, 0);
+ if (s->parent_cid != cur_pcid) {
+ /* CID not valid */
return 0;
+ }
}
#endif
- // CID valid
+ /* CID valid */
return 1;
}
@@ -251,14 +255,18 @@ static int vmdk_parent_open(BlockDriverState *bs)
return -1;
}
- if ((p_name = strstr(desc,"parentFileNameHint")) != NULL) {
+ p_name = strstr(desc, "parentFileNameHint");
+ if (p_name != NULL) {
char *end_name;
p_name += sizeof("parentFileNameHint") + 1;
- if ((end_name = strchr(p_name,'\"')) == NULL)
+ end_name = strchr(p_name, '\"');
+ if (end_name == NULL) {
return -1;
- if ((end_name - p_name) > sizeof (bs->backing_file) - 1)
+ }
+ if ((end_name - p_name) > sizeof(bs->backing_file) - 1) {
return -1;
+ }
pstrcpy(bs->backing_file, end_name - p_name + 1, p_name);
}
@@ -625,8 +633,9 @@ static int get_whole_cluster(BlockDriverState *bs,
if (bs->backing_hd) {
int ret;
- if (!vmdk_is_cid_valid(bs))
+ if (!vmdk_is_cid_valid(bs)) {
return -1;
+ }
/* floor offset to cluster */
offset -= offset % (extent->cluster_sectors * 512);
@@ -685,8 +694,9 @@ static int get_cluster_offset(BlockDriverState *bs,
int min_index, i, j;
uint32_t min_count, *l2_table, tmp = 0;
- if (m_data)
+ if (m_data) {
m_data->valid = 0;
+ }
if (extent->flat) {
*cluster_offset = extent->flat_start_offset;
return 0;
@@ -742,7 +752,7 @@ static int get_cluster_offset(BlockDriverState *bs,
return -1;
}
- // Avoid the L2 tables update for the images that have snapshots.
+ /* Avoid the L2 tables update for the images that have snapshots. */
*cluster_offset = bdrv_getlength(extent->file);
bdrv_truncate(
extent->file,
@@ -759,8 +769,9 @@ static int get_cluster_offset(BlockDriverState *bs,
* or inappropriate VM shutdown.
*/
if (get_whole_cluster(
- bs, extent, *cluster_offset, offset, allocate) == -1)
+ bs, extent, *cluster_offset, offset, allocate) == -1) {
return -1;
+ }
if (m_data) {
m_data->offset = tmp;
@@ -810,8 +821,9 @@ static int vmdk_is_allocated(BlockDriverState *bs, int64_t sector_num,
index_in_cluster = sector_num % extent->cluster_sectors;
n = extent->cluster_sectors - index_in_cluster;
- if (n > nb_sectors)
+ if (n > nb_sectors) {
n = nb_sectors;
+ }
*pnum = n;
return ret;
}
@@ -835,16 +847,19 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
sector_num << 9, 0, &cluster_offset);
index_in_cluster = sector_num % extent->cluster_sectors;
n = extent->cluster_sectors - index_in_cluster;
- if (n > nb_sectors)
+ if (n > nb_sectors) {
n = nb_sectors;
+ }
if (ret) {
/* if not allocated, try to read from parent image, if exist */
if (bs->backing_hd) {
- if (!vmdk_is_cid_valid(bs))
+ if (!vmdk_is_cid_valid(bs)) {
return -EINVAL;
+ }
ret = bdrv_read(bs->backing_hd, sector_num, buf, n);
- if (ret < 0)
+ if (ret < 0) {
return ret;
+ }
} else {
memset(buf, 0, 512 * n);
}
@@ -919,7 +934,8 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
sector_num += n;
buf += n * 512;
- // update CID on the first write every time the virtual disk is opened
+ /* update CID on the first write every time the virtual disk is
+ * opened */
if (!s->cid_updated) {
vmdk_write_cid(bs, time(NULL));
s->cid_updated = true;
@@ -940,8 +956,9 @@ static int vmdk_create_sparse(const char *filename, int64_t filesize)
filename,
O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
0644);
- if (fd < 0)
+ if (fd < 0) {
return -errno;
+ }
magic = cpu_to_be32(VMDK4_MAGIC);
memset(&header, 0, sizeof(header));
header.version = 1;
@@ -1384,16 +1401,16 @@ static QEMUOptionParameter vmdk_create_options[] = {
};
static BlockDriver bdrv_vmdk = {
- .format_name = "vmdk",
- .instance_size = sizeof(BDRVVmdkState),
- .bdrv_probe = vmdk_probe,
+ .format_name = "vmdk",
+ .instance_size = sizeof(BDRVVmdkState),
+ .bdrv_probe = vmdk_probe,
.bdrv_open = vmdk_open,
- .bdrv_read = vmdk_read,
- .bdrv_write = vmdk_write,
- .bdrv_close = vmdk_close,
- .bdrv_create = vmdk_create,
- .bdrv_flush = vmdk_flush,
- .bdrv_is_allocated = vmdk_is_allocated,
+ .bdrv_read = vmdk_read,
+ .bdrv_write = vmdk_write,
+ .bdrv_close = vmdk_close,
+ .bdrv_create = vmdk_create,
+ .bdrv_flush = vmdk_flush,
+ .bdrv_is_allocated = vmdk_is_allocated,
.create_options = vmdk_create_options,
};
^ permalink raw reply related [flat|nested] 21+ messages in thread* [Qemu-devel] [PATCH v6 12/12] block: add bdrv_get_allocated_file_size() operation
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (10 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 11/12] VMDK: fix coding style Fam Zheng
@ 2011-07-01 4:55 ` Fam Zheng
2011-07-01 5:25 ` [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Stefan Hajnoczi
12 siblings, 0 replies; 21+ messages in thread
From: Fam Zheng @ 2011-07-01 4:55 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
qemu-img.c wants to count allocated file size of image. Previously it
counts a single bs->file by 'stat' or Window API. As VMDK introduces
multiple file support, the operation becomes format specific with
platform specific meanwhile.
The functions are moved to block/raw-{posix,win32}.c and qemu-img.c calls
bdrv_get_allocated_file_size to count the bs. And also added VMDK code
to count his own extents.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block.c | 19 +++++++++++++++++++
block.h | 1 +
block/raw-posix.c | 21 +++++++++++++++++++++
block/raw-win32.c | 29 +++++++++++++++++++++++++++++
block/vmdk.c | 24 ++++++++++++++++++++++++
block_int.h | 1 +
qemu-img.c | 31 +------------------------------
7 files changed, 96 insertions(+), 30 deletions(-)
diff --git a/block.c b/block.c
index 24a25d5..9549b9e 100644
--- a/block.c
+++ b/block.c
@@ -1147,6 +1147,25 @@ int bdrv_truncate(BlockDriverState *bs, int64_t offset)
}
/**
+ * Length of a allocated file in bytes. Sparse files are counted by actual
+ * allocated space. Return < 0 if error or unknown.
+ */
+int64_t bdrv_get_allocated_file_size(BlockDriverState *bs)
+{
+ BlockDriver *drv = bs->drv;
+ if (!drv) {
+ return -ENOMEDIUM;
+ }
+ if (drv->bdrv_get_allocated_file_size) {
+ return drv->bdrv_get_allocated_file_size(bs);
+ }
+ if (bs->file) {
+ return bdrv_get_allocated_file_size(bs->file);
+ }
+ return -ENOTSUP;
+}
+
+/**
* Length of a file in bytes. Return < 0 if error or unknown.
*/
int64_t bdrv_getlength(BlockDriverState *bs)
diff --git a/block.h b/block.h
index 859d1d9..59cc410 100644
--- a/block.h
+++ b/block.h
@@ -89,6 +89,7 @@ int bdrv_write_sync(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
int bdrv_truncate(BlockDriverState *bs, int64_t offset);
int64_t bdrv_getlength(BlockDriverState *bs);
+int64_t bdrv_get_allocated_file_size(BlockDriverState *bs);
void bdrv_get_geometry(BlockDriverState *bs, uint64_t *nb_sectors_ptr);
void bdrv_guess_geometry(BlockDriverState *bs, int *pcyls, int *pheads, int *psecs);
int bdrv_commit(BlockDriverState *bs);
diff --git a/block/raw-posix.c b/block/raw-posix.c
index 4cd7d7a..911cc0d 100644
--- a/block/raw-posix.c
+++ b/block/raw-posix.c
@@ -791,6 +791,17 @@ static int64_t raw_getlength(BlockDriverState *bs)
}
#endif
+static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
+{
+ struct stat st;
+ BDRVRawState *s = bs->opaque;
+
+ if (fstat(s->fd, &st) < 0) {
+ return -errno;
+ }
+ return (int64_t)st.st_blocks * 512;
+}
+
static int raw_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
@@ -886,6 +897,8 @@ static BlockDriver bdrv_file = {
.bdrv_truncate = raw_truncate,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
.create_options = raw_create_options,
};
@@ -1154,6 +1167,8 @@ static BlockDriver bdrv_host_device = {
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
/* generic scsi device */
#ifdef __linux__
@@ -1269,6 +1284,8 @@ static BlockDriver bdrv_host_floppy = {
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
/* removable device support */
.bdrv_is_inserted = floppy_is_inserted,
@@ -1366,6 +1383,8 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
/* removable device support */
.bdrv_is_inserted = cdrom_is_inserted,
@@ -1489,6 +1508,8 @@ static BlockDriver bdrv_host_cdrom = {
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
/* removable device support */
.bdrv_is_inserted = cdrom_is_inserted,
diff --git a/block/raw-win32.c b/block/raw-win32.c
index 56bd719..91067e7 100644
--- a/block/raw-win32.c
+++ b/block/raw-win32.c
@@ -213,6 +213,31 @@ static int64_t raw_getlength(BlockDriverState *bs)
return l.QuadPart;
}
+static int64_t raw_get_allocated_file_size(BlockDriverState *bs)
+{
+ typedef DWORD (WINAPI * get_compressed_t)(const char *filename,
+ DWORD * high);
+ get_compressed_t get_compressed;
+ struct _stati64 st;
+ const char *filename = bs->filename;
+ /* WinNT support GetCompressedFileSize to determine allocate size */
+ get_compressed =
+ (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"),
+ "GetCompressedFileSizeA");
+ if (get_compressed) {
+ DWORD high, low;
+ low = get_compressed(filename, &high);
+ if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR) {
+ return (((int64_t) high) << 32) + low;
+ }
+ }
+
+ if (_stati64(filename, &st) < 0) {
+ return -1;
+ }
+ return st.st_size;
+}
+
static int raw_create(const char *filename, QEMUOptionParameter *options)
{
int fd;
@@ -257,6 +282,8 @@ static BlockDriver bdrv_file = {
.bdrv_write = raw_write,
.bdrv_truncate = raw_truncate,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
.create_options = raw_create_options,
};
@@ -419,6 +446,8 @@ static BlockDriver bdrv_host_device = {
.bdrv_read = raw_read,
.bdrv_write = raw_write,
.bdrv_getlength = raw_getlength,
+ .bdrv_get_allocated_file_size
+ = raw_get_allocated_file_size,
};
static void bdrv_file_init(void)
diff --git a/block/vmdk.c b/block/vmdk.c
index 6bf242c..32e39c0 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1373,6 +1373,29 @@ static int vmdk_flush(BlockDriverState *bs)
return ret;
}
+static int64_t vmdk_get_allocated_file_size(BlockDriverState *bs)
+{
+ int i;
+ int64_t ret = 0;
+ int64_t r;
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_get_allocated_file_size(bs->file);
+ if (ret < 0) {
+ return ret;
+ }
+ for (i = 0; i < s->num_extents; i++) {
+ if (s->extents[i].file == bs->file) {
+ continue;
+ }
+ r = bdrv_get_allocated_file_size(s->extents[i].file);
+ if (r < 0) {
+ return r;
+ }
+ ret += r;
+ }
+ return ret;
+}
static QEMUOptionParameter vmdk_create_options[] = {
{
@@ -1411,6 +1434,7 @@ static BlockDriver bdrv_vmdk = {
.bdrv_create = vmdk_create,
.bdrv_flush = vmdk_flush,
.bdrv_is_allocated = vmdk_is_allocated,
+ .bdrv_get_allocated_file_size = vmdk_get_allocated_file_size,
.create_options = vmdk_create_options,
};
diff --git a/block_int.h b/block_int.h
index e870c33..e13eb70 100644
--- a/block_int.h
+++ b/block_int.h
@@ -86,6 +86,7 @@ struct BlockDriver {
const char *protocol_name;
int (*bdrv_truncate)(BlockDriverState *bs, int64_t offset);
int64_t (*bdrv_getlength)(BlockDriverState *bs);
+ int64_t (*bdrv_get_allocated_file_size)(BlockDriverState *bs);
int (*bdrv_write_compressed)(BlockDriverState *bs, int64_t sector_num,
const uint8_t *buf, int nb_sectors);
diff --git a/qemu-img.c b/qemu-img.c
index 4f162d1..d7c999c 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -979,35 +979,6 @@ out:
return 0;
}
-#ifdef _WIN32
-static int64_t get_allocated_file_size(const char *filename)
-{
- typedef DWORD (WINAPI * get_compressed_t)(const char *filename, DWORD *high);
- get_compressed_t get_compressed;
- struct _stati64 st;
-
- /* WinNT support GetCompressedFileSize to determine allocate size */
- get_compressed = (get_compressed_t) GetProcAddress(GetModuleHandle("kernel32"), "GetCompressedFileSizeA");
- if (get_compressed) {
- DWORD high, low;
- low = get_compressed(filename, &high);
- if (low != 0xFFFFFFFFlu || GetLastError() == NO_ERROR)
- return (((int64_t) high) << 32) + low;
- }
-
- if (_stati64(filename, &st) < 0)
- return -1;
- return st.st_size;
-}
-#else
-static int64_t get_allocated_file_size(const char *filename)
-{
- struct stat st;
- if (stat(filename, &st) < 0)
- return -1;
- return (int64_t)st.st_blocks * 512;
-}
-#endif
static void dump_snapshots(BlockDriverState *bs)
{
@@ -1067,7 +1038,7 @@ static int img_info(int argc, char **argv)
bdrv_get_format(bs, fmt_name, sizeof(fmt_name));
bdrv_get_geometry(bs, &total_sectors);
get_human_readable_size(size_buf, sizeof(size_buf), total_sectors * 512);
- allocated_size = get_allocated_file_size(filename);
+ allocated_size = bdrv_get_allocated_file_size(bs);
if (allocated_size < 0) {
snprintf(dsize_buf, sizeof(dsize_buf), "unavailable");
} else {
^ permalink raw reply related [flat|nested] 21+ messages in thread* Re: [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support
2011-07-01 4:55 [Qemu-devel] [PATCH v6 00/12] Adding VMDK monolithic flat support Fam Zheng
` (11 preceding siblings ...)
2011-07-01 4:55 ` [Qemu-devel] [PATCH v6 12/12] block: add bdrv_get_allocated_file_size() operation Fam Zheng
@ 2011-07-01 5:25 ` Stefan Hajnoczi
12 siblings, 0 replies; 21+ messages in thread
From: Stefan Hajnoczi @ 2011-07-01 5:25 UTC (permalink / raw)
To: kwolf; +Cc: Fam Zheng, qemu-devel, hch
On Fri, Jul 1, 2011 at 5:55 AM, Fam Zheng <famcool@gmail.com> wrote:
> Changes from v5:
> 01/12: add vmdk_free_extents
> 05/12: parameter order fix in calling vmdk_add_extent in vmdk_opem_vmdk4
> 09/12: add bdrv_delete, bs->open_flags
> 12/12: change commit message tag. remove duplicated count of monolithicSparse
>
> Fam Zheng (12):
> VMDK: introduce VmdkExtent
> VMDK: bugfix, align offset to cluster in get_whole_cluster
> VMDK: probe for monolithicFlat images
> VMDK: separate vmdk_open by format version
> VMDK: add field BDRVVmdkState.desc_offset
> VMDK: flush multiple extents
> VMDK: move 'static' cid_update flag to bs field
> VMDK: change get_cluster_offset return type
> VMDK: open/read/write for monolithicFlat image
> VMDK: create different subformats
> VMDK: fix coding style
> block: add bdrv_get_allocated_file_size() operation
>
> block.c | 19 +
> block.h | 1 +
> block/raw-posix.c | 21 +
> block/raw-win32.c | 29 ++
> block/vmdk.c | 1385 +++++++++++++++++++++++++++++++++++++----------------
> block_int.h | 2 +
> qemu-img.c | 31 +--
> 7 files changed, 1051 insertions(+), 437 deletions(-)
Kevin, I'm close to happy on this, please take a look.
Stefan
^ permalink raw reply [flat|nested] 21+ messages in thread