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(-)
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]);
+ }
actions_count[ACTION_SUSPEND]--;
}
}
diff --git a/include/block/block-io.h b/include/block/block-io.h
index 1fa717a545a0..0e7032a23936 100644
--- a/include/block/block-io.h
+++ b/include/block/block-io.h
@@ -175,7 +175,7 @@ void *qemu_try_blockalign0(BlockDriverState *bs, size_t size);
void bdrv_enable_copy_on_read(BlockDriverState *bs);
void bdrv_disable_copy_on_read(BlockDriverState *bs);
-void bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event);
+void coroutine_mixed_fn bdrv_debug_event(BlockDriverState *bs, BlkdebugEvent event);
#define BLKDBG_EVENT(child, evt) \
do { \
diff --git a/include/block/block_int-common.h b/include/block/block_int-common.h
index c34c525fa6ba..1d4fd5094a5b 100644
--- a/include/block/block_int-common.h
+++ b/include/block/block_int-common.h
@@ -726,7 +726,8 @@ struct BlockDriver {
int coroutine_fn GRAPH_RDLOCK_PTR (*bdrv_co_check)(
BlockDriverState *bs, BdrvCheckResult *result, BdrvCheckMode fix);
- void (*bdrv_debug_event)(BlockDriverState *bs, BlkdebugEvent event);
+ void coroutine_mixed_fn (*bdrv_debug_event)(BlockDriverState *bs,
+ BlkdebugEvent event);
/* io queue for linux-aio */
void (*bdrv_io_plug)(BlockDriverState *bs);
--
2.38.1
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
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
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) coroutine_mixed_fn isn't a thing. I assume this depends on some patch you haven't sent yet? Kevin
© 2016 - 2024 Red Hat, Inc.