From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38732) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aknzA-0002Bj-2y for qemu-devel@nongnu.org; Tue, 29 Mar 2016 03:22:45 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aknz6-0002aB-R8 for qemu-devel@nongnu.org; Tue, 29 Mar 2016 03:22:44 -0400 Received: from barbershop.grep.be ([89.106.240.122]:54655) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aknz6-0002Zh-LO for qemu-devel@nongnu.org; Tue, 29 Mar 2016 03:22:40 -0400 Date: Tue, 29 Mar 2016 09:22:23 +0200 From: Wouter Verhelst Message-ID: <20160329072223.GB22386@grep.be> References: <1459161798-32120-1-git-send-email-den@openvz.org> <1459161798-32120-2-git-send-email-den@openvz.org> <56F92AE1.2040709@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56F92AE1.2040709@redhat.com> Subject: Re: [Qemu-devel] [Nbd] [PATCH 1/3] NBD proto: forbid TRIM command without negotiation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eric Blake Cc: nbd-general@lists.sourceforge.net, "Denis V. Lunev" , qemu-devel@nongnu.org, Pavel Borzenkov On Mon, Mar 28, 2016 at 07:00:17AM -0600, Eric Blake wrote: > On 03/28/2016 04:43 AM, Denis V. Lunev wrote: > > From: Pavel Borzenkov > > > > There is a loophole in the protocol that allows a client to send TRIM > > request even if support for it wasn't negotiated with the server. State > > explicitly that the client MUST NOT send such command without prior > > successful negotiation. > > > > Signed-off-by: Pavel Borzenkov > > Reviewed-by: Roman Kagan > > Signed-off-by: Denis V. Lunev > > CC: Wouter Verhelst > > CC: Eric Blake > > CC: Alex Bligh > > --- > > doc/proto.md | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/doc/proto.md b/doc/proto.md > > index 6d1cb34..d54ed19 100644 > > --- a/doc/proto.md > > +++ b/doc/proto.md > > @@ -471,6 +471,9 @@ The following request types exist: > > about the contents of the export affected by this command, until > > overwriting it again with `NBD_CMD_WRITE`. > > > > + A client MUST NOT send a trim request unless `NBD_FLAG_SEND_TRIM` > > + was set in the export flags field. > > + > > Do we also want to mention that the server SHOULD fail with EINVAL if > the client sends it anyway, and similarly if NBD_CMD_FLUSH was sent > without the appropriate export flag (but that the client should not rely > on that particular failure)? I think the protocol should mention that the server MAY fail with EINVAL, rather than SHOULD. Rationale: the robusness principle -- if you didn't negotiate it, you may end up with a server who doesn't know about the feature; but if it just so happens that the server does know about it even though you didn't negotiate it, there is little harm in it following up on the request. > But as this is a strict improvement, > Reviewed-by: Eric Blake -- < ron> I mean, the main *practical* problem with C++, is there's like a dozen people in the world who think they really understand all of its rules, and pretty much all of them are just lying to themselves too. -- #debian-devel, OFTC, 2016-02-12