|
From: | Vladimir Sementsov-Ogievskiy |
Subject: | Re: [Qemu-block] [PATCH v5 06/11] nbd: Minimal structured read for server |
Date: | Fri, 20 Oct 2017 22:30:03 +0300 |
User-agent: | Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
20.10.2017 22:11, Eric Blake wrote:
On 10/20/2017 02:03 PM, Vladimir Sementsov-Ogievskiy wrote:20.10.2017 01:26, Eric Blake wrote:From: Vladimir Sementsov-Ogievskiy <address@hidden> 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_lengthHere, successfully answering the client with an error message should NOT stop negotiating, so THIS return is correct.
ok, you are right
+ } 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).
yes. but it looks strange) Anyway, Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden> -- Best regards, Vladimir
[Prev in Thread] | Current Thread | [Next in Thread] |