1. Patchew
  2. QEMU
  3. [Qemu-devel] [PATCH v3 0/6] Optiomize VMDK I/O by allocating multiple clusters
  4. View patch

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

Ashijeet Acharya posted 6 patches 8 years, 10 months ago
[Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
Posted by Ashijeet Acharya 8 years, 10 months ago 
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 <ashijeetacharya@gmail.com>
---
 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;
+
         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 */
+        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
Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
Posted by Fam Zheng 8 years, 9 months ago 
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 <ashijeetacharya@gmail.com>
> ---
>  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
Re: [Qemu-devel] [PATCH v3 5/6] vmdk: Set maximum bytes allocated in one cycle
Posted by Ashijeet Acharya 8 years, 9 months ago 
On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng <famz@redhat.com> 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 <ashijeetacharya@gmail.com>
>> ---
>>  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
Posted by Ashijeet Acharya 8 years, 9 months ago 
On Fri, Apr 21, 2017 at 8:23 PM, Ashijeet Acharya
<ashijeetacharya@gmail.com> wrote:
> On Wed, Apr 19, 2017 at 6:30 PM, Fam Zheng <famz@redhat.com> 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 <ashijeetacharya@gmail.com>
>>> ---
>>>  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

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.