[Qemu-devel] [PATCH 5/7] qcow2: use byte-based read in qcow2_decompress_cluster

Vladimir Sementsov-Ogievskiy posted 7 patches 7 years, 3 months ago
[Qemu-devel] [PATCH 5/7] qcow2: use byte-based read in qcow2_decompress_cluster
Posted by Vladimir Sementsov-Ogievskiy 7 years, 3 months ago
We are gradually moving away from sector-based interfaces, towards
byte-based.  Get rid of it here too.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 block/qcow2.c | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index e9d24b801e..950b9f7ec6 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -3956,14 +3956,15 @@ fail:
 int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
 {
     BDRVQcow2State *s = bs->opaque;
-    int ret, csize, nb_csectors, sector_offset;
+    int ret, csize, nb_csectors;
     uint64_t coffset;
+    struct iovec iov;
+    QEMUIOVector local_qiov;
 
     coffset = cluster_offset & s->cluster_offset_mask;
     if (s->cluster_cache_offset != coffset) {
         nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
-        sector_offset = coffset & 511;
-        csize = nb_csectors * 512 - sector_offset;
+        csize = nb_csectors * 512 - (coffset & 511);
 
         /* Allocate buffers on first decompress operation, most images are
          * uncompressed and the memory overhead can be avoided.  The buffers
@@ -3981,14 +3982,17 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
             s->cluster_cache = g_malloc(s->cluster_size);
         }
 
+        iov.iov_base = s->cluster_data;
+        iov.iov_len = csize;
+        qemu_iovec_init_external(&local_qiov, &iov, 1);
+
         BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
-        ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
-                        nb_csectors);
+        ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov, 0);
         if (ret < 0) {
             return ret;
         }
         if (qcow2_decompress(s->cluster_cache, s->cluster_size,
-                             s->cluster_data + sector_offset, csize) < 0) {
+                             s->cluster_data, csize) < 0) {
             return -EIO;
         }
         s->cluster_cache_offset = coffset;
-- 
2.18.0


Re: [Qemu-devel] [Qemu-block] [PATCH 5/7] qcow2: use byte-based read in qcow2_decompress_cluster
Posted by Alberto Garcia 7 years, 3 months ago
On Thu 01 Nov 2018 07:27:36 PM CET, Vladimir Sementsov-Ogievskiy wrote:

> diff --git a/block/qcow2.c b/block/qcow2.c
> index e9d24b801e..950b9f7ec6 100644
> --- a/block/qcow2.c
> +++ b/block/qcow2.c
> @@ -3956,14 +3956,15 @@ fail:
>  int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
>  {
>      BDRVQcow2State *s = bs->opaque;
> -    int ret, csize, nb_csectors, sector_offset;
> +    int ret, csize, nb_csectors;
>      uint64_t coffset;
> +    struct iovec iov;
> +    QEMUIOVector local_qiov;
>  
>      coffset = cluster_offset & s->cluster_offset_mask;
>      if (s->cluster_cache_offset != coffset) {
>          nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
> -        sector_offset = coffset & 511;
> -        csize = nb_csectors * 512 - sector_offset;
> +        csize = nb_csectors * 512 - (coffset & 511);
>  
>          /* Allocate buffers on first decompress operation, most images are
>           * uncompressed and the memory overhead can be avoided.  The buffers
> @@ -3981,14 +3982,17 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
>              s->cluster_cache = g_malloc(s->cluster_size);
>          }
>  
> +        iov.iov_base = s->cluster_data;
> +        iov.iov_len = csize;
> +        qemu_iovec_init_external(&local_qiov, &iov, 1);
> +
>          BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
> -        ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
> -                        nb_csectors);
> +        ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov,
> 0);

I think you should annotate the function with coroutine_fn or use
bdrv_pread() instead.

Berto

Re: [Qemu-devel] [Qemu-block] [PATCH 5/7] qcow2: use byte-based read in qcow2_decompress_cluster
Posted by Vladimir Sementsov-Ogievskiy 7 years, 3 months ago
06.11.2018 16:53, Alberto Garcia wrote:
> On Thu 01 Nov 2018 07:27:36 PM CET, Vladimir Sementsov-Ogievskiy wrote:
>
>> diff --git a/block/qcow2.c b/block/qcow2.c
>> index e9d24b801e..950b9f7ec6 100644
>> --- a/block/qcow2.c
>> +++ b/block/qcow2.c
>> @@ -3956,14 +3956,15 @@ fail:
>>   int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
>>   {
>>       BDRVQcow2State *s = bs->opaque;
>> -    int ret, csize, nb_csectors, sector_offset;
>> +    int ret, csize, nb_csectors;
>>       uint64_t coffset;
>> +    struct iovec iov;
>> +    QEMUIOVector local_qiov;
>>   
>>       coffset = cluster_offset & s->cluster_offset_mask;
>>       if (s->cluster_cache_offset != coffset) {
>>           nb_csectors = ((cluster_offset >> s->csize_shift) & s->csize_mask) + 1;
>> -        sector_offset = coffset & 511;
>> -        csize = nb_csectors * 512 - sector_offset;
>> +        csize = nb_csectors * 512 - (coffset & 511);
>>   
>>           /* Allocate buffers on first decompress operation, most images are
>>            * uncompressed and the memory overhead can be avoided.  The buffers
>> @@ -3981,14 +3982,17 @@ int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset)
>>               s->cluster_cache = g_malloc(s->cluster_size);
>>           }
>>   
>> +        iov.iov_base = s->cluster_data;
>> +        iov.iov_len = csize;
>> +        qemu_iovec_init_external(&local_qiov, &iov, 1);
>> +
>>           BLKDBG_EVENT(bs->file, BLKDBG_READ_COMPRESSED);
>> -        ret = bdrv_read(bs->file, coffset >> 9, s->cluster_data,
>> -                        nb_csectors);
>> +        ret = bdrv_co_preadv(bs->file, coffset, csize, &local_qiov,
>> 0);
> I think you should annotate the function with coroutine_fn or use
> bdrv_pread() instead.
>
> Berto

it is called only from qcow2_co_preadv, so it's ok to move to 
already-in-coroutine behaviour. I'll add coroutine_fn if new version is 
needed, or it can be added inflight.

-- 
Best regards,
Vladimir