[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-block] [PATCH v6 22/42] block: Fix bdrv_get_allocated_file_siz

From: Max Reitz
Subject: Re: [Qemu-block] [PATCH v6 22/42] block: Fix bdrv_get_allocated_file_size's fallback
Date: Wed, 11 Sep 2019 08:20:12 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0

On 10.09.19 16:52, Kevin Wolf wrote:
> Am 09.08.2019 um 18:13 hat Max Reitz geschrieben:
>> If the driver does not implement bdrv_get_allocated_file_size(), we
>> should fall back to cumulating the allocated size of all non-COW
>> children instead of just bs->file.
>> Suggested-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Signed-off-by: Max Reitz <address@hidden>
> This smells like an overgeneralisation, but if we want to count all vmdk
> extents, the qcow2 external data file, etc. it's an improvement anyway.
> A driver that has a child that should not be counted must just remember
> to implement the callback.
> Let me think of an example... How about quorum, for a change? :-)
> Or the second blkverify child.
> Or eventually the block job filter nodes.

I actually think it makes sense for all of these nodes to report the sum
of all of their children’s allocated sizes.

If a quorum node has three children with allocated sizes of 3 MB, 1 MB,
and 2 MB, respectively (totally possible if some have explicit zeroes
and others don’t; it may also depend on the protocol, the filesystem,
etc.), then I think it makes most sense to report indeed 6 MB for the
quorum subtree as a whole.  What would you report?  3 MB?

> Ehm... Maybe I should just take back what I said first. It almost feels
> like it would be better if qcow2 and vmdk explicitly used a handler that
> counts all children (could still be a generic one in block.c) rather
> than having to remember to disable the functionality everywhere where we
> don't want to have it.

I don’t, because everywhere we don’t want this functionality, we still
need to choose a child.  This has to be done by the driver anyway.


> And please adjust the comment for bdrv_get_allocated_file_size(), it
> only talks about a single file as if trees didn't exist. Actually, it
> doesn't even seem so easy to define. Maybe primary node + storage nodes?
> Then vmdk needs to expose its extents as storage nodes (plural!), but
> in the long run that might be needed anyway.

Attachment: signature.asc
Description: OpenPGP digital signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]