Re: [Qemu-devel] [Nbd] [PATCH v2] doc: Allow NBD_CMD_FLAG_NO_HOLE during NBD_CMD_WRITE

[Wouter Verhelst]

On Tue, Apr 05, 2016 at 10:43:14AM -0600, Eric Blake wrote:
> On 04/05/2016 03:38 AM, Markus Pargmann wrote:
> > Hi,
> > 
> > On Monday 04 April 2016 16:15:43 Eric Blake wrote:
> >> qemu already has an existing server implementation option that will
> >> explicitly search the payload of NBD_CMD_WRITE for large blocks of
> >> zeroes, and punch holes in the underlying file.  For old clients
> >> that don't know how to use the new NBD_CMD_WRITE_ZEROES, this is a
> >> workaround to keep the server's destination file approximately as
> >> sparse as the client's source.  However, for new clients that know
> >> how to explicitly request holes, it is unnecessary overhead; and
> >> can lead to the server punching a hole and risking fragmentation or
> >> future ENOSPC even when the client explicitly wanted to write
> >> zeroes rather than a hole.  So it makes sense to let the new
> >> NBD_CMD_FLAG_NO_HOLE work for WRITE as well as WRITE_ZEROES.
> > 
> > From the commit message it sounds like this is only for new clients
> > supporting WRITE_ZEROES because for those we don't want to search
> > through all the data of normal WRITEs. If you don't need to set this for
> > each WRITE individually perhaps we could move it to the negotiation
> > part?
> 
> Interesting idea.  So we'd add a new NBD_OPT_XXX that lets the server
> know that "I plan on using WRITE_ZEROS and TRIM as the only places where
> I want you to trim, so you can avoid scanning for zeroes in WRITE"; the
> server replies with NBD_REP_ACK if it understands the client (in which
> case the server _should_ be advertising NBD_FLAG_SEND_WRITE_ZEROES
> and/or NBD_FLAG_SEND_TRIM), and with NBD_REP_ERR_UNSUP if it is too old
> (the server may still advertise TRIM, but probably should not advertise
> WRITE_ZEROES - we are still early enough that we could mandate that any
> server that supports WRITE_ZEROES also supports the new NBD_OPT_XXX).

Certainly.

However, I think a server should be allowed to reply to this
NBD_OPT_NO_AUTO_HOLE (or whatever we end up calling it) with
NBD_REP_ERR_POLICY -- i.e., it understands the request, but server-side
configuration forbids it to heed it.

This kind of stuff is *always* a trade-off. Someone low on diskspace
might want to force their server to scan for zeroes, in the
understanding that things might break.

[...]
> Does the idea of a new NBD_OPT_ make enough sense to write that up
> rather than mandating the use of NBD_CMD_FLAG_NO_HOLE with WRITE?

Yeah, it does to me. The client shouldn't have to care much about this
kind of stuff.

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