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

[Eric Blake]

On 10/20/2017 02:26 PM, Vladimir Sementsov-Ogievskiy wrote:
> 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_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 ...

Hmm.  For NBD_OPT_STARTTLS, NBD_REP_ERR_POLICY is an example of an
expected (error) reply.  Then again, we can't reach here on that reply,
because it would have already been handled in nbd_handle_reply_error().
Okay, I can see where you are coming from - since we already treated all
NBD_REP_ERR_* as rejection messages, anything else that the server sends
is unexpected if it is not ACK.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org

signature.asc
Description: OpenPGP digital signature