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

[Kevin Wolf <kwolf@redhat.com>]

Am 14.11.2011 04:04, schrieb Zhi Yong Wu:
> On Sat, Nov 12, 2011 at 12:47 AM, Stefan Hajnoczi
> <stefanha@linux.vnet.ibm.com> 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 <stefanha@linux.vnet.ibm.com>
>> ---
>>  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().

Kevin