Re: [Qemu-devel] [PATCH 2/2] block/io: use qemu_iovec_init_buf

[Vladimir Sementsov-Ogievskiy]

30.01.2019 0:41, Eric Blake wrote:
> On 1/29/19 8:38 AM, Vladimir Sementsov-Ogievskiy wrote:
>> Use new qemu_iovec_init_buf() instead of
>> qemu_iovec_init_external( ... , 1), which simplifies the code.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> ---
>>   block/io.c | 89 ++++++++++++------------------------------------------
>>   1 file changed, 20 insertions(+), 69 deletions(-)
>>
>> diff --git a/block/io.c b/block/io.c
>> index bd9d688f8b..80961910a6 100644
>> --- a/block/io.c
>> +++ b/block/io.c
>> @@ -842,17 +842,13 @@ static int bdrv_prwv_co(BdrvChild *child, int64_t 
>> offset,
>>   static int bdrv_rw_co(BdrvChild *child, int64_t sector_num, uint8_t *buf,
>>                         int nb_sectors, bool is_write, BdrvRequestFlags 
>> flags)
>>   {
>> -    QEMUIOVector qiov;
>> -    struct iovec iov = {
>> -        .iov_base = (void *)buf,
>> -        .iov_len = nb_sectors * BDRV_SECTOR_SIZE,
>> -    };
>> +    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, buf,
>> +                                            nb_sectors * BDRV_SECTOR_SIZE);
> 
> Okay, so you ARE using both the macro...
> 
>> @@ -977,17 +963,12 @@ int bdrv_pwritev(BdrvChild *child, int64_t offset, 
>> QEMUIOVector *qiov)
>>   
>>   int bdrv_pwrite(BdrvChild *child, int64_t offset, const void *buf, int 
>> bytes)
>>   {
>> -    QEMUIOVector qiov;
>> -    struct iovec iov = {
>> -        .iov_base   = (void *) buf,
>> -        .iov_len    = bytes,
>> -    };
>> +    QEMUIOVector qiov = QEMU_IOVEC_INIT_BUF(qiov, NULL, bytes);
> 
> Ouch - why are you passing NULL instead of buf?  But this answers why
> the macro had to cast - it is indeed casting away const.
> 
>> @@ -1229,9 +1209,7 @@ static int coroutine_fn 
>> bdrv_co_do_copy_on_readv(BdrvChild *child,
>>   
>>           if (ret <= 0) {
>>               /* Must copy-on-read; use the bounce buffer */
>> -            iov.iov_base = bounce_buffer;
>> -            iov.iov_len = pnum = MIN(pnum, MAX_BOUNCE_BUFFER);
>> -            qemu_iovec_init_external(&local_qiov, &iov, 1);
>> +            qemu_iovec_init_buf(&local_qiov, bounce_buffer, pnum);
> 
> ...and the function.
> 
> Ouch - why did you lose the assignment of pnum = MIN() here?
> 

Thank you, both hit the mark, I was inattentive.


-- 
Best regards,
Vladimir