Re: [Qemu-devel] [PATCH 01/11] block: switch blk_write_compressed() to byte-based interface

[Pavel Butsykin <pbutsykin@virtuozzo.com>]

On 13.06.2016 17:23, Eric Blake wrote:
> On 05/31/2016 03:15 AM, Denis V. Lunev wrote:
>> From: Pavel Butsykin <pbutsykin@virtuozzo.com>
>>
>> This is a preparatory patch, which continues the general trend of the transition
>> to the byte-based interfaces.
>>
>> Signed-off-by: Pavel Butsykin <pbutsykin@virtuozzo.com>
>> Signed-off-by: Denis V. Lunev <den@openvz.org>
>> CC: Jeff Cody <jcody@redhat.com>
>> CC: Markus Armbruster <armbru@redhat.com>
>> CC: Eric Blake <eblake@redhat.com>
>> CC: John Snow <jsnow@redhat.com>
>> CC: Stefan Hajnoczi <stefanha@redhat.com>
>> CC: Kevin Wolf <kwolf@redhat.com>
>> ---
>>   block/block-backend.c          | 8 ++++----
>>   block/io.c                     | 9 +++++----
>>   include/block/block.h          | 4 ++--
>>   include/sysemu/block-backend.h | 4 ++--
>>   qemu-img.c                     | 6 ++++--
>>   qemu-io-cmds.c                 | 2 +-
>>   6 files changed, 18 insertions(+), 15 deletions(-)
>>
>> diff --git a/block/block-backend.c b/block/block-backend.c
>> index 34500e6..3c1fc50 100644
>> --- a/block/block-backend.c
>> +++ b/block/block-backend.c
>> @@ -1477,15 +1477,15 @@ int coroutine_fn blk_co_pwrite_zeroes(BlockBackend *blk, int64_t offset,
>>                             flags | BDRV_REQ_ZERO_WRITE);
>>   }
>>
>> -int blk_write_compressed(BlockBackend *blk, int64_t sector_num,
>> -                         const uint8_t *buf, int nb_sectors)
>> +int blk_pwrite_compressed(BlockBackend *blk, int64_t offset, const void *buf,
>> +                          int count)
>
> Why are you switching the type of buf?  It's not necessarily wrong, but
> the commit message should call it out as intentional.

Here I just tried to make the interface like blk_pwrite, it has no other
meaning more..

>
>> -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
>> -                          const uint8_t *buf, int nb_sectors)
>> +int bdrv_pwrite_compressed(BlockDriverState *bs, int64_t offset,
>> +                           const void *buf, int count)
>>   {
>>       BlockDriver *drv = bs->drv;
>>       int ret;
>> @@ -1791,14 +1791,15 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
>>       if (!drv->bdrv_write_compressed) {
>>           return -ENOTSUP;
>>       }
>> -    ret = bdrv_check_request(bs, sector_num, nb_sectors);
>> +    ret = bdrv_check_byte_request(bs, offset, count);
>>       if (ret < 0) {
>>           return ret;
>>       }
>>
>>       assert(QLIST_EMPTY(&bs->dirty_bitmaps));
>>
>> -    return drv->bdrv_write_compressed(bs, sector_num, buf, nb_sectors);
>> +    return drv->bdrv_write_compressed(bs, offset >> BDRV_SECTOR_BITS, buf,
>> +                                      count >> BDRV_SECTOR_BITS);
>
> If you are going to shift right, you need to first assert that offset
> and count are aligned (and thus that our call to a sector interface
> isn't going to operate on the wrong data).  See for example commit 166fe960.
>

ok, thanks