On 19.08.20 12:46, Kevin Wolf wrote:
> Am 25.06.2020 um 17:21 hat Max Reitz geschrieben:
>> blkverify is a filter, so bdrv_get_allocated_file_size()'s default
>> implementation will return only the size of its filtered child.
>> However, because both of its children are disk images, it makes more
>> sense to sum both of their allocated sizes.
> 
> Hm, but so are the children of, say, backup-top. I don't think you're
> suggesting that backup-top should add the sizes of both images,

Can be argued either way.

I lean on the side of that it should not, because: The backup is
external.  The job is copying data off.  So it isn’t really directly
data that serves qemu, which can be seen from the fact that it’s never read.

Which is not the case for quorum and blkverify.  Though one can argue
that blkverify is different from quorum here in that it doesn’t read
data from the non-filtered child to serve a guest device, but just to
verify it node-internally.

> even
> though the backup job is actively increasing the allocated size of the
> non-primary node, much like blkverify.
> 
> So I believe returning only the allocated size of the primary child in
> blkverify would be more consistent with what we do elsewhere.

For me, blkverify is basically an archaic mode of quorum, and for quorum
it’s clear that we should sum the sizes.  Which is why I thought summing
the sizes would be more consistent.

But honestly, I just don’t care about blkverify whatsoever.  I don’t
believe anyone actually cares about whether what blkverify returns for
.bdrv_get_allocated_file_size() is consistent.  I believe we could
return 42 and nobody would bat an eyelash.

(But that’s the curse of this series.  I have to touch stuff that nobody
cares about, and then we have discussions on stuff nobody cares about.)

So from that POV I’m happy to drop this patch if it means there’s just
one less opportunity to have a discussion on blkverify.

Max