Re: [Qemu-devel] [PATCH 01/10] block/io: add bdrv_co_write_compressed

[Eric Blake]

On 05/14/2016 06:45 AM, Denis V. Lunev wrote:
> From: Pavel Butsykin <address@hidden>
> 
> This patch just adds the interface to the bdrv_co_write_compressed, which
> is currently not used but will be useful for safe implementation of the
> bdrv_co_write_compressed callback in format drivers.
> 
> 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>
> ---

> +++ b/block/io.c
> @@ -1828,8 +1828,8 @@ int bdrv_is_allocated_above(BlockDriverState *top,
>      return 0;
>  }
>  
> -int bdrv_write_compressed(BlockDriverState *bs, int64_t sector_num,
> -                          const uint8_t *buf, int nb_sectors)
> +int bdrv_co_write_compressed(BlockDriverState *bs, int64_t sector_num,
> +                             int nb_sectors, QEMUIOVector *qiov)

As long as we're adding a new public interface, I'd really like us to
make it byte-based.  int64_t sector_num might be better represented as a
byte offset, and int nb_sectors seems redundant with qiov->size.

>  {
>      BlockDriver *drv = bs->drv;
>      int ret;
> @@ -1837,7 +1837,7 @@ int bdrv_write_compressed(BlockDriverState *bs, int64_t 
> sector_num,
>      if (!drv) {
>          return -ENOMEDIUM;
>      }
> -    if (!drv->bdrv_write_compressed) {
> +    if (!drv->bdrv_co_write_compressed) {
>          return -ENOTSUP;
>      }
>      ret = bdrv_check_request(bs, sector_num, nb_sectors);
> @@ -1846,8 +1846,71 @@ int bdrv_write_compressed(BlockDriverState *bs, 
> int64_t sector_num,
>      }
>  
>      assert(QLIST_EMPTY(&bs->dirty_bitmaps));
> +    assert(qemu_in_coroutine());
> +
> +    return drv->bdrv_co_write_compressed(bs, sector_num, nb_sectors, qiov);

Of course, if you make the public interface byte-based, then calling
into the back end will have to scale back to sectors (after first
asserting that we aren't violating the scaling); see how Kevin did it in
commit 166fe9605.

> +}
> +
> +typedef struct BdrvWriteCompressedCo {
> +    BlockDriverState *bs;
> +    int64_t sector_num;

Again, I think a byte offset is smarter than a sector number.

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

signature.asc
Description: OpenPGP digital signature