Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
On Fri, Apr 21, 2017 at 8:23 PM, Ashijeet Acharya
wrote:
> On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng wrote:
>> On Sat, 04/01 20:14, Ashijeet Acharya wrote:
>>> Set the maximum bytes allowed to get allocated at once to be not more
>>> than the extent size boundary to handle writes at two separate extents
>>> appropriately.
>>>
>>> Signed-off-by: Ashijeet Acharya
>>> ---
>>> block/vmdk.c | 13 +++--
>>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/block/vmdk.c b/block/vmdk.c
>>> index a8babd7..9456ddd 100644
>>> --- a/block/vmdk.c
>>> +++ b/block/vmdk.c
>>> @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs,
>>> uint64_t offset,
>>> int64_t offset_in_cluster, n_bytes;
>>> uint64_t cluster_offset;
>>> uint64_t bytes_done = 0;
>>> +uint64_t extent_size;
>>> VmdkMetaData m_data;
>>> uint32_t total_alloc_clusters = 0;
>>>
>>> @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs,
>>> uint64_t offset,
>>> if (!extent) {
>>> return -EIO;
>>> }
>>> +extent_size = extent->end_sector * BDRV_SECTOR_SIZE;
>>
>> Maybe extent_end to be more accurate?
>
> Done
>
>>> +
>>> offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
>>> -n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
>>> - - offset_in_cluster);
>>> +
>>> +/* truncate n_bytes to first cluster because we need to perform
>>> COW */
>>
>> Makes sense, but shouldn't this be squashed into patch patch 3? Because it
>> looks
>> like it is fixing an intermediate bug.
>
> Did you mean that I should merge this whole patch into patch 3? Maybe
> moving it before patch 3 rather than squashing it make more sense?
Instead I have moved it before patch 3 in v4
Ashijeet
Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng wrote:
> On Sat, 04/01 20:14, Ashijeet Acharya wrote:
>> Set the maximum bytes allowed to get allocated at once to be not more
>> than the extent size boundary to handle writes at two separate extents
>> appropriately.
>>
>> Signed-off-by: Ashijeet Acharya
>> ---
>> block/vmdk.c | 13 +++--
>> 1 file changed, 11 insertions(+), 2 deletions(-)
>>
>> diff --git a/block/vmdk.c b/block/vmdk.c
>> index a8babd7..9456ddd 100644
>> --- a/block/vmdk.c
>> +++ b/block/vmdk.c
>> @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t
>> offset,
>> int64_t offset_in_cluster, n_bytes;
>> uint64_t cluster_offset;
>> uint64_t bytes_done = 0;
>> +uint64_t extent_size;
>> VmdkMetaData m_data;
>> uint32_t total_alloc_clusters = 0;
>>
>> @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs,
>> uint64_t offset,
>> if (!extent) {
>> return -EIO;
>> }
>> +extent_size = extent->end_sector * BDRV_SECTOR_SIZE;
>
> Maybe extent_end to be more accurate?
Done
>> +
>> offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
>> -n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
>> - - offset_in_cluster);
>> +
>> +/* truncate n_bytes to first cluster because we need to perform COW
>> */
>
> Makes sense, but shouldn't this be squashed into patch patch 3? Because it
> looks
> like it is fixing an intermediate bug.
Did you mean that I should merge this whole patch into patch 3? Maybe
moving it before patch 3 rather than squashing it make more sense?
Ashijeet
Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
On Sat, 04/01 20:14, Ashijeet Acharya wrote:
> Set the maximum bytes allowed to get allocated at once to be not more
> than the extent size boundary to handle writes at two separate extents
> appropriately.
>
> Signed-off-by: Ashijeet Acharya
> ---
> block/vmdk.c | 13 +++--
> 1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/block/vmdk.c b/block/vmdk.c
> index a8babd7..9456ddd 100644
> --- a/block/vmdk.c
> +++ b/block/vmdk.c
> @@ -1767,6 +1767,7 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t
> offset,
> int64_t offset_in_cluster, n_bytes;
> uint64_t cluster_offset;
> uint64_t bytes_done = 0;
> +uint64_t extent_size;
> VmdkMetaData m_data;
> uint32_t total_alloc_clusters = 0;
>
> @@ -1782,9 +1783,17 @@ static int vmdk_pwritev(BlockDriverState *bs, uint64_t
> offset,
> if (!extent) {
> return -EIO;
> }
> +extent_size = extent->end_sector * BDRV_SECTOR_SIZE;
Maybe extent_end to be more accurate?
> +
> offset_in_cluster = vmdk_find_offset_in_cluster(extent, offset);
> -n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
> - - offset_in_cluster);
> +
> +/* truncate n_bytes to first cluster because we need to perform COW
> */
Makes sense, but shouldn't this be squashed into patch patch 3? Because it looks
like it is fixing an intermediate bug.
> +if (offset_in_cluster > 0) {
> +n_bytes = MIN(bytes, extent->cluster_sectors * BDRV_SECTOR_SIZE
> + - offset_in_cluster);
> +} else {
> +n_bytes = MIN(bytes, extent_size - offset);
> +}
>
> ret = vmdk_alloc_cluster_offset(bs, extent, &m_data, offset,
> !(extent->compressed || zeroed),
> --
> 2.6.2
>
Fam
