On 10/20/2017 02:03 PM, Vladimir Sementsov-Ogievskiy wrote: > 20.10.2017 01:26, Eric Blake wrote: >> From: Vladimir Sementsov-Ogievskiy <vsement...@virtuozzo.com> >> >> Minimal implementation of structured read: one structured reply chunk, >> no segmentation. >> Minimal structured error implementation: no text message. >> Support DF flag, but just ignore it, as there is no segmentation any >> way. >>
>> +
>> + case NBD_OPT_STRUCTURED_REPLY:
>> + if (length) {
>> + ret = nbd_check_zero_length(client, length,
>> option, errp);
>
> here same thing like in previous patch, about success in
> nbd_check_zero_length
Here, successfully answering the client with an error message should NOT
stop negotiating, so THIS return is correct.
>
>> + } else if (client->structured_reply) {
>> + ret = nbd_negotiate_send_rep_err(
>> + client->ioc, NBD_REP_ERR_INVALID, option, errp,
>> + "structured reply already negotiated");
>> + } else {
>> + ret = nbd_negotiate_send_rep(client->ioc,
>> NBD_REP_ACK,
>> + option, errp);
>> + }
>
> you've dropped "if (ret < 0) { return ret }", but the following two
> lines should not be
> executed if ret < 0.. May be it doesn't matter (we will abort connection
> anyway after switch {}) but
> it looks strange.
>
>> + client->structured_reply = true;
>> + myflags |= NBD_FLAG_SEND_DF;
>> + break;
Indeed, these two lines are harmless due to the catch-all 'ret < 0'
after the switch at the tail end of the loop (which is why I dropped the
'if' here).
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
signature.asc
Description: OpenPGP digital signature
