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

[Max Reitz <mreitz@redhat.com>]

[-- Attachment #1.1: Type: text/plain, Size: 2421 bytes --]

On 31.01.20 18: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).

I don’t find it misleading, I just find it meaningless, which then makes
it open to interpretation (or maybe rather s/interpretation/wishful
thinking/).

> Refactor all uses
> to instead have a single function that returns multiple bits of
> information, with better naming and documentation.

It doesn’t make sense to me.  How exactly is it unwieldy?  In the sense
that we have to deal with multiple rather small implementation functions
rather than a big one per driver?  Actually, multiple small functions
sounds better to me – unless the three implementations share common code.

As for the callers, they only want a single flag out of the three, don’t
they?  If so, it doesn’t really matter for them.

In fact, I can imagine that drivers can trivially return
BDRV_ZERO_TRUNCATE information (because the preallocation mode is
fixed), whereas BDRV_ZERO_CREATE can be a bit more involved, and
BDRV_ZERO_OPEN could take even more time because some (constant-time)
inquiries have to be done.

And thus callers which just want the trivially obtainable
BDRV_ZERO_TRUNCATE info have to wait for the BDRV_ZERO_OPEN inquiry,
even though they don’t care about that flag.

So I’d leave it as separate functions so drivers can feel free to have
implementations for BDRV_ZERO_OPEN that take more than mere microseconds
but that are more accurate.

(Or maybe if you really want it to be a single functions, callers could
pass the mask of flags they care about.  If all flags are trivially
obtainable, the implementations would then simply create their result
mask and & it with the caller-given mask.  For implementations where
some branches could take a bit more time, those branches are only taken
when the caller cares about the given flag.  But again, I don’t
necessarily think having a single function is more easily handleable
than three smaller ones.)

Max


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]