[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] blkdebug: ignore invalid rules in non-coroutine context
From: |
Kevin Wolf |
Subject: |
Re: [PATCH] blkdebug: ignore invalid rules in non-coroutine context |
Date: |
Tue, 17 Jan 2023 18:21:16 +0100 |
Am 17.01.2023 um 16:44 hat Kevin Wolf geschrieben:
> 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.
In fact, this conflicts with a patch in my series:
[PATCH v2 13/14] block: Convert bdrv_debug_event() to co_wrapper_mixed
Resolving the conflict essentially reverts this one because after that
patch it actually is a coroutine_fn. So I may just drop this one agian.
Kevin