Re: [Qemu-devel] [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension

From: Pavel Borzenkov <pborzenkov@virtuozzo.com>
To: Wouter Verhelst <w@uter.be>
Cc: nbd-general@lists.sourceforge.net, Kevin Wolf <kwolf@redhat.com>,
	qemu-devel@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	"Denis V. Lunev" <den@openvz.org>
Subject: Re: [Qemu-devel] [Nbd] [PATCH 1/2] NBD proto: add WRITE_ZEROES extension
Date: Thu, 24 Mar 2016 14:35:06 +0300	[thread overview]
Message-ID: <20160324113506.GD24831@phobos.sw.ru> (raw)
In-Reply-To: <20160324082641.GF1590@grep.be>

On Thu, Mar 24, 2016 at 09:26:41AM +0100, Wouter Verhelst wrote:
> On Thu, Mar 24, 2016 at 10:57:06AM +0300, Pavel Borzenkov wrote:
> > On Wed, Mar 23, 2016 at 06:21:16PM +0100, Wouter Verhelst wrote:
> > > So, the semantics of your proposed WRITE_ZEROES are exactly the same as
> > > the WRITE command, except that no payload is sent?
> > > 
> > > In that case, I think it's slightly more sensible if we don't add a new
> > > command, but instead just have an NBD_CMD_FLAG_ZEROES added to the WRITE
> > > command instead. After all, they're going to be (mostly) the same
> > > anyway.
> > > 
> > > Did you propose a separate command for a specific reason that I'm
> > > missing (or forgetting), or is that just an oversight?
> > 
> > No, there is no specific reason. Looks like NBD_CMD_FLAG_ZEROES fits the
> > spec and implementations nicely. So I'll rewrite the extension and add
> > the flag instead of the whole command.
> 
> Actually, having given this some more thought...
> 
> There is at least one server-side implementation of nbd (mine) which
> silently ignores flags it doesn't know about. This isn't a problem for
> non-critical flags, but it could be a problem for a flag like this. Of
> course, a client shouldn't send a flag to a server which that server
> hasn't heard of, but mistakes do happen.
> 
> Do we want to keep that in mind? If so, we might want to keep it as a
> separate command after all.
> 
> OTOH, it could be said that silently ignoring unknown messages is a bug.
> I should probably just fix my implementation instead.

Since we are going to fix such ambiguities in the spec, fixing incorrect
implementations looks like the right thing to do.

But old unfixed versions which ignore unknown flags (or broken
implementation which send ZEROES without negotiating it) will still
exist.

So it looks safer to do it as a separate command. In this case we will
at least receive an error code back.

> 
> -- 
> < 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