A server may have a reason to reject a request for structured replies, beyond just not recognizing them as a valid request. It doesn't hurt us to continue talking to such a server; otherwise 'qemu-nbd --list' of such a server fails to display all possible details about the export.
Encountered when temporarily tweaking nbdkit to reply with NBD_REP_ERR_POLICY. Present since structured reply support was first added (commit d795299b reused starttls handling, but starttls has to reject all errors). Signed-off-by: Eric Blake <ebl...@redhat.com> --- nbd/client.c | 39 +++++++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 16 deletions(-) diff --git a/nbd/client.c b/nbd/client.c index 49bf9906f94b..92444f5e9a62 100644 --- a/nbd/client.c +++ b/nbd/client.c @@ -1,5 +1,5 @@ /* - * Copyright (C) 2016-2018 Red Hat, Inc. + * Copyright (C) 2016-2019 Red Hat, Inc. * Copyright (C) 2005 Anthony Liguori <anth...@codemonkey.ws> * * Network Block Device Client Side @@ -142,17 +142,19 @@ static int nbd_receive_option_reply(QIOChannel *ioc, uint32_t opt, return 0; } -/* If reply represents success, return 1 without further action. - * If reply represents an error, consume the optional payload of - * the packet on ioc. Then return 0 for unsupported (so the client - * can fall back to other approaches), or -1 with errp set for other - * errors. +/* + * If reply represents success, return 1 without further action. If + * reply represents an error, consume the optional payload of the + * packet on ioc. Then return 0 for unsupported (so the client can + * fall back to other approaches), where @strict determines if only + * ERR_UNSUP or all errors fit that category, or -1 with errp set for + * other errors. */ static int nbd_handle_reply_err(QIOChannel *ioc, NBDOptionReply *reply, - Error **errp) + bool strict, Error **errp) { char *msg = NULL; - int result = -1; + int result = strict ? -1 : 0; if (!(reply->type & (1 << 31))) { return 1; @@ -163,6 +165,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, NBDOptionReply *reply, error_setg(errp, "server error %" PRIu32 " (%s) message is too long", reply->type, nbd_rep_lookup(reply->type)); + result = -1; goto cleanup; } msg = g_malloc(reply->length + 1); @@ -170,6 +173,7 @@ static int nbd_handle_reply_err(QIOChannel *ioc, NBDOptionReply *reply, error_prepend(errp, "Failed to read option error %" PRIu32 " (%s) message: ", reply->type, nbd_rep_lookup(reply->type)); + result = -1; goto cleanup; } msg[reply->length] = '\0'; @@ -258,7 +262,7 @@ static int nbd_receive_list(QIOChannel *ioc, char **name, char **description, if (nbd_receive_option_reply(ioc, NBD_OPT_LIST, &reply, errp) < 0) { return -1; } - error = nbd_handle_reply_err(ioc, &reply, errp); + error = nbd_handle_reply_err(ioc, &reply, true, errp); if (error <= 0) { return error; } @@ -371,7 +375,7 @@ static int nbd_opt_info_or_go(QIOChannel *ioc, uint32_t opt, if (nbd_receive_option_reply(ioc, opt, &reply, errp) < 0) { return -1; } - error = nbd_handle_reply_err(ioc, &reply, errp); + error = nbd_handle_reply_err(ioc, &reply, true, errp); if (error <= 0) { return error; } @@ -546,12 +550,15 @@ static int nbd_receive_query_exports(QIOChannel *ioc, } } -/* nbd_request_simple_option: Send an option request, and parse the reply +/* + * nbd_request_simple_option: Send an option request, and parse the reply. + * @strict controls whether ERR_UNSUP or all errors produce 0 status. * return 1 for successful negotiation, * 0 if operation is unsupported, * -1 with errp set for any other error */ -static int nbd_request_simple_option(QIOChannel *ioc, int opt, Error **errp) +static int nbd_request_simple_option(QIOChannel *ioc, int opt, bool strict, + Error **errp) { NBDOptionReply reply; int error; @@ -563,7 +570,7 @@ static int nbd_request_simple_option(QIOChannel *ioc, int opt, Error **errp) if (nbd_receive_option_reply(ioc, opt, &reply, errp) < 0) { return -1; } - error = nbd_handle_reply_err(ioc, &reply, errp); + error = nbd_handle_reply_err(ioc, &reply, strict, errp); if (error <= 0) { return error; } @@ -595,7 +602,7 @@ static QIOChannel *nbd_receive_starttls(QIOChannel *ioc, QIOChannelTLS *tioc; struct NBDTLSHandshakeData data = { 0 }; - ret = nbd_request_simple_option(ioc, NBD_OPT_STARTTLS, errp); + ret = nbd_request_simple_option(ioc, NBD_OPT_STARTTLS, true, errp); if (ret <= 0) { if (ret == 0) { error_setg(errp, "Server don't support STARTTLS option"); @@ -695,7 +702,7 @@ static int nbd_receive_one_meta_context(QIOChannel *ioc, return -1; } - ret = nbd_handle_reply_err(ioc, &reply, errp); + ret = nbd_handle_reply_err(ioc, &reply, true, errp); if (ret <= 0) { return ret; } @@ -951,7 +958,7 @@ static int nbd_start_negotiate(AioContext *aio_context, QIOChannel *ioc, if (structured_reply) { result = nbd_request_simple_option(ioc, NBD_OPT_STRUCTURED_REPLY, - errp); + false, errp); if (result < 0) { return -EINVAL; } -- 2.21.0