Re: [Nbd] [PATCH/RFC] Add support for NBD_CMD_WRITE_ZEROES

[Wouter Verhelst <w@...112...>]

On Fri, Apr 15, 2016 at 04:50:45PM +0100, Alex Bligh wrote:
> 
> On 15 Apr 2016, at 15:10, Eric Blake <eblake@...696...> wrote:
> 
> > You completely ignore NBD_CMD_FLAG_NO_HOLE; you could obey that flag
> > (when set, do the calloc() here; when cleared, call into the trim code
> > if you can guarantee that you'll read zero after trims).
> 
> Sadly you cannot guarantee that you will read zeroes after
> exptrim is called. Exceptions include:
> 
> 1. F_COPYONWRITE being set (trim is ignored)

I've been wanting to modify copyonwrite so that when you TRIM, it clears
out the diff file, but not the original file.

That would still mean it doesn't zero out stuff, obviously.

> 2. The initial and final blocks if F_TREEFILES is set if either of
>    the start and end of the read is not aligned to TREEPAGESIZE

We could detect if that's the case, and split the request up into three:
head and tail with writes, middle part with treefile handling.

Due to the nature of how the treefiles stuff works, it's 100% guaranteed
that that would work.

> 3. Any underlying filesystems which don't support fallocate
>    with PUNCH_HOLE (we don't check the error return)

Would be easy to add that.

> 4. Probably something on Windows (!)

Gotta admit, I don't know what the semantics of the Windows punch_hole
equivalent are.

> > However, the client MAY set the command flag NBD_CMD_FLAG_NO_HOLEto
> > inform the server that the area MUST be fully provisioned, ensuring
> > that future writes to the same area will not cause fragmentation or
> > cause failure due to insufficient space.
> 
> So I could do this:
> 
> >  But if nothing
> > else, you have a bug for not permitting the NBD_CMD_FLAG_NO_HOLE (even
> > if you intend to ignore it by ALWAYS operating in no-hole mode, as was
> > done here).
> 
> Or I could do the physical write of zeroes and then call trim afterwards
> if NBD_CMD_FLAG_NO_HOLE is *not* set and trim is supported.
> 
> As fallocate() and friends are cheap, I think I will do that.

That gets awkward if you remember all the possible backing store modes.
I really should get started on that abstraction layer I was talking
about earlier.

I've been thinking we should also have the TRIM and WRITE_ZEROES
handling be combined into one set of functions. You'd have a "expclear"
function or some such which has two booleans:

- must_zero: when WRITE_ZEROES was called, set it to true. Operation
  must then set all affected bytes to zero.
- may_hole: when either TRIM was called or WRITE_ZEROES doesn't have the
  NO_HOLE flag set. Operation may deallocate the range if possible.

> Someone should really do this properly (which is non-trivial); I was
> really getting a baseline code example for the extension branch
> demonstration.

We should probably keep it at that for now -- possibly add a "todo" item
there.

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