On 26/05/2015 05:36, Fam Zheng wrote:
> Like bdrv_is_allocated_above, this function follows the backing chain until
> seeing
> BDRV_BLOCK_ALLOCATED. Base is not included.
>
> Reimplement bdrv_is_allocated on top.
>
> Signed-off-by: Fam Zheng <f...@redhat.com>
> ---
> block/io.c | 53
> ++++++++++++++++++++++++++++++++++++++++-----------
> include/block/block.h | 4 ++++
> 2 files changed, 46 insertions(+), 11 deletions(-)
>
> diff --git a/block/io.c b/block/io.c
> index e394d92..a0d9990 100644
> --- a/block/io.c
> +++ b/block/io.c
> @@ -1560,28 +1560,51 @@ static int64_t coroutine_fn
> bdrv_co_get_block_status(BlockDriverState *bs,
> return ret;
> }
>
> -/* Coroutine wrapper for bdrv_get_block_status() */
> -static void coroutine_fn bdrv_get_block_status_co_entry(void *opaque)
> +static int64_t coroutine_fn bdrv_co_get_block_status_above(BlockDriverState
> *bs,
> + BlockDriverState *base,
> + int64_t sector_num,
> + int nb_sectors,
> + int *pnum)
> +{
> + BlockDriverState *p;
> + int64_t ret;
> +
> + assert(bs != base);
> + for (p = bs; p != base; p = p->backing_hd) {
> + ret = bdrv_co_get_block_status(p, sector_num, nb_sectors, pnum);
Since pnum is an output parameter only, *pnum will be set by the last
call in the loop.
This is not what bdrv_is_allocated_above does: you have to set *pnum
(roughly) to the _smallest_ value returned by the calls. Consider this
case (base == NULL, bs->backing_hd->backing_hd == NULL):
1 2 3
123456789012345678901234567890
bs ...........AAAAAAAAAAAA.......
bs->backing_hd ...............AAAAAAAAAAAAAAA
Your code would return *pnum == 15, but the right result is *pnum == 11.
Paolo
> + if (ret < 0 || ret & BDRV_BLOCK_ALLOCATED) {
> + break;
> + }
> + }
> + return ret;
> +}
> +
> +/* Coroutine wrapper for bdrv_get_block_status_above() */
> +static void coroutine_fn bdrv_get_block_status_above_co_entry(void *opaque)
> {
> BdrvCoGetBlockStatusData *data = opaque;
> - BlockDriverState *bs = data->bs;
>
> - data->ret = bdrv_co_get_block_status(bs, data->sector_num,
> data->nb_sectors,
> - data->pnum);
> + data->ret = bdrv_co_get_block_status_above(data->bs, data->base,
> + data->sector_num,
> + data->nb_sectors,
> + data->pnum);
> data->done = true;
> }
>
> /*
> - * Synchronous wrapper around bdrv_co_get_block_status().
> + * Synchronous wrapper around bdrv_co_get_block_status_above().
> *
> - * See bdrv_co_get_block_status() for details.
> + * See bdrv_co_get_block_status_above() for details.
> */
> -int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
> - int nb_sectors, int *pnum)
> +int64_t bdrv_get_block_status_above(BlockDriverState *bs,
> + BlockDriverState *base,
> + int64_t sector_num,
> + int nb_sectors, int *pnum)
> {
> Coroutine *co;
> BdrvCoGetBlockStatusData data = {
> .bs = bs,
> + .base = base,
> .sector_num = sector_num,
> .nb_sectors = nb_sectors,
> .pnum = pnum,
> @@ -1590,11 +1613,11 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,
> int64_t sector_num,
>
> if (qemu_in_coroutine()) {
> /* Fast-path if already in coroutine context */
> - bdrv_get_block_status_co_entry(&data);
> + bdrv_get_block_status_above_co_entry(&data);
> } else {
> AioContext *aio_context = bdrv_get_aio_context(bs);
>
> - co = qemu_coroutine_create(bdrv_get_block_status_co_entry);
> + co = qemu_coroutine_create(bdrv_get_block_status_above_co_entry);
> qemu_coroutine_enter(co, &data);
> while (!data.done) {
> aio_poll(aio_context, true);
> @@ -1603,6 +1626,14 @@ int64_t bdrv_get_block_status(BlockDriverState *bs,
> int64_t sector_num,
> return data.ret;
> }
>
> +int64_t bdrv_get_block_status(BlockDriverState *bs,
> + int64_t sector_num,
> + int nb_sectors, int *pnum)
> +{
> + return bdrv_get_block_status_above(bs, bs->backing_hd,
> + sector_num, nb_sectors, pnum);
> +}
> +
> int coroutine_fn bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num,
> int nb_sectors, int *pnum)
> {
> diff --git a/include/block/block.h b/include/block/block.h
> index c1c963e..8a13bed 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h
> @@ -361,6 +361,10 @@ bool bdrv_unallocated_blocks_are_zero(BlockDriverState
> *bs);
> bool bdrv_can_write_zeroes_with_unmap(BlockDriverState *bs);
> int64_t bdrv_get_block_status(BlockDriverState *bs, int64_t sector_num,
> int nb_sectors, int *pnum);
> +int64_t bdrv_get_block_status_above(BlockDriverState *bs,
> + BlockDriverState *base,
> + int64_t sector_num,
> + int nb_sectors, int *pnum);
> int bdrv_is_allocated(BlockDriverState *bs, int64_t sector_num, int
> nb_sectors,
> int *pnum);
> int bdrv_is_allocated_above(BlockDriverState *top, BlockDriverState *base,
>
