Re: [Qemu-devel] [PATCH 10/50] block: Remove wr_highest_offset from BlockAcctStats

[Max Reitz <mreitz@redhat.com>]

On 2015-01-27 at 15:01, Eric Blake wrote:
> On 01/26/2015 09:02 AM, Max Reitz wrote:
>> BlockAcctStats contains statistics about the data transferred from and
>> to the device; wr_highest_offset does not fit in with the rest.
>>
>> Furthermore, those statistics are supposed to be specific for a certain
>> device and not necessarily for a BDS (see the comment above
>> bdrv_get_stats()); on the other hand, wr_highest_offset may be a rather
>> important information to know for each BDS. When BlockAcctStats is
>> finally removed from the BDS, we will want to keep wr_highest_offset in
>> the BDS.
> Yes, I recently did work in libvirt to expose wr_highest_offset of
> backing images during block commit (qemu still isn't populating it on
> images opened only for read, but the point remains that it is a
> statistic tied to the BDS, not the BB).
>
> On the other hand, even the other statistics might make sense on both
> BDS and BB level (at the BB level, how many bytes has the guest
> read/written; at the BDS level, how many bytes were serviced by the
> active layer vs. delegated to a backing layer).
>
> I'm not sure if we are set up for that fine of a level of reporting yet,
> but we shouldn't make it hard to implement later.  But for now, I agree
> with separating the definite BDS-only stat, leaving the rest of the
> struct usable for either BDS or BB.
>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block.c                    | 4 +++-
>>   block/accounting.c         | 9 ---------
>>   block/qapi.c               | 4 ++--
>>   include/block/accounting.h | 3 ---
>>   include/block/block_int.h  | 3 +++
>>   5 files changed, 8 insertions(+), 15 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> +++ b/include/block/block_int.h
>> @@ -366,6 +366,9 @@ struct BlockDriverState {
>>       /* I/O stats (display with "info blockstats"). */
>>       BlockAcctStats stats;
>>   
>> +    /* Highest sector index written to */
>> +    uint64_t wr_highest_sector;
> Umm, now would be a great time to track this in bytes instead of
> sectors, if that is not too difficult to do.

Fine with me, will do.

(on a different note, please refer to the "RESEND" thread or replace 
john@redhat.com by jsnow@redhat.com in the CC list (my bad))

Max