Re: [Qemu-block] [PATCH v5 08/11] nbd/client: refactor nbd_receive_starttls

[Vladimir Sementsov-Ogievskiy]

20.10.2017 01:26, Eric Blake wrote:
> From: Vladimir Sementsov-Ogievskiy <address@hidden>
>
> Split out nbd_request_simple_option to be reused for structured reply
> option.
>
> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v5: only check length for ACK responses
> v4: reduce redundant traces, typo fix in commit message
> ---
>   nbd/client.c     | 70 ++++++++++++++++++++++++++++++++++++++------------------
>   nbd/trace-events |  4 +---
>   2 files changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/nbd/client.c b/nbd/client.c
> index 50f36b511e..78a0e9cdc3 100644
> --- a/nbd/client.c
> +++ b/nbd/client.c
> @@ -508,35 +508,61 @@ static int nbd_receive_query_exports(QIOChannel *ioc,
>       }
>   }
>
> +/* nbd_request_simple_option: Send an option request, and parse the reply
> + * 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)
> +{
> +    nbd_opt_reply reply;
> +    int error;
> +
> +    if (nbd_send_option_request(ioc, opt, 0, NULL, errp) < 0) {
> +        return -1;
> +    }
> +
> +    if (nbd_receive_option_reply(ioc, opt, &reply, errp) < 0) {
> +        return -1;
> +    }
> +    error = nbd_handle_reply_err(ioc, &reply, errp);
> +    if (error <= 0) {
> +        return error;
> +    }
> +
> +    if (reply.type != NBD_REP_ACK) {
> +        error_setg(errp, "Server rejected request for option %d (%s) "

Looks like now it is not rejected (nbd_handle_reply_err return >0 only 
for successful replies), but it should be
Server replied ... with unexpected reply ...

with that fixed:
Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>




--
Best regards,
Vladimir