[Qemu-devel] Re: [PATCH 1/2] block: Add wr_highest_sector blockstat

[Kevin Wolf <kwolf@redhat.com>]

Am 28.04.2010 19:31, schrieb Luiz Capitulino:
> On Wed, 28 Apr 2010 17:56:20 +0200
> Kevin Wolf <kwolf@redhat.com> wrote:
> 
>> This adds the wr_highest_sector blockstat which implements what is generally
>> known as the high watermark. It is the highest offset of a sector written to
>> the respective BlockDriverState since it has been opened.
>>
>> The query-blockstat QMP command is extended to add this value to the result,
>> and also to add the statistics of the underlying protocol in a new "parent"
>> field. Note that to get the "high watermark" of a qcow2 image, you need to look
>> into the wr_highest_sector field of the parent (which can be a file, a
>> host_device, ...). The wr_highest_sector of the qcow2 BlockDriverState itself
>> is the highest offset on the _virtual_ disk that the guest has written to.
>>
>> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
>> ---
>>  block.c     |   76 +++++++++++++++++++++++++++++++++++++++++++++++-----------
>>  block_int.h |    1 +
>>  2 files changed, 62 insertions(+), 15 deletions(-)
>>
>> diff --git a/block.c b/block.c
>> index 91fecab..b75cef2 100644
>> --- a/block.c
>> +++ b/block.c
>> @@ -880,6 +880,10 @@ int bdrv_write(BlockDriverState *bs, int64_t sector_num,
>>          set_dirty_bitmap(bs, sector_num, nb_sectors, 1);
>>      }
>>  
>> +    if (bs->wr_highest_sector < sector_num + nb_sectors - 1) {
>> +        bs->wr_highest_sector = sector_num + nb_sectors - 1;
>> +    }
>> +
>>      return drv->bdrv_write(bs, sector_num, buf, nb_sectors);
>>  }
>>  
>> @@ -1523,6 +1527,10 @@ static void bdrv_stats_iter(QObject *data, void *opaque)
>>                          qdict_get_int(qdict, "wr_bytes"),
>>                          qdict_get_int(qdict, "rd_operations"),
>>                          qdict_get_int(qdict, "wr_operations"));
>> +    if (qdict_haskey(qdict, "parent")) {
>> +        QObject *parent = qdict_get(qdict, "parent");
>> +        bdrv_stats_iter(parent, mon);
>> +    }
>>  }
>>  
>>  void bdrv_stats_print(Monitor *mon, const QObject *data)
>> @@ -1530,6 +1538,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)
>>      qlist_iter(qobject_to_qlist(data), bdrv_stats_iter, mon);
>>  }
>>  
>> +static QObject* bdrv_info_stats_bs(BlockDriverState *bs)
>> +{
>> +    QObject *res;
>> +    QObject *parent = NULL;
>> +
>> +    if (bs->file) {
>> +        parent = bdrv_info_stats_bs(bs->file);
>> +    }
> 
>  Doesn't build, I guess you meant 'backing_hd'.

The patch is against git://repo.or.cz/qemu/kevin.git block, not master
(like most block related patches lately).

backing_hd is for qcow base images, so that's a different thing.

>> +
>> +    res = qobject_from_jsonf("{ 'device': %s, 'stats': {"
>> +                             "'rd_bytes': %" PRId64 ","
>> +                             "'wr_bytes': %" PRId64 ","
>> +                             "'rd_operations': %" PRId64 ","
>> +                             "'wr_operations': %" PRId64 ","
>> +                             "'wr_highest_offset': %" PRId64
>> +                             "} }",
>> +                             bs->device_name,
>> +                             bs->rd_bytes, bs->wr_bytes,
>> +                             bs->rd_ops, bs->wr_ops,
>> +                             bs->wr_highest_sector * 512);
>> +    if (parent) {
>> +        QDict *dict = qobject_to_qdict(res);
>> +        qdict_put_obj(dict, "parent", parent);
>> +    }
>> +
>> +    return res;
>> +}
>> +
>>  /**
>>   * bdrv_info_stats(): show block device statistics
>>   *
>> @@ -1544,19 +1580,34 @@ void bdrv_stats_print(Monitor *mon, const QObject *data)
>>   *     - "wr_bytes": bytes written
>>   *     - "rd_operations": read operations
>>   *     - "wr_operations": write operations
>> - * 
>> + *     - "wr_highest_offset": Highest offset of a sector written since the
>> + *       BlockDriverState has been opened
>> + *     - "parent": Contains recursively the statistics of the underlying
>> + *       protocol (e.g. the host file for a qcow2 image). If there is no
>> + *       underlying protocol, this field is omitted.
> 
>  Looks like you are not pushing 'protocol', we already have 'format'
> though (not sure if they overlap).

You're talking about a new key 'protocol'? I'm not sure what it should
describe, to be honest.

>  Also, 'device' is "" for 'parent'. You should consider not pushing the
> key in this case (and noting that it's optional in the doc above).

Ok, will change that for v2.

Kevin