On Mon, Jun 27, 2011 at 4:48 AM, Fam Zheng <famc...@gmail.com> wrote:
> - extent = s->extents;
> - extent->flat = false;
> - extent->file = bs->file;
> - extent->cluster_sectors = le32_to_cpu(header.granularity);
> - extent->l2_size = 1 << 9;
> - extent->l1_size = 1 << 6;
> - extent->sectors = le32_to_cpu(header.disk_sectors);
> - extent->end_sector = le32_to_cpu(header.disk_sectors);
> - extent->l1_table_offset = le32_to_cpu(header.l1dir_offset) << 9;
> - extent->l1_backup_table_offset = 0;
> - extent->l1_entry_sectors = extent->l2_size * extent->cluster_sectors;
Please move vmdk_add_extent() to the patch where you added this code.
A nice patch series minimizes the amount of code that gets added
temporarily and then removed again.
> +/* Create and append extent to the entext array. Return the added VmdkExtent
s/entext/extent/
> + * address. return NULL if allocation failed. */
> +static int 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 **new_extent)
> +{
> + VmdkExtent *extent, *p;
> + BDRVVmdkState *s = bs->opaque;
> +
> + p = qemu_realloc(s->extents, (s->num_extents + 1) * sizeof(VmdkExtent));
> + if (!p) {
> + return -ENOMEM;
> + }
qemu_realloc() never returns NULL.
> + s->extents = p;
> + 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;
> + if (new_extent) {
> + *new_extent = extent;
> + }
> + return 0;
Since this function cannot fail it would be simpler to make the return
value VmdkExtent *.
Stefan
