Re: [Qemu-devel] [PATCH 02/10] block: add .bdrv_co_is_allocated()

[Zhi Yong Wu]

On Sat, Nov 12, 2011 at 12:47 AM, Stefan Hajnoczi
<address@hidden> wrote:
> This patch adds the .bdrv_co_is_allocated() interface which is identical
> to .bdrv_is_allocated() but runs in coroutine context.  Running in
> coroutine context implies that other coroutines might be performing I/O
> at the same time.   Therefore it must be safe to run while the following
> BlockDriver functions are in-flight:
>
>    .bdrv_co_readv()
>    .bdrv_co_writev()
>    .bdrv_co_flush()
>    .bdrv_co_is_allocated()
>
> The new .bdrv_co_is_allocated() interface is useful because it can be
> used when a VM is running, whereas .bdrv_is_allocated() is a synchronous
> interface that does not cope with parallel requests.
>
> Signed-off-by: Stefan Hajnoczi <address@hidden>
> ---
>  block.c     |   37 +++++++++++++++++++++++++++++++++++++
>  block_int.h |    2 ++
>  2 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/block.c b/block.c
> index e6ac6d3..f8705b7 100644
> --- a/block.c
> +++ b/block.c
> @@ -1771,6 +1771,26 @@ int bdrv_has_zero_init(BlockDriverState *bs)
>     return 1;
>  }
>
> +typedef struct BdrvCoIsAllocatedData {
> +    BlockDriverState *bs;
> +    int64_t sector_num;
> +    int nb_sectors;
> +    int *pnum;
> +    int ret;
> +    bool done;
> +} BdrvCoIsAllocatedData;
> +
> +/* Coroutine wrapper for bdrv_is_allocated() */
> +static void coroutine_fn bdrv_is_allocated_co_entry(void *opaque)
> +{
> +    BdrvCoIsAllocatedData *data = opaque;
> +    BlockDriverState *bs = data->bs;
> +
> +    data->ret = bs->drv->bdrv_co_is_allocated(bs, data->sector_num,
> +                                              data->nb_sectors, data->pnum);
> +    data->done = true;
> +}
> +
>  /*
>  * Returns true iff the specified sector is present in the disk image. Drivers
>  * not implementing the functionality are assumed to not support backing 
> files,
> @@ -1786,6 +1806,23 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t 
> sector_num, int nb_sectors,
>        int *pnum)
>  {
>     int64_t n;
> +    if (bs->drv->bdrv_co_is_allocated) {
> +        Coroutine *co;
> +        BdrvCoIsAllocatedData data = {
> +            .bs = bs,
> +            .sector_num = sector_num,
> +            .nb_sectors = nb_sectors,
> +            .pnum = pnum,
> +            .done = false,
> +        };
> +
> +        co = qemu_coroutine_create(bdrv_is_allocated_co_entry);
> +        qemu_coroutine_enter(co, &data);
Since this main process will stop within qemu_coroutine_enter() until
bdrv_is_allocated_co_entry() is completed, three lines of condition
codes below are unnecessary, right?
> +        while (!data.done) {
> +            qemu_aio_wait();
> +        }
> +        return data.ret;
> +    }
>     if (!bs->drv->bdrv_is_allocated) {
>         if (sector_num >= bs->total_sectors) {
>             *pnum = 0;
> diff --git a/block_int.h b/block_int.h
> index f4547f6..1c1351c 100644
> --- a/block_int.h
> +++ b/block_int.h
> @@ -87,6 +87,8 @@ struct BlockDriver {
>     int coroutine_fn (*bdrv_co_flush)(BlockDriverState *bs);
>     int coroutine_fn (*bdrv_co_discard)(BlockDriverState *bs,
>         int64_t sector_num, int nb_sectors);
> +    int coroutine_fn (*bdrv_co_is_allocated)(BlockDriverState *bs,
> +        int64_t sector_num, int nb_sectors, int *pnum);
>
>     int (*bdrv_aio_multiwrite)(BlockDriverState *bs, BlockRequest *reqs,
>         int num_reqs);
> --
> 1.7.7.1
>
>
>



-- 
Regards,

Zhi Yong Wu