Re: [PATCH 06/26] blkdebug: add missing coroutine_fn annotations

[Kevin Wolf]

Am 05.10.2022 um 23:11 hat Paolo Bonzini geschrieben:
> Il mer 5 ott 2022, 06:32 Kevin Wolf <kwolf@redhat.com> ha scritto:
> 
> > Hm... blkdebug_debug_event() is called from bdrv_debug_event(), which is
> > not coroutine_fn. And I think that it's not coroutine_fn is correct:
> > For example, with 'qemu-img snapshot -c' you get img_snapshot() ->
> > bdrv_snapshot_create() -> qcow2_snapshot_create() ->
> > qcow2_alloc_clusters() -> BLKDBG_EVENT. I'm sure many other calls in
> > qcow2 can be reached from non-coroutine context as well.
> >
> > It almost looks like your code analysis didn't consider calls through
> > function pointers?
> >
> 
> No, that is not what happened. Rather it's that the analysis goes the
> other way round: because SUSPEND rules call qemu_coroutine_yield(),
> clang wants all the callers to be coroutine_fn too.

Ah, ok, makes sense. So checking the callers is indeed something that
requires manual review.

> It is technically incorrect that bdrv_debug_events sometimes are
> placed outside coroutine context, and QEMU would crash if those events
> were associated with a suspend rule.

Yes, looks like a bug. As long as it's only in blkdebug, we can probably
live with it (the easiest fix would probably be generating coroutine
wrappers for bdrv_debug_event(), too).

> I guess I (or you) can just drop this patch?

Yes, I can do that.

Kevin