[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/4] nbd: do qemu_coroutine_yield during tls han
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH 3/4] nbd: do qemu_coroutine_yield during tls handshake |
Date: |
Mon, 11 Feb 2019 15:55:15 -0600 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.4.0 |
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 <address@hidden>
> ---
> 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