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

[Zhi Yong Wu]

On Mon, Nov 14, 2011 at 4:50 PM, Kevin Wolf <address@hidden> wrote:
> Am 14.11.2011 04:04, schrieb 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?
>
> No, they are needed. We want to wait until the whole operation has
> completed, but qemu_coroutine_enter() returns after the first
> qemu_coroutine_yield().
got it. thanks.
>
> Kevin
>



-- 
Regards,

Zhi Yong Wu