Re: [Qemu-devel] [PATCH v3 06/19] block: remove bdrv_is_allocated_above/bdrv_co_is_allocated_above distinction

[Paolo Bonzini]

Il 29/07/2013 15:34, Kevin Wolf ha scritto:
> Am 25.07.2013 um 16:23 hat Paolo Bonzini geschrieben:
>> Now that bdrv_is_allocated detects coroutine context, the two can
>> use the same code.
>>
>> Reviewed-by: Eric Blake <address@hidden>
>> Signed-off-by: Paolo Bonzini <address@hidden>
>> ---
>>  block.c               | 46 ++++------------------------------------------
>>  block/commit.c        |  6 +++---
>>  block/mirror.c        |  4 ++--
>>  block/stream.c        |  4 ++--
>>  include/block/block.h |  4 ----
>>  5 files changed, 11 insertions(+), 53 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index dd4c570..1ee1d93 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -3058,10 +3058,10 @@ int bdrv_is_allocated(BlockDriverState *bs, int64_t 
>> sector_num, int nb_sectors,
>>   *  allocated/unallocated state.
>>   *
>>   */
>> -int coroutine_fn bdrv_co_is_allocated_above(BlockDriverState *top,
>> -                                            BlockDriverState *base,
>> -                                            int64_t sector_num,
>> -                                            int nb_sectors, int *pnum)
>> +int coroutine_fn bdrv_is_allocated_above(BlockDriverState *top,
>> +                                         BlockDriverState *base,
>> +                                         int64_t sector_num,
>> +                                         int nb_sectors, int *pnum)
> 
> This is no longer a coroutine_fn.

I'm always confused about must-yield vs. may-yield. :(

> However, if this was the only reason for making bdrv_is_allocated() a
> hybrid function, it may be easier to simply let the only caller
> (img_compare in qemu-img) run in a coroutine and drop the synchronous
> version entirely, keeping only a coroutine_fn.

The reason is to avoid having the same (IMHO pretty gratuitous)
distinction in get_block_status.  "qemu-img map" will also add another
synchronous user of get_block_status.

If we want to keep only the coroutine_fn, we should make all of qemu-img
run in a coroutine.

Paolo