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

[Eric Blake <eblake@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2919 bytes --]

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.


> -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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]