Am 20.07.2026 um 09:55 hat Marc-André Lureau geschrieben: > The functions call coroutine functions or are called by coroutine.
That it's called only by coroutines isn't a reason to make a function coroutine_fn as long as it theoretically could be called by non-coroutine code and it would still work fine (i.e. it doesn't yield). > Add an assert() in qcow2_do_close() code path which calls a > no_coroutine_fn bdrv_graph_wrlock_drained(). > > Signed-off-by: Marc-André Lureau <[email protected]> > diff --git a/block/parallels.c b/block/parallels.c > index 7a90fb5220b..46a6e8f2743 100644 > --- a/block/parallels.c > +++ b/block/parallels.c > @@ -142,8 +142,9 @@ static uint32_t host_cluster_index(BDRVParallelsState *s, > int64_t off) > return off / s->cluster_size; > } > > -static int64_t block_status(BDRVParallelsState *s, int64_t sector_num, > - int nb_sectors, int *pnum) > +static int64_t coroutine_fn > +block_status(BDRVParallelsState *s, int64_t sector_num, > + int nb_sectors, int *pnum) > { > int64_t start_off = -2, prev_end_off = -2; > Specifically this hunk is unclear to me. As far as I can see, block_status() can't yield. Why did you add the coroutine_fn marker here, but not e.g. for seek_to_sector() or cluster_remainder(), which are only called by block_status() and therefore also always only in coroutine context? I believe the right way to stay consistent is to leave all of them unmarked because there is no fundamental reason why they would only make sense in coroutine context. > diff --git a/block/qcow2.c b/block/qcow2.c > index 19271b10a49..161626fa3cb 100644 > --- a/block/qcow2.c > +++ b/block/qcow2.c > @@ -1287,9 +1287,10 @@ fail: > } > > /* s_locked specifies whether s->lock is held or not */ > -static void qcow2_update_options_commit(BlockDriverState *bs, > - Qcow2ReopenState *r, > - bool s_locked) > +static void coroutine_fn > +qcow2_update_options_commit(BlockDriverState *bs, > + Qcow2ReopenState *r, > + bool s_locked) > { > BDRVQcow2State *s = bs->opaque; > int i; > @@ -2126,7 +2127,8 @@ fail: > return ret; > } > > -static void qcow2_reopen_commit(BDRVReopenState *state) > +static void coroutine_fn > +qcow2_reopen_commit(BDRVReopenState *state) > { > BDRVQcow2State *s = state->bs->opaque; > This doesn't match the declaration of the .bdrv_reopen_commit callback in BlockDriver, and as far as I can see, the callers are actually guaranteed to _not_ be in coroutine context. If this is true, qcow2_update_options_commit() should probably be marked coroutine_mixed_fn and this one stay unannotated or no_coroutine_fn. The other parts of the patch looks correct. Kevin
