On Thu, May 15, 2014 at 03:02:48PM +0800, Fam Zheng wrote:
> @@ -397,6 +397,7 @@ static int vmdk_add_extent(BlockDriverState *bs,
> {
> VmdkExtent *extent;
> BDRVVmdkState *s = bs->opaque;
> + int64_t ret;
>
> if (cluster_sectors > 0x200000) {
> /* 0x200000 * 512Bytes = 1GB for one cluster is unrealistic */
> @@ -428,6 +429,12 @@ static int vmdk_add_extent(BlockDriverState *bs,
> extent->l2_size = l2_size;
> extent->cluster_sectors = flat ? sectors : cluster_sectors;
>
> + ret = bdrv_getlength(extent->file);
> + if (ret < 0) {
> + return ret;
We need to clean up here. extent isn't initialized fully and
s->num_extents needs to be decremented.
> + }
> + extent->next_cluster_sector = DIV_ROUND_UP(ret, BDRV_SECTOR_SIZE);
Should this be rounded up to clusters instead of just 512-byte sectors?
Otherwise the next allocated cluster might not be aligned.
> +
> if (s->num_extents > 1) {
> extent->end_sector = (*(extent - 1)).end_sector + extent->sectors;
> } else {
> @@ -954,57 +961,96 @@ static int vmdk_refresh_limits(BlockDriverState *bs)
> return 0;
> }
>
> +/**
> + * get_whole_cluster
> + *
> + * Copy backing file's cluster that covers @sector_num, otherwise write zero,
> + * to the cluster at @cluster_sector_num.
> + *
> + * If @skip_start < @skip_end, the relative range [@skip_start, @skip_end) is
> + * not copied or written, and leave it for call to write user data in the
> + * request.
skip_start/skip_end are in sectors, please include the unit in the
argument name.
