* [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support
@ 2011-06-28 1:32 Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 01/12] VMDK: introduce VmdkExtent Fam Zheng
` (11 more replies)
0 siblings, 12 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Changes from v4:
09/12: pstrcpy call off-by-one in vmdk_parse_description
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
BlockDriver: 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 | 1379 +++++++++++++++++++++++++++++++++++++----------------
block_int.h | 2 +
qemu-img.c | 31 +--
7 files changed, 1046 insertions(+), 436 deletions(-)
^ permalink raw reply [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v5 01/12] VMDK: introduce VmdkExtent
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
` (10 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 | 351 +++++++++++++++++++++++++++++++++++++++++-----------------
1 files changed, 250 insertions(+), 101 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 922b23d..49df1a5 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -60,7 +60,11 @@ typedef struct {
#define L2_CACHE_SIZE 16
-typedef struct BDRVVmdkState {
+typedef struct VmdkExtent {
+ BlockDriverState *file;
+ bool flat;
+ int64_t sectors;
+ int64_t end_sector;
int64_t l1_table_offset;
int64_t l1_backup_table_offset;
uint32_t *l1_table;
@@ -74,7 +78,12 @@ typedef struct BDRVVmdkState {
uint32_t l2_cache_counts[L2_CACHE_SIZE];
unsigned int cluster_sectors;
+} VmdkExtent;
+
+typedef struct BDRVVmdkState {
+ int num_extents;
uint32_t parent_cid;
+ VmdkExtent *extents;
} BDRVVmdkState;
typedef struct VmdkMetaData {
@@ -156,8 +165,8 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
static int vmdk_is_cid_valid(BlockDriverState *bs)
{
-#ifdef CHECK_CID
BDRVVmdkState *s = bs->opaque;
+#ifdef CHECK_CID
BlockDriverState *p_bs = bs->backing_hd;
uint32_t cur_pcid;
@@ -358,11 +367,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 +418,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.rgd_offset) << 9,
+ le64_to_cpu(header.gd_offset) << 9,
+ l1_size,
+ le32_to_cpu(header.num_gtes_per_gte),
+ le64_to_cpu(header.granularity));
+ if (extent->l1_entry_sectors <= 0) {
goto fail;
- s->l1_size = (bs->total_sectors + s->l1_entry_sectors - 1)
- / s->l1_entry_sectors;
- s->l1_table_offset = le64_to_cpu(header.rgd_offset) << 9;
- s->l1_backup_table_offset = le64_to_cpu(header.gd_offset) << 9;
-
+ }
// try to open parent images, if exist
if (vmdk_parent_open(bs) != 0)
goto fail;
@@ -405,41 +455,61 @@ static int vmdk_open(BlockDriverState *bs, int flags)
goto fail;
}
+ /* sum up the total sectors */
+ bs->total_sectors = 0;
+ for (i = 0; i < s->num_extents; i++) {
+ bs->total_sectors += s->extents[i].sectors;
+ }
+
/* read the L1 table */
- l1_size = s->l1_size * sizeof(uint32_t);
- s->l1_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file, s->l1_table_offset, s->l1_table, l1_size) != l1_size)
+ l1_size = extent->l1_size * sizeof(uint32_t);
+ extent->l1_table = qemu_malloc(l1_size);
+ if (bdrv_pread(bs->file,
+ extent->l1_table_offset,
+ extent->l1_table,
+ l1_size)
+ != l1_size) {
goto fail;
- for(i = 0; i < s->l1_size; i++) {
- le32_to_cpus(&s->l1_table[i]);
+ }
+ for (i = 0; i < extent->l1_size; i++) {
+ le32_to_cpus(&extent->l1_table[i]);
}
- if (s->l1_backup_table_offset) {
- s->l1_backup_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file, s->l1_backup_table_offset, s->l1_backup_table, l1_size) != l1_size)
+ if (extent->l1_backup_table_offset) {
+ extent->l1_backup_table = qemu_malloc(l1_size);
+ if (bdrv_pread(bs->file,
+ extent->l1_backup_table_offset,
+ extent->l1_backup_table,
+ l1_size)
+ != l1_size) {
goto fail;
- for(i = 0; i < s->l1_size; i++) {
- le32_to_cpus(&s->l1_backup_table[i]);
+ }
+ for (i = 0; i < extent->l1_size; i++) {
+ le32_to_cpus(&extent->l1_backup_table[i]);
}
}
- s->l2_cache = qemu_malloc(s->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
+ extent->l2_cache =
+ qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
return 0;
fail:
- qemu_free(s->l1_backup_table);
- qemu_free(s->l1_table);
- qemu_free(s->l2_cache);
+ for (i = 0; i < s->num_extents; i++) {
+ qemu_free(s->extents[i].l1_backup_table);
+ qemu_free(s->extents[i].l1_table);
+ qemu_free(s->extents[i].l2_cache);
+ }
+ qemu_free(s->extents);
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 +520,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 +535,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 +576,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 +600,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 +626,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 +641,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 +657,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 +748,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 +764,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 +966,15 @@ exit:
static void vmdk_close(BlockDriverState *bs)
{
+ int i;
BDRVVmdkState *s = bs->opaque;
- qemu_free(s->l1_table);
- qemu_free(s->l2_cache);
+ 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 int vmdk_flush(BlockDriverState *bs)
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v5 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 01/12] VMDK: introduce VmdkExtent Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 03/12] VMDK: probe for monolithicFlat images Fam Zheng
` (9 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 49df1a5..a2538be 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -511,21 +511,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 v5 03/12] VMDK: probe for monolithicFlat images
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 01/12] VMDK: introduce VmdkExtent Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 04/12] VMDK: separate vmdk_open by format version Fam Zheng
` (8 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
Probe as the same behavior as VMware does.
Recognize image as monolithicFlat descriptor file when the file is text
and the first effective line (not '#' leaded comment or space line) is
either 'version=1' or 'version=2'. No space or upper case charactors
accepted.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 44 ++++++++++++++++++++++++++++++++++++++++++--
1 files changed, 42 insertions(+), 2 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index a2538be..3bf8980 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -102,10 +102,50 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
return 0;
magic = be32_to_cpu(*(uint32_t *)buf);
if (magic == VMDK3_MAGIC ||
- magic == VMDK4_MAGIC)
+ magic == VMDK4_MAGIC) {
return 100;
- else
+ } else {
+ const char *p = (const char *)buf;
+ const char *end = p + buf_size;
+ while (p < end) {
+ if (*p == '#') {
+ /* skip comment line */
+ while (p < end && *p != '\n') {
+ p++;
+ }
+ p++;
+ continue;
+ }
+ if (*p == ' ') {
+ while (p < end && *p == ' ') {
+ p++;
+ }
+ /* skip '\r' if windows line endings used. */
+ if (p < end && *p == '\r') {
+ p++;
+ }
+ /* only accept blank lines before 'version=' line */
+ if (p == end || *p != '\n') {
+ return 0;
+ }
+ p++;
+ continue;
+ }
+ if (end - p >= strlen("version=X\n")) {
+ if (strncmp("version=1\n", p, strlen("version=1\n")) == 0 ||
+ strncmp("version=2\n", p, strlen("version=2\n")) == 0) {
+ return 100;
+ }
+ }
+ if (end - p >= strlen("version=X\r\n")) {
+ if (strncmp("version=1\r\n", p, strlen("version=1\r\n")) == 0 ||
+ strncmp("version=2\r\n", p, strlen("version=2\r\n")) == 0) {
+ return 100;
+ }
+ }
+ }
return 0;
+ }
}
#define CHECK_CID 1
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v5 04/12] VMDK: separate vmdk_open by format version
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (2 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 03/12] VMDK: probe for monolithicFlat images Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
` (7 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 | 195 +++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 126 insertions(+), 69 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 3bf8980..81c1054 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -443,74 +443,26 @@ static VmdkExtent *vmdk_add_extent(BlockDriverState *bs,
return extent;
}
-
-static int vmdk_open(BlockDriverState *bs, int flags)
+static int vmdk_init_tables(BlockDriverState *bs, VmdkExtent *extent)
{
- BDRVVmdkState *s = bs->opaque;
- uint32_t magic;
- int i;
- uint32_t l1_size, l1_entry_sectors;
- VmdkExtent *extent = NULL;
-
- if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic))
- goto fail;
-
- magic = be32_to_cpu(magic);
- if (magic == VMDK3_MAGIC) {
- VMDK3Header header;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header)) {
- goto fail;
- }
- extent = vmdk_add_extent(bs, bs->file, false,
- le32_to_cpu(header.disk_sectors),
- le32_to_cpu(header.l1dir_offset) << 9, 0,
- 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
- } else if (magic == VMDK4_MAGIC) {
- VMDK4Header header;
- if (bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header))
- != sizeof(header)) {
- goto fail;
- }
- l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
- * le64_to_cpu(header.granularity);
- l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
- / l1_entry_sectors;
- extent = vmdk_add_extent(bs, bs->file, false,
- le64_to_cpu(header.capacity),
- le64_to_cpu(header.rgd_offset) << 9,
- le64_to_cpu(header.gd_offset) << 9,
- l1_size,
- le32_to_cpu(header.num_gtes_per_gte),
- le64_to_cpu(header.granularity));
- if (extent->l1_entry_sectors <= 0) {
- goto fail;
- }
- // try to open parent images, if exist
- if (vmdk_parent_open(bs) != 0)
- goto fail;
- // write the CID once after the image creation
- s->parent_cid = vmdk_read_cid(bs,1);
- } else {
- goto fail;
- }
-
- /* sum up the total sectors */
- bs->total_sectors = 0;
- for (i = 0; i < s->num_extents; i++) {
- bs->total_sectors += s->extents[i].sectors;
- }
+ int ret;
+ int l1_size, i;
/* read the L1 table */
l1_size = extent->l1_size * sizeof(uint32_t);
extent->l1_table = qemu_malloc(l1_size);
- if (bdrv_pread(bs->file,
- extent->l1_table_offset,
- extent->l1_table,
- l1_size)
- != l1_size) {
+ if (!extent->l1_table) {
+ ret = -ENOMEM;
goto fail;
}
+ ret = bdrv_pread(bs->file,
+ extent->l1_table_offset,
+ extent->l1_table,
+ l1_size);
+ if (ret != l1_size) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail_l1;
+ }
for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_table[i]);
}
@@ -525,21 +477,126 @@ static int vmdk_open(BlockDriverState *bs, int flags)
goto fail;
}
for (i = 0; i < extent->l1_size; i++) {
+ if (!extent->l1_backup_table) {
+ ret = -ENOMEM;
+ goto fail_l1;
+ }
+ }
+ ret = bdrv_pread(bs->file,
+ extent->l1_backup_table_offset,
+ extent->l1_backup_table,
+ l1_size);
+ if (ret != l1_size) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ for (i = 0; i < extent->l1_size; i++) {
le32_to_cpus(&extent->l1_backup_table[i]);
}
}
extent->l2_cache =
qemu_malloc(extent->l2_size * L2_CACHE_SIZE * sizeof(uint32_t));
+ if (!extent->l2_cache) {
+ ret = -ENOMEM;
+ goto fail_l1b;
+ }
return 0;
+ fail_l1b:
+ qemu_free(extent->l1_backup_table);
+ fail_l1:
+ qemu_free(extent->l1_table);
fail:
- for (i = 0; i < s->num_extents; i++) {
- qemu_free(s->extents[i].l1_backup_table);
- qemu_free(s->extents[i].l1_table);
- qemu_free(s->extents[i].l2_cache);
+ return ret;
+}
+
+static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
+{
+ int ret;
+ uint32_t magic;
+ VMDK3Header header;
+ VmdkExtent *extent;
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
+ if (ret != sizeof(header)) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ extent = vmdk_add_extent(bs, bs->file, false, le32_to_cpu(header.disk_sectors),
+ le32_to_cpu(header.l1dir_offset) << 9, 0,
+ 1 << 6, 1 << 9, le32_to_cpu(header.granularity));
+ ret = vmdk_init_tables(bs, extent);
+ if (ret) {
+ return ret;
}
+ return 0;
+ fail:
qemu_free(s->extents);
- return -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 != sizeof(header)) {
+ ret = ret < 0 ? ret : -EIO;
+ goto fail;
+ }
+ l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
+ * le64_to_cpu(header.granularity);
+ l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
+ / l1_entry_sectors;
+ extent = vmdk_add_extent(bs, bs->file, false,
+ le64_to_cpu(header.capacity),
+ le64_to_cpu(header.rgd_offset) << 9,
+ le64_to_cpu(header.gd_offset) << 9,
+ l1_size,
+ le32_to_cpu(header.num_gtes_per_gte),
+ le64_to_cpu(header.granularity));
+ if (extent->l1_entry_sectors <= 0) {
+ ret = -EINVAL;
+ goto fail;
+ }
+ /* try to open parent images, if exist */
+ ret = vmdk_parent_open(bs);
+ if (ret) {
+ goto fail;
+ }
+ s->parent_cid = vmdk_read_cid(bs, 1);
+ ret = vmdk_init_tables(bs, extent);
+ if (ret) {
+ goto fail;
+ }
+ return 0;
+ fail:
+ qemu_free(s->extents);
+ return ret;
+}
+
+static int vmdk_open(BlockDriverState *bs, int flags)
+{
+ uint32_t magic;
+
+ if (bdrv_pread(bs->file, 0, &magic, sizeof(magic)) != sizeof(magic)) {
+ return -EIO;
+ }
+
+ magic = be32_to_cpu(magic);
+ if (magic == VMDK3_MAGIC) {
+ return vmdk_open_vmdk3(bs, flags);
+ } else if (magic == VMDK4_MAGIC) {
+ return vmdk_open_vmdk4(bs, flags);
+ } else {
+ return -EINVAL;
+ }
}
static int get_whole_cluster(BlockDriverState *bs,
@@ -626,11 +683,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;
}
}
@@ -641,7 +698,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 v5 05/12] VMDK: add field BDRVVmdkState.desc_offset
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (3 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 04/12] VMDK: separate vmdk_open by format version Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 06/12] VMDK: flush multiple extents Fam Zheng
` (6 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
There are several occurrence of magic number 0x200 as the descriptor
offset within mono sparse image file. This is not the case for images
with separate descriptor file. So a field is added to BDRVVmdkState to
hold the correct value.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 26 ++++++++++++++++----------
1 files changed, 16 insertions(+), 10 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 81c1054..c3f0ab6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -81,6 +81,7 @@ typedef struct VmdkExtent {
} VmdkExtent;
typedef struct BDRVVmdkState {
+ int desc_offset;
int num_extents;
uint32_t parent_cid;
VmdkExtent *extents;
@@ -160,10 +161,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";
@@ -185,10 +187,11 @@ static int vmdk_write_cid(BlockDriverState *bs, uint32_t cid)
{
char desc[DESC_SIZE], tmp_desc[DESC_SIZE];
char *p_name, *tmp_str;
+ BDRVVmdkState *s = bs->opaque;
- /* the descriptor offset = 0x200 */
- if (bdrv_pread(bs->file, 0x200, desc, DESC_SIZE) != DESC_SIZE)
- return -1;
+ if (bdrv_pread(bs->file, s->desc_offset, desc, DESC_SIZE) != DESC_SIZE) {
+ return -EIO;
+ }
tmp_str = strstr(desc,"parentCID");
pstrcpy(tmp_desc, sizeof(tmp_desc), tmp_str);
@@ -198,8 +201,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;
}
@@ -283,7 +287,6 @@ static int vmdk_snapshot_create(const char *filename, const char *backing_file)
ret = -errno;
goto fail;
}
- /* the descriptor offset = 0x200 */
if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
ret = -errno;
goto fail;
@@ -387,10 +390,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;
@@ -518,6 +522,7 @@ static int vmdk_open_vmdk3(BlockDriverState *bs, int flags)
VmdkExtent *extent;
BDRVVmdkState *s = bs->opaque;
+ s->desc_offset = 0x200;
ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
if (ret != sizeof(header)) {
ret = ret < 0 ? ret : -EIO;
@@ -545,6 +550,7 @@ static int vmdk_open_vmdk4(BlockDriverState *bs, int flags)
BDRVVmdkState *s = bs->opaque;
VmdkExtent *extent;
+ s->desc_offset = 0x200;
ret = bdrv_pread(bs->file, sizeof(magic), &header, sizeof(header));
if (ret != sizeof(header)) {
ret = ret < 0 ? ret : -EIO;
^ permalink raw reply related [flat|nested] 17+ messages in thread
* [Qemu-devel] [PATCH v5 06/12] VMDK: flush multiple extents
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (4 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
` (5 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 c3f0ab6..cf7370f 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1084,7 +1084,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 v5 07/12] VMDK: move 'static' cid_update flag to bs field
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (5 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 06/12] VMDK: flush multiple extents Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 08/12] VMDK: change get_cluster_offset return type Fam Zheng
` (4 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 cf7370f..10a8a9a 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -82,6 +82,7 @@ typedef struct VmdkExtent {
typedef struct BDRVVmdkState {
int desc_offset;
+ bool cid_updated;
int num_extents;
uint32_t parent_cid;
VmdkExtent *extents;
@@ -857,7 +858,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) {
@@ -904,9 +904,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 v5 08/12] VMDK: change get_cluster_offset return type
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (6 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
` (3 subsequent siblings)
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 UTC (permalink / raw)
To: qemu-devel; +Cc: kwolf, Fam Zheng, hch, stefanha
The return type of get_cluster_offset was an offset that use 0 to denote
'not allocated', this will be no longer true for flat extents, as we see
flat extent file as a single huge cluster whose offset is 0 and length
is the whole file length.
So now we use int return value, 0 means success and otherwise offset
invalid.
Signed-off-by: Fam Zheng <famcool@gmail.com>
---
block/vmdk.c | 73 +++++++++++++++++++++++++++++++---------------------------
1 files changed, 39 insertions(+), 34 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 10a8a9a..c84ea90 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -669,18 +669,23 @@ static int vmdk_L2update(VmdkExtent *extent, VmdkMetaData *m_data)
return 0;
}
-static uint64_t get_cluster_offset(BlockDriverState *bs,
+static int get_cluster_offset(BlockDriverState *bs,
VmdkExtent *extent,
VmdkMetaData *m_data,
- uint64_t offset, int allocate)
+ uint64_t offset,
+ int allocate,
+ uint64_t *cluster_offset)
{
unsigned int l1_index, l2_offset, l2_index;
int min_index, i, j;
uint32_t min_count, *l2_table, tmp = 0;
- uint64_t cluster_offset;
if (m_data)
m_data->valid = 0;
+ if (extent->flat) {
+ *cluster_offset = 0;
+ return 0;
+ }
l1_index = (offset >> 9) / extent->l1_entry_sectors;
if (l1_index >= extent->l1_size) {
@@ -725,21 +730,22 @@ static uint64_t get_cluster_offset(BlockDriverState *bs,
extent->l2_cache_counts[min_index] = 1;
found:
l2_index = ((offset >> 9) / extent->cluster_sectors) % extent->l2_size;
- cluster_offset = le32_to_cpu(l2_table[l2_index]);
+ *cluster_offset = le32_to_cpu(l2_table[l2_index]);
- if (!cluster_offset) {
- if (!allocate)
- return 0;
+ if (!*cluster_offset) {
+ if (!allocate) {
+ return -1;
+ }
// Avoid the L2 tables update for the images that have snapshots.
- cluster_offset = bdrv_getlength(extent->file);
+ *cluster_offset = bdrv_getlength(extent->file);
bdrv_truncate(
extent->file,
- cluster_offset + (extent->cluster_sectors << 9)
+ *cluster_offset + (extent->cluster_sectors << 9)
);
- cluster_offset >>= 9;
- tmp = cpu_to_le32(cluster_offset);
+ *cluster_offset >>= 9;
+ tmp = cpu_to_le32(*cluster_offset);
l2_table[l2_index] = tmp;
/* First of all we write grain itself, to avoid race condition
@@ -748,8 +754,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;
@@ -759,8 +765,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,
@@ -784,7 +790,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;
@@ -793,15 +798,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;
@@ -822,14 +825,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;
@@ -855,7 +859,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;
@@ -873,13 +877,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 v5 09/12] VMDK: open/read/write for monolithicFlat image
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (7 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 08/12] VMDK: change get_cluster_offset return type Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-29 15:57 ` Stefan Hajnoczi
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 10/12] VMDK: create different subformats Fam Zheng
` (2 subsequent siblings)
11 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 | 185 +++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 172 insertions(+), 13 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index c84ea90..8083c31 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;
@@ -390,9 +391,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;
}
@@ -588,6 +590,157 @@ 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 = 0;
+ int r;
+ char access[11];
+ char type[11];
+ char fname[512];
+ const char *p = desc;
+ int64_t sectors = 0;
+ int64_t flat_offset;
+
+ while (*p) {
+ if (strncmp(p, "RW", strlen("RW"))) {
+ goto next_line;
+ }
+ /* parse extent line:
+ * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
+ * or
+ * RW [size in sectors] SPARSE "file-name.vmdk"
+ */
+ flat_offset = -1;
+ sscanf(p, "%10s %lld %10s %512s",
+ access, §ors, type, fname);
+ if (!strcmp(type, "FLAT")) {
+ sscanf(p, "%10s %lld %10s %512s %lld",
+ access, §ors, type, fname, &flat_offset);
+ if (flat_offset == -1) {
+ return -EINVAL;
+ }
+ }
+
+ /* trim the quotation marks around */
+ if (fname[0] == '"') {
+ memmove(fname, fname + 1, strlen(fname) + 1);
+ if (fname[strlen(fname) - 1] == '"') {
+ fname[strlen(fname) - 1] = '\0';
+ }
+ }
+ if (!(strlen(access) && sectors && strlen(type) && strlen(fname))) {
+ goto next_line;
+ }
+ if (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) {
+ goto next_line;
+ }
+ if (strcmp(access, "RW")) {
+ goto next_line;
+ }
+ ret++;
+
+ /* save to extents array */
+ if (!strcmp(type, "FLAT")) {
+ /* FLAT extent */
+ char extent_path[PATH_MAX];
+ BlockDriverState *extent_file;
+ BlockDriver *drv;
+ VmdkExtent *extent;
+
+ extent_file = bdrv_new("");
+ drv = bdrv_find_format("file");
+ if (!drv) {
+ return -EINVAL;
+ }
+ path_combine(extent_path, sizeof(extent_path),
+ desc_file_path, fname);
+ r = bdrv_open(extent_file, extent_path,
+ BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
+ if (r) {
+ return -EINVAL;
+ }
+ extent = vmdk_add_extent(bs, extent_file, true, sectors,
+ 0, 0, 0, 0, sectors);
+ extent->flat_start_offset = flat_offset;
+ } else {
+ /* SPARSE extent, not supported for now */
+ fprintf(stderr,
+ "VMDK: Not supported extent type \"%s\""".\n", type);
+ return -ENOTSUP;
+ }
+next_line:
+ /* move to next line */
+ while (*p && *p != '\n') {
+ p++;
+ }
+ p++;
+ }
+ return 0;
+}
+
+static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
+{
+ int ret;
+ char buf[2048];
+ char ct[128];
+ BDRVVmdkState *s = bs->opaque;
+
+ ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
+ if (ret < 0) {
+ return ret;
+ }
+ buf[2047] = '\0';
+ if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
+ return -EINVAL;
+ }
+ if (strcmp(ct, "monolithicFlat")) {
+ fprintf(stderr,
+ "VMDK: Not supported image type \"%s\""".\n", ct);
+ return -ENOTSUP;
+ }
+ s->desc_offset = 0;
+ ret = vmdk_parse_extents(buf, bs, bs->file->filename);
+ if (ret) {
+ return ret;
+ }
+
+ /* try to open parent images, if exist */
+ if (vmdk_parent_open(bs)) {
+ qemu_free(s->extents);
+ return -EINVAL;
+ }
+ s->parent_cid = vmdk_read_cid(bs, 1);
+ return 0;
+}
+
static int vmdk_open(BlockDriverState *bs, int flags)
{
uint32_t magic;
@@ -602,7 +755,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);
}
}
@@ -683,7 +836,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;
}
@@ -836,16 +989,20 @@ static int vmdk_read(BlockDriverState *bs, int64_t sector_num,
/* if not allocated, try to read from parent image, if exist */
if (bs->backing_hd) {
if (!vmdk_is_cid_valid(bs))
- return -1;
+ return -EINVAL;
ret = bdrv_read(bs->backing_hd, sector_num, buf, n);
if (ret < 0)
- return -1;
+ return ret;
} else {
memset(buf, 0, 512 * n);
}
} else {
- if(bdrv_pread(bs->file, cluster_offset + index_in_cluster * 512, buf, n * 512) != n * 512)
- return -1;
+ ret = bdrv_pread(extent->file,
+ cluster_offset + index_in_cluster * 512,
+ buf, n * 512);
+ if (ret != n * 512) {
+ return ret;
+ }
}
nb_sectors -= n;
sector_num += n;
@@ -869,7 +1026,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) {
@@ -892,16 +1049,18 @@ static int vmdk_write(BlockDriverState *bs, int64_t sector_num,
n = nb_sectors;
}
- if (bdrv_pwrite(bs->file,
+ ret = bdrv_pwrite(extent->file,
cluster_offset + index_in_cluster * 512,
- buf, n * 512)
- != n * 512) {
- return -1;
+ buf,
+ n * 512);
+ if (ret != n * 512) {
+ ret = ret < 0 ? ret : 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
* [Qemu-devel] [PATCH v5 10/12] VMDK: create different subformats
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (8 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 11/12] VMDK: fix coding style Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation Fam Zheng
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 | 563 ++++++++++++++++++++++++++++++++++------------------------
block_int.h | 1 +
2 files changed, 333 insertions(+), 231 deletions(-)
diff --git a/block/vmdk.c b/block/vmdk.c
index 8083c31..c8f3536 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -154,8 +154,8 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
#define CHECK_CID 1
#define SECTOR_SIZE 512
-#define DESC_SIZE 20*SECTOR_SIZE // 20 sectors of 512 bytes each
-#define HEADER_SIZE 512 // first sector of 512 bytes
+#define DESC_SIZE (20 * SECTOR_SIZE) /* 20 sectors of 512 bytes each */
+#define HEADER_SIZE 512 /* first sector of 512 bytes */
static uint32_t vmdk_read_cid(BlockDriverState *bs, int parent)
{
@@ -227,167 +227,6 @@ static int vmdk_is_cid_valid(BlockDriverState *bs)
return 1;
}
-static int vmdk_snapshot_create(const char *filename, const char *backing_file)
-{
- int snp_fd, p_fd;
- int ret;
- uint32_t p_cid;
- char *p_name, *gd_buf, *rgd_buf;
- const char *real_filename, *temp_str;
- VMDK4Header header;
- uint32_t gde_entries, gd_size;
- int64_t gd_offset, rgd_offset, capacity, gt_size;
- char p_desc[DESC_SIZE], s_desc[DESC_SIZE], hdr[HEADER_SIZE];
- static const char desc_template[] =
- "# Disk DescriptorFile\n"
- "version=1\n"
- "CID=%x\n"
- "parentCID=%x\n"
- "createType=\"monolithicSparse\"\n"
- "parentFileNameHint=\"%s\"\n"
- "\n"
- "# Extent description\n"
- "RW %u SPARSE \"%s\"\n"
- "\n"
- "# The Disk Data Base \n"
- "#DDB\n"
- "\n";
-
- snp_fd = open(filename, O_RDWR | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE, 0644);
- if (snp_fd < 0)
- return -errno;
- p_fd = open(backing_file, O_RDONLY | O_BINARY | O_LARGEFILE);
- if (p_fd < 0) {
- close(snp_fd);
- return -errno;
- }
-
- /* read the header */
- if (lseek(p_fd, 0x0, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (read(p_fd, hdr, HEADER_SIZE) != HEADER_SIZE) {
- ret = -errno;
- goto fail;
- }
-
- /* write the header */
- if (lseek(snp_fd, 0x0, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (write(snp_fd, hdr, HEADER_SIZE) == -1) {
- ret = -errno;
- goto fail;
- }
-
- memset(&header, 0, sizeof(header));
- memcpy(&header,&hdr[4], sizeof(header)); // skip the VMDK4_MAGIC
-
- if (ftruncate(snp_fd, header.grain_offset << 9)) {
- ret = -errno;
- goto fail;
- }
- if (lseek(p_fd, 0x200, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (read(p_fd, p_desc, DESC_SIZE) != DESC_SIZE) {
- ret = -errno;
- goto fail;
- }
-
- if ((p_name = strstr(p_desc,"CID")) != NULL) {
- p_name += sizeof("CID");
- sscanf(p_name,"%x",&p_cid);
- }
-
- real_filename = filename;
- if ((temp_str = strrchr(real_filename, '\\')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, '/')) != NULL)
- real_filename = temp_str + 1;
- if ((temp_str = strrchr(real_filename, ':')) != NULL)
- real_filename = temp_str + 1;
-
- snprintf(s_desc, sizeof(s_desc), desc_template, p_cid, p_cid, backing_file,
- (uint32_t)header.capacity, real_filename);
-
- /* write the descriptor */
- if (lseek(snp_fd, 0x200, SEEK_SET) == -1) {
- ret = -errno;
- goto fail;
- }
- if (write(snp_fd, s_desc, strlen(s_desc)) == -1) {
- ret = -errno;
- goto fail;
- }
-
- gd_offset = header.gd_offset * SECTOR_SIZE; // offset of GD table
- rgd_offset = header.rgd_offset * SECTOR_SIZE; // offset of RGD table
- capacity = header.capacity * SECTOR_SIZE; // Extent size
- /*
- * Each GDE span 32M disk, means:
- * 512 GTE per GT, each GTE points to grain
- */
- gt_size = (int64_t)header.num_gtes_per_gte * header.granularity * SECTOR_SIZE;
- if (!gt_size) {
- ret = -EINVAL;
- goto fail;
- }
- gde_entries = (uint32_t)(capacity / gt_size); // number of gde/rgde
- gd_size = gde_entries * sizeof(uint32_t);
-
- /* write RGD */
- rgd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, rgd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
- if (read(p_fd, rgd_buf, gd_size) != gd_size) {
- ret = -errno;
- goto fail_rgd;
- }
- if (lseek(snp_fd, rgd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
- if (write(snp_fd, rgd_buf, gd_size) == -1) {
- ret = -errno;
- goto fail_rgd;
- }
-
- /* write GD */
- gd_buf = qemu_malloc(gd_size);
- if (lseek(p_fd, gd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- if (read(p_fd, gd_buf, gd_size) != gd_size) {
- ret = -errno;
- goto fail_gd;
- }
- if (lseek(snp_fd, gd_offset, SEEK_SET) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- if (write(snp_fd, gd_buf, gd_size) == -1) {
- ret = -errno;
- goto fail_gd;
- }
- ret = 0;
-
-fail_gd:
- qemu_free(gd_buf);
-fail_rgd:
- qemu_free(rgd_buf);
-fail:
- close(p_fd);
- close(snp_fd);
- return ret;
-}
-
static int vmdk_parent_open(BlockDriverState *bs)
{
char *p_name;
@@ -1076,68 +915,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;
@@ -1148,7 +951,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);
@@ -1206,28 +1008,320 @@ 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 (!fmt) {
+ fmt = "monolithicSparse";
+ }
+ if (!strcmp(fmt, "monolithicFlat") || !strcmp(fmt, "twoGbMaxExtentFlat")) {
+ bool split = strcmp(fmt, "monolithicFlat");
+ const char desc_line_templ[] = "RW %lld FLAT \"%s\" 0\n";
+ int64_t filesize = total_size;
+ int idx = 1;
+
+ if (backing_file) {
+ /* not supporting backing file for flat image */
+ return -ENOTSUP;
+ }
+ while (filesize > 0) {
+ char desc_line[1024];
+ char ext_filename[1024];
+ char desc_filename[1024];
+ int64_t size = filesize;
+
+ if (split && size > split_size) {
+ size = split_size;
+ }
+ if (split) {
+ sprintf(desc_filename, "%s-f%03d%s",
+ prefix, idx++, postfix);
+ sprintf(ext_filename, "%s%s",
+ path, desc_filename);
+ } else {
+ sprintf(desc_filename, "%s-flat%s",
+ prefix, postfix);
+ sprintf(ext_filename, "%s%s",
+ path, desc_filename);
+ }
+ if (vmdk_create_flat(ext_filename, size)) {
+ return -EINVAL;
+ }
+ filesize -= size;
+
+ /* Format description line */
+ snprintf(desc_line, 1024,
+ desc_line_templ, size / 512, desc_filename);
+ pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line);
+ }
+
+ /* generate descriptor file */
+ snprintf(desc, sizeof(desc), desc_template,
+ (unsigned int)time(NULL),
+ 0xffffffff,
+ fmt,
+ "",
+ ext_desc_lines,
+ (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+ total_size / (int64_t)(63 * 16 * 512));
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
+ if (fd < 0) {
+ return -errno;
+ }
+ ret = qemu_write_full(fd, desc, strlen(desc));
+ if (ret != strlen(desc)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = 0;
+ } else if (!strcmp(fmt, "monolithicSparse")
+ || !strcmp(fmt, "twoGbMaxExtentSparse")) {
+ int ret;
+ int fd = 0;
+ int idx = 1;
+ int64_t filesize = total_size;
+ const char desc_line_templ[] = "RW %lld SPARSE \"%s\"\n";
+ char desc_line[1024];
+ char desc_filename[1024];
+ char ext_filename[1024];
+ bool split = strcmp(fmt, "monolithicSparse");
+ char parent_desc_line[1024] = "";
+ uint32_t parent_cid = 0xffffffff;
+
+ if (backing_file) {
+ char parent_filename[1024];
+ BlockDriverState *bs = bdrv_new("");
+ ret = bdrv_open(bs, backing_file, 0, NULL);
+ if (ret != 0) {
+ bdrv_delete(bs);
+ return ret;
+ }
+ filesize = bdrv_getlength(bs);
+ parent_cid = vmdk_read_cid(bs, 0);
+ bdrv_delete(bs);
+ relative_path(parent_filename, 1024, filename, backing_file);
+ snprintf(parent_desc_line, sizeof(parent_desc_line),
+ "parentFileNameHint=\"%s\"", parent_filename);
+ }
+ while (filesize > 0) {
+ int64_t size = filesize;
+ if (split && size > split_size) {
+ size = split_size;
+ }
+
+ if (split) {
+ snprintf(desc_filename, 1024,
+ "%s-s%03d%s", prefix, idx++, postfix);
+ snprintf(ext_filename, 1024, "%s%s", path, desc_filename);
+ } else {
+ snprintf(desc_filename, 1024, "%s%s", prefix, postfix);
+ snprintf(ext_filename, 1024, "%s%s", path, desc_filename);
+ }
+ ret = vmdk_create_sparse(ext_filename, size);
+ if (ret != 0) {
+ return ret;
+ }
+ filesize -= size;
+ snprintf(desc_line, 1024,
+ desc_line_templ, size / 512, desc_filename);
+ pstrcat(ext_desc_lines, sizeof(ext_desc_lines), desc_line);
+ }
+ /* generate descriptor file */
+ snprintf(desc, sizeof(desc), desc_template,
+ (unsigned int)time(NULL),
+ parent_cid,
+ fmt,
+ parent_desc_line,
+ ext_desc_lines,
+ (flags & BLOCK_FLAG_COMPAT6 ? 6 : 4),
+ total_size / (int64_t)(63 * 16 * 512));
+ if (split) {
+ fd = open(
+ filename,
+ O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
+ 0644);
+ } else {
+ fd = open(
+ filename,
+ O_WRONLY | O_BINARY | O_LARGEFILE,
+ 0644);
+ }
+ if (fd < 0) {
+ return -errno;
+ }
+ if (!split && 0x200 != lseek(fd, 0x200, SEEK_SET)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = qemu_write_full(fd, desc, strlen(desc));
+ if (ret != strlen(desc)) {
+ ret = -errno;
+ goto exit;
+ }
+ ret = 0;
+ } else {
+ fprintf(stderr, "Vmdk: unsupported format (%s)\n", fmt);
+ return -ENOTSUP;
+ }
exit:
close(fd);
return ret;
@@ -1278,6 +1372,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
* [Qemu-devel] [PATCH v5 11/12] VMDK: fix coding style
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (9 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 10/12] VMDK: create different subformats Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation Fam Zheng
11 siblings, 0 replies; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 c8f3536..f472e39 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -101,8 +101,9 @@ static int vmdk_probe(const uint8_t *buf, int buf_size, const char *filename)
{
uint32_t magic;
- if (buf_size < 4)
+ if (buf_size < 4) {
return 0;
+ }
magic = be32_to_cpu(*(uint32_t *)buf);
if (magic == VMDK3_MAGIC ||
magic == VMDK4_MAGIC) {
@@ -177,9 +178,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;
@@ -195,9 +197,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);
@@ -217,13 +220,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;
}
@@ -238,14 +242,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);
}
@@ -612,8 +620,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);
@@ -672,8 +681,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;
@@ -729,7 +739,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,
@@ -746,8 +756,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;
@@ -797,8 +808,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;
}
@@ -822,16 +834,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);
}
@@ -906,7 +921,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;
@@ -927,8 +943,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;
@@ -1383,16 +1400,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 v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
` (10 preceding siblings ...)
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 11/12] VMDK: fix coding style Fam Zheng
@ 2011-06-28 1:32 ` Fam Zheng
2011-06-29 15:27 ` Stefan Hajnoczi
11 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2011-06-28 1:32 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 | 21 +++++++++++++++++++++
block_int.h | 1 +
qemu-img.c | 31 +------------------------------
7 files changed, 93 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 f472e39..33ed5b9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -1372,6 +1372,26 @@ 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++) {
+ r = bdrv_get_allocated_file_size(s->extents[i].file);
+ if (r < 0) {
+ return r;
+ }
+ ret += r;
+ }
+ return ret;
+}
static QEMUOptionParameter vmdk_create_options[] = {
{
@@ -1410,6 +1430,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 v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation Fam Zheng
@ 2011-06-29 15:27 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-06-29 15:27 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Tue, Jun 28, 2011 at 2:32 AM, Fam Zheng <famcool@gmail.com> wrote:
Please use "block:" as the commit message tag instead of BlockDriver.
Usually the easiest way to find out which tag to use it by doing
git-log(1) on the main file you have modified and looking at previous
commit messages.
> +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++) {
> + r = bdrv_get_allocated_file_size(s->extents[i].file);
> + if (r < 0) {
> + return r;
> + }
> + ret += r;
> + }
> + return ret;
> +}
Does this count bs->file twice for images without a separate descriptor file?
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
@ 2011-06-29 15:57 ` Stefan Hajnoczi
2011-06-30 1:57 ` Fam Zheng
0 siblings, 1 reply; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-06-29 15:57 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Tue, Jun 28, 2011 at 2:32 AM, Fam Zheng <famcool@gmail.com> wrote:
> +/* 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;
> + }
This can produce false positives because strstr(desc, opt_name) will
match anything that contains the opt_name string. Also we don't
verify that "=\"" follow the opt_name. Luckily we only use this for
"createType" which will hopefully never cause false positives.
> + 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 = 0;
> + int r;
> + char access[11];
> + char type[11];
> + char fname[512];
> + const char *p = desc;
> + int64_t sectors = 0;
> + int64_t flat_offset;
> +
> + while (*p) {
> + if (strncmp(p, "RW", strlen("RW"))) {
> + goto next_line;
> + }
This check is duplicated below and can be removed.
> + /* parse extent line:
> + * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
> + * or
> + * RW [size in sectors] SPARSE "file-name.vmdk"
> + */
> + flat_offset = -1;
> + sscanf(p, "%10s %lld %10s %512s",
> + access, §ors, type, fname);
Please check the sscanf(3) return value to ensure the format string
matched. The value of access[], type[], fname[], sectors will be
unchanged at the point where sscanf(3) fails so your checks will not
work.
Declared as char fname[512] but using "%512s" format string. The
format string needs to be 511 to leave space for the NUL terminator.
> + if (!strcmp(type, "FLAT")) {
> + sscanf(p, "%10s %lld %10s %512s %lld",
> + access, §ors, type, fname, &flat_offset);
> + if (flat_offset == -1) {
> + return -EINVAL;
> + }
> + }
> +
> + /* trim the quotation marks around */
> + if (fname[0] == '"') {
> + memmove(fname, fname + 1, strlen(fname) + 1);
This copies 1 byte too many, just strlen(fname) will do.
> + if (fname[strlen(fname) - 1] == '"') {
> + fname[strlen(fname) - 1] = '\0';
> + }
> + }
If starts as fname[] = {'"', '\0'} then this if statement checks
fname[-1] == '"'!
> + if (!(strlen(access) && sectors && strlen(type) && strlen(fname))) {
> + goto next_line;
> + }
These can be replaced by checking the sscanf(3) return value above.
Validating sectors is still a good idea though.
> + if (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) {
> + goto next_line;
> + }
> + if (strcmp(access, "RW")) {
> + goto next_line;
> + }
> + ret++;
This variable is unused.
> + /* save to extents array */
> + if (!strcmp(type, "FLAT")) {
> + /* FLAT extent */
> + char extent_path[PATH_MAX];
> + BlockDriverState *extent_file;
> + BlockDriver *drv;
> + VmdkExtent *extent;
> +
> + extent_file = bdrv_new("");
> + drv = bdrv_find_format("file");
> + if (!drv) {
bdrv_delete(extent_file);
> + return -EINVAL;
> + }
> + path_combine(extent_path, sizeof(extent_path),
> + desc_file_path, fname);
> + r = bdrv_open(extent_file, extent_path,
> + BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
We should honor the bs->open_flags. Otherwise
cache=none|writeback|writethrough|unsafe is ignored on the actual
extent files.
> + if (r) {
bdrv_delete(extent_file);
> + return -EINVAL;
> + }
> + extent = vmdk_add_extent(bs, extent_file, true, sectors,
> + 0, 0, 0, 0, sectors);
> + extent->flat_start_offset = flat_offset;
> + } else {
> + /* SPARSE extent, not supported for now */
> + fprintf(stderr,
> + "VMDK: Not supported extent type \"%s\""".\n", type);
> + return -ENOTSUP;
> + }
> +next_line:
> + /* move to next line */
> + while (*p && *p != '\n') {
> + p++;
> + }
> + p++;
> + }
> + return 0;
> +}
> +
> +static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
> +{
> + int ret;
> + char buf[2048];
> + char ct[128];
> + BDRVVmdkState *s = bs->opaque;
> +
> + ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
> + if (ret < 0) {
> + return ret;
> + }
> + buf[2047] = '\0';
> + if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
> + return -EINVAL;
> + }
> + if (strcmp(ct, "monolithicFlat")) {
> + fprintf(stderr,
> + "VMDK: Not supported image type \"%s\""".\n", ct);
> + return -ENOTSUP;
> + }
> + s->desc_offset = 0;
> + ret = vmdk_parse_extents(buf, bs, bs->file->filename);
> + if (ret) {
> + return ret;
> + }
> +
> + /* try to open parent images, if exist */
> + if (vmdk_parent_open(bs)) {
> + qemu_free(s->extents);
This duplicates extent freeing code from vmdk_close(). Please reuse
that (maybe by moving it into a vmdk_free_extents() function), it also
frees l1_table, l2_cache, and l1_backup_table.
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image
2011-06-29 15:57 ` Stefan Hajnoczi
@ 2011-06-30 1:57 ` Fam Zheng
2011-06-30 6:13 ` Stefan Hajnoczi
0 siblings, 1 reply; 17+ messages in thread
From: Fam Zheng @ 2011-06-30 1:57 UTC (permalink / raw)
To: Stefan Hajnoczi; +Cc: kwolf, qemu-devel, hch
On Wed, Jun 29, 2011 at 11:57 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
> On Tue, Jun 28, 2011 at 2:32 AM, Fam Zheng <famcool@gmail.com> wrote:
>> +/* 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;
>> + }
>
> This can produce false positives because strstr(desc, opt_name) will
> match anything that contains the opt_name string. Also we don't
> verify that "=\"" follow the opt_name. Luckily we only use this for
> "createType" which will hopefully never cause false positives.
>
>> + 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 = 0;
>> + int r;
>> + char access[11];
>> + char type[11];
>> + char fname[512];
>> + const char *p = desc;
>> + int64_t sectors = 0;
>> + int64_t flat_offset;
>> +
>> + while (*p) {
>> + if (strncmp(p, "RW", strlen("RW"))) {
>> + goto next_line;
>> + }
>
> This check is duplicated below and can be removed.
>
>> + /* parse extent line:
>> + * RW [size in sectors] FLAT "file-name.vmdk" OFFSET
>> + * or
>> + * RW [size in sectors] SPARSE "file-name.vmdk"
>> + */
>> + flat_offset = -1;
>> + sscanf(p, "%10s %lld %10s %512s",
>> + access, §ors, type, fname);
>
> Please check the sscanf(3) return value to ensure the format string
> matched. The value of access[], type[], fname[], sectors will be
> unchanged at the point where sscanf(3) fails so your checks will not
> work.
>
> Declared as char fname[512] but using "%512s" format string. The
> format string needs to be 511 to leave space for the NUL terminator.
>
>> + if (!strcmp(type, "FLAT")) {
>> + sscanf(p, "%10s %lld %10s %512s %lld",
>> + access, §ors, type, fname, &flat_offset);
>> + if (flat_offset == -1) {
>> + return -EINVAL;
>> + }
>> + }
>> +
>> + /* trim the quotation marks around */
>> + if (fname[0] == '"') {
>> + memmove(fname, fname + 1, strlen(fname) + 1);
>
> This copies 1 byte too many, just strlen(fname) will do.
I meant to copy the NULL terminator too.
>
>> + if (fname[strlen(fname) - 1] == '"') {
>> + fname[strlen(fname) - 1] = '\0';
>> + }
>> + }
>
> If starts as fname[] = {'"', '\0'} then this if statement checks
> fname[-1] == '"'!
>
>> + if (!(strlen(access) && sectors && strlen(type) && strlen(fname))) {
>> + goto next_line;
>> + }
>
> These can be replaced by checking the sscanf(3) return value above.
> Validating sectors is still a good idea though.
>
>> + if (strcmp(type, "FLAT") && strcmp(type, "SPARSE")) {
>> + goto next_line;
>> + }
>> + if (strcmp(access, "RW")) {
>> + goto next_line;
>> + }
>> + ret++;
>
> This variable is unused.
>
>> + /* save to extents array */
>> + if (!strcmp(type, "FLAT")) {
>> + /* FLAT extent */
>> + char extent_path[PATH_MAX];
>> + BlockDriverState *extent_file;
>> + BlockDriver *drv;
>> + VmdkExtent *extent;
>> +
>> + extent_file = bdrv_new("");
>> + drv = bdrv_find_format("file");
>> + if (!drv) {
>
> bdrv_delete(extent_file);
>
>> + return -EINVAL;
>> + }
>> + path_combine(extent_path, sizeof(extent_path),
>> + desc_file_path, fname);
>> + r = bdrv_open(extent_file, extent_path,
>> + BDRV_O_RDWR | BDRV_O_NO_BACKING, drv);
>
> We should honor the bs->open_flags. Otherwise
> cache=none|writeback|writethrough|unsafe is ignored on the actual
> extent files.
>
>> + if (r) {
>
> bdrv_delete(extent_file);
>
>> + return -EINVAL;
>> + }
>> + extent = vmdk_add_extent(bs, extent_file, true, sectors,
>> + 0, 0, 0, 0, sectors);
>> + extent->flat_start_offset = flat_offset;
>> + } else {
>> + /* SPARSE extent, not supported for now */
>> + fprintf(stderr,
>> + "VMDK: Not supported extent type \"%s\""".\n", type);
>> + return -ENOTSUP;
>> + }
>> +next_line:
>> + /* move to next line */
>> + while (*p && *p != '\n') {
>> + p++;
>> + }
>> + p++;
>> + }
>> + return 0;
>> +}
>> +
>> +static int vmdk_open_desc_file(BlockDriverState *bs, int flags)
>> +{
>> + int ret;
>> + char buf[2048];
>> + char ct[128];
>> + BDRVVmdkState *s = bs->opaque;
>> +
>> + ret = bdrv_pread(bs->file, 0, buf, sizeof(buf));
>> + if (ret < 0) {
>> + return ret;
>> + }
>> + buf[2047] = '\0';
>> + if (vmdk_parse_description(buf, "createType", ct, sizeof(ct))) {
>> + return -EINVAL;
>> + }
>> + if (strcmp(ct, "monolithicFlat")) {
>> + fprintf(stderr,
>> + "VMDK: Not supported image type \"%s\""".\n", ct);
>> + return -ENOTSUP;
>> + }
>> + s->desc_offset = 0;
>> + ret = vmdk_parse_extents(buf, bs, bs->file->filename);
>> + if (ret) {
>> + return ret;
>> + }
>> +
>> + /* try to open parent images, if exist */
>> + if (vmdk_parent_open(bs)) {
>> + qemu_free(s->extents);
>
> This duplicates extent freeing code from vmdk_close(). Please reuse
> that (maybe by moving it into a vmdk_free_extents() function), it also
> frees l1_table, l2_cache, and l1_backup_table.
>
> Stefan
>
--
Best regards!
Fam Zheng
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image
2011-06-30 1:57 ` Fam Zheng
@ 2011-06-30 6:13 ` Stefan Hajnoczi
0 siblings, 0 replies; 17+ messages in thread
From: Stefan Hajnoczi @ 2011-06-30 6:13 UTC (permalink / raw)
To: Fam Zheng; +Cc: kwolf, qemu-devel, hch
On Thu, Jun 30, 2011 at 2:57 AM, Fam Zheng <famcool@gmail.com> wrote:
> On Wed, Jun 29, 2011 at 11:57 PM, Stefan Hajnoczi <stefanha@gmail.com> wrote:
>> On Tue, Jun 28, 2011 at 2:32 AM, Fam Zheng <famcool@gmail.com> wrote:
>>> + /* trim the quotation marks around */
>>> + if (fname[0] == '"') {
>>> + memmove(fname, fname + 1, strlen(fname) + 1);
>>
>> This copies 1 byte too many, just strlen(fname) will do.
> I meant to copy the NULL terminator too.
Yes. The problem is the copying starts at fname + 1 but strlen(3)
starts at fname. So there is already an extra byte. The + 1 adds an
additional byte after the NUL.
Stefan
^ permalink raw reply [flat|nested] 17+ messages in thread
end of thread, other threads:[~2011-06-30 6:13 UTC | newest]
Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-28 1:32 [Qemu-devel] [PATCH v5 00/12] Adding VMDK monolithic flat support Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 01/12] VMDK: introduce VmdkExtent Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 02/12] VMDK: bugfix, align offset to cluster in get_whole_cluster Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 03/12] VMDK: probe for monolithicFlat images Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 04/12] VMDK: separate vmdk_open by format version Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 05/12] VMDK: add field BDRVVmdkState.desc_offset Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 06/12] VMDK: flush multiple extents Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 07/12] VMDK: move 'static' cid_update flag to bs field Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 08/12] VMDK: change get_cluster_offset return type Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 09/12] VMDK: open/read/write for monolithicFlat image Fam Zheng
2011-06-29 15:57 ` Stefan Hajnoczi
2011-06-30 1:57 ` Fam Zheng
2011-06-30 6:13 ` Stefan Hajnoczi
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 10/12] VMDK: create different subformats Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 11/12] VMDK: fix coding style Fam Zheng
2011-06-28 1:32 ` [Qemu-devel] [PATCH v5 12/12] BlockDriver: add bdrv_get_allocated_file_size() operation Fam Zheng
2011-06-29 15:27 ` Stefan Hajnoczi
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.