Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle

[Ashijeet Acharya]

On Fri, Apr 21, 2017 at 8:23 PM, Ashijeet Acharya
<address@hidden> wrote:
> On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng <address@hidden> 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 <address@hidden>
>>> ---
>>>  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