Re: [PATCH 10/17] block: Add new BDRV_ZERO_OPEN flag

[Eric Blake]

On 2/4/20 11:34 AM, Max Reitz wrote:

> > +++ b/include/block/block.h
> > @@ -105,6 +105,16 @@ typedef enum {
> >        * for drivers that set .bdrv_co_truncate.
> >        */
> >       BDRV_ZERO_TRUNCATE      = 0x2,
> > +
> > +    /*
> > +     * bdrv_known_zeroes() should include this bit if an image is
> > +     * known to read as all zeroes when first opened; this bit should
> > +     * not be relied on after any writes to the image.
>
> Is there a good reason for this?  Because to me this screams like we are
> going to check this flag without ensuring that the image has actually
> not been written to yet.  So if it’s generally easy for drivers to stop
> reporting this flag after a write, then maybe we should do so.

In patch 15 (implementing things in qcow2), I actually wrote the driver 
to return live results, rather than just open-time results, in part 
because writing the bit to persistent storage in qcow2 means that the 
bit must be accurate, without relying on the block layer's help.

But my pending NBD patch (not posted yet, but will be soon), the 
proposal I'm making for the NBD protocol itself is just open-time, not 
live, and so it would be more work than necessary to make the NBD driver 
report live results.

But it seems like it should be easy enough to also patch the block layer 
itself to guarantee that callers of bdrv_known_zeroes() cannot see this 
bit set if the block layer has been used in any non-zero transaction, by 
repeating the same logic as used in qcow2 to kill the bit (any 
write/write_compressed/bdrv_copy clear the bit, any trim clears the bit 
if the driver does not guarantee trim reads as zero, any truncate clears 
the bit if the driver does not guarantee truncate reads as zero, etc). 
Basically, the block layer would cache the results of .bdrv_known_zeroes 
during .bdrv_co_open, bdrv_co_pwrite() and friends would update that 
cache, and and bdrv_known_zeroes() would report the cached value rather 
than a fresh call to .bdrv_known_zeroes.

Are we worried enough about clients of this interface to make the block 
layer more robust?  (From the maintenance standpoint, the more the block 
layer guarantees, the easier it is to write code that uses the block 
layer; but there is the counter-argument that making the block layer 
track whether an image has been modified means a [slight] penalty to 
every write request to update the boolean.)

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org