Re: [Qemu-devel] [PATCH v3 14/38] block: Move BlockAcctStats into BlockBackend

[Eric Blake]

On 06/05/2015 04:47 AM, Alberto Garcia wrote:
> On Wed 03 Jun 2015 10:52:34 PM CEST, Eric Blake wrote:
> 
>>> As the comment above bdrv_get_stats() says, BlockAcctStats is
>>> something which belongs to the device instead of each
>>> BlockDriverState. This patch therefore moves it into the
>>> BlockBackend.
>>
>> Again, Berto may want to eventually report stats for both BDS and BB,
>> but that can come later.  For now, this is reasonable refactoring.
> 
> Yeah, this change makes sense and I was actually planning to do
> something similar in my patch series.
> 
> The only problem that I see with this is that the data is stored in the
> right place but the API is (still) wrong. query-blockstats queries BDSs,
> but the information we'll get in return it's either the stats from the
> BlockBackend (for root nodes) or all zeroes (for the rest), not the
> stats from the BDSs themselves.
> 
> Ideally we would need one way to query information from the BlockBackend
> (that we already have) and another way to query information from the BDS
> (that we don't have yet). But I guess we have to look for a way to do
> this without breaking the API compatibility.
> 
> And for the record, my priorities at the moment are the stats from the
> BlockBackend, not the BDS.

We already have:
query-block - reports stats on BB
query-blockstats - reports stats on BDS

I don't know if it makes more sense to have a single shared struct that
both calls can report, for the things that make sense in both places,
but I also know that in my current efforts to add write-threshold
support to libvirt, there were several stats that I wish were available
from the opposite command of where it is currently found.  Back-compat
says it is okay to duplicate output in multiple locations, but not to
completely move it from one to the other, although the effort of
duplication may be a maintenance nightmare.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature