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

[Pavel Butsykin]

On 13.06.2016 17:23, Eric Blake wrote:
> On 05/31/2016 03:15 AM, Denis V. Lunev wrote:
> > From: Pavel Butsykin <address@hidden>
> >
> > This is a preparatory patch, which continues the general trend of the transition
> > to the byte-based interfaces.
> >
> > Signed-off-by: Pavel Butsykin <address@hidden>
> > Signed-off-by: Denis V. Lunev <address@hidden>
> > CC: Jeff Cody <address@hidden>
> > CC: Markus Armbruster <address@hidden>
> > CC: Eric Blake <address@hidden>
> > CC: John Snow <address@hidden>
> > CC: Stefan Hajnoczi <address@hidden>
> > CC: Kevin Wolf <address@hidden>
> > ---
> >   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