12.01.2019 20:58, Eric Blake wrote:
> An upcoming patch will add the ability for qemu-nbd to list
> the services provided by an NBD server. Share the common
> code of the TLS handshake by splitting the initial exchange
> into a separate function, leaving only the export handling
> in the original function. Functionally, there should be no
> change in behavior in this patch, although some of the code
> motion may be difficult to follow due to indentation changes
> (view with 'git diff -w' for a smaller changeset).
>
> I considered an enum for the return code coordinating state
> between the two functions, but in the end just settled with
> ample comments.
>
> Signed-off-by: Eric Blake <ebl...@redhat.com>
> Reviewed-by: Richard W.M. Jones <rjo...@redhat.com>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com>
> Message-Id: <20181215135324.152629-17-ebl...@redhat.com>
> ---
> nbd/client.c | 144 +++++++++++++++++++++++++++++++----------------
> nbd/trace-events | 2 +-
> 2 files changed, 95 insertions(+), 51 deletions(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index fa931fd8e5d..5053433ea5e 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -813,21 +813,24 @@ static int nbd_negotiate_simple_meta_context(QIOChannel
> *ioc,
> return received;
> }
>
> -int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
> - const char *hostname, QIOChannel **outioc,
> - NBDExportInfo *info, Error **errp)
> +/*
> + * nbd_start_negotiate:
> + * Start the handshake to the server. After a positive return, the server
> + * is ready to accept additional NBD_OPT requests.
+ * @zeroes must be set to true before call !!!
> + * Returns: negative errno: failure talking to server
> + * 0: server is oldstyle, client must still parse export size
> + * 1: server is newstyle, but can only accept EXPORT_NAME
> + * 2: server is newstyle, but lacks structured replies
> + * 3: server is newstyle and set up for structured replies
> + */
> +static int nbd_start_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
> + const char *hostname, QIOChannel **outioc,
> + bool structured_reply, bool *zeroes,
> + Error **errp)
> {
> uint64_t magic;
> - bool zeroes = true;
> - bool structured_reply = info->structured_reply;
> - bool base_allocation = info->base_allocation;
>
> - trace_nbd_receive_negotiate(tlscreds, hostname ? hostname : "<null>");
> -
> - assert(info->name);
> - trace_nbd_receive_negotiate_name(info->name);
> - info->structured_reply = false;
> - info->base_allocation = false;
> + trace_nbd_start_negotiate(tlscreds, hostname ? hostname : "<null>");
or, a lot better:
+ *zeroes = true
>
> if (outioc) {
> *outioc = NULL;
> @@ -872,7 +875,7 @@ int nbd_receive_negotiate(QIOChannel *ioc,
> QCryptoTLSCreds *tlscreds,
> clientflags |= NBD_FLAG_C_FIXED_NEWSTYLE;
> }
> if (globalflags & NBD_FLAG_NO_ZEROES) {
> - zeroes = false;
> + *zeroes = false;
> clientflags |= NBD_FLAG_C_NO_ZEROES;
> }
> /* client requested flags */
[..]
> +/*
> + * nbd_receive_negotiate:
> + * Connect to server, complete negotiation, and move into transmission phase.
> + * Returns: negative errno: failure talking to server
> + * 0: server is connected
> + */
> +int nbd_receive_negotiate(QIOChannel *ioc, QCryptoTLSCreds *tlscreds,
> + const char *hostname, QIOChannel **outioc,
> + NBDExportInfo *info, Error **errp)
> +{
> + int result;
> + bool zeroes = true;
and then, this initialization may be dropped (or left as is, if gcc like it)
> + bool base_allocation = info->base_allocation;
> + uint32_t oldflags;
> +
> + assert(info->name);
> + trace_nbd_receive_negotiate_name(info->name);
> +
> + result = nbd_start_negotiate(ioc, tlscreds, hostname, outioc,
> + info->structured_reply, &zeroes, errp);
> +
--
Best regards,
Vladimir
