* [Qemu-devel] [PATCH v8 01/12] VMDK: introduce VmdkExtent
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
` (11 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 348 +++++++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 246 insertions(+), 102 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 922b23d..3b78583 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,13 @@ typedef struct BDRVVmdkState {
uint32_t l2_cache_counts[L2_CACHE_SIZE];
unsigned int cluster_sectors;
+} VmdkExtent;
+
+typedef struct BDRVVmdkState {
uint32_t parent_cid;
+ int num_extents;
+ /* Extent array with num_extents entries, ascend ordered by address */
+ VmdkExtent *extents;
} BDRVVmdkState;
typedef struct VmdkMetaData {
@@ -105,6 +115,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];
@@ -358,11 +381,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 +432,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;
@@ -406,40 +470,49 @@ static int vmdk_open(BlockDriverState *bs, int flags)
}
/* 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 +523,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 +538,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 +579,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 +603,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 +629,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 +644,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 +660,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 +751,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 +767,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 +969,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 01/12] VMDK: introduce VmdkExtent Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 03/12] VMDK: probe for monolithicFlat images Fam Zheng
` (10 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 3b78583..03a4619 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -514,21 +514,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 03/12] VMDK: probe for monolithicFlat images
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 01/12] VMDK: introduce VmdkExtent Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 04/12] VMDK: separate vmdk_open by format version Fam Zheng
` (9 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 45 +++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 43 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 03a4619..f8a815c 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -103,10 +103,51 @@ 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;
+ }
return 0;
+ }
}
#define CHECK_CID 1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v8 04/12] VMDK: separate vmdk_open by format version
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (2 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 03/12] VMDK: probe for monolithicFlat images Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
` (8 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 178 ++++++++++++++++++++++++++++++++++++---------------------
1 files changed, 112 insertions(+), 66 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index f8a815c..6d7b497 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -458,67 +458,20 @@ 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;
- }
+ 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) {
- goto fail;
+ ret = bdrv_pread(extent->file,
+ extent->l1_table_offset,
+ extent->l1_table,
+ l1_size);
+ if (ret < 0) {
+ goto fail_l1;
}
for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_table[i]);
@@ -526,12 +479,12 @@ static int vmdk_open(BlockDriverState *bs, int flags)
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;
+ ret = bdrv_pread(extent->file,
+ extent->l1_backup_table_offset,
+ extent->l1_backup_table,
+ l1_size);
+ if (ret < 0) {
+ goto fail_l1b;
}
for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_backup_table[i]);
@@ -541,9 +494,102 @@ static int vmdk_open(BlockDriverState *bs, int flags)
extent->l2_cache =
qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
return 0;
+ fail_l1b:
+ qemu_free(extent->l1_backup_table);
+ fail_l1:
+ qemu_free(extent->l1_table);
+ return ret;
+}
+
+static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+{
+ int ret;
+ uint32_t magic;
+ VMDK3Header header;
+ VmdkExtent *extent;
+
+ ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ if (ret < 0) {
+ 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) {
+ /* vmdk_init_tables cleans up on fail, so only free allocation of
+ * vmdk_add_extent here. */
+ goto fail;
+ }
+ return 0;
fail:
vmdk_free_extents(bs);
- return -1;
+ 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 < 0) {
+ 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:
+ vmdk_free_extents(bs);
+ 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,
@@ -630,11 +676,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;
}
}
@@ -645,7 +691,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 05/12] VMDK: add field BDRVVmdkState.desc_offset
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (3 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 04/12] VMDK: separate vmdk_open by format version Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 06/12] VMDK: flush multiple extents Fam Zheng
` (7 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 27 ++++++++++++++++++---------
1 files changed, 18 insertions(+), 9 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 6d7b497..529ae90 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -81,6 +81,7 @@ typedef struct VmdkExtent {
} VmdkExtent;
typedef struct BDRVVmdkState {
+ int desc_offset;
uint32_t parent_cid;
int num_extents;
/* Extent array with num_extents entries, ascend ordered by address */
@@ -175,10 +176,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";
@@ -200,10 +202,12 @@ 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;
+ memset(desc, 0, sizeof(desc));
+ 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);
@@ -213,8 +217,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;
}
@@ -402,10 +407,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;
@@ -506,8 +512,10 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
int ret;
uint32_t magic;
VMDK3Header header;
+ BDRVVmdkState *s = bs->opaque;
VmdkExtent *extent;
+ s->desc_offset = 0x200;
ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
if (ret < 0) {
goto fail;
@@ -539,6 +547,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 < 0) {
goto fail;
^ permalink raw reply related [flat|nested] 17+ messages in thread* [Qemu-devel] [PATCH v8 06/12] VMDK: flush multiple extents
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (4 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
` (6 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 529ae90..f6d2986 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1072,7 +1072,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 07/12] VMDK: move 'static' cid_update flag to bs field
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (5 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 06/12] VMDK: flush multiple extents Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 08/12] VMDK: change get_cluster_offset return type Fam Zheng
` (5 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 f6d2986..8dc58a8 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;
uint32_t parent_cid;
int num_extents;
/* Extent array with num_extents entries, ascend ordered by address */
@@ -853,7 +854,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) {
@@ -900,9 +900,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 08/12] VMDK: change get_cluster_offset return type
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (6 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
` (4 subsequent siblings)
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 79 ++++++++++++++++++++++++++++++---------------------------
1 files changed, 42 insertions(+), 37 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 8dc58a8..f637d98 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -665,26 +665,31 @@ 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) {
- return 0;
+ return -1;
}
l2_offset = extent->l1_table[l1_index];
if (!l2_offset) {
- return 0;
+ return -1;
}
for (i = 0; i < L2_CACHE_SIZE; i++) {
if (l2_offset == extent->l2_cache_offsets[i]) {
@@ -714,28 +719,29 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
l2_table,
extent->l2_size * sizeof(uint32_t)
) != extent->l2_size * sizeof(uint32_t)) {
- return 0;
+ return -1;
}
extent->l2_cache_offsets[min_index] = l2_offset;
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
@@ -744,8 +750,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;
@@ -755,8 +761,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,
@@ -780,7 +786,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;
@@ -789,15 +794,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;
@@ -818,14 +821,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;
@@ -851,7 +855,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;
@@ -869,13 +873,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (7 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 08/12] VMDK: change get_cluster_offset return type Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-08 12:55 ` Stefan Hajnoczi
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats Fam Zheng
` (3 subsequent siblings)
12 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 172 +++++++++++++++++++++++++++++++++++++++++++++++++++++-----
1 files changed, 159 insertions(+), 13 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index f637d98..2183ace 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;
@@ -407,9 +408,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;
}
@@ -584,6 +586,145 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
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;
+ VmdkExtent *extent;
+
+ path_combine(extent_path, sizeof(extent_path),
+ desc_file_path, fname);
+ ret = bdrv_file_open(&extent_file, extent_path, bs->open_flags);
+ if (ret) {
+ return ret;
+ }
+ extent = vmdk_add_extent(bs, extent_file, true, sectors,
+ 0, 0, 0, 0, sectors);
+ extent->flat_start_offset = flat_offset;
+ } else {
+ /* 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;
@@ -598,7 +739,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);
}
}
@@ -679,7 +820,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;
}
@@ -832,16 +973,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 < 0) {
+ return ret;
+ }
}
nb_sectors -= n;
sector_num += n;
@@ -865,7 +1010,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) {
@@ -888,16 +1033,17 @@ 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 < 0) {
+ 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] 17+ messages in thread* Re: [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-07-08 12:55 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-07-08 12:55 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Tue, Jul 5, 2011 at 12:31 PM, Fam Zheng <famcool@gmail.com> wrote:
> + ret = sscanf(p, "%10s %lld %10s %512s",
[...]
> + ret = sscanf(p, "%10s %lld %10s %511s %lld",
%512s -> %511s
But instead of duplicating the format string and sscanf(3), I suggest
doing sscanf(p, "%10s %lld %10s %511s %lld", ...) once only.
After it returns you can check:
if (ret < 4) {
...fail...
} else if (!strcmp(access, "FLAT")) {
if (ret != 5 || flat_offset < 0) {
...fail...
}
} else {
if (ret != 4) {
...fail...
}
}
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (8 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-08 15:29 ` Stefan Hajnoczi
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 11/12] VMDK: fix coding style Fam Zheng
` (2 subsequent siblings)
12 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 | 561 ++++++++++++++++++++++++++++++++++------------------------
block_int.h | 1 +
2 files changed, 330 insertions(+), 232 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 2183ace..0f51332 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -156,8 +156,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)
{
@@ -243,168 +243,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;
- }
- /* the descriptor offset = 0x200 */
- 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;
@@ -1059,68 +897,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;
@@ -1131,7 +933,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);
@@ -1189,28 +990,317 @@ 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;
+ }
+ /* the descriptor offset = 0x200 */
+ 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;
@@ -1253,6 +1343,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] 17+ messages in thread* Re: [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats Fam Zheng
@ 2011-07-08 15:29 ` Stefan Hajnoczi
2011-07-09 12:09 ` Fam Zheng
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-07-08 15:29 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Tue, Jul 5, 2011 at 12:31 PM, Fam Zheng <famcool@gmail.com> wrote:
> Add create option 'format', with enums:
The -drive format=... option exists in QEMU today to specify the image
format of a file. I think adding a format=... creation option may
lead to confusion.
How about subformat=... or type=...?
> Each creates a subformat image file. The default is monolithiSparse.
s/monolithiSparse/monolithicSparse/
> @@ -243,168 +243,6 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
> return 1;
> }
>
> -static int vmdk_snapshot_create(const char *filename, const char *backing_file)
Is this function really not needed anymore?
> @@ -1189,28 +990,317 @@ 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;
What is the point of setting filesize to zero?
> + 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;
errno is a global variable that may be modified by any errno-using
library function. Its value may be changed by close(2) (even if there
is no error closing the fd). Therefore please do return ret instead
of return -errno.
> }
> + close(fd);
> + return 0;
> +}
>
> - ret = 0;
> +static int filename_decompose(const char *filename, char *path, char *prefix,
> + char *postfix, int buf_len)
Memory sizes (e.g. buffer size) should be size_t (which is unsigned)
instead of int.
> +{
> + const char *p, *q;
> +
> + if (filename == NULL || !strlen(filename)) {
> + fprintf(stderr, "Vmdk: wrong filename (%s)\n", filename);
Printing filename doesn't make sense since filename is either NULL or
"". Also note that fprintf(..., "%s", NULL) is undefined and may
crash on some platforms (e.g. Solaris).
> + 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 {
No check for prefix buf_len here. Imagine filename has no '/', '\\',
or ':' but it does have a '.'. It is possible to overflow prefix.
> + 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)) {
Please don't hardcode buffer sizes, if one of path, prefix, postfix
ever needs to be changed then this code also needs to be updated. I
suggest defining a constant and using it everywhere.
> + 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];
Buffer sizes again.
> + int64_t size = filesize;
> +
> + if (split && size > split_size) {
> + size = split_size;
> + }
> + if (split) {
> + sprintf(desc_filename, "%s-f%03d%s",
> + prefix, idx++, postfix);
snprintf?
> + 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);
Here sizeof(desc_line) should be used as the buffer size to avoid
duplicating it. Same thing below in the code.
> + 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);
This assumes that the backing file is vmdk. Where was that checked?
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats
2011-07-08 15:29 ` Stefan Hajnoczi
@ 2011-07-09 12:09 ` Fam Zheng
0 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-09 12:09 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, qemu-devel, hch
On Fri, Jul 8, 2011 at 11:29 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Jul 5, 2011 at 12:31 PM, Fam Zheng <famcool@gmail.com> wrote:
>> Add create option 'format', with enums:
>
> The -drive format=... option exists in QEMU today to specify the image
> format of a file. I think adding a format=... creation option may
> lead to confusion.
>
> How about subformat=... or type=...?
>
>> Each creates a subformat image file. The default is monolithiSparse.
>
> s/monolithiSparse/monolithicSparse/
>
>> @@ -243,168 +243,6 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
>> return 1;
>> }
>>
>> -static int vmdk_snapshot_create(const char *filename, const char *backing_file)
>
> Is this function really not needed anymore?
>
I think so. This function is not used to create snapshot, actually
it's called for backing file. It copies header and L1 table, allocate
L2 tables and leave them zero. This is equivalent to creating a new
extent. The only difference is to set descriptor options parentCID and
parantFileNameHint.
>> @@ -1189,28 +990,317 @@ 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;
>
> What is the point of setting filesize to zero?
>
>> + 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;
>
> errno is a global variable that may be modified by any errno-using
> library function. Its value may be changed by close(2) (even if there
> is no error closing the fd). Therefore please do return ret instead
> of return -errno.
>
>> }
>> + close(fd);
>> + return 0;
>> +}
>>
>> - ret = 0;
>> +static int filename_decompose(const char *filename, char *path, char *prefix,
>> + char *postfix, int buf_len)
>
> Memory sizes (e.g. buffer size) should be size_t (which is unsigned)
> instead of int.
>
>> +{
>> + const char *p, *q;
>> +
>> + if (filename == NULL || !strlen(filename)) {
>> + fprintf(stderr, "Vmdk: wrong filename (%s)\n", filename);
>
> Printing filename doesn't make sense since filename is either NULL or
> "". Also note that fprintf(..., "%s", NULL) is undefined and may
> crash on some platforms (e.g. Solaris).
>
>> + 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 {
>
> No check for prefix buf_len here. Imagine filename has no '/', '\\',
> or ':' but it does have a '.'. It is possible to overflow prefix.
>
>> + 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)) {
>
> Please don't hardcode buffer sizes, if one of path, prefix, postfix
> ever needs to be changed then this code also needs to be updated. I
> suggest defining a constant and using it everywhere.
>
>> + 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];
>
> Buffer sizes again.
>
>> + int64_t size = filesize;
>> +
>> + if (split && size > split_size) {
>> + size = split_size;
>> + }
>> + if (split) {
>> + sprintf(desc_filename, "%s-f%03d%s",
>> + prefix, idx++, postfix);
>
> snprintf?
>
>> + 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);
>
> Here sizeof(desc_line) should be used as the buffer size to avoid
> duplicating it. Same thing below in the code.
>
>> + 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);
>
> This assumes that the backing file is vmdk. Where was that checked?
>
> Stefan
>
--
Best regards!
Fam Zheng
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v8 11/12] VMDK: fix coding style
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (9 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 10/12] VMDK: create different subformats Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 12/12] block: add bdrv_get_allocated_file_size() operation Fam Zheng
2011-07-08 15:40 ` [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Stefan Hajnoczi
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 0f51332..1084db8 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -102,8 +102,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) {
@@ -192,9 +193,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;
@@ -211,9 +213,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);
@@ -233,13 +236,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;
}
@@ -254,14 +258,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);
}
@@ -595,8 +603,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);
@@ -655,8 +664,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;
@@ -712,7 +722,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,
@@ -729,8 +739,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;
@@ -780,8 +791,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;
}
@@ -805,16 +817,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);
}
@@ -888,7 +903,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;
@@ -909,8 +925,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;
@@ -1354,16 +1371,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] 17+ messages in thread* [Qemu-devel] [PATCH v8 12/12] block: add bdrv_get_allocated_file_size() operation
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (10 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 11/12] VMDK: fix coding style Fam Zheng
@ 2011-07-05 11:31 ` Fam Zheng
2011-07-08 15:40 ` [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Stefan Hajnoczi
12 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-07-05 11:31 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 1084db8..a3c8709 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1343,6 +1343,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[] = {
{
@@ -1381,6 +1404,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] 17+ messages in thread* Re: [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support
2011-07-05 11:31 [Qemu-devel] [PATCH v8 00/12] Adding VMDK monolithic flat support Fam Zheng
` (11 preceding siblings ...)
2011-07-05 11:31 ` [Qemu-devel] [PATCH v8 12/12] block: add bdrv_get_allocated_file_size() operation Fam Zheng
@ 2011-07-08 15:40 ` Stefan Hajnoczi
12 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-07-08 15:40 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Tue, Jul 5, 2011 at 12:31 PM, Fam Zheng <famcool@gmail.com> wrote:
> Chnages from v7:
> 03/12: remove deadloop in probing descriptor file.
>
> 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 | 1361 +++++++++++++++++++++++++++++++++++++----------------
> block_int.h | 2 +
> qemu-img.c | 31 +--
> 7 files changed, 1024 insertions(+), 440 deletions(-)
Getting closer. Patch 10/12 is big and I see a lot of repetition in
the image creation process. I wonder if it helps to factor out
certain aspects such as filename and descriptor generation so that the
creation function doesn't become so large. The aim is to encapsulate
aspects of image creation cleanly so that the caller doesn't need to
keep state around and can use a simple interface to orchestrate image
creation.
Structuring is going to be important for future vmdk changes. We need
to introduce clean interfaces to separate subformats while keeping
common code shared as utility functions. Right now there is a lot of
if (flat) { ... } or if (!strcmp(..., "monolithicSparse")) { ... }.
The details of various formats are spread throughout the entire vmdk
codebase instead of encapsulated in one module each. This will become
even more important as you flesh out the support matrix for various
file formats. Something to keep in mind for future work after this
series.
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread