On 09.04.24 18:49, Eric Blake wrote:
On Tue, Apr 09, 2024 at 09:30:39AM +0300, Vladimir Sementsov-Ogievskiy wrote:
On 08.04.24 19:00, Eric Blake wrote:
nbd_negotiate() is already marked coroutine_fn. And given the fix in
the previous patch to have nbd_negotiate_handle_starttls not create
and wait on a g_main_loop (as that would violate coroutine
constraints), it is worth marking the rest of the related static
functions reachable only during option negotiation as also being
coroutine_fn.
Suggested-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Signed-off-by: Eric Blake <ebl...@redhat.com>
---
nbd/server.c | 102 +++++++++++++++++++++++++++++----------------------
1 file changed, 59 insertions(+), 43 deletions(-)
diff --git a/nbd/server.c b/nbd/server.c
index 98ae0e16326..1857fba51c1 100644
[..]
{
int rc;
g_autofree char *name = NULL;
@@ -755,7 +764,8 @@ struct NBDTLSServerHandshakeData {
Coroutine *co;
};
-static void nbd_server_tls_handshake(QIOTask *task, void *opaque)
+static coroutine_fn void
This is not, that's a callback for tls handshake, which is not coroutine
context as I understand.
The call chain is:
nbd_negotiate() - coroutine_fn before this patch
nbd_negotiate_options() - marked coroutine_fn here
nbd_negotiate_handle_starttls() - marked coroutine_fn here
qio_channel_tls_handshake() - in io/channel-tls.c; not marked
coroutine_fn, but
works both in or out of coroutine context
...
nbd_server_tls_handshake() - renamed in 1/2 of this series
without this hunk:
I can drop that particular marking. There are cases where the
callback is reached without having done the qemu_coroutine_yield() in
nbd_negotiate_handle_starttls; but there are also cases where the IO
took enough time that the coroutine yielded and it is that callback
that reawakens it.
The key thing for me is that in this case (when coroutine yielded),
nbd_server_tls_handshake() would finally be called from glib IO handler, set in
qio_channel_tls_handshake()
qio_channel_tls_handshake_task()
qio_channel_add_watch_full()
g_source_set_callback(source, (GSourceFunc)func, user_data, notify);
<<<
, which would definitely not be a coroutine context.
Do I understand correctly, that "coroutine_fn" means "only call from coroutine
context"[1], or does it mean "could be called from coroutine context"[2]?
The comment in osdep.h says:
* Mark a function that executes in coroutine context
|}
*
|
* Functions that execute in coroutine context cannot be called directly from
|
* normal functions. ...
So I assume, we mean [1].
Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@yandex-team.ru>
Thanks.
--
Best regards,
Vladimir
