On 2/11/19 6:56 AM, Vladimir Sementsov-Ogievskiy wrote: > We always call qio_channel_tls_handshake in nbd from couroutine. Take > benefit of it and just yield instead of creating personal main loop. > > Mark and rename the function and it's callers correspondingly and > trace-points too. > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> > --- > nbd/client.c | 26 +++++++++----------------- > nbd/common.c | 6 ++---- > nbd/server.c | 45 +++++++++++++++++---------------------------- > nbd/trace-events | 15 +++++++-------- > 4 files changed, 35 insertions(+), 57 deletions(-) > > diff --git a/nbd/client.c b/nbd/client.c > index 2ba2220a4a..e3919be30e 100644 > --- a/nbd/client.c > +++ b/nbd/client.c > @@ -578,13 +578,14 @@ static int nbd_request_simple_option(QIOChannel *ioc, > int opt, Error **errp) > return 1; > } > > -static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, > - QCryptoTLSCreds *tlscreds, > - const char *hostname, Error **errp) > +static QIOChannel *nbd_co_receive_starttls(
Missing coroutine_fn ? > + QIOChannel *ioc, QCryptoTLSCreds *tlscreds, const char *hostname, > + Error **errp) > { > int ret; > QIOChannelTLS *tioc; > - struct NBDTLSHandshakeData data = { 0 }; > + > + assert(qemu_in_coroutine()); Again, I'm not sure these assertions add much. > > ret = nbd_request_simple_option(ioc, NBD_OPT_STARTTLS, errp); Should we also be marking these helper functions as coroutine_fn by the end of the series, once all callers are marked that way? > if (ret <= 0) { > @@ -601,23 +602,13 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, > return NULL; > } > qio_channel_set_name(QIO_CHANNEL(tioc), "nbd-client-tls"); > - data.loop = g_main_loop_new(g_main_context_default(), FALSE); > trace_nbd_receive_starttls_tls_handshake(); > qio_channel_tls_handshake(tioc, > nbd_tls_handshake, > - &data, > + qemu_coroutine_self(), > NULL, > NULL); > - > - if (!data.complete) { > - g_main_loop_run(data.loop); > - } > - g_main_loop_unref(data.loop); > - if (data.error) { > - error_propagate(errp, data.error); > - object_unref(OBJECT(tioc)); > - return NULL; > - } > + qemu_coroutine_yield(); Nice. > +++ b/nbd/server.c > @@ -668,16 +668,15 @@ static int nbd_negotiate_handle_info(NBDClient *client, > uint16_t myflags, > > /* Handle NBD_OPT_STARTTLS. Return NULL to drop connection, or else the > * new channel for all further (now-encrypted) communication. */ > -static QIOChannel *nbd_negotiate_handle_starttls(NBDClient *client, > - Error **errp) > +static QIOChannel coroutine_fn *nbd_co_negotiate_handle_starttls( Awkward split of the return type; the coroutine_fn should instead be placed after the *, as in: block/mirror.c:static MirrorOp *coroutine_fn active_write_prepare(... > + NBDClient *client, Error **errp) > { > QIOChannel *ioc; > QIOChannelTLS *tioc; > - struct NBDTLSHandshakeData data = { 0 }; All uses of this type have been deleted; you should also remove it from nbd-internal.h. > @@ -1093,7 +1082,7 @@ static int nbd_negotiate_options(NBDClient *client, > uint16_t myflags, > return -EINVAL; > } > > - trace_nbd_negotiate_options_check_option(option, > + trace_nbd_co_negotiate_options_check_option(option, > nbd_opt_lookup(option)); Indentation looks off. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature