Re: [Qemu-devel] [RFC] block: Removed coroutine ownership assumption

[Jan Kiszka]

On 2012-06-22 08:44, Peter A. G. Crosthwaite wrote:
> The block layer assumes that it is the only user of coroutines -
> The qemu_in_coroutine() is used to determine if a function is in one of the
> block layers coroutines, which is flawed. I.E. If a client (e.g. a device or
> a machine model) of the block layer uses couroutine itself, the block layer
> will identify the callers coroutines as its own, and may falsely yield the
> calling coroutine (instead of creating its own to yield).
> 
> AFAICT, there are no conflicts in the QEMU master here yet, but its kind of an
> issue, as anyone who comes along and used coroutines and the block layer
> together is going to run into some very obscure and hard to debug race
> conditions.

Not sure if I understood the intention yet: Is this supposed to fix an
issue with the current usage of coroutines or to extend their usage
beyond that? In the latter case, please don't do this. We'd rather like
to get rid of them long term.

Jan

> 
> Signed-off-by: Peter A. G. Crosthwaite <address@hidden>
> ---
>  block.c |    8 ++++----
>  1 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 0acdcac..b50af15 100644
> --- a/block.c
> +++ b/block.c
> @@ -380,7 +380,7 @@ int bdrv_create(BlockDriver *drv, const char* filename,
>          return -ENOTSUP;
>      }
>  
> -    if (qemu_in_coroutine()) {
> +    if (0) {
>          /* Fast-path if already in coroutine context */
>          bdrv_create_co_entry(&cco);
>      } else {
> @@ -1590,7 +1590,7 @@ static int bdrv_rw_co(BlockDriverState *bs, int64_t 
> sector_num, uint8_t *buf,
>          bdrv_io_limits_disable(bs);
>      }
>  
> -    if (qemu_in_coroutine()) {
> +    if (0) {
>          /* Fast-path if already in coroutine context */
>          bdrv_rw_co_entry(&rwco);
>      } else {
> @@ -3813,7 +3813,7 @@ int bdrv_flush(BlockDriverState *bs)
>          .ret = NOT_DONE,
>      };
>  
> -    if (qemu_in_coroutine()) {
> +    if (0) {
>          /* Fast-path if already in coroutine context */
>          bdrv_flush_co_entry(&rwco);
>      } else {
> @@ -3874,7 +3874,7 @@ int bdrv_discard(BlockDriverState *bs, int64_t 
> sector_num, int nb_sectors)
>          .ret = NOT_DONE,
>      };
>  
> -    if (qemu_in_coroutine()) {
> +    if (0) {
>          /* Fast-path if already in coroutine context */
>          bdrv_discard_co_entry(&rwco);
>      } else {
> 

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux