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