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

2018-11-06 Thread Vladimir Sementsov-Ogievskiy 
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(_qiov, , 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, _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

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

2018-11-06 Thread Alberto Garcia 
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(_qiov, , 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, _qiov,
> 0);

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

Berto

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

2018-11-01 Thread Vladimir Sementsov-Ogievskiy 
We are gradually moving away from sector-based interfaces, towards
byte-based.  Get rid of it here too.

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
 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(_qiov, , 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, _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