Hi Charlie, Many thanks for this patch series.
On Mon, Aug 05, 2013 at 08:44:05PM +0200, Charlie Shepherd wrote: > This patch converts the .bdrv_open, .bdrv_file_open and .bdrv_create members > of struct BlockDriver > to be explicitly annotated as coroutine_fn, rather than yielding dynamically > depending on whether > they are executed in a coroutine context or not. The commit message is incomplete. You also convert brdv_read and brdv_write. Moreover: > diff --git a/block/ssh.c b/block/ssh.c > index d7e7bf8..66ac4c1 100644 > --- a/block/ssh.c > +++ b/block/ssh.c > @@ -748,7 +748,7 @@ static int return_true(void *opaque) > return 1; > } > > -static coroutine_fn void set_fd_handler(BDRVSSHState *s) > +static void set_fd_handler(BDRVSSHState *s) > { > int r; > IOHandler *rd_handler = NULL, *wr_handler = NULL; > @@ -769,7 +769,7 @@ static coroutine_fn void set_fd_handler(BDRVSSHState *s) > qemu_aio_set_fd_handler(s->sock, rd_handler, wr_handler, return_true, > co); > } > > -static coroutine_fn void clear_fd_handler(BDRVSSHState *s) > +static void clear_fd_handler(BDRVSSHState *s) > { > DPRINTF("s->sock=%d", s->sock); > qemu_aio_set_fd_handler(s->sock, NULL, NULL, NULL, NULL); > diff --git a/include/block/coroutine_int.h b/include/block/coroutine_int.h > index f133d65..d0ab27d 100644 > --- a/include/block/coroutine_int.h > +++ b/include/block/coroutine_int.h > @@ -48,6 +48,6 @@ Coroutine *qemu_coroutine_new(void); > void qemu_coroutine_delete(Coroutine *co); > CoroutineAction qemu_coroutine_switch(Coroutine *from, Coroutine *to, > CoroutineAction action); > -void coroutine_fn qemu_co_queue_run_restart(Coroutine *co); > +void qemu_co_queue_run_restart(Coroutine *co); > > #endif Adding coroutine_fn where it is necessary seems straightforward to me: just follow the "callers of coroutine_fn should be coroutine_fn" rule (assuming you got it right and did not over-annotate). On the other hand, you should probably explain in the commit message why you *remove* those three coroutine_fn annotations. Best, -- Gabriel