Re: [Qemu-devel] [PATCH v6 22/42] block: Fix bdrv_get_allocated_file_size's fallback

[Kevin Wolf]

Am 11.09.2019 um 08:20 hat Max Reitz geschrieben:
> 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.

Hm... Yes, in a way. But not much more than it would make sense to
report the sum of the sizes of all images in the whole backing chain
(this is a useful thing to ask for, just maybe not the right thing to
return for a low-level interface). But I can accept that it's maybe a
bit more expected for quorum and blkverify than for COW images.

If you include the block job filter nodes, I have to disagree, though.
If mirror_top_bs (or any other job filter) sits in the middle of the
source chain, then I certainly don't want to see the target size added
to it.

> 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?

Do it's the quorum way: Just vote!

No, you're right, of course. -ENOTSUP is probably the only other thing
you could do then.

> > 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.

Well, by default the primary child, which should cover like 90% of the
drivers?

Kevin

signature.asc
Description: PGP signature