13.06.2019 1:09, Max Reitz wrote:
> With bdrv_filtered_rw_bs(), we can easily handle this default filter
> behavior in bdrv_co_block_status().
>
> blkdebug wants to have an additional assertion, so it keeps its own
> implementation, except bdrv_co_block_status_from_file() needs to be
> inlined there.
>
> Suggested-by: Eric Blake <eblake@redhat.com>
> Signed-off-by: Max Reitz <mreitz@redhat.com>
> ---
> include/block/block_int.h | 22 -----------------
> block/blkdebug.c | 7 ++++--
> block/blklogwrites.c | 1 -
> block/commit.c | 1 -
> block/copy-on-read.c | 2 --
> block/io.c | 51 +++++++++++++--------------------------
> block/mirror.c | 1 -
> block/throttle.c | 1 -
> 8 files changed, 22 insertions(+), 64 deletions(-)
>
> diff --git a/include/block/block_int.h b/include/block/block_int.h
> index cfefb00104..431fa38ea0 100644
> --- a/include/block/block_int.h
> +++ b/include/block/block_int.h
> @@ -1203,28 +1203,6 @@ void bdrv_format_default_perms(BlockDriverState *bs, BdrvChild *c,
> uint64_t perm, uint64_t shared,
> uint64_t *nperm, uint64_t *nshared);
>
> -/*
> - * Default implementation for drivers to pass bdrv_co_block_status() to
> - * their file.
> - */
> -int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
> - bool want_zero,
> - int64_t offset,
> - int64_t bytes,
> - int64_t *pnum,
> - int64_t *map,
> - BlockDriverState **file);
> -/*
> - * Default implementation for drivers to pass bdrv_co_block_status() to
> - * their backing file.
> - */
> -int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
> - bool want_zero,
> - int64_t offset,
> - int64_t bytes,
> - int64_t *pnum,
> - int64_t *map,
> - BlockDriverState **file);
> const char *bdrv_get_parent_name(const BlockDriverState *bs);
> void blk_dev_change_media_cb(BlockBackend *blk, bool load, Error **errp);
> bool blk_dev_has_removable_media(BlockBackend *blk);
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index efd9441625..7950ae729c 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -637,8 +637,11 @@ static int coroutine_fn blkdebug_co_block_status(BlockDriverState *bs,
> BlockDriverState **file)
> {
> assert(QEMU_IS_ALIGNED(offset | bytes, bs->bl.request_alignment));
> - return bdrv_co_block_status_from_file(bs, want_zero, offset, bytes,
> - pnum, map, file);
> + assert(bs->file && bs->file->bs);
> + *pnum = bytes;
> + *map = offset;
> + *file = bs->file->bs;
> + return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
> }
>
> static void blkdebug_close(BlockDriverState *bs)
> diff --git a/block/blklogwrites.c b/block/blklogwrites.c
> index eb2b4901a5..1eb4a5c613 100644
> --- a/block/blklogwrites.c
> +++ b/block/blklogwrites.c
> @@ -518,7 +518,6 @@ static BlockDriver bdrv_blk_log_writes = {
> .bdrv_co_pwrite_zeroes = blk_log_writes_co_pwrite_zeroes,
> .bdrv_co_flush_to_disk = blk_log_writes_co_flush_to_disk,
> .bdrv_co_pdiscard = blk_log_writes_co_pdiscard,
> - .bdrv_co_block_status = bdrv_co_block_status_from_file,
>
> .is_filter = true,
> .strong_runtime_opts = blk_log_writes_strong_runtime_opts,
> diff --git a/block/commit.c b/block/commit.c
> index ec5a8c8edf..a5b58eadeb 100644
> --- a/block/commit.c
> +++ b/block/commit.c
> @@ -257,7 +257,6 @@ static void bdrv_commit_top_child_perm(BlockDriverState *bs, BdrvChild *c,
> static BlockDriver bdrv_commit_top = {
> .format_name = "commit_top",
> .bdrv_co_preadv = bdrv_commit_top_preadv,
> - .bdrv_co_block_status = bdrv_co_block_status_from_backing,
> .bdrv_refresh_filename = bdrv_commit_top_refresh_filename,
> .bdrv_child_perm = bdrv_commit_top_child_perm,
>
> diff --git a/block/copy-on-read.c b/block/copy-on-read.c
> index 88e1c1f538..5a292de000 100644
> --- a/block/copy-on-read.c
> +++ b/block/copy-on-read.c
> @@ -161,8 +161,6 @@ static BlockDriver bdrv_copy_on_read = {
> .bdrv_eject = cor_eject,
> .bdrv_lock_medium = cor_lock_medium,
>
> - .bdrv_co_block_status = bdrv_co_block_status_from_file,
> -
> .bdrv_recurse_is_first_non_filter = cor_recurse_is_first_non_filter,
>
> .has_variable_length = true,
> diff --git a/block/io.c b/block/io.c
> index 14f99e1c00..0a832e30a3 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1998,36 +1998,6 @@ typedef struct BdrvCoBlockStatusData {
> bool done;
> } BdrvCoBlockStatusData;
>
> -int coroutine_fn bdrv_co_block_status_from_file(BlockDriverState *bs,
> - bool want_zero,
> - int64_t offset,
> - int64_t bytes,
> - int64_t *pnum,
> - int64_t *map,
> - BlockDriverState **file)
> -{
> - assert(bs->file && bs->file->bs);
> - *pnum = bytes;
> - *map = offset;
> - *file = bs->file->bs;
> - return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
> -}
> -
> -int coroutine_fn bdrv_co_block_status_from_backing(BlockDriverState *bs,
> - bool want_zero,
> - int64_t offset,
> - int64_t bytes,
> - int64_t *pnum,
> - int64_t *map,
> - BlockDriverState **file)
> -{
> - assert(bs->backing && bs->backing->bs);
> - *pnum = bytes;
> - *map = offset;
> - *file = bs->backing->bs;
> - return BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
> -}
> -
> /*
> * Returns the allocation status of the specified sectors.
> * Drivers not implementing the functionality are assumed to not support
> @@ -2068,6 +2038,7 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
> BlockDriverState *local_file = NULL;
> int64_t aligned_offset, aligned_bytes;
> uint32_t align;
> + bool has_filtered_child;
>
> assert(pnum);
> *pnum = 0;
> @@ -2093,7 +2064,8 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
>
> /* Must be non-NULL or bdrv_getlength() would have failed */
> assert(bs->drv);
> - if (!bs->drv->bdrv_co_block_status) {
> + has_filtered_child = bs->drv->is_filter && bdrv_filtered_rw_child(bs);
> + if (!bs->drv->bdrv_co_block_status && !has_filtered_child) {
> *pnum = bytes;
> ret = BDRV_BLOCK_DATA | BDRV_BLOCK_ALLOCATED;
> if (offset + bytes == total_size) {
> @@ -2114,9 +2086,20 @@ static int coroutine_fn bdrv_co_block_status(BlockDriverState *bs,
> aligned_offset = QEMU_ALIGN_DOWN(offset, align);
> aligned_bytes = ROUND_UP(offset + bytes, align) - aligned_offset;
>
> - ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset,
> - aligned_bytes, pnum, &local_map,
> - &local_file);
> + if (bs->drv->bdrv_co_block_status) {
> + ret = bs->drv->bdrv_co_block_status(bs, want_zero, aligned_offset,
> + aligned_bytes, pnum, &local_map,
> + &local_file);
> + } else {
> + /* Default code for filters */
> +
> + local_file = bdrv_filtered_rw_bs(bs);
> + assert(local_file);
> +
> + *pnum = aligned_bytes;
> + local_map = aligned_offset;
> + ret = BDRV_BLOCK_RAW | BDRV_BLOCK_OFFSET_VALID;
I now in a little doubt:
What is real difference between RAW for filters and UNALLOCATED for qcow2 (when we
should look at backing) ?
> + }
> if (ret < 0) {
> *pnum = 0;
> goto out;
> diff --git a/block/mirror.c b/block/mirror.c
> index 3d767e3030..71bd7f7625 100644
> --- a/block/mirror.c
> +++ b/block/mirror.c
> @@ -1484,7 +1484,6 @@ static BlockDriver bdrv_mirror_top = {
> .bdrv_co_pwrite_zeroes = bdrv_mirror_top_pwrite_zeroes,
> .bdrv_co_pdiscard = bdrv_mirror_top_pdiscard,
> .bdrv_co_flush = bdrv_mirror_top_flush,
> - .bdrv_co_block_status = bdrv_co_block_status_from_backing,
> .bdrv_refresh_filename = bdrv_mirror_top_refresh_filename,
> .bdrv_child_perm = bdrv_mirror_top_child_perm,
>
> diff --git a/block/throttle.c b/block/throttle.c
> index de1b6bd7e8..32ec56db0f 100644
> --- a/block/throttle.c
> +++ b/block/throttle.c
> @@ -269,7 +269,6 @@ static BlockDriver bdrv_throttle = {
> .bdrv_reopen_prepare = throttle_reopen_prepare,
> .bdrv_reopen_commit = throttle_reopen_commit,
> .bdrv_reopen_abort = throttle_reopen_abort,
> - .bdrv_co_block_status = bdrv_co_block_status_from_file,
>
> .bdrv_co_drain_begin = throttle_co_drain_begin,
> .bdrv_co_drain_end = throttle_co_drain_end,
>
--
Best regards,
Vladimir