Re: [PATCH] blkdebug: ignore invalid rules in non-coroutine context

[Kevin Wolf <kwolf@redhat.com>]

Am 15.12.2022 um 14:02 hat Paolo Bonzini geschrieben:
> blkdebug events can be called from either non-coroutine or coroutine
> contexts.  However, suspend actions only make sense from within
> a coroutine.  Currently, using those action would lead to an abort() in
> qemu_coroutine_yield() ("Co-routine is yielding to no one").  Catch them
> and print an error instead.
> 
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  block.c                          |  2 +-
>  block/blkdebug.c                 | 10 ++++++++--
>  include/block/block-io.h         |  2 +-
>  include/block/block_int-common.h |  3 ++-
>  4 files changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/block.c b/block.c
> index 3f2bd128570e..49c66475c73e 100644
> --- a/block.c
> +++ b/block.c
> @@ -6334,7 +6334,7 @@ BlockStatsSpecific *bdrv_get_specific_stats(BlockDriverState *bs)
>      return drv->bdrv_get_specific_stats(bs);
>  }
>  
> -void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event)
> +void coroutine_mixed_fn bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event)
>  {
>      IO_CODE();
>      if (!bs || !bs->drv || !bs->drv->bdrv_debug_event) {
> diff --git a/block/blkdebug.c b/block/blkdebug.c
> index 4265ca125e25..ce297961b7db 100644
> --- a/block/blkdebug.c
> +++ b/block/blkdebug.c
> @@ -31,6 +31,7 @@
>  #include "block/qdict.h"
>  #include "qemu/module.h"
>  #include "qemu/option.h"
> +#include "qemu/error-report.h"
>  #include "qapi/qapi-visit-block-core.h"
>  #include "qapi/qmp/qdict.h"
>  #include "qapi/qmp/qlist.h"
> @@ -837,7 +838,7 @@ static void process_rule(BlockDriverState *bs, struct BlkdebugRule *rule,
>      }
>  }
>  
> -static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
> +static void coroutine_mixed_fn blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
>  {
>      BDRVBlkdebugState *s = bs->opaque;
>      struct BlkdebugRule *rule, *next;
> @@ -855,7 +856,12 @@ static void blkdebug_debug_event(BlockDriverState *bs, BlkdebugEvent event)
>      }
>  
>      while (actions_count[ACTION_SUSPEND] > 0) {
> -        qemu_coroutine_yield();
> +        if (qemu_in_coroutine()) {
> +            qemu_coroutine_yield();
> +        } else {
> +            error_report("Non-coroutine event %s cannot suspend\n",
> +                         BlkdebugEvent_lookup.array[event]);

error_report() already adds a newline, so we shouldn't have an "\n"
here.

> +        }
>          actions_count[ACTION_SUSPEND]--;
>      }
>  }

Thanks, fixed this up and applied to the block branch.

Kevin