Re: [PATCH 09/17] block: Refactor bdrv_has_zero_init{,_truncate}

[Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>]

31.01.2020 20:44, Eric Blake wrote:
> Having two slightly-different function names for related purposes is
> unwieldy, especially since I envision adding yet another notion of
> zero support in an upcoming patch.  It doesn't help that
> bdrv_has_zero_init() is a misleading name (I originally thought that a
> driver could only return 1 when opening an already-existing image
> known to be all zeroes; but in reality many drivers always return 1
> because it only applies to a just-created image).  Refactor all uses
> to instead have a single function that returns multiple bits of
> information, with better naming and documentation.

Sounds good

> 
> No semantic change, although some of the changes (such as to qcow2.c)
> require a careful reading to see how it remains the same.
> 

...

> diff --git a/include/block/block.h b/include/block/block.h
> index 6cd566324d95..a6a227f50678 100644
> --- a/include/block/block.h
> +++ b/include/block/block.h

Hmm, header file in the middle of the patch, possibly you don't use
[diff]
     orderFile = scripts/git.orderfile

in git config.. Or it is broken.

> @@ -85,6 +85,28 @@ typedef enum {
>       BDRV_REQ_MASK               = 0x3ff,
>   } BdrvRequestFlags;
> 
> +typedef enum {
> +    /*
> +     * bdrv_known_zeroes() should include this bit if the contents of
> +     * a freshly-created image with no backing file reads as all
> +     * zeroes without any additional effort.  If .bdrv_co_truncate is
> +     * set, then this must be clear if BDRV_ZERO_TRUNCATE is clear.

I understand that this is preexisting logic, but could I ask: why? What's wrong
if driver can guarantee that created file is all-zero, but is not sure about
file resizing? I agree that it's normal for these flags to have the same value,
but what is the reason for this restriction?..

So, the only possible combination of flags, when they differs, is create=0 and
truncate=1.. How is it possible?

> +     * Since this bit is only reliable at image creation, a driver may
> +     * return this bit even for existing images that do not currently
> +     * read as zero.
> +     */
> +    BDRV_ZERO_CREATE        = 0x1,
> +
> +    /*
> +     * bdrv_known_zeroes() should include this bit if growing an image
> +     * with PREALLOC_MODE_OFF (either with no backing file, or beyond
> +     * the size of the backing file) will read the new data as all
> +     * zeroes without any additional effort.  This bit only matters
> +     * for drivers that set .bdrv_co_truncate.
> +     */
> +    BDRV_ZERO_TRUNCATE      = 0x2,
> +} BdrvZeroFlags;
> +

...


-- 
Best regards,
Vladimir